Skip to content

Conversation

@TomaSajt
Copy link
Contributor

@TomaSajt TomaSajt commented Jan 7, 2025

Closes #371692

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. labels Jan 7, 2025
@JohnRTitor
Copy link
Member

JohnRTitor commented Jan 7, 2025

CC @jsoo1 @baloo for testing

Also looks like a thousand rebuild, let's just Hydra eat it on master. Most Rust packages are leaf anyway.

@TomaSajt
Copy link
Contributor Author

TomaSajt commented Jan 7, 2025

I wonder which package causes this many rebuilds....

@jsoo1
Copy link
Contributor

jsoo1 commented Jan 7, 2025

Here's a minimal reproducer:

repro
with (import (builtins.fetchTarball {
  url = "https://github.com/NixOS/nixpkgs/archive/0402340d487eef0ac6e2e3989d8a9ec962c380ba.tar.gz";
  sha256 = "sha256:0h42w0gq4zl4195rvsa67c1zsfky4crhp2n24108cxqw79bg73if";
}) { });

let
  src = fetchFromGitHub {
    owner = "jsoo1";
    repo = "cargo-vendor-reproducer";
    rev = "main";
    hash = "sha256-7ubp31NkdfUgOv613ZCq+JzwixtoCPXw5oh7w2evxMo=";
  };
in

rustPlatform.buildRustPackage {
  name = "cargo-vendor-reproducer";
  version = "0.1.0";

  inherit src;

  cargoLock = {
    lockFile = "${src}/Cargo.lock";
    outputHashes."tap-1.0.1" = "sha256-n+jLAQKNTi1S1kJMjTjyvnrPpC378Ohh+R6UyvxUluU=";
  };
}
$ nix build -L -f repro.nix
cargo-vendor-reproducer> Running phase: unpackPhase
cargo-vendor-reproducer> unpacking source archive /nix/store/n90z34sxshlifsxr6yi79p7lz9ikkffa-source
cargo-vendor-reproducer> source root is source
cargo-vendor-reproducer> Executing cargoSetupPostUnpackHook
cargo-vendor-reproducer> Finished cargoSetupPostUnpackHook
cargo-vendor-reproducer> Running phase: patchPhase
cargo-vendor-reproducer> Executing cargoSetupPostPatchHook
cargo-vendor-reproducer> Validating consistency between /private/tmp/nix-build-cargo-vendor-reproducer.drv-0/source/Cargo.lock and /private/tmp/nix-build-cargo-vendor-reproducer.drv-0/cargo-vendor-dir/Cargo.lock
cargo-vendor-reproducer> Finished cargoSetupPostPatchHook
cargo-vendor-reproducer> Running phase: updateAutotoolsGnuConfigScriptsPhase
cargo-vendor-reproducer> Running phase: configurePhase
cargo-vendor-reproducer> Running phase: buildPhase
cargo-vendor-reproducer> Executing cargoBuildHook
cargo-vendor-reproducer> cargoBuildHook flags: -j 12 --target aarch64-apple-darwin --offline --profile release
cargo-vendor-reproducer> error: failed to load source for dependency `tap`
cargo-vendor-reproducer> Caused by:
cargo-vendor-reproducer>   Unable to update https://github.com/jsoo1/tap.git?branch=jsoo1%2Ftap%2Freproducer%2Fnixpkgs%2Fbug#f5315f0f
cargo-vendor-reproducer> Caused by:
cargo-vendor-reproducer>   can't checkout from 'https://github.com/jsoo1/tap.git': you are in the offline mode (--offline)
error: builder for '/nix/store/fw81s68jb82vaa5pfck9vkpdfd6msw21-cargo-vendor-reproducer.drv' failed with exit code 101;
       last 10 log lines:
       > Running phase: buildPhase
       > Executing cargoBuildHook
       > cargoBuildHook flags: -j 12 --target aarch64-apple-darwin --offline --profile release
       > error: failed to load source for dependency `tap`
       >
       > Caused by:
       >   Unable to update https://github.com/jsoo1/tap.git?branch=jsoo1%2Ftap%2Freproducer%2Fnixpkgs%2Fbug#f5315f0f
       >
       > Caused by:
       >   can't checkout from 'https://github.com/jsoo1/tap.git': you are in the offline mode (--offline)
       For full logs, run 'nix log /nix/store/fw81s68jb82vaa5pfck9vkpdfd6msw21-cargo-vendor-reproducer.drv'.

$ cat $(nix build --print-out-paths --no-link -L -f repro.nix cargoDeps)/tap-1.0.1/.cargo-config
[source."https://github.com/jsoo1/tap.git?branch=jsoo1%2Ftap%2Freproducer%2Fnixpkgs%2Fbug"]
git = "https://github.com/jsoo1/tap.git"
branch = "jsoo1%2Ftap%2Freproducer%2Fnixpkgs%2Fbug"
replace-with = "vendored-sources"

And with this PR:

with (import (builtins.fetchTarball {
  url = "https://github.com/TomaSajt/nixpkgs/archive/50c11e3b2e96c487c2c43f127b9e1b05b6f7c9c3.tar.gz";
  sha256 = "sha256:0s1d3p7981iwr5lv7hj4mxma2gh0jakqiinzm74m48ddv816svsf";
}) { });

let
  src = fetchFromGitHub {
    owner = "jsoo1";
    repo = "cargo-vendor-reproducer";
    rev = "main";
    hash = "sha256-7ubp31NkdfUgOv613ZCq+JzwixtoCPXw5oh7w2evxMo=";
  };
in

rustPlatform.buildRustPackage {
  name = "cargo-vendor-reproducer";
  version = "0.1.0";

  inherit src;

  cargoLock = {
    lockFile = "${src}/Cargo.lock";
    outputHashes."tap-1.0.1" = "sha256-n+jLAQKNTi1S1kJMjTjyvnrPpC378Ohh+R6UyvxUluU=";
  };
}

Building this PR does not fix the bug:

$ nix build -L -f repro.nix
cargo-vendor-reproducer> Running phase: unpackPhase
cargo-vendor-reproducer> unpacking source archive /nix/store/n90z34sxshlifsxr6yi79p7lz9ikkffa-source
cargo-vendor-reproducer> source root is source
cargo-vendor-reproducer> Executing cargoSetupPostUnpackHook
cargo-vendor-reproducer> Finished cargoSetupPostUnpackHook
cargo-vendor-reproducer> Running phase: patchPhase
cargo-vendor-reproducer> Executing cargoSetupPostPatchHook
cargo-vendor-reproducer> Validating consistency between /private/tmp/nix-build-cargo-vendor-reproducer.drv-0/source/Cargo.lock and /private/tmp/nix-build-cargo-vendor-reproducer.drv-0/cargo-vendor-dir/Cargo.lock
cargo-vendor-reproducer> Finished cargoSetupPostPatchHook
cargo-vendor-reproducer> Running phase: updateAutotoolsGnuConfigScriptsPhase
cargo-vendor-reproducer> Running phase: configurePhase
cargo-vendor-reproducer> Running phase: buildPhase
cargo-vendor-reproducer> Executing cargoBuildHook
cargo-vendor-reproducer> cargoBuildHook flags: -j 12 --target aarch64-apple-darwin --offline --profile release
cargo-vendor-reproducer> error: failed to load source for dependency `tap`
cargo-vendor-reproducer> Caused by:
cargo-vendor-reproducer>   Unable to update https://github.com/jsoo1/tap.git?branch=jsoo1%2Ftap%2Freproducer%2Fnixpkgs%2Fbug#f5315f0f
cargo-vendor-reproducer> Caused by:
cargo-vendor-reproducer>   can't checkout from 'https://github.com/jsoo1/tap.git': you are in the offline mode (--offline)
error: builder for '/nix/store/fw81s68jb82vaa5pfck9vkpdfd6msw21-cargo-vendor-reproducer.drv' failed with exit code 101;
       last 10 log lines:
       > Running phase: buildPhase
       > Executing cargoBuildHook
       > cargoBuildHook flags: -j 12 --target aarch64-apple-darwin --offline --profile release
       > error: failed to load source for dependency `tap`
       >
       > Caused by:
       >   Unable to update https://github.com/jsoo1/tap.git?branch=jsoo1%2Ftap%2Freproducer%2Fnixpkgs%2Fbug#f5315f0f
       >
       > Caused by:
       >   can't checkout from 'https://github.com/jsoo1/tap.git': you are in the offline mode (--offline)
       For full logs, run 'nix log /nix/store/fw81s68jb82vaa5pfck9vkpdfd6msw21-cargo-vendor-reproducer.drv'.

$ cat $(nix build --no-link --print-out-paths -L -f repro.nix cargoDeps)/tap-1.0.1/.cargo-config
[source."https://github.com/jsoo1/tap.git?branch=jsoo1%2Ftap%2Freproducer%2Fnixpkgs%2Fbug"]
git = "https://github.com/jsoo1/tap.git"
branch = "jsoo1%2Ftap%2Freproducer%2Fnixpkgs%2Fbug"
replace-with = "vendored-sources"

Copy link
Contributor

@jsoo1 jsoo1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be helping something but is not a fix for the original bug (yet).

@TomaSajt
Copy link
Contributor Author

TomaSajt commented Jan 7, 2025

@jsoo1
at this point this PR does not fix importCargoLock (cargoLock = { ... }),
only fetchCargoVendor (useFetchCargoVendor = true; cargoHash = "...";).
I'll be pushing a fix for importCargoLock soon

@TomaSajt TomaSajt changed the title rustPlatform.fetchCargoVendor: support lockfile v4 escaping rustPlatform.{fetchCargoVendor,importCargoLock}: support lockfile v4 escaping Jan 7, 2025
@nix-owners nix-owners bot requested review from figsoda, winterqt and zowoq January 7, 2025 20:17
@winterqt winterqt marked this pull request as draft January 7, 2025 20:25
@winterqt winterqt marked this pull request as ready for review January 7, 2025 20:25
@TomaSajt TomaSajt force-pushed the fetch-cargo-vendor-url-decode branch 3 times, most recently from eee5432 to 9bc5a80 Compare January 7, 2025 21:51
@TomaSajt TomaSajt marked this pull request as draft January 7, 2025 21:58
@github-actions github-actions bot added 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. and removed 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. labels Jan 7, 2025
@TomaSajt TomaSajt force-pushed the fetch-cargo-vendor-url-decode branch from 9bc5a80 to cd18394 Compare January 7, 2025 22:02
@TomaSajt TomaSajt marked this pull request as ready for review January 7, 2025 22:09
Copy link
Member

@baloo baloo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested last commit to fix #371692.

Thank you!

Copy link
Contributor

@jsoo1 jsoo1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also verified with the reproducer from above. The following builds:

with (import (builtins.fetchTarball {
  url = "https://github.com/TomaSajt/nixpkgs/archive/cd183949171cd4eaed4cd19e38bd52b74c36ecbd.tar.gz";
  sha256 = "sha256:0a5csv2ln3bcd84x9afml2hihwa86cs5qfrmimfbcz02wqxlin35";
}) { });

let
  src = fetchFromGitHub {
    owner = "jsoo1";
    repo = "cargo-vendor-reproducer";
    rev = "main";
    hash = "sha256-7ubp31NkdfUgOv613ZCq+JzwixtoCPXw5oh7w2evxMo=";
  };
in

rustPlatform.buildRustPackage {
  name = "cargo-vendor-reproducer";
  version = "0.1.0";

  inherit src;

  cargoLock = {
    lockFile = "${src}/Cargo.lock";
    outputHashes."tap-1.0.1" = "sha256-n+jLAQKNTi1S1kJMjTjyvnrPpC378Ohh+R6UyvxUluU=";
  };
}

Copy link
Member

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6119 rebuild now, welp need staging/staging-next.

@JohnRTitor JohnRTitor marked this pull request as draft January 8, 2025 05:50
@JohnRTitor JohnRTitor force-pushed the fetch-cargo-vendor-url-decode branch from cd18394 to 67a8220 Compare January 8, 2025 05:53
@JohnRTitor JohnRTitor changed the base branch from master to staging-next January 8, 2025 05:54
@JohnRTitor JohnRTitor force-pushed the fetch-cargo-vendor-url-decode branch from 67a8220 to 0bfdb03 Compare January 8, 2025 05:55
@JohnRTitor JohnRTitor marked this pull request as ready for review January 8, 2025 06:03
@JohnRTitor
Copy link
Member

Built a few packages, looks good to me so let's merge.

@JohnRTitor JohnRTitor merged commit 640ab88 into NixOS:staging-next Jan 8, 2025
33 of 37 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Jan 8, 2025

@emilazy
Copy link
Member

emilazy commented Jan 8, 2025

Did this fix anything urgent or unbreak failing builds on -next? In general we don’t merge large rebuilds for things other than fixes for staging-next problems into the branch without coordination in the Matrix room unless it’s early enough in the cycle that it’s not throwing away any builds; see the contributor documentation.

(~6k rebuilds at this stage in the cycle is probably small enough that this isn’t a big problem, to be clear; I just want to check you’re aware of the general procedures here to ward off any future mass‐rebuild problems.)

@JohnRTitor
Copy link
Member

I know staging-next branch is used for fixing Hydra builds. While this fix isn't "so urgent" persay, as more and more packages adopt lockFile version 4, this could have become an issue when updating packages. So I wanted to get this within current staging-next cycle.

Checked the diff, most Rust packages are here just leafs, so I thought we shouldn't worry so much.

@emilazy
Copy link
Member

emilazy commented Jan 8, 2025

We’ve had a lot of occurrences in the past where a problematic number of rebuilds was merged into staging-next to avoid delays. Those rebuilds slow down staging cycles which contributes to delaying other changes, and they often increase the number of regressions that have to be dealt with. That’s why we strengthened the note to ask in the Matrix room before merging any large‐rebuild PR to staging-next that doesn’t fix builds. (FWIW, my understanding is that the Hydra bottlenecks are mostly related to job count rather than build time or number of downstream dependencies.)

I think no harm done in this case but I’m letting you know so that you can coordinate on Matrix for next time. Evaluations of staging-next are only triggered manually too, so not coordinating can result in more wasted builds.

@JohnRTitor
Copy link
Member

Will keep that in mind, thanks.

@TomaSajt TomaSajt deleted the fetch-cargo-vendor-url-decode branch June 19, 2025 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rustPlatform.buildRustPackage: lockFile does not escape git branches as expected

5 participants