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

Tracking Issue for cargo config #9301

Open
13 tasks
ehuss opened this issue Mar 25, 2021 · 6 comments
Open
13 tasks

Tracking Issue for cargo config #9301

ehuss opened this issue Mar 25, 2021 · 6 comments
Labels
A-configuration Area: cargo config files and env vars C-tracking-issue Category: A tracking issue for something unstable. S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. Z-cargo-config Nightly: cargo config subcommand

Comments

@ehuss
Copy link
Contributor

ehuss commented Mar 25, 2021

Summary

Original issue: #2362
Implementation: #9302
Design doc: https://hackmd.io/@ehuss/rkjp32Fg_
Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#cargo-config
Issues: Z-cargo-config Nightly: cargo config subcommand

cargo config is a new subcommand for displaying config values.

Unresolved issues

  • The way environment variables are handled is awkward. When querying a specific value, it can read the environment variable. But when displaying maps, it cannot. This is due to the lazy nature of how environment variables are loaded. This may be impossible to solve (due to the case and dash/underscore translations). Is the way it is displayed reasonable? TOML displays all CARGO_* environment variables as comments, and JSON just prints a note after the first line on stderr (which hopefully shouldn't interfere with parsing).
  • A primary use case for this feature is to collect information in issue reports to help diagnose an issue. However, by default cargo config get also includes sensitive information like token. Should there be some way to redact or remove sensitive things?
  • Determine how to display manpages for nested subcommands.

Future extensions

  • set and delete subcommands. Needs a comment-preserving toml editor.
  • edit subcommand which launches an editor. There are some decisions to be made on how it decides which file to edit.
  • Display warnings for unknown keys. This would require having a global registry/schema of the config structure.
  • Quoted keys aren't handled as CLI input. For example, config get 'target."cfg(target_os=\"linux\")".runner' Unfortunately toml doesn't expose the parsing functions to make this easy. Could do from_str("{} = 1", key), and then collect all the key components. I wouldn't want that in the normal Config.get() path, though, for performance. I think it is fine to stabilize without this, since it can be added later, and is probably not a use case that I can imagine anyone needing.
  • Other formats mentioned in the design doc: toml-value, toml-table, sh-value. The only one that seems useful to me is sh-value, but I'm uncertain how that could handle maps (maybe just error in that case?).
  • --show-origin and --merged=no only supports TOML. I don't know how JSON could display either of those, and I don't think it is important.
  • The design doc mentioned a --includes=no flag to disable include handling. I'm not sure if that is really necessary. The --merged=no flag will show includes and where values are coming from.
  • Support displaying the "default" of values. This would require a global registry/schema. This also may be difficult, since TOML doesn't have a way to display none/null/unset, and some defaults are complicated.
  • cargo config does not fetch values from Cargo.toml. This could be confusing for things like profiles. There could be a flag like --merge-manifest or --manifest=Cargo.toml to support that?
  • Some config values are complex like rustflags, which is merged across multiple config values and environment variables. naively fetching build.rustflags won't necessarily get the flags cargo actually uses. Maybe config get could have special "computed" keys that take this into consideration? (see design doc)

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

@coolreader18
Copy link

This isn't actually a backwards-compatible change - the old cargo-config is now completely overriden, even when -Zunstable-options isn't passed.

tmplt added a commit to rtic-scope/cargo-rtic-scope that referenced this issue Jun 16, 2021
@ehuss ehuss added the Z-cargo-config Nightly: cargo config subcommand label Oct 22, 2021
bors bot added a commit to intellij-rust/intellij-rust that referenced this issue Apr 29, 2022
8494: CARGO: Retrieve config information r=vlad20012 a=avrong

Adds the ability to get config properties using `cargo config get` subcommand tracked in rust-lang/cargo#2362. It can be improved in the future as rust-lang/cargo#9301 features will be implemented.

Currently, it retrieves the config values of a specified path and parses them into a tree. Methods like `getBuildTarget` and `getEnvParams` return specific structured data

Co-authored-by: vlad20012 <[email protected]>
@ehuss ehuss added S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. labels Apr 25, 2023
@epage epage moved this to Unstable, baking in Cargo Roadmap Sep 5, 2023
@RalfJung
Copy link
Member

It looks like there currently is no way to distinguish "this config key does not exist" from other errors like non-existing manifest files and things like that. Are there plans to improve that?

@ehuss
Copy link
Contributor Author

ehuss commented Oct 27, 2023

Sorry, I don't understand the question. cargo config does not read manifest files, and should work fine without them. Can you clarify what the exact scenario you are looking at, the command you are running, and your use case?

@RalfJung
Copy link
Member

This came up in rust-lang/miri#3078. We want to check whether build-std is set. However when build-std is not set, cargo fails with error code 101 -- which is very non-specific and could also indicate all sorts of other problems. (Manifest file was a bad example though it seems.) Ideally we could differentiate "build-std not set" from other things that could have gone wrong.

@bukowa
Copy link

bukowa commented Jun 24, 2024

Design Doc link shows 404

I think simple config show --json that would allow piping to jq can be a good idea.

@ehuss
Copy link
Contributor Author

ehuss commented Jun 24, 2024

Thanks, the link is now fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars C-tracking-issue Category: A tracking issue for something unstable. S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. Z-cargo-config Nightly: cargo config subcommand
Projects
Status: Unstable, baking
Development

No branches or pull requests

4 participants