rust: Write config.toml not config #321095
Conversation
Seeing the following new warnings pop up on stderr when cargo was bumped to 1.78: ``` warning: `/build/.cargo/config` is deprecated in favor of `config.toml` note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml` ``` which happens to break commitmsgfmt builds in nix (NixOS#320294). closes NixOS#320294
|
Tried running nixpkgs-review but that failed with merge conflicts :/ |
wegank
added
the
12.approvals: 1
|
It feels like this change caused cargoDeps hashes to be invalidated. Why was this merged without a review from a codeowner? |
|
My bad, should I provide a revert ? |
|
For codeowners to decide. |
|
Yes, this was irresponsible to merge in its current state, and I'm sorry I didn't see it in my inbox. For now, let's just revert the whole thing to avoid any breakages making it to master. |
|
My apologies for the extra work. |
|
We should be able to pick all the changes except the one to |
|
We can provide a jobset on hydra.nixos.org for the |
|
Maybe moving it (config -> config.toml) in |
|
Any change to the output of the FODs will require hash changes, which is why I said we should first just change everything but the FODs. |
| @@ -22,17 +22,17 @@ cargoSetupPostUnpackHook() { | |||
| mkdir .cargo | |||
| fi | |||
|
|
|||
| config="$cargoDepsCopy/.cargo/config"; | |||
| config="$cargoDepsCopy/.cargo/config.toml"; | |||
There was a problem hiding this comment.
| config="$cargoDepsCopy/.cargo/config.toml"; | |
| mv "$cargoDepsCopy/.cargo/config" "$cargoDepsCopy/.cargo/config.toml" | |
| config="$cargoDepsCopy/.cargo/config.toml"; |
I actually mean something like this, i.e. not moving it in the FOD output, but moving it after the FOD is unpacked and copied and made writable
There was a problem hiding this comment.
Actually this wasn't the problem, the patch that caused FOD changes was the fetch-cargo-tarball one!
|
Bummer, I would have expected ofborg or something to catch this. I would actually prefer dealing with the hash change instead of layering fixes/hacks. Especially if upstream rust is doing this then it would be easier to avoid papering over things like this. |
|
Nix FODs aren't really designed for hashes to ever change — even if we fix all the hashes in Nixpkgs, we silently break all out of tree FODs, which means most out of tree Cargo hashes. Justifying a FOD hash change is an extremely high bar — there's a reason we have fetchpatch2. (Generally introducing a new API with new hashes like that has fewer negative consequences.) |
|
Hmm I'm confused about the revert though, I didn't have any need to update the cargo hash in commitmsgmft and had successfull build/test there. |
|
Did you try busting the hash with |
This is exactly why FOD hash changes are so problematic — Nix won't ever rebuild a FOD it's built before, so you don't get to find out your FODs are broken for other people unless they tell you. |
Ah nope.
Yep it makes sense now. I was only having issues with commitmsgmft tests. I did end up rebuilding icedtea-web iirc and that ended up rebuilding Firefox and a bunch of other stuff, but I suppose that is the same situation... |
|
I'll give this another go withess disruptive changes then :D |
Does it apply here? A "fetchCargo2"? |
|
We could, although I'm not sure it's worth it just for this… |
Description of changes
Changes the rust infra to write cargo config to .cargo/config.toml insted of .cargo/config since the latter is deprecated (since 1.38?) and is now warning on stderr.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.