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

Paths starting with "//?/" on Windows are local #320

Conversation

Torbjorn-Svensson
Copy link
Contributor

The prefix might be used to support paths longer than MAX_PATH.
https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#win32-file-namespaces

@github-actions
Copy link

github-actions bot commented Sep 30, 2023

Test Results

   25 files  ±0     25 suites  ±0   11m 19s ⏱️ +34s
2 156 tests ±0  2 110 ✅ ±0  46 💤 ±0  0 ❌ ±0 
2 200 runs  ±0  2 154 ✅ ±0  46 💤 ±0  0 ❌ ±0 

Results for commit f08103b. ± Comparison against base commit e2267e3.

♻️ This comment has been updated with latest results.

@Torbjorn-Svensson Torbjorn-Svensson force-pushed the pr/support-windows-long-path-prefix branch from dd1fe91 to c358544 Compare October 1, 2023 10:35
@Torbjorn-Svensson Torbjorn-Svensson force-pushed the pr/support-windows-long-path-prefix branch from c358544 to 43bf83e Compare November 3, 2023 18:26
@HannesWell HannesWell force-pushed the pr/support-windows-long-path-prefix branch from 43bf83e to 7070f88 Compare November 5, 2023 12:13
@HannesWell
Copy link
Member

This change makes the Path constructor handle //?/, but I wonder shouldn't the //?/ prefix be stored somehow in order to make it available when the file-system is queried eventually?
But adding it to the device is probably not the right thing to do.

@Torbjorn-Svensson
Copy link
Contributor Author

This change makes the Path constructor handle //?/, but I wonder shouldn't the //?/ prefix be stored somehow in order to make it available when the file-system is queried eventually?
But adding it to the device is probably not the right thing to do.

Adding it to the device is not the correct thing to do and I'm not sure if it makes sense to save it.
The main use for it, as far as I know, is to avoid the MAX_PATH length limitation. I suppose the JVM is already doing something along these lines internally, as my testing shows that java applications does not have problem of managing deep file trees.

@Torbjorn-Svensson Torbjorn-Svensson force-pushed the pr/support-windows-long-path-prefix branch from 7070f88 to 5926533 Compare January 6, 2024 18:25
@Torbjorn-Svensson
Copy link
Contributor Author

Rebased PR on 1d9eb47.
Anything I need to do prior to getting this merged?

@vogella
Copy link
Contributor

vogella commented Jan 15, 2024

@HannesWell any more concerns or can this be merged?

@HannesWell HannesWell force-pushed the pr/support-windows-long-path-prefix branch from 5926533 to eb9d47a Compare January 15, 2024 19:51
Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply.

This change makes the Path constructor handle //?/, but I wonder shouldn't the //?/ prefix be stored somehow in order to make it available when the file-system is queried eventually?
But adding it to the device is probably not the right thing to do.

Adding it to the device is not the correct thing to do and I'm not sure if it makes sense to save it. The main use for it, as far as I know, is to avoid the MAX_PATH length limitation. I suppose the JVM is already doing something along these lines internally, as my testing shows that java applications does not have problem of managing deep file trees.

That's right. I tested it and with the standard Java File or Path API one can handle files and directories straight forward without that prefix (unlike for example the explorer). So it does not need to be added. And with JDK22 Java will also strip it off:
https://jdk.java.net/22/release-notes#JDK-8287843

So in general this change is fine, only the test should be slightly adjusted.

Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all good now.
Thank you for this contribution.

@HannesWell HannesWell merged commit cfb0c9e into eclipse-equinox:master Jan 15, 2024
23 of 24 checks passed
@Torbjorn-Svensson Torbjorn-Svensson deleted the pr/support-windows-long-path-prefix branch January 16, 2024 07:34
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