nabijaczleweli / cargo-update

A cargo subcommand for checking and applying updates to installed executables
MIT License
1.22k stars 42 forks source link

Installing from source causes `cargo install` PATH warning on Windows due to path canonicalization #252

Closed CAD97 closed 7 months ago

CAD97 commented 7 months ago

Example:

> cargo install-update -i cargo-docs-rs
    Polling registry 'https://index.crates.io/'.

Package        Installed  Latest   Needs update
cargo-docs-rs  No         v0.1.10  Yes

Installing cargo-docs-rs
 INFO resolve: Resolving package: 'cargo-docs-rs'
 WARN The package cargo-docs-rs v0.1.10 will be installed from source (with cargo)
    Updating crates.io index
  Installing cargo-docs-rs v0.1.10
    Updating crates.io index
    Finished release [optimized] target(s) in 0.59s
  Installing \\?\D:\.rust\cargo\bin\cargo-docs-rs.exe
   Installed package `cargo-docs-rs v0.1.10` (executable `cargo-docs-rs.exe`)
warning: be sure to add `\\?\D:\.rust\cargo\bin` to your PATH to be able to run the installed binaries
 INFO Cargo finished successfully
 INFO Done in 2.2362273s

Updated 1 package.
Overall updated 1 package: cargo-docs-rs.

The UNC-style path \\?\D:\.rust\cargo\bin is not on my PATH, but D:\.rust\cargo\bin is. Cargo (and cargo binstall) correctly is not displaying this warning itself; this warning is coming from the cargo install-update wrapper.

nabijaczleweli commented 7 months ago

This looks like binstall output. Can you repro this with cargo-binstall --roots {whatever your cargo dir is configured to} --no-confirm --version =0.1.10 --force --quiet cargo-docs-rs?

NobodyXu commented 7 months ago

warning: be sure to add \\?\D:\.rust\cargo\bin to your PATH to be able to run the installed binaries

I think this is from cargo-install.

Cargo-binstall output looks like this

INFO Cargo finished successfully

It's always <log_level> <msg>

CAD97 commented 7 months ago

QuickInstall is now providing cargo-docs-rs@0.1.10 so I can't easily reproduce with this crate again. However, I have previously verified that neither cargo install nor cargo binstall print the warning on their own, only when orchestrated through cargo install-update. This is a message that cargo install produces, but it doesn't without specific prompting.

I think I figured out what's going on, though: you're passing a normalized root, thus a root in UNC path format, and cargo install doesn't do a normalized-prefix check, only a prefix check. Previously: https://github.com/rust-lang/cargo/issues/13378

CAD97 commented 7 months ago

Oh and for clarity, my $CARGO_HOME is D:\.rust\cargo, i.e. not in the canonicalized extended-length format. This is probably due to a path.canonicalize() somewhere in cargo-install-update, when it really should only be doing CARGO_HOME.join("bin") without any canonicalization.

nabijaczleweli commented 7 months ago

Yeah, that was gonna be my guess as well – we run cargo-binstall --roots {cargo-dir} ... or cargo install --root {cargo-dir} ..., and cargo-dir is canonicalised, which yields the \\?\ prefix.

I was gonna say I hadn't seen this warning before, even with

>echo %PATH% | tr ';' '\n' | grep cargo
D:\Users\nabijaczleweli\.cargo\bin
d:\Users\nabijaczleweli\.cargo\bin

but I just reproed it with cargo install --root \\?\D:\Users\nabijaczleweli\.cargo treesize so maybe I just never noticed it or whatever.

nabijaczleweli commented 7 months ago

@CAD97 can you try the current master branch (at least 3cc63719ab83cf9760fd4e09cf72f9eb9136771a)?

nabijaczleweli commented 6 months ago

Fix released in v13.4.0