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

environment variables are not parsed correctly #14381

Open
GuillaumeGomez opened this issue Aug 9, 2024 · 8 comments · May be fixed by #14376
Open

environment variables are not parsed correctly #14381

GuillaumeGomez opened this issue Aug 9, 2024 · 8 comments · May be fixed by #14376
Labels
A-environment-variables Area: environment variables A-rustflags Area: rustflags C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.

Comments

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Aug 9, 2024

Problem

RUSTFLAGS="--output '/home/somewhere/out file'" cargo doc

With this command, the --output command will receive /home/somewhere/out and not /home/somewhere/out file, which is problematic

Steps

Add any flag with whitespace characters to environment variables.

Possible Solution(s)

Correctly parse arguments passed through environment variables.

Notes

No response

Version

No response

@GuillaumeGomez GuillaumeGomez added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Aug 9, 2024
@weihanglo
Copy link
Member

While not too convenient, can you use CARGO_ENCODED_RUSTFLAGS?

@GuillaumeGomez
Copy link
Member Author

I didn't know about CARGO_ENCODED_RUSTFLAGS and CARGO_ENCODED_RUSTDOCFLAGS. Why is it not the default behaviour?

@weihanglo
Copy link
Member

I was not there and have no relevant context. CARGO_ENCODED_RUSTDOCFLAGS seems to be added because of the lossy RUSTFLAGS issue was found during a discussion of an unrelated issue.

@weihanglo weihanglo added A-environment-variables Area: environment variables A-rustflags Area: rustflags labels Aug 9, 2024
@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Aug 9, 2024

Seems like I just stepped into quite a mess. 😆

@ehuss ehuss added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` and removed C-bug Category: bug labels Aug 10, 2024
@ehuss
Copy link
Contributor

ehuss commented Aug 10, 2024

Just wanted to note that we generally wanted to avoid any parsing of RUSTFLAGS. Over the years, we have introduced several alternatives (some documented here).

Another alternative is to support TOML instead of shlex as suggested here.

Although not a strong concern, I think it is worth being wary that shell quoting styles are not universal. The shlex docs themselves don't document the exact grammar that it supports.

I'm also not convinced that it wouldn't be a breaking change to change the syntax for RUSTFLAGS.

I am slightly inclined to close this as "won't fix" since there are several alternatives that already exist.

@GuillaumeGomez
Copy link
Member Author

We could specify which quotes are supported and what would be the end result. Because currently, this is an issue you discover randomly, which is pretty bad.

@workingjubilee
Copy link
Member

With this command, the --output command will receive /home/somewhere/out and not /home/somewhere/out, which is problematic

Er, you may wish to amend your report given that currently you are technically reporting that "/home/somewhere/out" is a different string from `"/home/somewhere/out", which... is not the case and also not, I think, what you wanted to report.

@GuillaumeGomez
Copy link
Member Author

Oh indeed. I updated the issue. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-environment-variables Area: environment variables A-rustflags Area: rustflags C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants