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

src: don't error at startup when cwd doesn't exist #1194

Merged
merged 2 commits into from
Mar 19, 2015

Conversation

bnoordhuis
Copy link
Member

The current working directory may not exist when iojs starts up. Don't
treat that as an error because it's still possible to do many useful
things, like evaluating a command line script or starting a REPL.

Fixes: #1184

R=@mscdex?

https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/326/

@rvagg
Copy link
Member

rvagg commented Mar 18, 2015

lgtm

@bnoordhuis
Copy link
Member Author

Looks like you can't rmdir the cwd on Windows, I'll update the tests. It seems I broke parallel/test-process-argv-0 on Windows. I'll look into that, I anticipated breakage of that nature.

Anyone know what's up with Solaris? The rmdir calls are failing with EINVAL. /cc @iojs/platform-solaris

@jbergstroem
Copy link
Member

The problem on Solaris/Illumos is that you can't remove a directory you're currently standing in. Manpage here: https://smartos.org/man/2/rmdir.

@bnoordhuis
Copy link
Member Author

It seems I accidentally - or let's say: coincidentally - fixed an ancient Windows bug where process.argv[0] wasn't properly expanded. New CI: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/330/

@jbergstroem
Copy link
Member

LGTM after fix. Bonus points for tap output :shipit:

The current working directory may not exist when iojs starts up.  Don't
treat that as an error because it's still possible to do many useful
things, like evaluating a command line script or starting a REPL.

This commit also fixes an age-old Windows bug where process.argv[0] was
not properly expanded, that's why the parallel/test-process-argv-0 test
gets an update as well.

Fixes: nodejs#1184
PR-URL: nodejs#1194
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
The current working directory may not exist when the REPL starts up.
Don't treat that as an error because it's still possible to do many
useful things.  This is like the previous commit but for the REPL.

Fixes: nodejs#1184
PR-URL: nodejs#1194
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
@bnoordhuis bnoordhuis closed this Mar 19, 2015
@bnoordhuis bnoordhuis deleted the fix-issue-1184 branch March 19, 2015 01:12
@bnoordhuis bnoordhuis merged commit 2b2e48a into nodejs:v1.x Mar 19, 2015
@rvagg rvagg mentioned this pull request Mar 19, 2015
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.

3 participants