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.

Sign up for GitHub

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

Jump to bottom

RUSTFLAGS via cargo config #513

Open
aumetra opened this issue Oct 25, 2023 · 4 comments · May be fixed by #574
Open

RUSTFLAGS via cargo config #513

aumetra opened this issue Oct 25, 2023 · 4 comments · May be fixed by #574
Labels
feature request New feature or request

Comments

@aumetra
Copy link
Contributor

aumetra commented Oct 25, 2023

I tried to integrate cargo-dist into my release workflow and hit a roadblock in form of my required rustflags.

In my cargo config I set a rustflag to compile the uuid crate with unstable features to be able to use UUIDv7.

cargo-dist's usage of the environment variable completely overwrites the cargo configuration though.

Right now I'm resorting to allowing a dirty CI state and adding the RUSTFLAGS env-var with the appropriate value, since cargo-dist preserves those.

I personally see two possible solutions to this:

  1. cargo-dist parses the cargo config and appends any rustflags to the environment variable it sets
  2. cargo-dist adds the ability to add arbitrary environment variables

Both of these would be nice features tbh
@ashleygwilliams ashleygwilliams added the feature request New feature or request label Oct 25, 2023
@Gankra
Copy link
Member

Gankra commented Oct 25, 2023

The offending code, with comment giving some context:

cargo-dist/cargo-dist/src/tasks.rs

Lines 1960 to 1969 in 1fe2cb9

let mut rustflags = std::env::var("RUSTFLAGS").unwrap_or_default();
// FIXME: is there a more principled way for us to add things to RUSTFLAGS
// without breaking everything. Cargo has some builtin ways like keys
// in [target...] tables that will get "merged" with the flags it wants
// to set. More blunt approaches like actually setting the environment
// variable I think can result in overwriting flags other places set
// (which is defensible, having spaghetti flags randomly injected by
// a dozen different tools is a build maintenance nightmare!)

I hadn't considered straight up reading the flags out of Cargo's config, that's an interesting idea... (wonder if it's exposed in guppy/cargo-metadata already...)

@Gankra
Copy link
Member

Gankra commented Oct 25, 2023

Adding arbitrary env vars is also probably just the more useful/general/simple feature tho

@aumetra
Copy link
Contributor Author

aumetra commented Oct 27, 2023

I would take the issue for arbitrary env vars, if that's fine

@Gankra
Copy link
Member

Gankra commented Oct 30, 2023

@aumetra absolutely!

@aumetra aumetra linked a pull request Nov 17, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Expose setting to set arbitrary environment variables aumetra/cargo-dist
3 participants
@Gankra @ashleygwilliams @aumetra