rustPlatform.fetchCargoTarball: support pname+version #332975
Conversation
eclairevoyant
force-pushed
the
fetchcargotarball-pname
branch
from
32e4fb2
to
28dc9f4
Compare
eclairevoyant
changed the title
eclairevoyant
force-pushed
the
fetchcargotarball-pname
branch
from
28dc9f4
to
cb33492
Compare
ofborg
bot
added
8.has: clean-up
10.rebuild-darwin: 1-10
10.rebuild-linux: 11-100
labels
eclairevoyant
force-pushed
the
fetchcargotarball-pname
branch
from
cb33492
to
b37eefd
Compare
|
Funnily enough, it seem like there's several packages that already assumed that Someone actually almost noticed this in |
eclairevoyant
force-pushed
the
fetchcargotarball-pname
branch
from
5870d93
to
e878dfe
Compare
|
Okay, this now has one rebuild (the nixpkgs manual) which makes sense and I can live with. Result of 1 package built:
|
ofborg
bot
added
10.rebuild-darwin: 0
10.rebuild-linux: 1-10
10.rebuild-linux: 1
and removed
10.rebuild-darwin: 1-10
10.rebuild-linux: 11-100
labels
eclairevoyant
changed the title
|
Thanks for agreeing borgo |
|
It's certainly possible to follow up with a treewide in another PR, but I didn't want to balloon the scope here too much, or make it harder to review or merge without conflicts. I'm sure there are many packages that don't set |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/4496 |
|
|
||
| inherit (hash_) outputHashAlgo outputHash; | ||
|
|
||
| impureEnvVars = lib.fetchers.proxyImpureEnvVars ++ [ "NIX_CRATES_INDEX" ]; |
There was a problem hiding this comment.
Future: document this environment variable somewhere. Maybe in the Rust section of the nixpkgs-manual?
| cat >$CARGO_HOME/config.toml <<EOF | ||
| [source.crates-io] | ||
| replace-with = 'mirror' | ||
| [source.mirror] | ||
| registry = "$NIX_CRATES_INDEX" | ||
| EOF |
There was a problem hiding this comment.
Needs one more level of indent.
There was a problem hiding this comment.
The indentation error is present in the base branch. , but sure, I can fix it
Actually if I indent this then the contents of the toml file will be indented.
The only line that's indentable is L93 but that might look even more confusing 🫤
There was a problem hiding this comment.
Wow, load-bearing odd indentation. Thanks Bash.
| echo | ||
| echo "ERROR: The Cargo.lock file doesn't exist" | ||
| echo | ||
| echo "Cargo.lock is needed to make sure that cargoHash/cargoSha256 doesn't change" | ||
| echo "when the registry is updated." | ||
| echo | ||
|
|
||
| exit 1 |
There was a problem hiding this comment.
In Nix files, I do like my shell to have two-space indent. .editorconfig says four, but @emilazy and I might connive to effect a two-space-coup on shell.
There was a problem hiding this comment.
If you hide whitespace (https://github.com/NixOS/nixpkgs/pull/332975/files?diff=unified&w=1) you'll see I didn't substantively touch these lines at all. These lines are indented in the diff, but only by the same amount that the surroundings are indented (and was done by nixfmt, not manually).
There was a problem hiding this comment.
Oh yes, this is just me noticing formatting due to, well, the formatting being impacted with nixfmt.
| @@ -26,7 +26,7 @@ stdenv.mkDerivation (finalAttrs: { | |||
| }; | |||
|
|
|||
| cargoDeps = rustPlatform.fetchCargoTarball { | |||
| inherit (finalAttrs) pname version src; | |||
| inherit (finalAttrs) src; | |||
There was a problem hiding this comment.
According to the PR headmatter, this is removed in order to not cause hash mismatches.
eclairevoyant
force-pushed
the
fetchcargotarball-pname
branch
from
e878dfe
to
5ff3fef
Compare
| cat >$CARGO_HOME/config.toml <<EOF | ||
| [source.crates-io] | ||
| replace-with = 'mirror' | ||
| [source.mirror] | ||
| registry = "$NIX_CRATES_INDEX" | ||
| EOF |
There was a problem hiding this comment.
Wow, load-bearing odd indentation. Thanks Bash.
eclairevoyant
force-pushed
the
fetchcargotarball-pname
branch
from
5ff3fef
to
0278111
Compare
This is done because the existing fetchCargoTarball does not support pname or version, resulting in a vendor tarball with the name cargo-deps-vendor.tar.gz. Since adding pname+version support would change the name of the derivation, and therefore its hash, we remove existing usages to avoid treewide breakage.
eclairevoyant
force-pushed
the
fetchcargotarball-pname
branch
from
0278111
to
e273bc8
Compare
Description of changes
Ths should result in exactly one rebuild (the manual).
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.