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

Do not pass --target if user is not using --target #121

Merged
merged 1 commit into from
Aug 7, 2021

Conversation

taiki-e
Copy link
Contributor

@taiki-e taiki-e commented Aug 7, 2021

When --target flag is passed, cargo does not pass RUSTFLAGS to rustc when building proc-macro and build script even if the host and target triples are the same. Therefore, if we always pass --target to cargo, tools such as coverage that require RUSTFLAGS do not work for tests run by trybuild.

To avoid that problem, this patch change to not pass --target to cargo if we know that it has not been passed.

Cargo does not have a way to tell the build script whether --target has been passed or not, so we use the following heuristic:

  • The host and target triples are the same.
  • And RUSTFLAGS is available when building the build script.

Note that the second is when building, not when running. This is due to:

Fixes #115


Tested with a case where using cargo-llvm-cov and a case where using llvm-cov directly without any third party tools. See taiki-e/cargo-llvm-cov#44 (comment) for one of the cases I actually tested.

I have not tested the other tools, but it is likely that they will need some change like taiki-e/cargo-llvm-cov#44 on their side.

I also tested a case of #90, with a way discripted in #90.

Tested with cargo test --target x86_64-unknown-linux-gnu inside a crate that has [build] target = "thumbv7m-none-eabi" in .cargo/config.

// been passed or not, so we use the following heuristic:
//
// - The host and target triples are the same.
// - And RUSTFLAGS is available when *building* the build script.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this shouldn't be a problem because the build script will be rebuilt when RUSTFLAGS is changed, but I may have missed something.

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is great.

@dtolnay dtolnay merged commit 111abc4 into dtolnay:master Aug 7, 2021
@taiki-e taiki-e deleted the target branch August 7, 2021 17:39
bors bot added a commit to taiki-e/cargo-llvm-cov that referenced this pull request Aug 7, 2021
44: Support trybuild r=taiki-e a=taiki-e

Fixes #32

~~The following patch is needed until dtolnay/trybuild#121 is merged:~~ 

UPDATE: dtolnay/trybuild#121 merged and released in trybuild 1.0.44.


Co-authored-by: Taiki Endo <te316e89@gmail.com>
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.

Interaction with -Z instrument-coverage
2 participants