cargo-dist
cargo-dist copied to clipboard
Expose setting to set arbitrary environment variables
This PR implements the environment-variables field under [workspace.metadata.dist], allowing arbitrary environment variables to be set during the CI runs.
These are set via the top-level env field in GitHub Actions.
(Kinda) closes #513
If you look at the new snapshot, you might see that the key of the YAML is also quoted, I was thinking about how to get rid of that before remembering that YAML is horrible and actually just allows just about any horribleness you could think of.
And since YAML is quite literally a JSON superset, quoted keys should be 100% a-okay.
So yeah, that should be fine.
for the record, the magic incantation to avoid quotes is |safe:
https://github.com/axodotdev/cargo-dist/blob/9be2d30c4ac6ca58c0aa1cdb8510358121da569b/cargo-dist/templates/ci/github_ci.yml.j2#L345
but yeah we avoid using that unless absolutely necessary because ✨reliability
for the record, the magic incantation to avoid quotes is
|safe
Oh, huh.. I never used minijinja before (or jinja in general), that's definitely good to know
So! while you were gone we just landed a big new prototype feature for "generic" builds with all the gorey details of cflags and whatnot, so cargo-dist is More Than Ever in the business of managing env vars for you. As such, it would be great if this feature was adjusted from "sets in CI" (making it not help with local builds) to "sets it for all cargo-dist builds" (so it will work for local cargo-dist builds). As a plus, no more github_ci.yml.j2 edits for you! All the horrors get to be mine still! 🐱
Just checking, is there any reason why setting these env-vars locally, and not only-in-ci would be an issue for your usecase?
https://github.com/axodotdev/cargo-dist/blob/078d5cb7574a46750434a044bf34a65ee3756060/cargo-dist/src/generic_build.rs#L92-L102
https://github.com/axodotdev/cargo-dist/blob/078d5cb7574a46750434a044bf34a65ee3756060/cargo-dist/src/cargo_build.rs#L33
Just checking, is there any reason why setting these env-vars locally, and not only-in-ci would be an issue for your usecase?
There shouldn't be, no. I was just very laser-focussed on getting it to work the way I patched it into my workflow right now, forgetting about Potential other solutions, heh
Just a small question about the parts you linked: the first linked piece of code builds a command that then invokes cargo-dist again, running the second part of the code, right?
I just wanna make sure I understood correctly (sorry if the question is a little silly, GitHub mobile's Code browsing UX isn't the best.. and it's 4AM for me 🧍♀️)
Just a small question about the parts you linked: the first linked piece of code builds a command that then invokes cargo-dist again, running the second part of the code, right?
Sorry about the delay, also hit by "it's late and weekend". So no, the two linked pieces of code are parallel paths in cargo-dist. The first one is the new "generic build" backend, which exists for invoking things like makefiles. The second one is the original "cargo" back end which is building up an invocation of "cargo build" (and any other tools).
I'll admit it's a bit more complex to merge random env-vars here than set them in the CI itself... hmm I wonder if we can get away with mutating our own process' environment earlier on...
Or are you also looking to set RUSTFLAGS? (And want to set that via the environment rather than via .cargo/config.toml?)
Actually, that was the main motivator behind this PR. cargo-dist sets some RUSTFLAGS (last time I checked at least) via the environment, which then overwrites the flags set in the cargo config (because cargo doesn't to unification in this case).
But cargo-dist preserves the environment variable. Hence this PR that allows me to set the RUSTFLAGS via the environment instead.
Got it! That makes sense. In that case, what about this - on this line: https://github.com/axodotdev/cargo-dist/blob/main/cargo-dist/src/cargo_build.rs#L33
You can add a new check to query these environment variables you've added. In that case, I'd say the priorities should look like this:
- First, prefer the environment (like now)
- Then environment configured via
Cargo.toml - Then the default
Some variant of this would be good for https://github.com/axodotdev/cargo-dist/issues/788
Sorry, I will have a look soon. My time schedule just has been a bit tight lately and I lost track of things
oh haha, this was much more "i dropped the ball and should help you" than vice versa, no worries :)