Node: Possible Windows Node 6 regression with spawn and a substed drive

[xzyfer]

• Version:
  v6.0.0
• Platform:
  Windows 32 and 64 bit
• Subsystem:

I've noticed a minor breaking change in either child_process.spawn or path.resolve (currently unclear), on Windows with Node 6. This issue appears to crop up when a Node script is execute from a SUBST'd path.

The production steps are tricky so I have created a reproduction in a repo that runs its tests in AppVeyor.

[xzyfer]

This may be related to #5950.

[saper]

Wow! I'm amazed.

[saper]

@xzyfer can you rename the issue to "substed" ? (or reference "drive created by subst.exe") - only few MS-DOS die-hards like me remember, what a "substed drive" is.

[xzyfer]

_updated_

[saper]

I have built node 7.0.0 (fa542ebcfd93c4d3e933f3dee986679f412c58b7) and the issue is still there. Reverting de1dc0ae2eb52842b5c5c974090123a64c3a594c fixes the issue - npm test done one a substed drive passes.

[xzyfer]

Nice find @saper

[jasnell]

Yep, https://github.com/nodejs/node/commit/de1dc0ae2eb52842b5c5c974090123a64c3a594c fixed another bug but led to some other regressions. There is work underway to find a workaround that would address both sets of concerns but it's non-trivial. This is queued up already for @nodejs/ctc discussion this week to see if we need to revert or find another path forward.

[kzc]

Is SUBST the Windows equivalent of a symlink?

mkdir c:\projects
cd c:\projects
git clone https://github.com/xzyfer/node-6-native-ext-bug.git
cd node-6-native-ext-bug
subst s: c:\projects
npm install
npm test

If you cd to s:\node-6-native-ext-bug does npm test run successfully?

[saper]

Subst is the ancient feature, coming from MSDOS times. It is more like Unix nullfs - mounting some subdirectory under the new name. But Unix does not have drive letters.

and yes, it failes when you are "on" a substed drive. Compiling from C:\projects works.

[kzc]

Thanks @saper. In the DOS days I'm sure I used SUBST but knowledge of it was pushed out of my brain.

So a SUBST'd drive is similar to a symlink. However unlike a symlink, it changes the _original source_ directory into the logical link and treats the new drive as the realpath? How odd. This seems backwards.

If this same example were run with node 5.x or earlier, does npm test still run within the subst'd directory s:\node-6-native-ext-bug? If my theory is correct it won't.

[saper]

Yes, all the tests ran correctly.

realpath is nothing magical, it tries to do its job to some extent. Node uses libuv, and its implementation of realpath for Windows uses GetFinalPathNameByHandle Windows API, which is not enough in this case. One had to use QueryDosDevice API. But there is also JOIN.EXE to makes things even harder. (Or ASSIGN.EXE, which seems to be gone from the most recent Windows releases).

[kzc]

@saper Okay this should be fixable. To your point, libuv fs__realpath_handle should implement the algorithm in http://pdh11.blogspot.ca/2009/05/pathcanonicalize-versus-what-it-says-on.html for realpathing of SUBST drives which should fix the situation.

Can you please confirm that if you use windows symlinks instead of SUBST - does npm test work in both the original and symlinked directory with node 6.0.0? Not knowing anything about Windows symlinks, google tells me Admin privileges may be required.

[saper]

Well, yes. But in general I avoid using realpath as much as possible. node-sass has been using it to get the one true path of the node executable and we gave up https://github.com/sass/node-sass/issues/1323.

[kzc]

Alright then, this appears to be a Windows libuv realpath issue, not a module resolution problem.

[saper]

No, that's not true. The fix in the module is incorrect. It aims at improving __dirname, it breaks things instead.

[kzc]

After the SUBST command is run, what is fs.realpathSync("c:/projects/node-6-native-ext-bug")? (assuming I got the Windows path format right)

Edit: In the new node 6.0.0 module resolution scheme the main script is still realpathed to get the right directory, but the other modules are not. This is why I think it's a libuv realpath issue if the expression above returns the wrong path.

[saper]

C:\Projects\node-6-native-ext-bug>v:\sw\node\Release\node -p fs.realpathSync('s:\\node-6-native-ext-bug')
C:\Projects\node-6-native-ext-bug

C:\Projects\node-6-native-ext-bug>v:\sw\node\Release\node -p fs.realpathSync('c:\\projects\\node-6-native-ext-bug')
C:\Projects\node-6-native-ext-bug

