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

docs: warn users about non-ASCII paths on build #16735

Closed

Conversation

mmarchini
Copy link
Contributor

The build breaks if there's a non-ASCII character on the path to the building
directory.

Ref: #16278
Ref: #14336

Checklist
Affected core subsystem(s)

doc

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Nov 4, 2017
@bnoordhuis
Copy link
Member

#14336 was ostensibly addressed by #16047 although I won't vouch it always work everywhere now, see discussion in the PR.

@mmarchini
Copy link
Contributor Author

mmarchini commented Nov 6, 2017

Apparently, #16047 addresses only make lint, but make -j4 is also crashing. Just tested on Ubuntu 16.04 and it crashes if there's a Unicode character on the path.

LD_LIBRARY_PATH=/home/matheus/workspace/sthima/tmp/joão/node/out/Release/lib.host:/home/matheus/workspace/sthima/tmp/joão/node/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../deps/v8/src/inspector; mkdir -p /home/matheus/workspace/sthima/tmp/joão/node/out/Release/obj/gen/src/inspector/protocol /home/matheus/workspace/sthima/tmp/joão/node/out/Release/obj/gen/include/inspector; python ../../third_party/inspector_protocol/CodeGenerator.py --jinja_dir ../../third_party --output_base "/home/matheus/workspace/sthima/tmp/joão/node/out/Release/obj/gen/src/inspector" --config inspector_protocol_config.json
Failed to parse config file: 'ascii' codec can't decode byte 0xc3 in position 37: ordinal not in range(128)

deps/v8/src/inspector/protocol_generated_sources.target.mk:16: recipe for target '1ddbb19584026854c12fe832a3659fb93afcac00.intermediate' failed
make[1]: *** [1ddbb19584026854c12fe832a3659fb93afcac00.intermediate] Error 1
rm 1ddbb19584026854c12fe832a3659fb93afcac00.intermediate
Makefile:81: recipe for target 'node' failed
make: *** [node] Error 2

Path used was: /home/matheus/workspace/sthima/tmp/joão/node.

@bnoordhuis
Copy link
Member

Ah, that's been fixed upstream (link) but hasn't made its way downstream yet.

@mmarchini
Copy link
Contributor Author

Tested the upstream fix on Ubuntu 16.04 and Windows 10. Ubuntu is working, but Windows is still breaking:

image
(https://gist.github.com/anonymous/875aac0ab3e4dfa219854b691f8d3431)

Probably unrelated to #14336 as it seems to be a Windows-only problem with non-ASCII characters, I'll open a new issue for this.

BUILDING.md Outdated
@@ -97,7 +97,8 @@ More Developer Tools...`. This step will install `clang`, `clang++`, and
* After building, you may want to setup [firewall rules](tools/macosx-firewall.sh)
to avoid popups asking to accept incoming network connections when running tests:

If the path to your build directory contains a space, the build will likely fail.
If the path to your build directory contains a space or a non-ASCII character, the
build will likely fail.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not put this into the unix section, experiences from recent Code & Learn events have shown that we are not usually that far away from this working.

The only issue I know of is fixed by v8/v8@744b49e, which comes with the V8 6.4 upgrade (#17489).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense.

PR updated with the warning removed from the Unix section, keeping it only on the Windows section

The build breaks if there's a non-ASCII character on the path to the building
directory.

Ref: nodejs#16278
Ref: nodejs#14336
@mmarchini mmarchini force-pushed the nonascii-path-build-warning branch from 0a6f858 to 0e90f1a Compare December 12, 2017 19:18
@joyeecheung joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 17, 2018
@joyeecheung
Copy link
Member

@joyeecheung
Copy link
Member

Landed in c134ff2, thanks!

joyeecheung pushed a commit that referenced this pull request Jan 18, 2018
The build breaks if there's a non-ASCII character on
the path to the building directory.

PR-URL: #16735
Refs: #16278
Refs: #14336
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 18, 2018
evanlucas pushed a commit that referenced this pull request Jan 30, 2018
The build breaks if there's a non-ASCII character on
the path to the building directory.

PR-URL: #16735
Refs: #16278
Refs: #14336
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 30, 2018
The build breaks if there's a non-ASCII character on
the path to the building directory.

PR-URL: #16735
Refs: #16278
Refs: #14336
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 27, 2018
The build breaks if there's a non-ASCII character on
the path to the building directory.

PR-URL: #16735
Refs: #16278
Refs: #14336
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 27, 2018
The build breaks if there's a non-ASCII character on
the path to the building directory.

PR-URL: #16735
Refs: #16278
Refs: #14336
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
This was referenced Feb 27, 2018
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
The build breaks if there's a non-ASCII character on
the path to the building directory.

PR-URL: nodejs#16735
Refs: nodejs#16278
Refs: nodejs#14336
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants