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

'grunt create-sim' fails on Windows with "Error: spawn EINVAL" #359

Closed
fgamador opened this issue Jul 14, 2024 · 6 comments
Closed

'grunt create-sim' fails on Windows with "Error: spawn EINVAL" #359

fgamador opened this issue Jul 14, 2024 · 6 comments

Comments

@fgamador
Copy link

When using a recent version of Node on Windows, 'grunt create-sim' fails with "Error: spawn EINVAL". I found that this is a known issue with recent versions of Node and can be fixed by adding shell: true to the options passed to child_process.spawn in perennial/js/common/execute.js (line 52). I tried it, and it works.

C:\Users\franz\Dev\NodeProjects\phetsims\perennial (main -> origin) ([email protected])
λ grunt create-sim --repo=my-first-sim --author="Franz Amador"
Running "create-sim" task
Greetings Franz Amador!
creating sim with repository name my-first-sim
wrote ../my-first-sim/.gitignore
[...]
wrote ../my-first-sim/tsconfig.json
info: npm update in ../my-first-sim
Fatal error: Perennial task failed:
Error: spawn EINVAL
at ChildProcess.spawn (node:internal/child_process:421:11)
at Object.spawn (node:child_process:761:9)
at C:\Users\franz\Dev\NodeProjects\phetsims\perennial\js\common\execute.js:52:40
at new Promise ()
at module.exports (C:\Users\franz\Dev\NodeProjects\phetsims\perennial\js\common\execute.js:45:10)
at C:\Users\franz\Dev\NodeProjects\phetsims\perennial\js\common\npmUpdateDirectory.js:28:11
at C:\Users\franz\Dev\NodeProjects\phetsims\perennial\node_modules\async-mutex\lib\Mutex.js:23:66
at Semaphore. (C:\Users\franz\Dev\NodeProjects\phetsims\perennial\node_modules\async-mutex\lib\Semaphore.js:37:46)
at step (C:\Users\franz\Dev\NodeProjects\phetsims\perennial\node_modules\tslib\tslib.js:195:27)
at Object.next (C:\Users\franz\Dev\NodeProjects\phetsims\perennial\node_modules\tslib\tslib.js:176:57)
Full Error details:
Error: spawn EINVAL

@fgamador
Copy link
Author

This may also fix #357.

@zepumph zepumph self-assigned this Jul 15, 2024
@zepumph
Copy link
Member

zepumph commented Jul 15, 2024

I was able to reproduce after upgrading my node version which was on 18.16.

Here is a limited patch that gets much working, but there are other usages of child processes that may want coverage (quake/aqua/weddell).

Furthermore, we likely want to test on windows and only optionally add this based on platform.

