Add try_canonicalize to rustc_fs_util and use it over fs::canonicalize

[Zoxc]

This adds try_canonicalize which tries to call fs::canonicalize, but falls back to std::path::absolute if it fails. Existing canonicalize calls are replaced with it. fs::canonicalize is not guaranteed to work on Windows.

[rustbot]

r? @WaffleLapkin

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

These commits modify the Cargo.lock file. Random changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
This was probably unintentional and should be reverted before this PR is merged.

If this was intentional then you can ignore this comment.

These commits modify compiler targets.
(See the Target Tier Policy.)

[WaffleLapkin]

r? compiler

[ChrisDenton]

Some Windows background info:

• The canonicalize function will fail on filesystem drivers that don't fully implement the kernel interface (most notably ImDisk RAM drives).
• Paths starting with \\?\ (which canonicalize returns) are sometimes necessary to avoid path length limitations. However, std will now take care of this automatically.
• std::path::absolute gets an absolute path without touching the filesystem, thus it should never fail on a valid path. However, it does not resolve symlinks.

So trying fs::canonicalize and falling back to path::absolute will suite most cases. The only issue I see would be if it's vital for symlinks to be resolved. (I did have a patch that would make canonicalize work even in the presence of ImDisk but people were understandably nervous of the weird paths it produces in that case).

[eholk]

fs::canonicalize is not guaranteed to work on Windows.

Is this something that's fixable? Or would the fix just be to move try_canonicalize to the standard library?

[eholk]

@bors r+

[bors]

📌 Commit 4f7cd3d has been approved by eholk

It is now in the queue for this repository.

[ChrisDenton]

fs::canonicalize is not guaranteed to work on Windows.

Is this something that's fixable?

It's fixable if either a) we fix all the faulty drivers in the world or b) are ok with returning some weird paths. The first would be fairly difficult to do and the second is seen as being far from ideal.

Or would the fix just be to move try_canonicalize to the standard library?

We could have a function that attempts to clean up the . and .. parts from paths in whatever way works (which I think is what people usually want) but that doesn't promise to resolve symlinks.

[Zoxc]

I'm starting to think that std::fs::canonicalize should be changed to be best effort. I'm not sure what use cases there are where you need it to fail. It can't be used to resolve aliasing in general.

[ChrisDenton]

That would be a breaking change so isn't on the table. More over there are cases (e.g. in security contexts) where you do actually want to resolve symlinks. So it is a useful function. A new function would be a better way forward.

[Zoxc]

GetFinalPathNameByHandle is documented to have a fallback path for SMB, do you know if this can silently fail to resolve symlinks / reparse points?

I guess we kind of need a function which returns absolute paths which can resolve .. to make paths more user presentable. std::path::absolute would be a faster alternative for internal use, but it wouldn't be suitable if you want to remove .. so it's less of a std::fs::canonicalize drop-in.

[ChrisDenton]

If an SMB server can't resolve a component it will return an error (although of course this is ultimately up to the remote server, which can just straight up lie if it wants to).

std::path::absolute works on paths that don't exist. This is a big benefit over alternatives and sometimes is the only option (e.g. before you create the file/directories). Properly resolving .. on Unix systems requires accessing the filesystem so, indeed, absolute cannot be used for that case. I'd agree that a different function would be required for that.