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

git dependency revs do not affect crate resolution as I might expect #6921

Open
brson opened this issue May 9, 2019 · 9 comments
Open

git dependency revs do not affect crate resolution as I might expect #6921

brson opened this issue May 9, 2019 · 9 comments
Labels
A-git Area: anything dealing with git C-enhancement Category: enhancement S-needs-rfc Status: Needs an RFC to make progress.

Comments

@brson
Copy link
Contributor

brson commented May 9, 2019

TiKV is a workspace consisting of a number of crates. Many of those crates depend on other crates that are only available as git dependencies, and those git deps are depended on by multiple crates in the workspace.

e.g. tikv the library has a tipb = { git = "https://github.com/pingcap/tipb.git" } dependency, and so does the cop_datatype crate in the same workspace.

There are many instances of this in the workspace. Each git dep must be resolved to an exact rev or the build will break. Today those revs are specified only in the workspace lockfile and not in the manifests, where they should be (if someone were to completely regenerate the lockfile or use tikv or any of its libraries as libraries the build would fail).

In trying to move the git revs into the manifest it is clear that rev must be specified for every crate that depends on it. I had hoped that specifying the rev in a single place would "narrow" the selection of every instance of that git dep to the same revision, but instead we see multiple versions of the git deps being selected.

That is I would like to write in the tikv manifest tipb = { git = "https://github.com/pingcap/tipb.git", rev = "abdce" }, while leaving all the other manifests tipb = { git = "https://github.com/pingcap/tipb.git" } and have them resolve to the same crate.

So the end result is that every crate in the workspace that shares a git dep must independently specify the same revision, and so far there is no appetite to do that in the project.

(On the other hand, I can easily see how every crate should specify the version it needs since every crate can be published and used independently. But still, it would be more palatable for my present use case if "no rev" was like "rev=*" and all the identical git repos resolved to the most specific).

We also tried using [patch] for this purpose, which doesn't seem quite what it's intended for, and ran into #5478. So right now it looks to me that if we want to move the "rev" requirement out of the lockfile we must duplicate the rev in every project with the same git dep.

So I'm sure "rev" will ever work this way, but I am raising the issue to see if there are any suggestions how to specify the rev in a single place outside of the lockfile.

@brson brson added the C-bug Category: bug label May 9, 2019
@brson
Copy link
Contributor Author

brson commented May 9, 2019

cc tikv/tikv#4283

@alexcrichton
Copy link
Member

This is currently intended behavior in that Cargo treats a bare url and a url plus a revision as two distinct sources, compiling them as different crates instead of unifying them. I think the best solution for this would be something like #5471 where you only specify the dependency once in your workspace Cargo.toml and then all other crates would have something like tipb = { whatever-the-workspace-says = true }.

This is also a bummer today for crates.io deps when you have to bump the version requirement everywhere, so it's definitely not the first time this has come up!

@ehuss ehuss added A-git Area: anything dealing with git and removed C-bug Category: bug labels May 31, 2019
@kvark
Copy link

kvark commented Jan 22, 2021

This is very much a problem for gfx-rs ecosystem as well. All of gfx, wgpu, and wgpu-rs, want to depend on Naga. During development, we want to have the same git revision as a dependency, and updating it all across the crates is just inconvenient. We came up with a process where we tag the revisions meant to be propagated (and use a shorter tag instead of a revision), but it's still pretty sad.

@alexcrichton does this behavior have to be the intended one? My mental model/expectation was different: that there is a git source, and different revisions are just considered breaking/uncompatible by Cargo, exactly the same way as crates breaking revisions are incompatible. Why does cargo have to treat the bare git url with different revisions as a distinct sources?

@alexcrichton
Copy link
Member

@kvark this is baked so deeply in Cargo it would be a pretty large breaking change to alter this. Can you comment about whether my previous proposed solution, that is rust-lang/rfcs#2906, would solve this issue for you?

@kvark
Copy link

kvark commented Jan 22, 2021

@alexcrichton I skimmed through it, and I don't think it solves our case. For us, it's not a single workspace. gfx, wgpu, wgpu-rs - all separate repositories with their own workspaces. What I want to have, ideally, is the following:

  • gfx depends on naga rev=XXX
  • wgpu depends on gfx rev=YYY, also depends on naga, but without revision. Every time the gfx revision YYY is changed, I want Cargo to merge the "rev=XXX" from gfx and "*" from wgpu, making all things depend on rev=XXX.
  • similarly, wgpu-rs depends on wgpu rev=ZZZ, also depends on naga without exact revision.

@alexcrichton
Copy link
Member

Ah ok yeah if they're in separate repositories/workspaces this doesn't help. I would recommend sticking with tags in that case for now or something similar. Either that or if you'd like you can write an RFC for a new feature of Cargo to support this.

@brson
Copy link
Contributor Author

brson commented Jan 22, 2021

@alexcrichton the solution in the RFC looks like it would work for tikv and a number of other projects that i hack on. Dependency duplication and management in big workspaces is definitely one of the biggest cargo papercuts i've been running into lately. Thanks.

@epage
Copy link
Contributor

epage commented Nov 1, 2023

Workspace inheritance is now stable.

In theory, a resolver aware of public/private dependencies would help with the wgpu case (#6129). rust-lang/rfcs#3516 is up that removes that part of the proposal though. An alternative future possibility is to allow specifying that dependency information is to be extracted from a public dependency.

So something like

wgpu = { git = "...", rev = "..." }
naga = { git.from = ["wgpu"] }

See also #7497

@epage
Copy link
Contributor

epage commented Nov 1, 2023

#7497 is almost a duplicate of this. The slight difference is that #7497 includes unifying two dependency specs to the same dependency, even if tag vs branch was used. This is focusing on the user problem of wanting to keep git dependency specs in-sync with each other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-git Area: anything dealing with git C-enhancement Category: enhancement S-needs-rfc Status: Needs an RFC to make progress.
Projects
None yet
Development

No branches or pull requests

5 participants