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

update-api-docs.js: execa returns error if file path has space(s) in it #22512

Closed
nfmohit opened this issue May 21, 2020 · 1 comment · Fixed by #22513
Closed

update-api-docs.js: execa returns error if file path has space(s) in it #22512

nfmohit opened this issue May 21, 2020 · 1 comment · Fixed by #22513
Labels
[Type] Bug An existing feature does not function as intended [Type] Build Tooling Issues or PRs related to build tooling

Comments

@nfmohit
Copy link
Member

nfmohit commented May 21, 2020

Describe the bug
This issue refers to the bin/api-docs/update-api-docs.js file. In this script, we're using execa to execute docgen, specifically in this line.

If your path to the WordPress installation has spaces in it, execa fails with an error code 127. When you run node ./bin/api-docs/update-api-docs.js, here's what the entire error message looks like:

Error: Command failed with exit code 127: /Users/nahid/Sites/Local by Flywheel/hdev-single/app/public/wp-content/plugins/gutenberg/node_modules/.bin/docgen packages/annotations/src/store/selectors.js --output docs/designers-developers/developers/data/data-core-annotations.md --to-token --use-token "Autogenerated selectors|../../../../packages/annotations/src/store/selectors.js" --ignore "/unstable|experimental/i"
/bin/sh: /Users/nahid/Sites/Local: No such file or directory
    at makeError (/Users/nahid/Sites/Local by Flywheel/hdev-single/app/public/wp-content/plugins/gutenberg/node_modules/execa/lib/error.js:58:11)
    at handlePromise (/Users/nahid/Sites/Local by Flywheel/hdev-single/app/public/wp-content/plugins/gutenberg/node_modules/execa/index.js:114:26)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async Transform.<anonymous> (/Users/nahid/Sites/Local by Flywheel/hdev-single/app/public/wp-content/plugins/gutenberg/bin/api-docs/update-api-docs.js:207:5) {
  shortMessage: 'Command failed with exit code 127: /Users/nahid/Sites/Local by Flywheel/hdev-single/app/public/wp-content/plugins/gutenberg/node_modules/.bin/docgen packages/annotations/src/store/selectors.js --output docs/designers-developers/developers/data/data-core-annotations.md --to-token --use-token "Autogenerated selectors|../../../../packages/annotations/src/store/selectors.js" --ignore "/unstable|experimental/i"',
  command: '/Users/nahid/Sites/Local by Flywheel/hdev-single/app/public/wp-content/plugins/gutenberg/node_modules/.bin/docgen packages/annotations/src/store/selectors.js --output docs/designers-developers/developers/data/data-core-annotations.md --to-token --use-token "Autogenerated selectors|../../../../packages/annotations/src/store/selectors.js" --ignore "/unstable|experimental/i"',
  exitCode: 127,
  signal: undefined,
  signalDescription: undefined,
  stdout: '',
  stderr: '/bin/sh: /Users/nahid/Sites/Local: No such file or directory',
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}

As you can see in the above error message, this specific test site of mine exists in a folder which contains "Local by Flywheel", i.e. with spaces in it. I understand having such an environment/site path is not the best practice, but I think we should do something for contributors who does have such a setup.

I did some research and found a (sort of) relevant issue in execa: sindresorhus/execa#408. Then, I landed upon this issue in node: nodejs/node#6803 and upon further reading, it appears that it is a limitation within child_processes, and the documentation suggests quoting the path if it has spaces in it.

I'll submit a PR with the path quoted so that it works with such a file structure setup.

To reproduce
Steps to reproduce the behaviour:

  1. Set up a local WordPress installation with spaces in its file path, for example, /Users/<username>/Sites/Word Press (ugh naughty!)
  2. Set up the Gutenberg repository as a plugin in that installation.
  3. Run node ./bin/api-docs/update-api-docs.js.
  4. Reproduce the above-mentioned error.

Expected behavior
It shouldn't return such an error merely for the file path.

Editor version (please complete the following information):

  • WordPress version: 5.4.1
  • Does the website has Gutenberg plugin installed, or is it using the block editor that comes by default? "gutenberg plugin"
  • If the Gutenberg plugin is installed, which version is it? 8.1.0

Desktop (please complete the following information):

  • OS: macOS
  • Browser: Google Chrome
  • Version 81.0.4044.138
@nfmohit
Copy link
Member Author

nfmohit commented May 21, 2020

I've submitted #22513 aiming to address this.

@nfmohit nfmohit linked a pull request May 21, 2020 that will close this issue
6 tasks
@nfmohit nfmohit added [Type] Bug An existing feature does not function as intended [Type] Build Tooling Issues or PRs related to build tooling labels May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant