Add a general mechanism of setting RUSTFLAGS in Cargo for the root crate only

[ridwanabdillahi]

This RFC adds support In Cargo for setting RUSTFLAGS via a new command line options, --rustflags <RUSTFLAGS> that only applies to the current crate being built.

Rendered

[rustbot]

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[joshtriplett]

I do think we should have a mechanism for this, but I think this option name would be confusing; it doesn't give any indication that it applies just to the root crate, or that it's different than RUSTFLAGS in any way.

[thomcc]

We have rustflags that impact ABI, and that would be unsound to only compile one crate with. For example, -Csoft-float on some targets, -Ctarget-feature (and -Ctarget-cpu by extension), and likely others (several -Z flags are this way, for sure).

I think this RFC should have some consideration of this, which I don't see.

[ridwanabdillahi]

@joshtriplett

Thanks for taking a look, I understand the concern and I'm fine to update the name of the option to make it clearer. Any thoughts on a suitable replacement? root-crate-rustflags, current-crate-rustflags?

@thomcc

Thanks for the comment, this issue exists today in Cargo via the cargo rustc subcommand which passes rustflags to rustc only for the root crate being compiled. This potential issue is not mentioned in the RFC and definitely should be. I'll update the Drawbacks section to explicitly mention this scenario and the harm that can be done by certain rustflags. Thank you for pointing this out.

[nrc]

We have rustflags that impact ABI, and that would be unsound to only compile one crate with. For example, -Csoft-float on some targets, -Ctarget-feature (and -Ctarget-cpu by extension), and likely others (several -Z flags are this way, for sure).

It's worth mentioning, but I wouldn't be too concerned about it - both setting rust_flags and setting ABI flags are advanced features and naive users are unlikely to stumble into an error here, so I'm fine with this being an 'at your own risk' kind of feature. The other side of the trade-off is that any mechanism for checking this sort of thing would need a list of 'risky' flags which would need to be kept in sync between rustc and Cargo, which is intrinsically fragile. More philosophically, the less Cargo knows about rustc, the better - separation of concerns and all that.

[thomcc]

In principal I agree, but it is worth noting -Ctarget-cpu=native is among the most common uses of RUSTFLAGS in my experience, even among new users -- it's even in many guides to place in RUSTFLAGS (usually when installing their crate). Most are not aware it has impact on ABI.

This isn't to say that cargo should know about it, but just that it's not as much of an advanced-user feature as you suggest.

[joshtriplett]

@ridwanabdillahi At the risk of making this slightly more complex: we have various things in Cargo for matching crates and setting profile information on them, and I'm wondering if we could use similar logic, so that it's possible to say "use these rustflags for this crate, and these rustflags for the top-level crate".

[ridwanabdillahi]

@joshtriplett thanks for your response!

I'm wondering if we could use similar logic, so that it's possible to say "use these rustflags for this crate, and these rustflags for the top-level crate"

I believe this is possible via the profile-rustflags unstable Cargo toml feature that exists today. This allows a user to specify which rustflags are enabled for a specific profile and can also specify the crates that should have these rustflags set.

The point of this RFC was to remove the need to specify a given profile when setting the rustflags that should be enabled for a given crate. This way any Rust developer that would like to set rustflags can regardless of the profile being used when running a build/test.

I understand this RFC adds to the complexity of setting rustflags by adding yet another supported way that is drastically different from the existing scenarios, i.e. cargo cli option. There is also an alternative listed in the RFC of expanding on the already supported build.rustflags config section to allow for specifying crates to match against in the same manner as the profile.<profile>.package.<crate> manifest key. Thoughts on this?

[jschwe]

I do think we should have a mechanism for this, but I think this option name would be confusing; it doesn't give any indication that it applies just to the root crate, or that it's different than RUSTFLAGS in any way.

I agree. All the other options which have rustflags in the name modify the rustflags for the crate and all dependencies.
I have two different suggestions:

a) rename the flag to --local-rustflags or similar to make it clear that it doesn't affect everything
b) adjust cargo rustc to optionally take a positional parameter build | test | bench | .... which extends cargo rustc to also support the other cargo commands. If the new positional parameter is missing, then cargo rustc behaves as it currently does, which implicitly is the same as the new cargo rustc build. To pass parameters to e.g. the test binary, this would work the same way as proposed in the current version of this RFC, i.e. cargo rustc test <params> -- <rustflags> ; -- <flags to the test binary>. I'm not a huge fan of the semicolon though (in both approaches).

I'm wondering if we could use similar logic, so that it's possible to say "use these rustflags for this crate, and these rustflags for the top-level crate".

To allow approach b) to support this I guess we could add another parameter to cargo rustc like --downstream-rustflags=<crate_name_regex>=<rustflags_for_selected_crates>

In general, I think none of this is impossible today - One could write a RUSTC_WRAPPER script that modifies rustflags on certain crates. It's just quite a lot of effort.

[joshtriplett]

I don't have a strong preference on the bikeshed color for this. Anything like root-rustflags or top-rustflags that gives an indication of this only applying to one crate seems fine.

[epage]

I feel like tying the definition of "root package" to the primary package or the default-members doesn't seem like the right way to go.

Unfortunately, I'm not sure of anything else. Maybe "whatever is 'selected' on the command-line" which will be inferred from the primary package or default-members but allows a user finer control (cargo build --package foo would make foo a root for that invocation)

[epage]

I think some terminology is being mixed here. A "package" corresponds to a Cargo.toml file while a "crate" is one build target within a package, whether that is the bin, the lib, an example, or one of the 20 integration test bins.

Which does raise the question of how this might want to be applied for different build target types.

[epage]

@rfcbot fcp postpone

Based on our discussion in today's Cargo team meeting, I'm proposed we postpone this RFC until someone can pick it up and drive it to completion.

In particular, areas that need addressing

• Naming
• Which build units this gets applied to
• Documenting this within the general precedence of RUSTFLAGS
• Documenting how this interacts with cargo rustc and the relationship between this flag and that command
• More clearly specifying the CLI flag and the trade offs for that design
  • e.g. why value terminator over a shlex::joined string or other designs
  • Can you repeat the flag?

This runs into a bit of the same problem as --artifact-dir in trying to guess users intent. This is less of an issue with profile.rustflags and at one point in the conversation, there was interest in stabilizing profile-rustflags first to see how much of a use case is left. However, one blocker for profile-rustflags is we are uncomfortable putting rustflags in Cargo.toml, wanting to leave that to the CLI and .cargo/config.toml.

There is some debate over whether a CLI flag or a config is better for this. The CLI flag might be easier to design. In the past though, there has been concern over the growth of Cargo's CLI and how that makes it less approachable.

[rfcbot]

Team member @epage has proposed to postpone this. The next step is review by the rest of the tagged team members:

• @Eh2406
• @Muscraft
• @arlosi
• @ehuss
• @epage
• @joshtriplett
• @weihanglo

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to postpone, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This is now postponed.