uv-resolver: make source=git more structured

[BurntSushi]

This splits out the git fields into an inline table instead of smushing
them into a single URL.

I'm unsure about this change, because it seems like we are using the git
URLs in other user facing ways. For example, some of the snapshot diffs
look like this:

     Installed 2 packages in [TIME]
      - project==0.1.0 (from file://[TEMP_DIR]/)
      + project==0.1.0 (from file://[TEMP_DIR]/)
-     + uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979?rev=0.0.1#0dacfd662c64cb4ceb16e6cf65a157a8b715b979)
+     + uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979)

which seems non-ideal. It should be possible to make the lock file use
structured fields while keeping the URL for terser output as above, but
I'm not sure that it's worth it.

This builds on top of #4627

[konstin]

The new snapshot looks more correct to me, losing the duplication it had

[charliermarsh]

I can own this one.

[charliermarsh]

Why was Rev removed?

[BurntSushi]

Because, at least in this change, "rev" is no longer a distinct kind. It is strictly duplicative with the fact that the lock file always records a revision regardless of what the underlying kind is.

[charliermarsh]

But how does it record the revision that was requested by the user?

[BurntSushi]

Ah I see. I believe that is exactly the thing I am unsure about here. As in, I don't think I grok why we need to keep track of what the user requested specifically in the lock file, since it is presumably in pyproject.toml. But yeah, if we do need to keep track of what the user requested in the lock file, specifically for git (but notably not for other things), then we will indeed need to capture some extra state here somehow. Ideally without duplication the revision hash.

[charliermarsh]

So if the user has rev = "refs/foo" in their requirements, and we lock with that request, and then they re-lock with the same requirements... We want to know that the locked requirement matches rev = "refs/foo" even if the commit that maps to that ref has changed (unless the user passed --upgrade).

[BurntSushi]

OK, so my mistake here is assuming that "revision" and "precise sha1" were equivalent. They aren't, because a revision might be a named reference to something. I'll add this back.

[BurntSushi]

@charliermarsh OK, I've updated this PR to include the revision in the lock file so that doesn't get dropped.

I think the main issue remaining here is that some of the snapshot diffs for uv pip compile output look wrong due to the URLs changing. I think the root cause is that, since the lock file no longer uses a single URL string to represent all of the git information, things external to the lock data structure can't reuse it. So we might need a step that flattens the new structured representation to the URL format for uv pip compile?

We could also just keep the URL representation in the lock file and not use structured data. I wouldn't mind that personally.

[charliermarsh]

Should we omit the revision if they are the same?

[BurntSushi]

So I am in favor of this, but I think it then becomes ambiguous. Specifically, I'm thinking about this part in the code:

uv/crates/uv-resolver/src/lock.rs

Lines 1372 to 1396 in 65cbeb9

	Git {
	git,
	commit,
	subdirectory,
	tag,
	branch,
	revision,
	} => {
	let precise: GitSha =
	commit
	.parse()
	.map_err(|err| LockErrorKind::InvalidGitSourceRevision {
	given: commit.to_string(),
	err,
	})?;
	let kind = match (tag, branch, revision) {
	(None, None, None) => GitSourceKind::DefaultBranch,
	(Some(tag), None, None) => GitSourceKind::Tag(tag),
	(None, Some(branch), None) => GitSourceKind::Branch(branch),
	(None, None, Some(revision)) => GitSourceKind::Rev(revision),
	_ => {
	let kind = LockErrorKind::InvalidGitSourceTooMany;
	return Err(kind.into());
	}
	};

Namely, this is where we deserialize our structured git data back into a Source. Currently, when branch, tag and revision are all missing, we assume "default branch" semantics. Which means that if we drop revision = "..." when it's identical to commit = "...", we won't be able to distinguish between "user specified a specific revision" and "user asked for the default branch."

[BurntSushi]

@charliermarsh Other than preference, I think this was the main issue preventing me from merging this PR. I'm not sure that this is correct.

(I do kinda prefer the URL approach since it's consistent with what we do in other places.)

[charliermarsh]

Ok, let's run with URL. It seems fine to me. Feel free to close.