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

Add a build.cargoflags configuration key #8127

Closed
phil-opp opened this issue Apr 17, 2020 · 3 comments · Fixed by #8393
Closed

Add a build.cargoflags configuration key #8127

phil-opp opened this issue Apr 17, 2020 · 3 comments · Fixed by #8393
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@phil-opp
Copy link
Contributor

Describe the problem you are trying to solve

The cargo command line API has some -Z flags that allow to opt-in to unstable functionality. Examples are -Z build-std, which recompiles the sysroot for custom targets, or -Z features=build_dep, which fixes the long-standing problem tracked by #5730.

The problem with these flags is that they need to be explicitly passed to every invocation of cargo, which has many downsides:

  • It makes the build/check command much more cumbersome to type.
  • Your project might not build if you forget to pass the flags (e.g. with -Z build-std).
    • This means that you need to adjust all cargo invocations in CI scripts etc.
    • Users of your (nightly) crate need to use the adjusted build command too.
    • Tools like rust-analyzer also need to be adjusted to pass the required flag.
  • The changed build command is not recorded in the git history. So if you check out an old commit at some point, you need to manually remember which build command was used at that time.

As a result of these downsides, many people refrain from using unstable command line features for their (nightly) projects. This means that the features are less tested by the community before they are stabilized at some point.

Describe the solution you'd like

To solve these problems, I think there should be a way to enable cargo command line flags from a configuration file. One possibility for this could be to introduce a new build.cargoflags key in .cargo/config files, similar to the existing build.rustflags and build.rustdocsflags keys. This would be a relatively simple change that solves all the mentioned problems.

Notes

Given that it's already possible to enable unstable features of the rustc command line API through build.rustflags, there is already a precedent for enabling unstable command line features from .cargo/config files.

@phil-opp phil-opp added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Apr 17, 2020
@bearcage
Copy link
Contributor

It looks like there's some precedent for a different approach to globally enabling cargo Z flags in how mtime_on_use is configured. Doing this for all unstable flags would (I think) be as simple as modifying the snippet linked above to something like this:

if let Some(unstable_key) = key.strip_prefix("unstable.") {
    if let Err(err) = self.unstable_flags.add(unstable_key, value) {
        // error out on invalid flag? probably a good idea.
    }
}

I put together #8393 to see how that'd work, and I think it'd satisfy what you're trying to do @phil-opp -- lmk if you agree or if you would still prefer to see something like a cargoflags config entry.

@phil-opp
Copy link
Contributor Author

phil-opp commented Jun 22, 2020

Yes, I think this would solve my issue. I didn't know about the unstable table, but I really like the approach. Thanks a lot for tackling this!

@bearcage
Copy link
Contributor

Glad to hear it -- no problem!

I didn't know about unstable either until I saw mtime-on-use doing it, and I figured... why not?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants