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

Windows. Incorrect Module.id with mounted network disk #7294

Closed
tsofist opened this issue Jun 13, 2016 · 9 comments
Closed

Windows. Incorrect Module.id with mounted network disk #7294

tsofist opened this issue Jun 13, 2016 · 9 comments
Labels
module Issues and PRs related to the module subsystem. windows Issues and PRs related to the Windows platform.

Comments

@tsofist
Copy link

tsofist commented Jun 13, 2016

  • Version: v6.2.1, v6.2.0
  • Platform: Windows 10, x64

Steps:

  • Mount network drive V:\ = \\CASTOR\
  • Install any npm-package (e.g. ajv) to \\CASTOR\scope\
  • Launch locally installed node with cwd: V:\scope\ (or launch node from network disk: V:\scope\node.exe)
  • Write:require("ajv"); console.log(Object.keys(require.cache)[0])

Output
node v6.1.0 (and previous): V:\scope\node_modules\ajv\lib\ajv.js
node v6.2.0 (and v6.2.1): \\CASTOR\scope\node_modules\ajv\lib\ajv.js

process.cwd() in all versions: V:\\scope

@mscdex mscdex added module Issues and PRs related to the module subsystem. windows Issues and PRs related to the Windows platform. labels Jun 13, 2016
@mscdex
Copy link
Contributor

mscdex commented Jun 13, 2016

Can you launch node with --preserve-symlinks and try again? I suspect 95b7560 is the culprit.

@tsofist
Copy link
Author

tsofist commented Jun 13, 2016

Yes, with --preserve-symlinks it works correctly

@mscdex
Copy link
Contributor

mscdex commented Jun 14, 2016

/cc @jasnell

@joshgav
Copy link
Contributor

joshgav commented Jun 14, 2016

/cc @orangemocha

@tsofist
Copy link
Author

tsofist commented Jun 19, 2016

I found similar problem with __filename and __dirname.
node v6.1.0 (and previous): V:\scope\main.js
node v6.2.0 (and v6.2.1): \\CASTOR\scope\main.js

Should I write a new issue for this?

@bzoz
Copy link
Contributor

bzoz commented Jun 22, 2016

b488b19 is responsible for this - it changed how fs.realpath works. It landed in v6.

At the same time change to how modules filenames are resolved was introduced in de1dc0a. It switched from fs.realpath to path.resolve. Those two changes together made module filenames resolve as before realpath implementation change.

Now 5d38d54 put change from fs.realpath to path.resolve behind --preserve-symlinks switch. And so, instead of having this issue with v6.0, with fs.realpath change, we have it now. It is just another effect of fs.realpath change which is documented as breaking.

I don't know if we should fix this. Old realpath implementation on Windows didn't check if the drive itself is a "symlink" - mapped network share or effect of callingsubst. IMHO current implementation is correct. If needed, one can use --preserve-symlinks to make modules file name resolve as they used to before. Alternatively, one can use require.resolve to get the filename used as keys to require.cache.

Of course, we could also try to fix this. For Windows, we could make require fall back to old realpath implementation if new one gives filename on different drive or on a network share. The question is, should we do this?

@tsofist
Copy link
Author

tsofist commented Jun 22, 2016

I wrote this module (t.js):

console.log(
    "__filename:      " + __filename + "\r\n" +
    "__dirname:       " + __dirname + "\r\n" +
    "path.resolve():  " + require("path").resolve() + "\r\n" +
    "module.id:       " + module.id + "\r\n" +
    "module.filename: " + module.filename
);

Run node from V:\Work\IDEA\projects\efrond\nas\~ mount from \\CASTOR-VSRV\Development\Work\IDEA\projects\efrond\nas\~

Output
node v5.9.1:

__filename:      V:\Work\IDEA\projects\efrond\nas\~\t.js
__dirname:       V:\Work\IDEA\projects\efrond\nas\~
path.resolve():  V:\Work\IDEA\projects\efrond\nas\~
module.id:       V:\Work\IDEA\projects\efrond\nas\~\t.js
module.filename: V:\Work\IDEA\projects\efrond\nas\~\t.js

node v6.0.0:

__filename:      V:\Work\IDEA\projects\efrond\nas\~\t.js
__dirname:       V:\Work\IDEA\projects\efrond\nas\~
path.resolve():  V:\Work\IDEA\projects\efrond\nas\~
module.id:       V:\Work\IDEA\projects\efrond\nas\~\t.js
module.filename: V:\Work\IDEA\projects\efrond\nas\~\t.js

node v6.2.1:

__filename:      \\CASTOR-VSRV\Development\Work\IDEA\projects\efrond\nas\~\t.js
__dirname:       \\CASTOR-VSRV\Development\Work\IDEA\projects\efrond\nas\~
path.resolve():  V:\Work\IDEA\projects\efrond\nas\~
module.id:       \\CASTOR-VSRV\Development\Work\IDEA\projects\efrond\nas\~\t.js
module.filename: \\CASTOR-VSRV\Development\Work\IDEA\projects\efrond\nas\~\t.js

node v6.2.1 --preserve-symlinks:

__filename:      V:\Work\IDEA\projects\efrond\nas\~\t.js
__dirname:       V:\Work\IDEA\projects\efrond\nas\~
path.resolve():  V:\Work\IDEA\projects\efrond\nas\~
module.id:       V:\Work\IDEA\projects\efrond\nas\~\t.js
module.filename: V:\Work\IDEA\projects\efrond\nas\~\t.js

I do not think it's ok.
Information about the module becomes grease, moreover, it is not obvious.

@bzoz
Copy link
Contributor

bzoz commented Jun 23, 2016

There is more discussion about this real path issue here: #7175.

@Loghorn
Copy link
Contributor

Loghorn commented Jul 8, 2016

My situation is even worse, if possible. I'm using Visual Studio Code to develop and debug node projects.
I frequently switch project, so to simplify my workflow I use subst to make the root of the project act like a disk drive: this makes path shorter (with the side effect of reducing the Windows long path problem) and allows me to quickly change project simply by typing the drive letter.
I recently upgraded to node v6.3.0 and I can't set breakpoint in VS Code anymore, because the modules are loaded with their 'real' path instead of the substituted one.
--preserve-symlinks doesn't resolve the situation: the main file loading ignores the flag and because all subsequent require use a relative path all the files are loaded with the real path, making impossible setting breakpoints (that needs the absolute path of the file)

bzoz added a commit to JaneaSystems/node that referenced this issue Aug 5, 2016
This reverts parts of nodejs@b488b19
restoring javascript implementation of realpath and realpathSync.

Fixes: nodejs#7175
Fixes: nodejs#6861
Fixes: nodejs#7294
Fixes: nodejs#7192
Fixes: nodejs#7044
Fixes: nodejs#6624
Fixes: nodejs#6978
@bzoz bzoz closed this as completed in 08996fd Aug 12, 2016
cjihrig pushed a commit that referenced this issue Aug 15, 2016
This reverts parts of b488b19
restoring javascript implementation of realpath and realpathSync.

Fixes: #7175
Fixes: #6861
Fixes: #7294
Fixes: #7192
Fixes: #7044
Fixes: #6624
Fixes: #6978
PR-URL: #7899
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

5 participants