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

node: warn if cwd is inaccessible during bootstrap #12022

Closed

Conversation

Fishrock123
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

bootstrap

This was added/fixed in 2c6f79c and seems like it could be quite confusing, so... maybe we should warn? ¯\_(ツ)_/¯

I don't really know how to trigger this though, so no test... for now?

cc @bnoordhuis, maybe this isn't a good idea, idk.

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Mar 24, 2017
@Fishrock123 Fishrock123 force-pushed the warn-on-inaccesible-cwd branch from cee2c9b to f381a70 Compare March 24, 2017 15:54
@addaleax
Copy link
Member

I don't really know how to trigger this though, so no test... for now?

mkdir /tmp/a; cd /tmp/a; rmdir /tmp/a; node -p 'module.paths'

@Fishrock123
Copy link
Contributor Author

Ah ok, will write a test in a bit.

@@ -374,6 +374,9 @@
threw = false;
} finally {
if (threw) {
process.emitWarning('The Current Working Directory was inaccessible.\n',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you lowercase Current Working Directory.

@@ -374,6 +374,9 @@
threw = false;
} finally {
if (threw) {
process.emitWarning('The Current Working Directory was inaccessible.\n',
'Falling back to the executable\'s directory...',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you drop the last two dots.

@@ -374,6 +374,9 @@
threw = false;
} finally {
if (threw) {
process.emitWarning('The Current Working Directory was inaccessible.\n',
Copy link
Member

Choose a reason for hiding this comment

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

comma at the end of this line... did you mean to put a + here instead?

@Fishrock123
Copy link
Contributor Author

ok, looks like there's more to it than this. Something loads path during startup which explodes regardless:

path.js:1163
          cwd = process.cwd();
                        ^

Error: ENOENT: no such file or directory, uv_cwd
    at Object.resolve (path.js:1163:25)
    at Function.Module._initPaths (module.js:642:22)
    at module.js:683:8
    at NativeModule.compile (bootstrap_node.js:548:7)
    at Function.NativeModule.require (bootstrap_node.js:489:18)
    at evalScript (bootstrap_node.js:390:33)
    at run (bootstrap_node.js:121:11)
    at run (bootstrap_node.js:445:7)
    at startup (bootstrap_node.js:120:9)
    at bootstrap_node.js:560:3

@Fishrock123
Copy link
Contributor Author

I think the solution there would probably be to overload process.cwd() entirely on load?

@Fishrock123 Fishrock123 force-pushed the warn-on-inaccesible-cwd branch from f381a70 to f14c96f Compare March 27, 2017 16:43
@cjihrig
Copy link
Contributor

cjihrig commented Apr 11, 2017

The cwd being removed is only supported for evaluating code. Not running "normally." I added a known issue test in #12343. We could add a better message for other scenarios, or set the cwd like we do for eval script.

cjihrig added a commit to cjihrig/node that referenced this pull request Apr 18, 2017
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Refs: nodejs#12022
PR-URL: nodejs#12343
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BridgeAR BridgeAR added the stalled Issues and PRs that are stalled. label Aug 26, 2017
@BridgeAR
Copy link
Member

Ping @Fishrock123

@BridgeAR
Copy link
Member

Closing this due to a long inactivity. Please feel free to reopen if you want to follow up on this @Fishrock123

@BridgeAR BridgeAR closed this Sep 12, 2017
Trott pushed a commit to Trott/io.js that referenced this pull request Sep 29, 2017
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Refs: nodejs#12022
PR-URL: nodejs#12343
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2017
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Backport-PR-URL: #15479
Refs: #12022
PR-URL: #12343
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Backport-PR-URL: #15479
Refs: #12022
PR-URL: #12343
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. stalled Issues and PRs that are stalled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants