cargo-chef icon indicating copy to clipboard operation
cargo-chef copied to clipboard

package <packageName> is specified twice in the lockfile

Open mladedav opened this issue 2 years ago • 9 comments
trafficstars

This is possibly the same issue as #179

We have a project with a workspace with crate X. One of the projects depends on an older version of X for compatibility (so it uses local X as well as an older version from crates).

When I run cargo chef cook, the lock file gets rewritten and both packages get their versions changed to 0.0.1 so cargo then complains that the same package is there twice.

I think the correct thing to do here would be to check not only the name but also the version of a package before changing it in Cargo.lock. I think that should be sufficient since there can not be the same package with multiple versions (e.g. one local one from crates).

mladedav avatar Jun 30 '23 06:06 mladedav

First of all: what version of cargo-chef are you using?

Are both of those packages local packages (i.e. path dependencies or vendored)?

LukeMathWalker avatar Jun 30 '23 07:06 LukeMathWalker

Sorry for omitting that, cargo-chef-chef 0.1.61, rust stable 1.69.0

I have:

When I skimmed the code I think that the issue is because here the code checks just for the crate name and so it masks the version of both instances of X. I think it should mask only the latter.

mladedav avatar Jun 30 '23 07:06 mladedav

As you can see in the snippet you linked, that only applies to local crates, so it should not touch your [email protected] if it's pulled from crates.io. Can you share a reproducible example of the failure?

LukeMathWalker avatar Jun 30 '23 07:06 LukeMathWalker

I might be missing something, but to me it seems that the code I linked checks only the name. So if I have a local and remote crate with the same name, both will be considered local by that function.

I don't have anything I could share right now, I'll try to create something later today.

mladedav avatar Jun 30 '23 07:06 mladedav

It does only look at the name, but it's just invoked on local manifests.

LukeMathWalker avatar Jun 30 '23 09:06 LukeMathWalker

It changes the version for all packages that have the same name as one with a local manifest. The issue is that the same package can be used with multiple versions where one may be local and the other may be pulled from a repository. It then changes versions in the lockfile for packages that are not local.

I've tried to create a minimal example and although I think it is the same issue, it seems to manifest with a different build error. Here is the repo.

The error is different, but both versions of the log crate are overwritten in the lockfile, while I believe only the local one should be (only version 1.0.0). I've used the log crate simply because it's the first one I've found that has no dependencies for simplicity.

I've also noticed that when the dependencies use versions to distinguish multiple versions of the same crate the versions there are not replaced:

[[package]]
name = "binary"
version = "0.0.1"
dependencies = ["log 0.4.19", "log 1.0.0"]

mladedav avatar Jun 30 '23 21:06 mladedav

OK, I understand now the issue—thanks a lot for the reproduction!

LukeMathWalker avatar Jul 01 '23 12:07 LukeMathWalker

I've also tried to create a fix here, however I'm pretty unhappy with the code as it is so I didn't open PR yet. Aside from the obvious things like some unwraps, I'm not sure this is the best way to go about this. I'd be happy to finish it if this is the way you think it should go or if I should check another way to provide already parsed information to the function changing the versions.

The main issue I've encountered is with versions from workspaces, because in mask_local_crate_versions we have only parsed manifests which may contain workspace = true instead of dependency version, but at that point we don't have the workspace package readily available.

I think the manifests could be filled with workspace information when reading them, or the workspace package could be sent in as a separate argument. I don't

Also is there a reason local dependencies are searched for by their name (or possibly name and version) instead of checking whether the dependency toml object contains path? Alternatively in Cargo.lock by checking for omission of source. The only issue I can think of is the mentioned dependencies key inside a package in Cargo.lock where the dependencies are still specified only by name if this is not ambiguous, or by name x.y.z like log 0.4.19 in case version is needed to distinguish. This is further complicated by the fact that a Cargo.lock can contain two dependencies in the same version from different sources (example in different branch in the repro repo) where even the source should be checked, but if it is local, it seems it is omitted again.

mladedav avatar Jul 01 '23 16:07 mladedav

@LukeMathWalker What do you think about the possibility of rewriting the manifests where there is workspace = true to dependencies with actual versions? Would there be a problem with discarding the information that it has been inherited?

mladedav avatar Jul 19 '23 15:07 mladedav