node module,win: fix long path resolve

As reported here, there is a problem loading modules from long paths on Windows. This change fixes it by adding the required \\?\ prefix, with which, resolving long paths will work regardless of whether the LongPathsEnabled value is set or not.

Fixes: https://github.com/nodejs/node/issues/50753

Dec 08 '23 13:12 StefanStojanovic

FYI @nodejs/loaders @nodejs/fs

Dec 08 '23 16:12 richardlau

Can we add a test so we don't regress?

Of course. I'll add it next week.

Dec 08 '23 18:12 StefanStojanovic

Can we add a test so we don't regress?

Test's added.

Dec 15 '23 10:12 StefanStojanovic

@joyeecheung do you think this can land as a fix with a regression test added and then a follow-up PR can be opened for porting path.toNamespacedPath() logic and using it in here, but potentially in other places missing it as well?

Feb 08 '24 11:02 StefanStojanovic

Yes, with a TODO/FIXME comment.

Feb 08 '24 13:02 joyeecheung

@joyeecheung LGTY now?

Feb 09 '24 09:02 StefanStojanovic

CI: https://ci.nodejs.org/job/node-test-pull-request/57224/

Feb 20 '24 22:02 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/57499/

Feb 29 '24 11:02 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/57500/

Feb 29 '24 11:02 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/57517/

Mar 01 '24 00:03 nodejs-github-bot

Commit Queue failed

- Loading data for nodejs/node/pull/51097
✔  Done loading data for nodejs/node/pull/51097
----------------------------------- PR info ------------------------------------
Title      module,win: fix long path resolve (#51097)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     StefanStojanovic:mefi-resolve-long-path -> nodejs:main
Labels     c++, fs, needs-ci
Commits    1
 - module,win: fix long path resolve
Committers 1
 - StefanStojanovic 
PR-URL: https://github.com/nodejs/node/pull/51097
Fixes: https://github.com/nodejs/node/issues/50753
Reviewed-By: Joyee Cheung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51097
Fixes: https://github.com/nodejs/node/issues/50753
Reviewed-By: Joyee Cheung 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - module,win: fix long path resolve
   ℹ  This PR was created on Fri, 08 Dec 2023 13:28:00 GMT
   ✔  Approvals: 1
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/51097#pullrequestreview-1891732903
   ✘  GitHub CI is still running
   ℹ  Last Full PR CI on 2024-03-01T00:41:05Z: https://ci.nodejs.org/job/node-test-pull-request/57517/
- Querying data for job/node-test-pull-request/57517/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/8113120227

Mar 01 '24 15:03 nodejs-github-bot

@joyeecheung I see that the commit queue failed, but the reason GitHub CI is still running is unclear to me because all of the checks have already passed. Can you retrigger the commit queue or do something else to unblock this? Thanks in advance!

Mar 04 '24 14:03 StefanStojanovic

CI: https://ci.nodejs.org/job/node-test-pull-request/57845/

Mar 19 '24 18:03 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/57848/

Mar 19 '24 21:03 nodejs-github-bot

@joyeecheung can you add the commit-queue label to try to land this again? Last time it failed to do so...

Apr 01 '24 10:04 StefanStojanovic

CI: https://ci.nodejs.org/job/node-test-pull-request/58036/

Apr 01 '24 17:04 nodejs-github-bot

@StefanStojanovic @joyeecheung any chance of getting this released in one of the next patch or minor releases?

It's been a while since my original issue in Nov 2023 and users keep running into this (eg. with pnpm) and commenting that it's an open bug but the PR hasn't been merged yet.

Apr 03 '24 20:04 karlhorky

Commit Queue failed

- Loading data for nodejs/node/pull/51097
✔  Done loading data for nodejs/node/pull/51097
----------------------------------- PR info ------------------------------------
Title      module,win: fix long path resolve (#51097)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     StefanStojanovic:mefi-resolve-long-path -> nodejs:main
Labels     c++, fs, needs-ci
Commits    1
 - module,win: fix long path resolve
Committers 1
 - StefanStojanovic 
PR-URL: https://github.com/nodejs/node/pull/51097
Fixes: https://github.com/nodejs/node/issues/50753
Reviewed-By: Joyee Cheung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51097
Fixes: https://github.com/nodejs/node/issues/50753
Reviewed-By: Joyee Cheung 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - module,win: fix long path resolve
   ℹ  This PR was created on Fri, 08 Dec 2023 13:28:00 GMT
   ✔  Approvals: 1
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/51097#pullrequestreview-1891732903
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-04-01T17:51:22Z: https://ci.nodejs.org/job/node-test-pull-request/58036/
- Querying data for job/node-test-pull-request/58036/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/8596048782

Apr 08 '24 07:04 nodejs-github-bot

Commit Queue failed

Loading data for nodejs/node/pull/51097 ✔ Done loading data for nodejs/node/pull/51097 ----------------------------------- PR info ------------------------------------ Title module,win: fix long path resolve (#51097) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch StefanStojanovic:mefi-resolve-long-path -> nodejs:main Labels c++, fs, needs-ci Commits 1

module,win: fix long path resolve Committers 1

StefanStojanovic PR-URL: https://github.com/nodejs/node/pull/51097 Fixes: https://github.com/nodejs/node/issues/50753 Reviewed-By: Joyee Cheung ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/51097 Fixes: https://github.com/nodejs/node/issues/50753 Reviewed-By: Joyee Cheung

⚠ Commits were pushed since the last approving review: ⚠ - module,win: fix long path resolve ℹ This PR was created on Fri, 08 Dec 2023 13:28:00 GMT ✔ Approvals: 1 ✔ - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/51097#pullrequestreview-1891732903 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-04-01T17:51:22Z: https://ci.nodejs.org/job/node-test-pull-request/58036/

Querying data for job/node-test-pull-request/58036/ ✔ Last Jenkins CI successful

✔ Aborted git node land session in /home/runner/work/node/node/.ncu https://github.com/nodejs/node/actions/runs/8596048782

@joyeecheung do you see why this couldn't land from this log?

Apr 08 '24 07:04 StefanStojanovic

CI: https://ci.nodejs.org/job/node-test-pull-request/58206/

Apr 08 '24 08:04 nodejs-github-bot

Landed in 45f0dd019250945bafa56e14d187cbcd7793f2b6

Apr 08 '24 14:04 nodejs-github-bot

It looks like the Node.js v22 PR contains the fix "module,win: fix long path resolve (by @StefanStojanovic) https://github.com/nodejs/node/pull/51097":

https://github.com/nodejs/node/pull/52505#:~:text=%5B45f0dd0192%5D%20%2D%20module%2Cwin%3A%20fix%20long%20path%20resolve%20(Stefan%20Stojanovic)%20%2351097

So maybe Node.js has support for long paths on Windows now!

(make sure that you have LongPathsEnabled set to true on the Windows machine you test on)

May 02 '24 17:05 karlhorky