Skip to content
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

nix build: replace fetchCargoTarball with importCargoLock #31825

Merged
merged 1 commit into from Mar 25, 2024

Conversation

mukilan
Copy link
Member

@mukilan mukilan commented Mar 22, 2024

importCargoLock allows us to use the existing Cargo.lock file. This means we no longer need to update the sha256 hash whenever the dependencies are upgraded. It also integrates with nix's rustToolchain support via cargoSetupHooks and automatically vendors the dependencies, allowing us to simplify the logic for filterlock derivation.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because the only modify files related to build support for Nix.

[`importCargoLock`][1] allows us to use the existing Cargo.lock
file. This means we no longer need to update the sha256 hash
whenever the dependencies are upgraded. It also integrates
with nix's rustToolchain support via `cargoSetupHooks` and
automatically vendors the dependencies, allowing us to simplify
the logic for `filterlock` derivation.

[1]: https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/rust.section.md#vendoring-of-dependencies-vendoring-of-dependencies

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
@mukilan mukilan requested a review from delan March 22, 2024 13:33
@sagudev
Copy link
Member

sagudev commented Mar 22, 2024

This is also written on provided link:

If a Cargo.lock file is available, you can alternatively use the importCargoLock function. In contrast to fetchCargoTarball, this function does not require a hash (unless git dependencies are used)

but this might not apply to crown?

@mukilan
Copy link
Member Author

mukilan commented Mar 23, 2024

@sagudev, I'm not sure I follow, are you suggesting that we use importCargoLock for crown as well? IIUC, buildRustPackage internally uses importCargoLock , passing it the lockFileContents. The reason we don't directly pass the lockfile is because we don't want all Servo's dependencies ending up in nix store.

Let me know if I misunderstood your comment.

@sagudev
Copy link
Member

sagudev commented Mar 23, 2024

I was just asking, but now I see that hash was only used for filterlock, while I thought it was used for crown. My bad.

Copy link
Sponsor Member

@delan delan left a comment

Choose a reason for hiding this comment

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

Thanks!

@delan delan added this pull request to the merge queue Mar 25, 2024
Merged via the queue into servo:main with commit c50df5c Mar 25, 2024
9 checks passed
ektuu pushed a commit to ektuu/servo that referenced this pull request Mar 25, 2024
…31825)

[`importCargoLock`][1] allows us to use the existing Cargo.lock
file. This means we no longer need to update the sha256 hash
whenever the dependencies are upgraded. It also integrates
with nix's rustToolchain support via `cargoSetupHooks` and
automatically vendors the dependencies, allowing us to simplify
the logic for `filterlock` derivation.

[1]: https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/rust.section.md#vendoring-of-dependencies-vendoring-of-dependencies

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
ektuu pushed a commit to ektuu/servo that referenced this pull request Mar 28, 2024
…31825)

[`importCargoLock`][1] allows us to use the existing Cargo.lock
file. This means we no longer need to update the sha256 hash
whenever the dependencies are upgraded. It also integrates
with nix's rustToolchain support via `cargoSetupHooks` and
automatically vendors the dependencies, allowing us to simplify
the logic for `filterlock` derivation.

[1]: https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/rust.section.md#vendoring-of-dependencies-vendoring-of-dependencies

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants