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

Deduplicate PATH entries added during build (e.g. ~/.cargo/bin) #2848

Closed
dbr opened this issue Sep 14, 2021 · 2 comments · Fixed by #2849
Closed

Deduplicate PATH entries added during build (e.g. ~/.cargo/bin) #2848

dbr opened this issue Sep 14, 2021 · 2 comments · Fixed by #2849

Comments

@dbr
Copy link

dbr commented Sep 14, 2021

Problem
When running cargo build (or cargo check, cargo clippy), the $PATH env var contains ~/.cargo/bin twice. My $PATH contains it once (by sourcing ~/.cargo/env as per standard rustup install)

This addition to $PATH is mostly harmless, except when:

  1. A dependent package has a build.rs containing cargo:rerun-if-env-changed=PATH
  2. User switches between running cargo build and using something like rust-analyzer

The problem being:

  1. As user is editing, rust-analyzer is building and writes the "non-duplicated" $PATH into the fingerprinting file
  2. User switches to terminal and runs cargo build or similar, cargo adds the duplicated entry and the fingerprinter detects that $PATH changes and triggers a rebuild
  3. All downstream crates of the dependent package trbuild
  4. Switching back to rust-analyzer, it also rebuilds for the same reason

In short, the different $PATH causes lots of additional rebuilds (in some cases)

One example of this being pyo3:
PyO3/pyo3#1708
..which does have to re-run if $PATH changes, because a different Python interpreter might be added via a virtualenv

Steps (simpler)
Smallest reproduction case I can think of is:

dbr:~/pathexample$ cat Cargo.toml 
[package]
name = "pathexample"
version = "0.1.0"
edition = "2018"

build = "build.rs"

[dependencies]
dbr:~/pathexample$ cat build.rs 
fn main(){
    dbg!(env!("PATH"));
    panic!();
}
dbr:~/pathexample$ cargo build
Dupe path found /home/dbr/.cargo/bin
   Compiling pathexample v0.1.0 (/home/dbr/pathexample)
error: failed to run custom build command for `pathexample v0.1.0 (/home/dbr/pathexample)`

Caused by:
  process didn't exit successfully: `/home/dbr/pathexample/target/debug/build/pathexample-2395f9cd1cf04944/build-script-build` (exit code: 101)
  --- stderr
  [build.rs:2] env!("PATH") = "/home/dbr/.cargo/bin:/home/dbr/.cargo/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games"
  thread 'main' panicked at 'explicit panic', build.rs:3:5
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
dbr:~/pathexample$ echo $PATH
/home/dbr/.cargo/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games

(Noting the [build.rs:2] env!("PATH") contains /home/dbr/.cargo/bin twice, whereas echo $PATH is "correct")

If I bypass rustup (..I think..) by setting $PATH to only include a specific toolchain, the duplication doesn't occur:

dbr:~/pathexample$ env PATH=/home/dbr/.rustup/toolchains/1.54-x86_64-unknown-linux-gnu/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games cargo build
   Compiling pathexample v0.1.0 (/home/dbr/pathexample)
error: failed to run custom build command for `pathexample v0.1.0 (/home/dbr/pathexample)`

Caused by:
  process didn't exit successfully: `/home/dbr/pathexample/target/debug/build/pathexample-875cafc10592a6c9/build-script-build` (exit status: 101)
  --- stderr
  [build.rs:2] env!("PATH") = "/home/dbr/.rustup/toolchains/1.54-x86_64-unknown-linux-gnu/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games"
  thread 'main' panicked at 'explicit panic', build.rs:3:5
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Steps (real world)
Larger repro is to:

  1. Make a simple crate with pyo3 = "*" as a dependency
  2. run an editor with rust-analyzer and make some trivial changes
  3. switch to a terminal and run the following:
CARGO_LOG=cargo::core::compiler::fingerprint=info cargo build

..and note the lines similar to:

INFO  cargo::core::compiler::fingerprint]     err: env var `PATH` changed: previously Some("/home/dbr/.cargo/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games"), now Some("/home/dbr/.cargo/bin:/home/dbr/.cargo/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games")

Possible Solution(s)
🤷

Notes

Output of rustup --version:

$ rustup --version
rustup 1.24.3 (ce5817a94 2021-05-31)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.51.0 (2fd73fabe 2021-03-23)`

Output of rustup show:

$ rustup show
Default host: x86_64-unknown-linux-gnu
rustup home:  /home/dbr/.rustup

installed toolchains
--------------------

stable-x86_64-unknown-linux-gnu
nightly-2021-04-15-x86_64-unknown-linux-gnu
nightly-2021-07-05-x86_64-unknown-linux-gnu
nightly-x86_64-unknown-linux-gnu
1.46-x86_64-unknown-linux-gnu
1.48-x86_64-unknown-linux-gnu
1.51-x86_64-unknown-linux-gnu (default)
1.53-x86_64-unknown-linux-gnu
1.54-x86_64-unknown-linux-gnu

installed targets for active toolchain
--------------------------------------

wasm32-unknown-unknown
x86_64-unknown-linux-gnu

active toolchain
----------------

1.51-x86_64-unknown-linux-gnu (default)
rustc 1.51.0 (2fd73fabe 2021-03-23)
@kinnison
Copy link
Contributor

I believe the approach which would work here would be in:

https://github.com/rust-lang/rustup/blob/master/src/env_var.rs#L24-L37

We should, for each value in the new values we want to prepend, if they exist in the old values, remove them first from there. This would be fairly easy to do by means of a .filter() on the split_paths().collect() chain.

If someone wants to give this a go, I'm happy to mentor/help.

@kinnison kinnison changed the title Duplicate ~/.cargo/bin entries in builds Deduplicate PATH entries added during build (e.g. ~/.cargo/bin) Sep 15, 2021
@chansuke
Copy link
Contributor

I would like to send PR

jonhoo pushed a commit to jonhoo/rustup.rs that referenced this issue Apr 14, 2022
The current logic forces nested invocations to execute `cargo` and
friends from `$CARGO_HOME/bin`. This makes rustup break in environments
where the appropriate rustup proxies to use happen to be installed
elsewhere (earlier on `$PATH`), and using the binaries in
`$CARGO_HOME/bin` would not work correctly.

It also ensures that Rustup won't change `$PATH` "just for the heck of
it", which _should_ help reduce unnecessary re-compilations when
downstream build logic notices that `$PATH` changes (since it will no
longer).

Helps with rust-lang#2848.

Fixes rust-lang/cargo#7431.
Darunada pushed a commit to Darunada/rustup that referenced this issue Feb 25, 2023
The current logic forces nested invocations to execute `cargo` and
friends from `$CARGO_HOME/bin`. This makes rustup break in environments
where the appropriate rustup proxies to use happen to be installed
elsewhere (earlier on `$PATH`), and using the binaries in
`$CARGO_HOME/bin` would not work correctly.

It also ensures that Rustup won't change `$PATH` "just for the heck of
it", which _should_ help reduce unnecessary re-compilations when
downstream build logic notices that `$PATH` changes (since it will no
longer).

Helps with rust-lang#2848.

Fixes rust-lang/cargo#7431.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants