-
-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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.
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
cargo-update: init at 1.5.2 #40575
cargo-update: init at 1.5.2 #40575
Conversation
@GrahamcOfBorg build cargo-update |
Failure on x86_64-darwin (full log) Attempted: cargo-update Partial log (click to expand)
|
Looks like you need to add curl as a dependency. |
Success on x86_64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
Actually that's probably not needed. I think we need to make curl propagate in cargo: https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/rust/default-crate-overrides.nix#L22 /cc @Mic92 |
Failure on aarch64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
What and where should I do this? |
@@ -0,0 +1,27 @@ | |||
{ stdenv, defaultCrateOverrides, fetchFromGitHub, cmake, libssh2, libgit2, openssl, zlib }: | |||
|
|||
((import ./cargo-update.nix).cargo_update {}).override { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look right. Why not use rustPlatform.buildRustPackage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in the PR description: I need to use carnix
because rustPlatform.buildRustPackage
depends on having an up-to-date Cargo.lock
at the root of the src
, but in the repo is no Cargo.lock
checked in.
@GrahamcOfBorg build cargo-update |
Failure on x86_64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
Failure on x86_64-darwin (full log) Attempted: cargo-update Partial log (click to expand)
|
Failure on aarch64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
@P-E-Meunier do you have any idea? you seem very involved in carnix in nixpkgs |
As mentioned |
f845b77
to
3989742
Compare
Actually curl-sys needs an entry in default-crate-overrides.nix. You can find plenty of examples already in that file. |
Actually there is already an entry for |
3989742
to
75561ad
Compare
Yes. It also has |
@GrahamcOfBorg build cargo-update |
Failure on x86_64-darwin (full log) Attempted: cargo-update Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
@@ -19,8 +19,7 @@ in | |||
crateBin = [ { name = "cargo-vendor"; path = "src/main.rs"; } ]; | |||
}; | |||
curl-sys = attrs: { | |||
buildInputs = [ pkgconfig zlib ]; | |||
propagatedBuiltInputs = [ curl ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this changed? It might fix the darwin build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @matthewbauer who added this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That wasn't on purpose, fixed it
Success on aarch64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
75561ad
to
ccea395
Compare
Wait @Gerschtli, this is going to conflict with #40587. Why not wait until that one is accepted? |
Sure I can revert this change, but we can now test if this change will fix the builds, just to be save |
ccea395
to
4222a7d
Compare
0471786
to
dd0576b
Compare
#40808 got merged and includes the fix. |
What is currently preventing, that this PR gets merged? |
@matthewbauer @Mic92 @P-E-Meunier : What is currently preventing, that this PR gets merged? |
Whats the status of this PR? |
ping |
@GrahamcOfBorg build cargo-update |
Failure on x86_64-darwin (full log) Attempted: cargo-update Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
dd0576b
to
4e348c1
Compare
Just rebased it to current master, but I can't see the cause of this build failure on darwin, because curl is via |
@GrahamcOfBorg build cargo-update |
Success on x86_64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: cargo-update Partial log (click to expand)
|
Failure on x86_64-darwin (full log) Attempted: cargo-update Partial log (click to expand)
|
Let's merge this and fix darwin later. |
Motivation for this change
Adds cargo-update at version 1.5.2 with
carnix
because of missingCargo.lock
in repository.Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)