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

cargo config get doesn't seem to pick up RUSTFLAGS or RUSTDOCFLAGS #12087

Open
LukeMathWalker opened this issue May 5, 2023 · 13 comments
Open
Labels
A-documenting-cargo-itself Area: Cargo's documentation A-rustflags Area: rustflags C-bug Category: bug E-hard Experience: Hard S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. Z-cargo-config Nightly: cargo config subcommand

Comments

@LukeMathWalker
Copy link

LukeMathWalker commented May 5, 2023

Problem

cargo config get build.rustdocflags and cargo config get build.rustflags do not seem to behave as I would have expected.
If the respective environment variables are set (i.e. RUSTDOCFLAGS and RUSTFLAGS) but no value is set in .cargo/config.toml, I'd expect the environment variable value to be returned.

Instead, the command fails with:

error: config value `build.rustdocflags` is not set

Steps

export RUSTDOCFLAGS="--cfg tokio_unstable"
cargo -Z unstable-options config get build.rustdocflags

and

export RUSTFLAGS="--cfg tokio_unstable"
cargo -Z unstable-options config get build.rustflags

Possible Solution(s)

No response

Notes

Is this intended behaviour?

Version

No response

@LukeMathWalker LukeMathWalker added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels May 5, 2023
@weihanglo
Copy link
Member

I would say it is kinda intentional. The Cargo configuration system only cares about environment variables starting with CARGO_12. Or more precisely, those that listed in the doc are officially supported; those not may be subject to change.

Try this instead:

CARGO_BUILD_RUSTFLAGS="--cfg tokio_unstable" cargo +nightly -Z unstable-options config get build.rustflags

However, we must admit that the story around rustflags management is a bit messy.

Footnotes

  1. https://github.com/rust-lang/cargo/blob/2d693e20ea2dd3a55af94c56372c960503a6c673/src/cargo/util/config/key.rs#L31

  2. https://doc.rust-lang.org/cargo/reference/config.html#environment-variables

@LukeMathWalker
Copy link
Author

I see, but the docs do mention RUSTFLAGS as well:

build.rustflags

    Type: string or array of strings
    Default: none
    Environment: CARGO_BUILD_RUSTFLAGS or CARGO_ENCODED_RUSTFLAGS or RUSTFLAGS

Extra command-line flags to pass to rustc. The value may be an array of strings or a space-separated string.

@LukeMathWalker
Copy link
Author

The usecase here is being able to accurately predict what extra flags will be passed when invoking rustc or rustdoc without actually performing the invocation.
If cargo config get shows a value that differs from what cargo actually does, its value is greatly diminished.

@weihanglo weihanglo added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. A-rustflags Area: rustflags Z-cargo-config Nightly: cargo config subcommand A-documenting-cargo-itself Area: Cargo's documentation and removed S-triage Status: This issue is waiting on initial triage. S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. labels May 6, 2023
@weihanglo
Copy link
Member

weihanglo commented May 6, 2023

Since RUSTFLAGS is essentially not a cargo configuration, I guess it won't be included in cargo config get in the near future. #11452 (comment) also pointed out this issue. This comment also listed ways to configure rustflags.

For now, I'll triage this issue as A-documenting-cargo-itself Area: Cargo's documentation . Like you just said in #12087 (comment), we can improve the doc of build.rustflags a bit. We can state that setting CARGO_ENCODED_RUSTFLAGS or RUSTFLAGS won't set the config value. Instead, we can say they affect build.rustflags.

@weihanglo weihanglo added the S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review label May 6, 2023
@Bhardwaj-Himanshu
Copy link
Contributor

After going through the discussion above, a with very little knowledge of what's going on, I think you guys are talking about changing this-

build.rustflags

    Type: string or array of strings
    Default: none
    Environment: CARGO_BUILD_RUSTFLAGS or CARGO_ENCODED_RUSTFLAGS or RUSTFLAGS

Extra command-line flags to pass to rustc. The value may be an array of strings or a space-separated string.

to-

build.rustflags

    Type: string or array of strings
    Default: none
   --> Environment: CARGO_BUILD_RUSTFLAGS or CARGO_ENCODED_RUSTFLAGS or RUSTFLAGS <-- N-stating this line    

Extra command-line flags to pass to rustc. The value may be an array of strings or a space-separated string.

But Idk what N-stating a line means, so if you could shower some more light on the issue,I would be happy to do the doc work for this issue!
Thanks

@LukeMathWalker
Copy link
Author

LukeMathWalker commented May 7, 2023

I'd like to understand this better:

Since RUSTFLAGS is essentially not a cargo configuration

Are we saying that cargo ignores entirely RUSTFLAGS and it's rustc that picks it up on its own? If that's the case, I agree, this is a documentation issue.
But if that is not the case (i.e. cargo does look at RUSTFLAGS and performs some behaviour according to its value), I think the documentation is correct and this should be treated as a bug in cargo config.

My impression is that the latter is happening.

@weihanglo
Copy link
Member

By configuration I mean Cargo's configuration system specifically, which serializes equivalent CARGO_* environment variables. Cargo has some custom serde rules around that. RUSTFLAGS is one of the additional environment variables that affects rustc invocations. That's why I don't see it as a bug in the "Cargo Configuration System". It is definitely a mess of handling RUSTFLAGS in general.

@LukeMathWalker
Copy link
Author

I see what you mean, but I disagree with the premise of what the "Cargo Configuration System" is.
As a user of the tool, the configuration for cargo is the global set of inputs that influence the behaviour of the tool, regardless of how that translates in the internals of the tool itself. Based on that premise, I do see RUSTFLAGS as a part of cargo's configuration system.

In an "ideal" scenario, the mechanism powering cargo config get would be the same mechanism used by the rest of cargo to fetch configuration values, thus ensuring that nothing is taken into account by other cargo commands that is not reflected by cargo config itself.

I also do understand that the whole situation is fairly messy and it probably requires some design work to be sorted out, and that's OK.

@weihanglo
Copy link
Member

120% agree. Users don't and shouldn't care about what a cargo configuration really is. My suggestion was clarifying that RUSTFLAGS won't reflect in the system but still affect the behavior.

Thinking of it a bit more, we want to answer users what and how rustc got those flags eventually. It could be lots of work to hack the config system doing so. Doc still needs some tweaks but doesn't effectively resolve this issue though.

Thanks for calling out the confusion as an end user!

@weihanglo
Copy link
Member

I got an idea for this. Since rustflags is an important config for Cargo, we could have a separate cargo config rustflags unstable subcommand, showing the unmerged and merged flags with the origins.

This is not yet accepted by the Cargo team but is definitely not a bad idea to me.

@weihanglo weihanglo added E-hard Experience: Hard E-easy Experience: Easy and removed E-hard Experience: Hard labels May 27, 2023
@cnpryer
Copy link
Contributor

cnpryer commented Jul 16, 2023

I got an idea for this. Since rustflags is an important config for Cargo, we could have a separate cargo config rustflags unstable subcommand, showing the unmerged and merged flags with the origins.

This is not yet accepted by the Cargo team but is definitely not a bad idea to me.

Is there any interest in this?

@LukeMathWalker
Copy link
Author

Most definitely—it would be a solution here.

@weihanglo
Copy link
Member

weihanglo commented Jul 16, 2023

To avoid any misleading, I'll relabel this issue for some solution like cargo config rustflags. And the solution might involve lots of different place in the codebase, so it might be a bit not easy.

@rustbot label +S-needs-design -S-accepted +E-hard -E-easy

@rustbot rustbot added E-hard Experience: Hard S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review E-easy Experience: Easy labels Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation A-rustflags Area: rustflags C-bug Category: bug E-hard Experience: Hard S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. Z-cargo-config Nightly: cargo config subcommand
Projects
None yet
Development

No branches or pull requests

5 participants