SpacingBat3 / ReForged

A set of :electron: Electron Forge tools, makers and publishers.
https://spacingbat3.github.io/ReForged/
Other
25 stars 5 forks source link

`Unable to parse '-version'` #10

Closed sbuller closed 1 year ago

sbuller commented 1 year ago

https://github.com/SpacingBat3/ReForged/blob/9f0d60ef83adf60a5d5355f036a74d63749e63d7/makers/appimage/src/utils.ts#L207

Clearing the env here leads to there being no PATH available, and it's interfering with looking up mksquashfs.

SpacingBat3 commented 1 year ago

https://github.com/SpacingBat3/ReForged/blob/9f0d60ef83adf60a5d5355f036a74d63749e63d7/makers/appimage/src/utils.ts#L207

Clearing the env here leads to there being no PATH available, and it's interfering with looking up mksquashfs.

I've checked this on Node v20.2.0 running on Arch Linux and it seems that's not the case for me – this is what I get on REPL:

> child_process.execFileSync("mksquashfs", ["-version"], {env:{}}).toString()
'mksquashfs version 4.6.1 (2023/03/25)\n' +
  'copyright (C) 2023 Phillip Lougher <phillip@squashfs.org.uk>\n' +
  '\n' +
  'This program is free software; you can redistribute it and/or\n' +
  'modify it under the terms of the GNU General Public License\n' +
  'as published by the Free Software Foundation; either version 2,\n' +
  'or (at your option) any later version.\n' +
  '\n' +
  'This program is distributed in the hope that it will be useful,\n' +
  'but WITHOUT ANY WARRANTY; without even the implied warranty of\n' +
  'MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\n' +
  'GNU General Public License for more details.\n'

…but I'll also verify if PATH is preserved (with env) and how.

SpacingBat3 commented 1 year ago

OK, so env is empty for me as expected. So I guess it behaves the way I imagined – because process is not being run in shell (or at least it shouldn't be according to docs), I thought Node.js is handling binary resolution using process.env.PATH while passing empty env to the child process.

Anyway, I think the try...catch here is redundant, I think it is safe to rely on Node.js errors unless I'll be relying another way to fetch version from the binary (by reading ELF with fs API maybe?). I'll probably remove it, for sake of readability.

sbuller commented 1 year ago

On Fedora I'm getting this:

$ node
Welcome to Node.js v18.16.0.
Type ".help" for more information.
> child_process.execFileSync("mksquashfs", ["-version"], {env:{}})
Uncaught:
<ref *1> Error: spawnSync mksquashfs ENOENT
    at __node_internal_captureLargerStackTrace (node:internal/errors:490:5)
    at __node_internal_errnoException (node:internal/errors:620:12)
    at Object.spawnSync (node:internal/child_process:1110:20)
    at spawnSync (node:child_process:871:24)
    at Object.execFileSync (node:child_process:914:15) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'spawnSync mksquashfs',
  path: 'mksquashfs',
  spawnargs: [ '-version' ],
  error: [Circular *1],
  status: null,
  signal: null,
  output: null,
  pid: 0,
  stdout: null,
  stderr: null
}
> child_process.execFileSync("mksquashfs", ["-version"], {})
<Buffer 6d 6b 73 71 75 61 73 68 66 73 20 76 65 72 73 69 6f 6e 20 34 2e 35 2e 31 20 28 32 30 32 32 2f 30 33 2f 31 37 29 0a 63 6f 70 79 72 69 67 68 74 20 28 43 ... 510 more bytes>
>
SpacingBat3 commented 1 year ago

Oh, I see now an issue – Node.js doesn't seems to use the PATH set by user on empty env, I wonder what source it picks for PATH instead… I guess it might use some system-wide defaults? Anyway, at least I know PATH might be required to be passed further, I guess I might add an exception for it.