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

Inconsistent resolution of path "C:" with spawn on Windows #7215

Closed
ScottFreeCode opened this issue Jun 8, 2016 · 4 comments
Closed

Inconsistent resolution of path "C:" with spawn on Windows #7215

ScottFreeCode opened this issue Jun 8, 2016 · 4 comments
Labels
child_process Issues and PRs related to the child_process subsystem. path Issues and PRs related to the path subsystem. windows Issues and PRs related to the Windows platform.

Comments

@ScottFreeCode
Copy link

  • Version: 6.2.1
  • Platform: Windows 8.1, 64-bit
  • Subsystem: child_process, path

On Windows, when Node is run from another Node instance using child_process.spawn, path.resolve("C:") yields "C:". Calling path.resolve("C:") from a Node script run directly yields the current working directory. Changing directories, even to the current working directory, causes further calls to path.resolve("C:") to yield the current working directory.

This issue was originally reported by @sttk to mochajs/mocha#2242

Reproduce with the following code:

test.js

const path = require("path")

console.log(path.resolve("C:"))
console.log(process.cwd())

process.chdir(process.cwd())

console.log(path.resolve("C:"))
console.log(process.cwd())

wrap.js

var spawn = require('child_process').spawn,
  path = require('path'),
  args = [path.join(__dirname, 'test.js')]

spawn(process.execPath, args, { stdio: 'inherit' })

Calling node test.js prints the current working directory four times; calling node wrap.js prints "C:" once followed by the current working directory three times.

@mscdex mscdex added child_process Issues and PRs related to the child_process subsystem. windows Issues and PRs related to the Windows platform. path Issues and PRs related to the path subsystem. labels Jun 8, 2016
@RReverser
Copy link
Member

Generally that's how DOS / Windows always worked - there are separate current working directories per-disk, so when you were not specifying some path (even if \), disk itself resolves to that current directory.

However, I agree that it might be confusing behavior in context of Node.

@ScottFreeCode
Copy link
Author

Yeah, I get that C: is the current working directory for the C disk; what I don't get is that when Node first spawns a process the current working directory for the current disk is not set to the same as the general current working directory, whereas they're the same in all other circumstances I can find (although I guess to really dig into it I may have to check the behavior in other programs/languages that can spawn processes and resolve C:...). Is it normal Windows behavior that by default the general current working directory is inherited but the per-disk current working directory is not inherited even for the current disk, thus causing the two to differ when (and only when) a process is first spawned?

@izatop
Copy link

izatop commented Jun 22, 2016

I have same problem when scripts resolves path via npm run. You may use path.join instead of path.resolve, this solves issue.

jasongin added a commit to jasongin/nodejs that referenced this issue Sep 15, 2016
The `path.resolve()` function when given just a drive letter such as
"C:" tries to get a drive-specific CWD, but that isn't available in
cases when the process is not launched via cmd.exe and the process
CWD has not been explicitly set on that drive.

This change adds a fallback to the process CWD, if the process CWD
happens to be on the resolved drive letter. If the process CWD is on
another drive, then a drive-specific CWD cannot be resolved and
defaults to the drive's root as before.

Based on experimentation, the fixed behavior matches that of other
similar path resolution implementations on Windows I checked: .NET's
`System.IO.Path.GetFullPath()` and Python's `os.path.abspath()`.

In the automated path test cases the issue doesn't occur when the
tests are run normally from cmd.exe. But it did cause an assertion
when running the tests from PowerShell, that is fixed by this change.

Fixes: nodejs#7215
@ScottFreeCode
Copy link
Author

I love how that commit/PR explains and answers everything I was wondering about how and why this was happening, as well as confirming the expected behavior based on other systems (including Microsoft's own .Net!) and providing a simple fix. 👍

jasnell pushed a commit that referenced this issue Sep 29, 2016
The `path.resolve()` function when given just a drive letter such as
"C:" tries to get a drive-specific CWD, but that isn't available in
cases when the process is not launched via cmd.exe and the process
CWD has not been explicitly set on that drive.

This change adds a fallback to the process CWD, if the process CWD
happens to be on the resolved drive letter. If the process CWD is on
another drive, then a drive-specific CWD cannot be resolved and
defaults to the drive's root as before.

Based on experimentation, the fixed behavior matches that of other
similar path resolution implementations on Windows I checked: .NET's
`System.IO.Path.GetFullPath()` and Python's `os.path.abspath()`.

In the automated path test cases the issue doesn't occur when the
tests are run normally from cmd.exe. But it did cause an assertion
when running the tests from PowerShell, that is fixed by this change.

PR-URL: #8541
Fixes: #7215
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit that referenced this issue Oct 11, 2016
The `path.resolve()` function when given just a drive letter such as
"C:" tries to get a drive-specific CWD, but that isn't available in
cases when the process is not launched via cmd.exe and the process
CWD has not been explicitly set on that drive.

This change adds a fallback to the process CWD, if the process CWD
happens to be on the resolved drive letter. If the process CWD is on
another drive, then a drive-specific CWD cannot be resolved and
defaults to the drive's root as before.

Based on experimentation, the fixed behavior matches that of other
similar path resolution implementations on Windows I checked: .NET's
`System.IO.Path.GetFullPath()` and Python's `os.path.abspath()`.

In the automated path test cases the issue doesn't occur when the
tests are run normally from cmd.exe. But it did cause an assertion
when running the tests from PowerShell, that is fixed by this change.

PR-URL: #8541
Fixes: #7215
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. path Issues and PRs related to the path subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants