Document that cargo sets CARGO for build scripts
#4764
Merged
Conversation
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
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
@bors: r+ Looks good! As to whether that's expected to work, who knows! It's not like we made a decision one way or another about stuff like that... In any case it seems reasonable to at least attempt to get things to "work by default" more often, so where that's possible seems like we should? |
|
📌 Commit 94db2d1 has been approved by |
bors
added a commit
that referenced
this pull request
Nov 29, 2017
Document that cargo sets `CARGO` for build scripts This documents and tests the current behavior. However, I have two questions: * Do we support recursively invoking Cargo from the build script? * Do we support recursively invoking Cargo on the crate's own sources (ie, stuff inside `CARGO_HOME` which gets extracted from `.crate` files, and which in general is not byte-equal to the original sources). Here's an interesting problem that Servo faced: * Servo is a workspace. * Servo's `CARGO_HOME` is inside sources. * One of Servo's deps uses cbindgen from build.rs * cbindgen [invokes](https://github.com/eqrion/cbindgen/blob/ef3bd8d307f01ad1171ab69cf911b712fbd73583/src/bindgen/cargo/cargo_metadata.rs#L106) Cargo to read Cargo.toml from a crate inside CARGO_HOME (which is inside worksapce). This all failed to work as expected, because Cargo invoked by bindgen thought that workspace configuration was wrong. The problem was fixed on the Servo side by using `exclude` key for workspace. But do we actually guarantee that this is supposed to work? :) If we do, we might want to apply fix suggested by @nox and avoid looking for workspace root if we are inside `CARGO_HOME`. Note that cbindgen will probably be broken for crates which are workspaces themselves, because we rewrite members to path dependencies, but, if you have an explicit `members` key, `cargo metadata` inside such workspace in `CARGO_HOME` will complain. Original IRC discussion: https://botbot.me/mozilla/cargo/2017-11-29/?msg=94061970&page=3 cc @nox
Ok, I'll log an issue then: I am to overloaded to fix it myself at the moment :( |
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This documents and tests the current behavior.
However, I have two questions:
Do we support recursively invoking Cargo from the build script?
Do we support recursively invoking Cargo on the crate's own sources (ie, stuff inside
CARGO_HOMEwhich gets extracted from.cratefiles, and which in general is not byte-equal to the original sources).Here's an interesting problem that Servo faced:
CARGO_HOMEis inside sources.This all failed to work as expected, because Cargo invoked by bindgen thought that workspace configuration was wrong.
The problem was fixed on the Servo side by using
excludekey for workspace.But do we actually guarantee that this is supposed to work? :) If we do, we might want to apply fix suggested by @nox and avoid looking for workspace root if we are inside
CARGO_HOME.Note that cbindgen will probably be broken for crates which are workspaces themselves, because we rewrite members to path dependencies, but, if you have an explicit
memberskey,cargo metadatainside such workspace inCARGO_HOMEwill complain.Original IRC discussion: https://botbot.me/mozilla/cargo/2017-11-29/?msg=94061970&page=3
cc @nox