Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Error: spawn EINVAL #83

Closed
Badisi opened this issue Apr 12, 2024 · 11 comments · Fixed by #84
Closed

[BUG] Error: spawn EINVAL #83

Badisi opened this issue Apr 12, 2024 · 11 comments · Fixed by #84

Comments

@Badisi
Copy link

Badisi commented Apr 12, 2024

Issue

I'm playing with the following github action:

  prebuild:
    runs-on: ${{ matrix.os }}
    ...
    - name: Setup node
      uses: actions/setup-node@v4
      with:
        node-version: 20
        architecture: ${{ matrix.arch }}
    ...
    - name: Prebuild
      run: prebuildify --target 12.0.0 --napi --strip

The main idea is to run prebuildify on macos-latest:x64, ubuntu-latest:x64, windows-latest:x64 and windows-latest:x86.

Everything works fine except for the windows-latest:x86 where I get the following issue:

node:internal/child_process:414
    throw errnoException(err, 'spawn');
    ^

Error: spawn EINVAL
    at ChildProcess.spawn (node:internal/child_process:414:11)
    at Object.spawn (node:child_process:761:9)
    at D:\a\my-project\node_modules\prebuildify\index.js:230:22
    at D:\a\my-project\node_modules\mkdirp-classic\index.js:30:20
    at FSReqCallback.oncomplete (node:fs:192:23) {
  errno: -4071,
  code: 'EINVAL',
  syscall: 'spawn'

Investigation

The code in prebuildify index.js:230 is:

var child = proc.spawn(opts.nodeGyp, args, {
  cwd: opts.cwd,
  env: opts.env,
  stdio: opts.quiet ? 'ignore' : 'inherit'
})

After debugging I can see that the actual value used for opts.nodeGyp is "node-gyp.cmd":

{
  ...
  stdio: 'inherit',
  args: [
    'node-gyp.cmd',
    'rebuild',
    '--target=12.0.0',
    '--devdir=C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\prebuildify\\node',
    '--arch=ia32',
    '--release'
  ],
  detached: false,
  envPairs: [
    ...,
    ... 98 more items
  ],
  file: 'node-gyp.cmd',
  windowsHide: false,
  windowsVerbatimArguments: false
}

Solution

There seems to have some issues with spawn on Windows and most of the time people suggests to use shell: true to solve it.

So I monkey patched the previous code in prebuildify like this:

var child = proc.spawn(opts.nodeGyp, args, {
  shell: true,
  cwd: opts.cwd,
  env: opts.env,
  stdio: opts.quiet ? 'ignore' : 'inherit'
})

and it indeed solved the issue.

@kylegoetz
Copy link

I am also getting this when using tree-sitter's package-npm workflow. Exact same issue, only happening when building for windows 64bit. Not an issue for macOS and linux ubuntu

@vweevers
Copy link
Member

Which Node.js versions have this issue? Only 20?

@derevnjuk
Copy link
Contributor

@vweevers It relates to versions 18 and 20, which received some security fixes.

@vweevers
Copy link
Member

PR is welcome to add shell: true. We might want to investigate why it happens, but shell shouldn't have side effects AFAICT so that's a good short-term fix.

@Badisi
Copy link
Author

Badisi commented Apr 15, 2024

Same as @derevnjuk, I can confirm the issue exists with node 18 and 20.

As for the side effects, I can also confirm that using shell: true on every platforms worked fine in my case (so no side effects).

@derevnjuk
Copy link
Contributor

@Badisi It seems it only affects Windows x86, but I'm not sure. Have you tested it?

derevnjuk added a commit to derevnjuk/prebuildify that referenced this issue Apr 15, 2024
derevnjuk added a commit to derevnjuk/prebuildify that referenced this issue Apr 15, 2024
derevnjuk added a commit to derevnjuk/prebuildify that referenced this issue Apr 15, 2024
@Badisi
Copy link
Author

Badisi commented Apr 15, 2024

@derevnjuk,
Yes, my github action is running on macos-latest:x64, ubuntu-latest:x64, windows-latest:x64 and windows-latest:x86, and only windows-latest:x86 was failing.

Adding shell: true was enough to make it work and had no impacts on the other platforms.

@derevnjuk
Copy link
Contributor

@Badisi, thanks! 🙏🏾 I have opened the PR and am waiting for the review and feedback.

@derevnjuk
Copy link
Contributor

@vweevers the root cause is straightforward: Node.js team forbids the use of .bat and .cmd without the shell option, as seen here: nodejs/node@6627222409. For details, please refer to the following issues: nodejs/node#52470 and node-red/node-red#4652.

@kylegoetz
Copy link

Which Node.js versions have this issue? Only 20?

21.7.3 is what's used in my workflow

Screen Shot 2024-04-15 at 12 03 11 PM

derevnjuk added a commit to derevnjuk/prebuildify that referenced this issue Apr 15, 2024
derevnjuk added a commit to derevnjuk/prebuildify that referenced this issue Apr 15, 2024
@derevnjuk
Copy link
Contributor

Could you please provide an update on this issue? I believe this, along with the similar issue prebuild/node-gyp-build#68, should be considered critical bugs.

sebastiaanspeck added a commit to sebastiaanspeck/tree-sitter-magik that referenced this issue Apr 17, 2024
vweevers pushed a commit that referenced this issue Apr 17, 2024
TylerJang27 added a commit to trunk-io/plugins that referenced this issue Apr 23, 2024
Offers some temporary fixes to make `main` green and provide a migration
guide until we have a long-term story for #735. Also includes a node fix
with [executing `.bat`
files](https://github.com/trunk-io/plugins/actions/runs/8702103641/job/23865448625)
related to prebuild/prebuildify#83
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants