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

buildRustPackage: introduce --no-merge-sources #48274

Closed
wants to merge 2 commits into from

Conversation

@ljli
Copy link
Contributor

@ljli ljli commented Oct 12, 2018

The update for cargo-vendor is needed to get access to the --no-merge-sources flag.
The other commit just exposes the flag for the builder, with the necessary changes to make it work.
Should close #30742.

The core problem in my understanding is that in the rust packaging world a crate can be referenced
by it's source, name and version. Sources are for example, the crates.io registry or a git repository containing a rust crate. cargo-vendor downloads a dependency closure for a crate to a local directory and while doing so tries to merge it into a single source. Since different sources can contain a crate with the same name and version and need not agree about what it is, this can not always work. This is the source of the found duplicate version error some people encounter when trying to package a crate. --no-merge-sources just keeps crates apart depending on the source. For backwards compatibility reasons (i think) this isn't the default in cargo-vendor and unless we want to fix the hashes for all rust packages using buildRustPackage in nixpkgs we probably do not want to make it the default either.

I choose the names like cargoVendorNoMergeSources rather mechanical and am not opinionated about them.

Can be used like this:

rustPlatform.buildRustPackage {
  # ...
  cargoVendorNoMergeSources = true;
  # ...
}
ljli added 2 commits Oct 12, 2018
Give the option to use cargo-vendor with the flag --no-merge-sources
within the buildRustPackage infrastructure.
@ljli ljli changed the title Cargo vendor multi sources buildRustPackage: introduce --no-merge-sources Oct 12, 2018
@xeji xeji requested a review from Mic92 Oct 12, 2018
@boomshroom
Copy link
Contributor

@boomshroom boomshroom commented Oct 23, 2018

I tried using this to build a project with duplicate dependencies (relibc), but after trying to build the vendored dependencies, the actual output directory doesn't exist.

@ljli
Copy link
Contributor Author

@ljli ljli commented Oct 25, 2018

I had tested it with spotifyd. Can you paste the relevant nix expressions and what exactly you did?

@ljli
Copy link
Contributor Author

@ljli ljli commented Oct 26, 2018

The commit id you reference in my repo is not of this PR but of a previous WIP.

@boomshroom
Copy link
Contributor

@boomshroom boomshroom commented Oct 26, 2018

🤦‍♂️

Now I'm just running into problems with not being able to find ./include.sh even though its hash-bang is /usr/bin/env bash. I can even completely run the build inside a shell, but nix-build just doesn't want to work.

@ljli
Copy link
Contributor Author

@ljli ljli commented Oct 27, 2018

I'm not sure I can follow you, do you attribute any problems to this PR? If so, please give complete reproduction steps.

@boomshroom
Copy link
Contributor

@boomshroom boomshroom commented Oct 27, 2018

No. It's a separate problem.

@@ -42,7 +43,12 @@ stdenv.mkDerivation {
${cargoUpdateHook}
mkdir -p $out
cargo vendor $out | cargo-vendor-normalise > config
${if flagNoMergeSources

This comment has been minimized.

@Mic92

Mic92 Oct 28, 2018
Contributor

Ideally this would not be a flag at all. Because it sounds to me like something we want to have by default.

This comment has been minimized.

@ljli

ljli Oct 28, 2018
Author Contributor

Afaik there is no good reason to merge the packages into a single source. Probably ~80 packages need to get their hash updated in nixpkgs and out-of-tree uses might break, if we use the flag unconditionally, but we could just run cargo-vendor without the flag and if the duplicate version error, occurs re-run it with the flag, that way everything should stay the same.

This comment has been minimized.

@Mic92

Mic92 Oct 28, 2018
Contributor

Would re-running cause cargo-vendor to download all dependencies twice? Otherwise this might be not a bad idea.

cargo vendor $out | cargo-vendor-normalise > config
${if flagNoMergeSources
then ''
cargo vendor --no-merge-sources $out > config

This comment has been minimized.

@Mic92

Mic92 Oct 28, 2018
Contributor

Is this incompatible with our normalize script?

This comment has been minimized.

@ljli

ljli Oct 28, 2018
Author Contributor

Currently it is incompatible. Is there any reason to believe the formatting details will change, other than that they don't guarantee it won't? Did we actually observe this? It shouldn't be a problem to adapt the script though.

This comment has been minimized.

@Mic92

Mic92 Oct 28, 2018
Contributor

We talked to the author of cargo-vendor and there is no guarantee that the output will stay the same after updating cargo-vendor. By having the script we can make sure it will or we can create workarounds.

@sjmackenzie
Copy link
Contributor

@sjmackenzie sjmackenzie commented May 5, 2019

Is this going in? Getting this error now.

@marsam
Copy link
Contributor

@marsam marsam commented Jan 10, 2020

Closing because it has become outdated. cargo-vendor was integrated into Cargo 1.37 and removed the flag --no-merge-sources rust-lang/cargo#6869

@marsam marsam closed this Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants
You can’t perform that action at this time.