Conversation
symphorien
force-pushed
the
non-broken-fetchcargo
branch
2 times, most recently
from
4fe2f8f to
b5a1b02
Compare
GrahamcOfBorg
added
6.topic: nixos
GrahamcOfBorg
added
10.rebuild-darwin: 0
There was a problem hiding this comment.
Do we really have an incompatibility compared to the current state? Is the output different for packages that build at the moment? I would like to enable this by default otherwise.
There was a problem hiding this comment.
Otherwise a warning or should be printed otherwise people will never know about this option or bother to migrate.
There was a problem hiding this comment.
The incompatibility is: this changes the expected cargoSha256. So two approaches:
- we use the new scheme forcibly from now on and break every current hash
- or we deprecate the old behavior, let say one nixos release for people to switch to the new behavior, and then remove the old behavior.
This PR implements the second approach.
I agree about the warning.
There was a problem hiding this comment.
So you would not just add this:
[source.crates-io]
registry = 'https://github.com/rust-lang/crates.io-index'
replace-with = 'vendored-sources'
[source.vendored-sources]
directory = '$(pwd)/$cargoDepsCopy'
to keep the checksum of existing packages in tact and then append the new content as required by the Cargo.toml?
There was a problem hiding this comment.
Or is there a problem with this approach?
There was a problem hiding this comment.
the right approach is to remove the config when it only contains crates.io. But this is a great idea indeed. I just pushed a commit which should not break any cargoSha256. It is a mass rebuild though. (it rebuilds cargo)
GrahamcOfBorg
removed
6.topic: nixos
|
Here is a new commit which does not change any cargosha256 and is thus backward compatible. |
symphorien
force-pushed
the
non-broken-fetchcargo
branch
from
0246c0e to
dcec4dd
Compare
GrahamcOfBorg
added
10.rebuild-darwin: 101-500
bkchr
left a comment
bkchr
left a comment
There was a problem hiding this comment.
LGTM, especially that you save the generated config.
There was a problem hiding this comment.
I added type checking here and reformatted the code with black + some minor readability improvements.
There was a problem hiding this comment.
I added this as a test, because it makes use of the new .cargo/config
|
@GrahamcOfBorg build alacritty |
|
Can you rebase this on staging? |
|
Unexpected error: command failed with exit code 1 on x86_64-darwin (full log) Attempted: alacritty Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: alacritty Partial log (click to expand)
|
|
@GrahamcOfBorg build fd |
|
It would be great if someone with macOS could look at the failure |
|
@GrahamcOfBorg build fd |
|
@GrahamcOfBorg build alacritty |
|
too bad, staging will never finish in time. |
|
Success on x86_64-linux (full log) Attempted: fd Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: alacritty Partial log (click to expand)
|
|
@Mic92 alacritty doesn't build on darwin AFAIK. |
|
Timed out, unknown build status on x86_64-darwin (full log) Attempted: fd Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: fd Partial log (click to expand)
|
|
Timed out, unknown build status on x86_64-darwin (full log) Attempted: alacritty Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: alacritty Partial log (click to expand)
|
|
@LnL7 ok, however could you just check if the cargo vendor normalization works for you? ofBorg returned an failure on macOS. Just take any rust package and change the |
Thanks to NixOS#46362 We can now support git dependencies!
| < $reference $out/bin/cargo-vendor-normalise > test; | ||
| cmp test $reference | ||
| ''; | ||
| buildInputs = [ (python3.withPackages(ps: [ ps.toml ])) ]; |
There was a problem hiding this comment.
I just realized we cannot use the results python3.withPackages to replace the shebang of the shell script on macOS, instead we must call the wrapped python executable and pass cargo-vendor-normalise.py as the first argument.
On macOS shebangs must be binaries and cannot be scripts with shebangs itself.
There was a problem hiding this comment.
this is what @infinisil got on his machine: https://hastebin.com/uxihohicax.sql
|
Added a commit that fixes this. Trying an Alacritty build on Darwin with a slightly changed dependency hash leads to: So the hashes are indeed the same :) I cherry-picked these commits to master so I wouldn't have to build all of stdenv (branch) |
|
Alacritty fully builds on the cherry-picked master branch even :) |
|
@infinisil thanks! |
|
So this is the "right way" to wrap python scripts... |
|
Config looked at https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/rust/default.nix#L60 by name |
8 participants
Motivation for this change
This builds on the work of @erictapen #45889
In this comment #45889 (comment) @Mic92 suggested the output of cargo vendor (which is not guaranteed to be stable by upstream) be filtered/normalised to ensure we get a fixed output derivation, somewhat like fetchpatch does.
This is an attempt at implementing it.
What is normalized:
what is not normalised
fixes #41518
Things done
Tested that nix-du can build with
useRealVendorConfig = trueand that I can build the updated habitatfrom this PR #46138 without the cargo config hack they are forced to use.
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"(nothing changes)./result/bin/)nix path-info -Sbefore and after)