Skip to content

Commit

Permalink
add shell:true on windows only for bash runnables, phetsims/perennial…
Browse files Browse the repository at this point in the history
…#359

Signed-off-by: Michael Kauzmann <[email protected]>
  • Loading branch information
zepumph committed Jan 30, 2025
1 parent 5cc3579 commit 71e4eb8
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 12 deletions.
6 changes: 4 additions & 2 deletions js/grunt/Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -731,8 +731,10 @@ Updates the normal automatically-generated files for this repository. Includes:
// Include the --repo flag
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'
const isWindows = /^win/.test( process.platform );
const spawned = child_process.spawn( isWindows ? 'grunt.cmd' : 'grunt', args, {
cwd: '../perennial',
shell: isWindows // shell is required for a NodeJS security update, see https://github.com/phetsims/perennial/issues/359
} );
grunt.log.debug( `running grunt ${argsString} in ../${repo}` );

Expand Down
13 changes: 5 additions & 8 deletions js/grunt/copySupplementalPhetioFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ const handleJSDOC = async buildDir => {
}
}

const getArgs = explain => [
const getJSDocArgs = explain => [
'../chipper/node_modules/jsdoc/jsdoc.js',
...( explain ? [ '-X' ] : [] ),
...JSDOC_FILES,
Expand All @@ -533,21 +533,18 @@ const handleJSDOC = async buildDir => {
'--readme', JSDOC_README_FILE
];


// FOR DEBUGGING JSDOC:
// uncomment this line, and run it from the top level of a sim directory
// console.log( 'node', getArgs( false ).join( ' ' ) );
// console.log( 'node', getJSDocArgs( false ).join( ' ' ) );

// First we tried to run the jsdoc binary as the cmd, but that wasn't working, and was quite finicky. Then @samreid
// found https://stackoverflow.com/questions/33664843/how-to-use-jsdoc-with-gulp which recommends the following method
// (node executable with jsdoc js file)
await execute( 'node', getArgs( false ), process.cwd(), {
shell: true
} );
await execute( 'node', getJSDocArgs( false ), process.cwd() );

// Running with explanation -X appears to not output the files, so we have to run it twice.
const explanation = ( await execute( 'node', getArgs( true ), process.cwd(), {
shell: true
} ) ).trim();
const explanation = ( await execute( 'node', getJSDocArgs( true ), process.cwd() ) ).trim();

// Copy the logo file
const imageDir = `${buildDir}doc/images`;
Expand Down
7 changes: 5 additions & 2 deletions js/grunt/lint.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const { ESLint } = require( 'eslint' ); // eslint-disable-line require-statement
const fs = require( 'fs' );

const DEBUG_MARKER = 'eslint:cli-engine';
const nxpCommand = /^win/.test( process.platform ) ? 'npx.cmd' : 'npx';
const npxCommand = /^win/.test( process.platform ) ? 'npx.cmd' : 'npx';

// Print formatted errors and warning to the console.
async function consoleLogResults( results ) {
Expand Down Expand Up @@ -118,8 +118,11 @@ function runEslint( repos, options ) {
}

// 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, {
const eslint = spawn( npxCommand, args, {
cwd: '../chipper',

// A shell is required for npx because the runnable is a shell script. see https://github.com/phetsims/perennial/issues/359
shell: /^win/.test( process.platform ),
env: env // Use the prepared environment
} );

Expand Down

0 comments on commit 71e4eb8

Please sign in to comment.