C:\Projects\node-6-native-ext-bug>subst
S:\: => C:\projects

So, realpath seems to work fine here.

[kzc]

Okay, there goes that theory.

If the same test is run with path.resolve instead of fs.realpathSync do you get the same result?

[saper]

C:\Projects\node-6-native-ext-bug>v:\sw\nodebrk -p path.resolve('s:\\node-6-native-ext-bug')
s:\node-6-native-ext-bug

C:\Projects\node-6-native-ext-bug>v:\sw\nodebrk -p path.resolve('c:\\projects\\node-6-native-ext-bug')
c:\projects\node-6-native-ext-bug

C:\Projects\node-6-native-ext-bug>subst
S:\: => C:\projects

[kzc]

I see. libuv handles SUBST correctly. Thanks.

[kzc]

Would it possible to get the value of require.resolve('buffertools') with node 6.0.0 when the command is run from C:\Projects\node-6-native-ext-bug and then from s:\node-6-native-ext-bug?

[saper]

C:\Projects\node-6-native-ext-bug>v:\sw\nodebrk.exe -p require.resolve('buffertools')
C:\Projects\node-6-native-ext-bug\node_modules\buffertools\buffertools.js

C:\Projects\node-6-native-ext-bug>s:

S:\>cd node-6-native-ext-bug

S:\node-6-native-ext-bug>v:\sw\nodebrk.exe -p require.resolve('buffertools')
S:\node-6-native-ext-bug\node_modules\buffertools\buffertools.js

[saper]

I think this problem is pretty easy to see from the npm test output:

Spawning: node S:\node-6-native-ext-bug\loader.js
Executing: C:\Projects\node-6-native-ext-bug\loader.js
Requiring: C:\Projects\node-6-native-ext-bug\node_modules\buffertools\build\Release\buffertools.node

The above lines run in a fresh copy of node (spawned) and have isMain true.
https://github.com/nodejs/node/commit/de1dc0ae2eb52842b5c5c974090123a64c3a594c#diff-d1234a869b3d648ebfcdce5a76747d71R117 when isMain is true, we are using realpath to convert the path to the One True Path.

Then a module is included, and isMain ceases to be true now:

Requiring: S:\node-6-native-ext-bug\subloader.js
Executing: S:\node-6-native-ext-bug\subloader.js
Requiring: S:\node-6-native-ext-bug\node_modules\buffertools\build\Release\buffertools.node

Now we take another code path https://github.com/nodejs/node/commit/de1dc0ae2eb52842b5c5c974090123a64c3a594c#diff-d1234a869b3d648ebfcdce5a76747d71R119 and we are not converting the path to the One True Path.

Therefore another attempt to load the binary module is undertook, which brings a whole thing down.

[kzc]

/cc @dlongley @lamara

[MylesBorins]

/cc @alexanderGugel

[jasnell]

See: https://github.com/nodejs/node/pull/6537

[dlongley]

@saper,

Could you just add an additional require to get around the isMain issue?

[dlongley]

I'd be +1 to removing the isMain flag and never using realpath at all, by default. Not sure what the consequences of that would be, however.

[saper]

@dlongley node needs to use realpath not to try to load the same binary module twice. It's clunky, but that's how it currently works...

[dlongley]

@saper,

node needs to use realpath not to try to load the same binary module twice. It's clunky, but that's how it currently works...

Then we should only do realpath on binary modules.

[kzc]

@saper Earlier in this thread I asked if the same test case worked with node 6.0.0 with a symlink instead of SUBST. Just wanted to confirm that this is true or if I misinterpreted you. Just trying to determine the scope of the issue and whether symlinks are a workaround.

[saper]

I have not tried to reproduce it with symlinks before, I have only used SUBST.EXE.

But yesterday I wrote a test case for node to check for this issue and I have made the test case use MKLINK.EXE (the symbolic linking on Windows) if it gets administrative privileges. The symptoms are the same (it does not matter if one uses SUBST or MKLINK).

[evanlucas]

@xzyfer can you please try v6.2.0 and see if the problem still exists? Thanks!

[xzyfer]

@evanlucas I can confirm this is fixed in Node v6.2.0 in both my reduced reproduction and in the larger node-sass ci.

[evanlucas]

Glad to hear. Closing as this has been fixed. Thanks!!