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

Make profile name and for_host target available as environment variables #10387

Closed
wants to merge 1 commit into from

Conversation

kamulos
Copy link

@kamulos kamulos commented Feb 13, 2022

Motivation

I want to use custom profiles to create special targets like coverage or sanitized. In a mixed Rust and C program this requires per profile control over the environment (CFLAGS and more), but also over the rustc command line parameters. Using the rustc-wrapper and rustc-workspace-wrapper this can be controlled very fine grained, but there is information missing:

  • The profile name I am compiling for to set the environment and rustc flags accordingly
  • The compile mode: I want to be able to skip setting some options for build.rs and proc-macro compilations

Approach

This PR makes the cargo profile name available as the environment variable CARGO_PROFILE_NAME. The target.for_host() is made available as CARGO_FOR_HOST. This seems to be true for build scripts and false in other cases.

Discussion

  • Obviously the env variable names are flawed and can be improved
  • for_host seems to be true for build scripts and false otherwise, but I am not sure if this is correct
  • For proc-macro crates both variables are just not set
  • There is Tracking Issue for profile-rustflags #10271, which at some point in the future will make part of this easier. But this will still lack the full control especially over the environment, which is important for the C compilation flags.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 13, 2022
@alexcrichton
Copy link
Member

For the profile name in an environment variable, I unfortunately don't think that will work because if the profile name changes and nothing else about a profile changes I don't believe that the build script is re-run (or at least it shouldn't be). The name isn't factored into the hash (or so I seem to recall) and I don't think it's something we want to factor into the hash. In general we try to not use names like that any more since specific configuration options are more important than trying to pick a name everyone in the ecosystem agrees on.

For the for_host variable I think your use case is unfortunately something not really supported right now. Cargo needs more internal refactorings to support use cases like this. Right now if there's something like cargo build and a dependency is used both in the final output and for a proc-macro/build-script I believe that dependency is built once and used in both locations, but that's not what you want for things like coverage. There are other PRs and issues about trying to fix this, but unfortunately I think it's a large-ish refactoring internally to fix this and also has user-facing ramifications that need to be considered carefully.

@alexcrichton
Copy link
Member

Thanks for the PR, but I'm going to be stepping down from the Cargo team so I'm going to un-assign myself from this. The Cargo team will help review this when they get a chance.

@alexcrichton alexcrichton removed their assignment Mar 8, 2022
@kamulos
Copy link
Author

kamulos commented Mar 8, 2022

This is not good anyways 😅 Thank you for your work!

@kamulos kamulos closed this Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants