rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.75k stars 2.42k forks source link

Support tag/branch & rev at the same time for git dependencies #13142

Open dpc opened 11 months ago

dpc commented 11 months ago

Problem

Caused by:
  dependency (xyz) specification is ambiguous. Only one of `branch`, `tag` or `rev` is allowed

In some contexts it is useful to specify the exact rev to ensure a given revisions, but keep the tag/branch for human readability and preventing mistakes.

Proposed Solution

rev is great to ensure a given revision in projects that require it, but it's not very human-readable. Having both would be ideal in some context, but then it would be great if cargo actually did some checking that they actually match (otherwise the human-readable one could be a mistake and be worse than not being there at all).

The behavior I'm thinking of:

If rev is used with tag/branch, the rev should be used and cargo should verify that the rev is part of the history of the tag/branch (if it can) and print a warning (but not error) if it can't (--frozen etc.). cargo should only fail if it was able to check the history and rev and tag/branch don't actually match.

Users that don't want this behavior, can just not use both and retain current behavior.

Notes

No response

epage commented 11 months ago

Why is a Cargo.lock insufficient for this? This sounds like an alternative form of locking.

the rev should be used and cargo should verify that the rev is part of the history of the tag/branch (if it can) and print a warning (but not error) if it can't (--frozen etc.). cargo should only fail if it was able to check the history and rev and tag/branch don't actually match.

If I understand our current git behavior and git in general, we have the potential to move to shallow clones but this would prevent us from being able to do so, at least in this scenario.

dpc commented 11 months ago

Why is a Cargo.lock insufficient for this? This sounds like an alternative form of locking.

Some reasons:

Cargo.lock is ignored when a library is used as a dependency.

Cargo.lock is a mechanical lock file, hard to read, people don't pay attention to it during code reviews (as much). In Cargo.toml there's a semantic meaning. rev = means "you must use this revision, no updates supported/desired in this use-case", branch = means you should use stuff from this brunch, and potentially things might get updated there".

rev is very useful, but it just suffers from not being very human readable and hard to verify. Pairing it with something more readable just helps humans review updates and go about their work.

If I understand our current git behavior and git in general, we have the potential to move to shallow clones but this would prevent us from being able to do so, at least in this scenario.

Yes. I think that's the downside. But, I would be willing to pay that cost for these few deps that need to be git deps and that's the reason I mention that maybe cargo could verify rev to tag/branch correspondence only sometimes. Because once it's verified it's job done - human did not make a mistake and things are great, and dependency is pinned to the rev.

epage commented 11 months ago

Cargo.lock is ignored when a library is used as a dependency.

registry dependencies also cannot have git dependencies.

workspace dependencies will already ensure that they are in sync.

So this is exclusive to git/path dependencies where a common workspace is involved. Do I have that right?

epage commented 11 months ago

If rev is used with tag/branch, the rev should be used and cargo should verify that the rev is part of the history of the tag/branch

Generally tags are assumed and treated to be immutable, so do we need it for that? And if you want a human readable name for a rev, wouldn't tagging it be more appropriate or do you not have control over that repo and you aren't on one of their sanctioned releases?

dpc commented 11 months ago

registry dependencies also cannot have git dependencies.

I must admit we're doing lots of git dependencies... :D ... unfortunately. Basically we have a a project that depends on open source project and we often need to pin to a given git rev, and that internally often needs to pin things to git rev ... it's fun. That's the reason I'm aware of that usability issue.

So this is exclusive to git/path dependencies where a common workspace is involved. Do I have that right?

I'm not sure what do you mean. This generally applies to any git dependency. In any project that needs a git dependency to use ref= it's hard for the human to know immediately what does a git hash points to.

The whole feature is mostly a UX improvement.

Generally tags are assumed and treated to be immutable, so do we need it for that?

It's true that usually tags are immutable, so generally this wouldn't be that much of an issue. But it occurred to me that sometimes people do use rev for "security" reason and in that case having both rev and tag, instead of tag might be desired as well.