Skip to content

Comments

Use std::path::absolute instead of canonicalize#117

Merged
Byron merged 3 commits intoByron:mainfrom
ChrisDenton:absolute
Jan 19, 2026
Merged

Use std::path::absolute instead of canonicalize#117
Byron merged 3 commits intoByron:mainfrom
ChrisDenton:absolute

Conversation

@ChrisDenton
Copy link
Contributor

@ChrisDenton ChrisDenton commented Jan 19, 2026

canonicalize can fail on some devices. This PR instead uses std::path::absolute which has similar normalisation properties but will not fail for any valid path. The only real difference between absolute and canonicalize is that absolute does not resolve symlinks and does not produce \\?\ paths (unless they're passed in as such). Note that dunce is still used in case the user passed in a path starting with \\?\.

@Byron
Copy link
Owner

Byron commented Jan 19, 2026

Thanks a lot for contributing!

The green CI is deceptive unfortunately and it wasn't easy to reproduce, but when doing this…

cargo clippy  --target aarch64-pc-windows-msvc --features shellexecute-on-windows

…I could see what I suspected:

warning: current MSRV (Minimum Supported Rust Version) is `1.62.0` but this item is stable since `1.79.0`
  --> src/windows.rs:72:16
   |
72 |     let path = std::path::absolute(path)?;
   |                ^^^^^^^^^^^^^^^^^^^

I think this is fine, as the different MSRV is behind a feature toggle, but it shows CI is definitely under-tested.

Let me see if I can make this a bit safer.

@ChrisDenton
Copy link
Contributor Author

Ah, thanks for catching that! I admit I was a bit confused for a sec. If it's fine to raise the MSRV for this feature only then that's great! Alternatively, as absolute is just a wrapper around GetFullPathNameW, I could just manually call that. But it is more involved (and unsafe of course) so it wouldn't be my first preference. Or of course this could wait until the MSRV is high enough.

@Byron
Copy link
Owner

Byron commented Jan 19, 2026

Oh, you fixed CI for me, nice, thanks!

Like I said, the MSRV is for the non-feature toggled version of the crate only, so features can come with their own MSRVs. All good.

@Byron Byron merged commit 20ea175 into Byron:main Jan 19, 2026
7 checks passed
@ChrisDenton ChrisDenton deleted the absolute branch January 19, 2026 07:54
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.

2 participants