Subject: [PATCH] shell:true, https://github.com/phetsims/density-buoyancy-common/issues/256
---
Index: chipper/js/scripts/precommit-hook-multi.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/scripts/precommit-hook-multi.js b/chipper/js/scripts/precommit-hook-multi.js
--- a/chipper/js/scripts/precommit-hook-multi.js	(revision facc2134c559512fa92b8fd827474fccd75b935a)
+++ b/chipper/js/scripts/precommit-hook-multi.js	(date 1721060039292)
@@ -37,7 +37,7 @@
     const execOnRepos = async ( repoSubset, command ) => {
 
       const promises = repoSubset.map( repo => {
-        return new Promise( resolve => child_process.exec( command, { cwd: repo }, error => resolve( error ) ) );
+        return new Promise( resolve => child_process.exec( command, { cwd: repo, shell: true }, error => resolve( error ) ) );
       } );
       const results = await Promise.all( promises );
 
Index: perennial-alias/js/grunt/checkoutMainAll.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/perennial-alias/js/grunt/checkoutMainAll.js b/perennial-alias/js/grunt/checkoutMainAll.js
--- a/perennial-alias/js/grunt/checkoutMainAll.js	(revision 2557afe5afbed8d751a67156d99087a79a658314)
+++ b/perennial-alias/js/grunt/checkoutMainAll.js	(date 1721060039288)
@@ -24,7 +24,7 @@
   for ( let i = 0; i < gitRoots.length; i++ ) {
     const filename = gitRoots[ i ]; // Don't change to const without rewrapping usages in the closure
     if ( filename !== 'babel' && grunt.file.isDir( `../${filename}` ) && grunt.file.exists( `../${filename}/.git` ) ) {
-      child_process.exec( command, { cwd: `../${filename}` }, error => {
+      child_process.exec( command, { cwd: `../${filename}`, shell: true }, error => {
         if ( error ) {
           grunt.log.writeln( `error in ${command} for repo ${filename}` );
         }
Index: chipper/js/grunt/lint.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/grunt/lint.js b/chipper/js/grunt/lint.js
--- a/chipper/js/grunt/lint.js	(revision facc2134c559512fa92b8fd827474fccd75b935a)
+++ b/chipper/js/grunt/lint.js	(date 1721059933191)
@@ -120,6 +120,7 @@
     // It is nice to use our own spawn here instead of execute() so we can stream progress updates as it runs.
     const eslint = spawn( nxpCommand, args, {
       cwd: '../chipper',
+      shell: true,
       env: env // Use the prepared environment
     } );
 
Index: chipper/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/grunt/Gruntfile.js b/chipper/js/grunt/Gruntfile.js
--- a/chipper/js/grunt/Gruntfile.js	(revision facc2134c559512fa92b8fd827474fccd75b935a)
+++ b/chipper/js/grunt/Gruntfile.js	(date 1721059720503)
@@ -732,7 +732,8 @@
       const args = [ `--repo=${repo}`, ...process.argv.slice( 2 ) ];
       const argsString = args.map( arg => `"${arg}"` ).join( ' ' );
       const spawned = child_process.spawn( /^win/.test( process.platform ) ? 'grunt.cmd' : 'grunt', args, {
-        cwd: '../perennial'
+        cwd: '../perennial',
+        shell: true
       } );
       grunt.log.debug( `running grunt ${argsString} in ../${repo}` );
 
Index: perennial-alias/js/common/execute.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/perennial-alias/js/common/execute.js b/perennial-alias/js/common/execute.js
--- a/perennial-alias/js/common/execute.js	(revision 2557afe5afbed8d751a67156d99087a79a658314)
+++ b/perennial-alias/js/common/execute.js	(date 1721059665376)
@@ -51,7 +51,8 @@
 
     const childProcess = child_process.spawn( cmd, args, {
       cwd: cwd,
-      env: options.childProcessEnv
+      env: options.childProcessEnv,
+      shell: true
     } );
 
     childProcess.on( 'error', error => {
Index: perennial-alias/js/grunt/checkoutShas.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/perennial-alias/js/grunt/checkoutShas.js b/perennial-alias/js/grunt/checkoutShas.js
--- a/perennial-alias/js/grunt/checkoutShas.js	(revision 2557afe5afbed8d751a67156d99087a79a658314)
+++ b/perennial-alias/js/grunt/checkoutShas.js	(date 1721060039284)
@@ -39,7 +39,7 @@
       //cp.exec('foocommand', { cwd: 'path/to/dir/' }, callback);
       //http://stackoverflow.com/questions/14026967/calling-child-process-exec-in-node-as-though-it-was-executed-in-a-specific-folde
       const command = `git checkout ${toMain ? 'main' : dependencies[ property ].sha}`;
-      child_process.exec( command, { cwd: `../${property}` }, ( error1, stdout1, stderr1 ) => {
+      child_process.exec( command, { cwd: `../${property}`, shell: true }, ( error1, stdout1, stderr1 ) => {
         assert( !error1, `error in ${command} for repo ${property}` );
         grunt.log.writeln( 'Finished checkout.' );
         grunt.log.writeln( stdout1 );

@zepumph
Copy link
Member

zepumph commented Jul 15, 2024

Elevating priority since this totally breaks all our build tooling on windows after node upgrade.

zepumph added a commit that referenced this issue Jul 15, 2024
zepumph added a commit to phetsims/chipper that referenced this issue Jul 15, 2024
@zepumph
Copy link
Member

zepumph commented Jul 15, 2024

@samreid investigated and were able to trace it down to runnables that were not binaries (like bash scripts). For example grunt and npx. This helped us target exactly when we need the shell:true workaround. @fgamador, can you please check to see if this is fixed for you on main?

@fgamador
Copy link
Author

Yup, that fixes it. Thanks!

@zepumph
Copy link
Member

zepumph commented Jul 16, 2024

Excellent! I appreciate your quick response. Please do let us know if you run into anything else. We definitely want to continue improving our open source experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants