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

Prefer File.getCanonicalFile over Path.normalize #79

Closed

Conversation

alexarchambault
Copy link
Collaborator

Fixes #78.

@alexarchambault

This comment has been minimized.

The former normalizes Windows 8.3 paths to long paths, whereas the
latter doesn't.
Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

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

Looks good to me. Having a test for it, would make me more comfortable.

@alexarchambault
Copy link
Collaborator Author

Having a test for it, would make me more comfortable.

Same for me, actually… I'll try to add one the next time I'll boot my Windows machine.

@alexarchambault
Copy link
Collaborator Author

alexarchambault commented Jun 8, 2021

Ok, I added a test… and now I'm shared about merging this 😬

File.getCanonicalFile depends on what's on disk, so that two calls of os.Path(…).toString may not return the same value (if we pass it a short form, it will return a longer one, but only when the corresponding file exists), which could be surprising.

But a worse issue is that it seems that File.getCanonicalFile caches its values: if it's called with a short form before the file exists, it will later always return the short form, even if we create the underlying file later on.

Path.normalize doesn't suffer those quirks, AFAIK.

And fwiw, a similar issue can be reproduced on macOS (with its "case-insensitive but case-preserving" file systems), using just the case:

$ mkdir Foo && amm -c 'println((os.pwd / "foo").toIO.getCanonicalFile)'
/Users/alexandre/test/Foo

$ rmdir Foo && amm -c 'println((os.pwd / "foo").toIO.getCanonicalFile); os.makeDir(os.pwd / "Foo"); println((os.pwd / "foo").toIO.getCanonicalFile)'
/Users/alexandre/test/foo
/Users/alexandre/test/foo

$ ls
Foo

@lefou
Copy link
Member

lefou commented Jun 8, 2021

Ok, I added a test… and now I'm shared about merging this grimacing

Do you mean "scared"?

File.getCanonicalFile depends on what's on disk, so that two calls of os.Path(…).toString may not return the same value (if we pass it a short form, it will return a longer one, but only when the corresponding file exists), which could be surprising.

Oh, as I commented on #78, I'd rather remove any dependency on disk state, and I was under the impression, you're going to always normalize to the long form. In that regard, I don't understand the test case, at least, it test something different.

But a worse issue is that it seems that File.getCanonicalFile caches its values: if it's called with a short form before the file exists, it will later always return the short form, even if we create the underlying file later on.

Path.normalize doesn't suffer those quirks, AFAIK.

Better, we don't merge this.

@alexarchambault
Copy link
Collaborator Author

It'd be better not to merge, yes.

(And it seems there's a need for a library providing true canonical paths, without those caching quirks…)

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.

File.getCanonicalFile "normalizes" more than Path.normalize on Windows
2 participants