fix: exe path on WSL

[dreisel]

on wsl the path needs to be re-written to a windows path

[malept]

Thanks for the pull request. I'd be much more comfortable with this change if we could test this somehow.

[dreisel]

Agree, but I wasn't sure exactly how to test it, I'm currently using it on WSL and it works fine,
and there is always a fallback incase the command fails / missing.

Would appreciate suggestions.

[dsanders11]

Made a couple of comments on the code, one of them is definitely a functional issue.

[dsanders11]

Seems like this is a typo, ** => &&?

[dreisel]

absolutely :|

[dsanders11]

I think this PR would be a bit easier to read if getQualifiedPath was renamed to getWslPath and the check for WSL was pushed to this line, as:

const args = [wsl ? getWslPath(exe) : exe]

[dreisel]

agree, fixed

[dreisel]

fixes #65

[dreisel]

@dsanders11 fixes were made

[dsanders11]

@dsanders11 fixes were made

Thanks, changes look good. Unfortunately I have no power here, was just commenting as I'd added the original feature. @malept will still need to be comfortable with it. Perhaps now that it's more clearly and cleanly confined to WSL that will be more reassuring.

[dreisel]

@malept any updates?

[andreineculau]

@dreisel Never a good idea to gulp an error :)
Our app name contains spaces so spawnSync returns

{
  "status": 1,
  "signal": null,
  "output": [
    null,
    "",
    "wslpath: /tmp/electron-packager/win32-x64/Foo Bar-win32-x64/Foo Bar.exe: Invalid argument\n"
  ],
  "pid": 6117,
  "stdout": "",
  "stderr": "wslpath: /tmp/electron-packager/win32-x64/Foo Bar-win32-x64/Foo Bar.exe: Invalid argument\n"
}

but you default silently to the linux path, which fails (main issue of this PR).

So

1. need to handle whitespaces in arguments to wslpath, BUT probably also to escape whitespaces of the response (see wslpath Handle/Escape Whitespaces microsoft/WSL#3713 )
2. if status is non-zero, print and throw

I'm in the process of fixing the above myself, but just wanted to dump these findings first. In the 1:1000000 chance that this PR gets merged :) Anytime now :)

LATER EDIT:

 const os = require('os')
 const path = require('path')
-const { spawn, spawnSync } = require('child_process')
+const { exec, execSync } = require('child_process')

 const pairSettings = ['version-string']
 const singleSettings = ['file-version', 'product-version', 'icon', 'requested-execution-level']
 const noPrefixSettings = ['application-manifest']

 const isWsl = process.platform !== 'win32' && os.release().endsWith('Microsoft')

 function getWslPath(exe) {
-  const winPath = spawnSync('wslpath', ["-w", exe], {encoding: 'utf8'});
-  if (winPath.status != 0) {
-    return exe
-  }
-
-  // removing trailing "\n" from output
-  return winPath.stdout.toString("utf8").replace("\n", "");
+  let winPath = execSync(`wslpath -w "${exe}"`, {encoding: 'utf8'});
+  winPath = winPath.replace("\n", "");
+  return winPath;
 }

 module.exports = async (exe, options) => {
   let rcedit = path.resolve(__dirname, '..', 'bin', process.arch === 'x64' ? 'rcedit-x64.exe' : 'rcedit.exe')
-  const args = [isWsl ? getWslPath(exe) : exe]
+  exe = isWsl ? getWslPath(exe) : exe;
+  let args = [`"${exe}"`]

   for (const name of pairSettings) {
     if (options[name]) {
       for (const [key, value] of Object.entries(options[name])) {
-        args.push(`--set-${name}`, key, value)
+        args.push(`--set-${name}`, key, `"${value}"`)
       }
     }
   }

   for (const name of singleSettings) {
     if (options[name]) {
-      args.push(`--set-${name}`, options[name])
+      args.push(`--set-${name}`, `"${options[name]}"`)
     }
   }

   for (const name of noPrefixSettings) {
     if (options[name]) {
-      args.push(`--${name}`, options[name])
+      args.push(`--${name}`, `"${options[name]}"`)
     }
   }

+  args = args.join(' ')
+
   const spawnOptions = {
     env: { ...process.env }
   }
@@ -57,7 +54,7 @@ module.exports = async (exe, options) => {
   }

   return new Promise((resolve, reject) => {
-    const child = spawn(rcedit, args, spawnOptions)
+    const child = exec(`${rcedit} ${args}`, spawnOptions)
     let stderr = ''
     let error = null

@@ -77,7 +74,7 @@ module.exports = async (exe, options) => {
       } else if (code === 0) {
         resolve()
       } else {
-        let message = `rcedit.exe failed with exit code ${code}`
+        let message = `${rcedit} failed with exit code ${code}`
         stderr = stderr.trim()
         if (stderr) {
           message += `. ${stderr}`

[dreisel]

@andreineculau the notion of gulping the error was to keep the effect to a minimum.
but I agree, updated the PR.

[andreineculau]

@dreisel FYI I paused the efforts after posting the patch, but I bumped into other issues and I know now that my patch is incorrect/incomplete. Will come back when I have something better to share.

[andreineculau]

I have updated my diff above with a fully working version.
But why exec -> spawn you might wonder? I can't answer except I got issues related to escaping. Using nodejs 10 btw (legacy codebase).

For anyone coming here thinking that it's only node-rcedit that needs WSL support, you have to know it's also Squirrel for Windows that needs patching if you're packaging the electron app into an installer Squirrel/Squirrel.Windows#1683 .

[dreisel]

@andreineculau do you wan't to open a PR to my branch?

[andreineculau]

I don't see a need. You can take all the credit ;) PS: It would have been nice with a CI pipeline to test WSL.

[electron-bot]

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