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

Fix unwanted rebuilds in xtask commands #1559

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nicholasbishop
Copy link
Member

Recently, cargo xtask test started doing a lot of rebuilding. Possibly this started with cargo 1.85, but I'm not sure.

The behavior was: cargo xtask test builds xtask and all of its dependencies in order to launch xtask. Then it would rebuild everything again to run the test command. If you then ran cargo xtask test a second time, everything would again be rebuilt. In other words, the build cache essentially wasn't working.

Fix by clearing all CARGO env vars in fix_nested_cargo_env.

Also update command_to_string to exclude cleared env vars, otherwise the command logging gets quite verbose.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

Rather than maintaining a list of all variables cleared by
`fix_nested_cargo_env`, just skip over all variables that are cleared by
checking if the value is `Some`.

That PATH var is still excluded by name since it's set (usually to a long value)
rather than cleared.
Recently, `cargo xtask test` started doing a lot of rebuilding. Possibly this
started with cargo 1.85, but I'm not sure.

The behavior was: `cargo xtask test` builds `xtask` and all of its dependencies
in order to launch `xtask`. Then it would rebuild everything again to run the
test command. If you then ran `cargo xtask test` a second time, everything would
again be rebuilt. In other words, the build cache essentially wasn't working.

Fix by clearing all `CARGO` env vars in `fix_nested_cargo_env`.
@nicholasbishop nicholasbishop requested a review from phip1611 March 1, 2025 23:37
@phip1611
Copy link
Contributor

phip1611 commented Mar 2, 2025

I ran this with rust 1.82 und 1.85. The following env vars are deleted

1.82

Deleting cargo env CARGO=/nix/store/9dp7c0afzkkqalj5035ycz7pgb7p9m39-cargo-1.82.0-x86_64-unknown-linux-gnu/bin/cargo
Deleting cargo env CARGO_MANIFEST_DIR=/home/pschuster/dev/other/uefi-rs/xtask
Deleting cargo env CARGO_PKG_AUTHORS=
Deleting cargo env CARGO_PKG_DESCRIPTION=
Deleting cargo env CARGO_PKG_HOMEPAGE=
Deleting cargo env CARGO_PKG_LICENSE=
Deleting cargo env CARGO_PKG_LICENSE_FILE=
Deleting cargo env CARGO_PKG_NAME=xtask
Deleting cargo env CARGO_PKG_README=
Deleting cargo env CARGO_PKG_REPOSITORY=
Deleting cargo env CARGO_PKG_RUST_VERSION=
Deleting cargo env CARGO_PKG_VERSION=0.0.0
Deleting cargo env CARGO_PKG_VERSION_MAJOR=0
Deleting cargo env CARGO_PKG_VERSION_MINOR=0
Deleting cargo env CARGO_PKG_VERSION_PATCH=0
Deleting cargo env CARGO_PKG_VERSION_PRE=

1.85

Deleting cargo env CARGO=/home/pschuster/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo
Deleting cargo env CARGO_HOME=/home/pschuster/.cargo
Deleting cargo env CARGO_MANIFEST_DIR=/home/pschuster/dev/other/uefi-rs/xtask
Deleting cargo env CARGO_MANIFEST_PATH=/home/pschuster/dev/other/uefi-rs/xtask/Cargo.toml
Deleting cargo env CARGO_PKG_AUTHORS=
Deleting cargo env CARGO_PKG_DESCRIPTION=
Deleting cargo env CARGO_PKG_HOMEPAGE=
Deleting cargo env CARGO_PKG_LICENSE=
Deleting cargo env CARGO_PKG_LICENSE_FILE=
Deleting cargo env CARGO_PKG_NAME=xtask
Deleting cargo env CARGO_PKG_README=
Deleting cargo env CARGO_PKG_REPOSITORY=
Deleting cargo env CARGO_PKG_RUST_VERSION=
Deleting cargo env CARGO_PKG_VERSION=0.0.0
Deleting cargo env CARGO_PKG_VERSION_MAJOR=0
Deleting cargo env CARGO_PKG_VERSION_MINOR=0
Deleting cargo env CARGO_PKG_VERSION_PATCH=0
Deleting cargo env CARGO_PKG_VERSION_PRE=

The effective difference is that CARGO_HOME and CARGO_MANIFEST_PATH have been added.

Given these facts, I don't exactly understand the cause of the rebuilds. But thank you for fixing it.

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