Make cargo forward pre-existing CARGO if set#11285
Merged
bors merged 1 commit intorust-lang:masterfrom Nov 2, 2022
Merged
Conversation
|
r? @weihanglo (rust-highfive has picked a reviewer for you, use r? to override) |
Currently, Cargo will always set `$CARGO` to point to what it detects its own path to be (using `std::env::current_exe`). Unfortunately, this runs into trouble when Cargo is used as a library, or when `current_exe` is not actually the binary itself (e.g., when invoked through Valgrind or `ld.so`), since `$CARGO` will not point at something that can be used as `cargo`. This, in turn, means that users can't currently rely on `$CARGO` to do the right thing, and will sometimes have to invoke `cargo` directly from `$PATH` instead, which may not reflect the `cargo` that's currently in use. This patch makes Cargo re-use the existing value of `$CARGO` if it's already set in the environment. For Cargo subcommands, this will mean that the initial invocation of `cargo` in `cargo foo` will set `$CARGO`, and then Cargo-as-a-library inside of `cargo-foo` will inherit that (correct) value instead of overwriting it with the incorrect value `cargo-foo`. For other execution environments that do not have `cargo` in their call stack, it gives them the opportunity to set a working value for `$CARGO`. One note about the implementation of this is that the test suite now needs to override `$CARGO` explicitly so that the _user's_ `$CARGO` does not interfere with the contents of the tests. It _could_ remove `$CARGO` instead, but overriding it seemed less error-prone. Fixes rust-lang#10119. Fixes rust-lang#10113.
Contributor
Author
|
I hate Windows paths.. Pushed a test change that should hopefully fix the issue. |
Contributor
Author
|
Huzzah, looks like that did it! |
arlosi
reviewed
Oct 27, 2022
| via config. | ||
| * `CARGO` - If set, Cargo will forward this value instead of setting it | ||
| to its own auto-detected path when it builds crates and when it | ||
| executes build scripts and external subcommands. This value is not |
Contributor
There was a problem hiding this comment.
Should we include that the value, if set, should be the path to the cargo binary performing the build?
Contributor
Author
There was a problem hiding this comment.
I don't think that's necessary. It may also be confusing phrasing for tools that use Cargo as a library but do not otherwise have a cargo binary to point to (e.g., because they're not cargo subcommands).
Member
|
@bors r+ |
Contributor
Contributor
Contributor
|
☀️ Test successful - checks-actions |
weihanglo
added a commit
to weihanglo/rust
that referenced
this pull request
Nov 2, 2022
14 commits in 7e484fc1a766f56dbc95380f45719698e0c82749..810cbad9a123ad4ee0a55a96171b8f8478ff1c03 2022-10-27 15:20:57 +0000 to 2022-11-02 21:04:31 +0000 - Update curl-sys (rust-lang/cargo#11326) - Mention fix on build script deadlock (rust-lang/cargo#11325) - Make cargo forward pre-existing CARGO if set (rust-lang/cargo#11285) - Clean up workspace dependencies after cargo remove (rust-lang/cargo#11242) - Update the outdated link for rust-semverver (rust-lang/cargo#11322) - Fix broken link to compilation entry point (rust-lang/cargo#11317) - Only remove fingerprints and build script artifacts of the requested package (rust-lang/cargo#10621) - Newer anyhow features are required (rust-lang/cargo#11316) - Clean stale git temp files (rust-lang/cargo#11308) - Report crate size on package and publish (rust-lang/cargo#11270) - add a note that some warnings (and/or errors) can be auto-fixed (rust-lang/cargo#10989) - Update libcurl (rust-lang/cargo#11307) - artifact deps shoud works when target field specified coexists with `optional = true` (rust-lang/cargo#11183) - Fix singular verb in tests page (rust-lang/cargo#11300)
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 3, 2022
Update cargo 14 commits in 7e484fc1a766f56dbc95380f45719698e0c82749..810cbad9a123ad4ee0a55a96171b8f8478ff1c03 2022-10-27 15:20:57 +0000 to 2022-11-02 21:04:31 +0000 - Update curl-sys (rust-lang/cargo#11326) - Mention fix on build script deadlock (rust-lang/cargo#11325) - Make cargo forward pre-existing CARGO if set (rust-lang/cargo#11285) - Clean up workspace dependencies after cargo remove (rust-lang/cargo#11242) - Update the outdated link for rust-semverver (rust-lang/cargo#11322) - Fix broken link to compilation entry point (rust-lang/cargo#11317) - Only remove fingerprints and build script artifacts of the requested package (rust-lang/cargo#10621) - Newer anyhow features are required (rust-lang/cargo#11316) - Clean stale git temp files (rust-lang/cargo#11308) - Report crate size on package and publish (rust-lang/cargo#11270) - add a note that some warnings (and/or errors) can be auto-fixed (rust-lang/cargo#10989) - Update libcurl (rust-lang/cargo#11307) - artifact deps shoud works when target field specified coexists with `optional = true` (rust-lang/cargo#11183) - Fix singular verb in tests page (rust-lang/cargo#11300) r? `@ghost`
bors
pushed a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 5, 2022
20 commits in 7e484fc1a766f56dbc95380f45719698e0c82749..9286a1beba5b28b115bad67de2ae91fb1c61eb0b 2022-10-27 15:20:57 +0000 to 2022-11-04 06:41:49 +0000 - chore: Upgrade dependencies (rust-lang/cargo#11328) - Clean more aggressively in CI (rust-lang/cargo#11335) - Remove remove_dir_all (rust-lang/cargo#11333) - test(publish): Cover more wait-for-publish cases (rust-lang/cargo#11327) - Revert rust-lang/cargo#11183 (rust-lang/cargo#11331) - fix(semver-check): adapt to a different error for variant not covered (rust-lang/cargo#11332) - Update curl-sys (rust-lang/cargo#11326) - Mention fix on build script deadlock (rust-lang/cargo#11325) - Make cargo forward pre-existing CARGO if set (rust-lang/cargo#11285) - Clean up workspace dependencies after cargo remove (rust-lang/cargo#11242) - Update the outdated link for rust-semverver (rust-lang/cargo#11322) - Fix broken link to compilation entry point (rust-lang/cargo#11317) - Only remove fingerprints and build script artifacts of the requested package (rust-lang/cargo#10621) - Newer anyhow features are required (rust-lang/cargo#11316) - Clean stale git temp files (rust-lang/cargo#11308) - Report crate size on package and publish (rust-lang/cargo#11270) - add a note that some warnings (and/or errors) can be auto-fixed (rust-lang/cargo#10989) - Update libcurl (rust-lang/cargo#11307) - artifact deps shoud works when target field specified coexists with `optional = true` (rust-lang/cargo#11183) - Fix singular verb in tests page (rust-lang/cargo#11300)
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 5, 2022
Update cargo 20 commits in 7e484fc1a766f56dbc95380f45719698e0c82749..9286a1beba5b28b115bad67de2ae91fb1c61eb0b 2022-10-27 15:20:57 +0000 to 2022-11-04 06:41:49 +0000 - chore: Upgrade dependencies (rust-lang/cargo#11328) - Clean more aggressively in CI (rust-lang/cargo#11335) - Remove remove_dir_all (rust-lang/cargo#11333) - test(publish): Cover more wait-for-publish cases (rust-lang/cargo#11327) - Revert rust-lang/cargo#11183 (rust-lang/cargo#11331) - fix(semver-check): adapt to a different error for variant not covered (rust-lang/cargo#11332) - Update curl-sys (rust-lang/cargo#11326) - Mention fix on build script deadlock (rust-lang/cargo#11325) - Make cargo forward pre-existing CARGO if set (rust-lang/cargo#11285) - Clean up workspace dependencies after cargo remove (rust-lang/cargo#11242) - Update the outdated link for rust-semverver (rust-lang/cargo#11322) - Fix broken link to compilation entry point (rust-lang/cargo#11317) - Only remove fingerprints and build script artifacts of the requested package (rust-lang/cargo#10621) - Newer anyhow features are required (rust-lang/cargo#11316) - Clean stale git temp files (rust-lang/cargo#11308) - Report crate size on package and publish (rust-lang/cargo#11270) - add a note that some warnings (and/or errors) can be auto-fixed (rust-lang/cargo#10989) - Update libcurl (rust-lang/cargo#11307) - artifact deps shoud works when target field specified coexists with `optional = true` (rust-lang/cargo#11183) - Fix singular verb in tests page (rust-lang/cargo#11300) r? `@ghost`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, Cargo will always set
$CARGOto point to what it detects its own path to be (usingstd::env::current_exe). Unfortunately, this runs into trouble when Cargo is used as a library, or whencurrent_exeis not actually the binary itself (e.g., when invoked through Valgrind orld.so), since$CARGOwill not point at something that can be used ascargo. This, in turn, means that users can't currently rely on$CARGOto do the right thing, and will sometimes have to invokecargodirectly from$PATHinstead, which may not reflect thecargothat's currently in use.This patch makes Cargo re-use the existing value of
$CARGOif it's already set in the environment. For Cargo subcommands, this will mean that the initial invocation ofcargoincargo foowill set$CARGO, and then Cargo-as-a-library inside ofcargo-foowill inherit that (correct) value instead of overwriting it with the incorrect valuecargo-foo. For other execution environments that do not havecargoin their call stack, it gives them the opportunity to set a working value for$CARGO.One note about the implementation of this is that the test suite now needs to override
$CARGOexplicitly so that the user's$CARGOdoes not interfere with the contents of the tests. It could remove$CARGOinstead, but overriding it seemed less error-prone.Fixes #10119.
Fixes #10113.