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

fix: check bash path before invocation #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

antongolub
Copy link

Hi there.

Current bash-glob impl does not verify that bash bin exists and sometimes an unhandled exception is thrown. This PR brings the missed check.

$ npx handpick --target=releaseDependencies && npx multi-semantic-release -d
npx: installed 26 in 3.643s
- Hand picking DIRTY releaseDependencies via NPM
✔ Hand picking DIRTY releaseDependencies via NPM
multi-semantic-release version: 2.4.5
semantic-release version: 17.1.1
flags: {
  "d": true,
  "sequentialInit": false,
  "firstParent": false,
  "debug": false
}
[multi-semantic-release]: TypeError [ERR_INVALID_ARG_TYPE]: The "file" argument must be of type string. Received type undefined
    at validateString (internal/validators.js:112:11)
    at normalizeSpawnArguments (child_process.js:398:3)
    at spawnSync (child_process.js:557:16)
    at Function.spawnSync [as sync] (/builds/project/node_modules/bash-glob/node_modules/cross-spawn/index.js:46:14)
    at Function.glob.sync (/builds/project/node_modules/bash-glob/index.js:189:18)
    at /builds/project/node_modules/bash-glob/index.js:160:29
    at Array.reduce (<anonymous>)
    at Function.glob.sync (/builds/project/node_modules/bash-glob/index.js:159:20)
    at getWorkspacesYarn (/builds/project/node_modules/multi-semantic-release/lib/getWorkspacesYarn.js:26:26)
    at module.exports (/builds/project/node_modules/multi-semantic-release/bin/runner.js:22:17) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Originally posted by @younes200 in dhoulb/multi-semantic-release#35 (comment)

bash-glob/index.js

var bashPath = require('bash-path');
//...
var cp = spawn(bashPath, cmd(pattern, options), options);

cross-spawn.index.js

var cp = require('child_process');
var parse = require('./lib/parse');
var cpSpawnSync = cp.spawnSync;
//...

// Parse the arguments
parsed = parse(command, args, options);

// Spawn the child process
result = cpSpawnSync(parsed.command, parsed.args, parsed.options);

@antongolub
Copy link
Author

@jonschlinkert,

could you take a look?

@jonschlinkert
Copy link
Member

jonschlinkert commented Oct 8, 2020 via email

BREAKING CHANGE: require NodeJS >= 8
@antongolub
Copy link
Author

@jonschlinkert,

these things are still needed: #4, micromatch/bash-path#1

@antongolub
Copy link
Author

@jonschlinkert,

one more ping.

@antongolub
Copy link
Author

@jonschlinkert,

@antongolub
Copy link
Author

antongolub commented Apr 22, 2021

@jonschlinkert,

PR is still waiting for your consideration.

@antongolub
Copy link
Author

@jonschlinkert,

Travis-ci.org is shutting down. Let's migrate to gh-actions/travis-ci.com or just merge this PR while tests are green.

Hi! It seems there are still plenty of builds running. We're pushing back the shutdown date to June 15th. Please consider migrating to travis-ci.com.

@antongolub
Copy link
Author

@jonschlinkert,
any progress on this?

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 this pull request may close these issues.

2 participants