Skip to content

PrepareCheckout non-head references #1403

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

Merged
merged 5 commits into from
Jun 20, 2024

Conversation

Fernthedev
Copy link
Contributor

@Fernthedev Fernthedev commented Jun 16, 2024

This PR adds support for checking non-head references such as commits, tags, branches etc. using repo.find_reference() in PrepareCheckout.

While I preferred using Reference as a parameter rather than &BStr, I kept running into lifetime issues related to borrows with self.repo in PrepareCheckout.

At the time of writing this, it seems this solution is incomplete as the checkout does not set the git repo to checkout the reference. Specifically (excuse my lack of git terminology) git status is still set to track the HEAD reference it seems. However, the files are correctly cloned.

git status
On branch master
Your branch is up to date with 'origin/master'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   qpm.shared.json
        modified:   src/logger.cpp

I hope this PR isn't too messy, though feel free to ask for changes to follow the repo's code style. Thanks!

In reference to discussion #1389

Research

How does Git do it?

The ls-refs call of git clone --branch status <gitoxide-url>, thus --branch definitely assures anything with that name (if the protocol supports filtering).

0014command=ls-refs
0024agent=git/2.39.3.(Apple.Git-146)0016object-format=sha100010009peel
000csymrefs
000bunborn
0014ref-prefix HEAD
001bref-prefix refs/heads/
0016ref-prefix status
001bref-prefix refs/status
0020ref-prefix refs/tags/status
0021ref-prefix refs/heads/status
0023ref-prefix refs/remotes/status
0028ref-prefix refs/remotes/status/HEAD
001aref-prefix refs/tags/
0000

With --single-branch also specified, we get:

0014command=ls-refs
0024agent=git/2.39.3.(Apple.Git-146)0016object-format=sha100010009peel
000csymrefs
000bunborn
0014ref-prefix HEAD
001bref-prefix refs/heads/
0016ref-prefix status
001bref-prefix refs/status
0020ref-prefix refs/tags/status
0021ref-prefix refs/heads/status
0023ref-prefix refs/remotes/status
0028ref-prefix refs/remotes/status/HEAD
001aref-prefix refs/tags/
0000

So it's the same, but what's different is the list of wants, which now is very short, matching 954bab9f16839002a02ac809c7ca3184e8588261 refs/heads/status

0011command=fetch
0024agent=git/2.39.3.(Apple.Git-146)
0016object-format=sha1
0001
000dthin-pack000finclude-tag
000dofs-delta
0032want 954bab9f16839002a02ac809c7ca3184e8588261
0009done
0000

Lastly, without --branch --single-branch the ls-refs call looks like this:

0014command=ls-refs
0024agent=git/2.39.3.(Apple.Git-146)0016object-format=sha100010009peel
000csymrefs
000bunborn
0014ref-prefix HEAD
001bref-prefix refs/heads/
001aref-prefix refs/tags/
0000

With a long want list.

How does Gix do it?

By default, it auto-generates the ref-prefix from refspecs, creating an ls-refs call similar to what Git produces.

0014command=ls-refs
001bagent=git/oxide-0.63.0
0001000csymrefs
0009peel
000bunborn
001bref-prefix refs/heads/
0014ref-prefix HEAD
001aref-prefix refs/tags/
0000

Conclusion

  • --single-branch - trims the want-list to only the branch we would check out
  • --branch - controls the ref (i.e. possibly refs) that is checked out

Tasks

  • RefMap can receive any prefix to be sure the branch makes it into the set of returned refs automatically
  • checkout the named branch (with correct branch configuration)
  • add --ref to the gix CLI

Postponed: single-ref support

Implementing this should be fairly simple, but also might take longer if the parts don't work as anticipated. Let's leave it for another day or to a request.

  • single-ref support
    • affect negotiation
    • affect refspec updates
  • add --single-ref to the gix CLI to make this feature more broadly available (and verifiable by hand)

@Byron
Copy link
Member

Byron commented Jun 16, 2024

Thanks for giving it a shot!

I strongly recommend to try and find existing tests to adapt and to test the new functionality with them. This also allows to double-check the outcome. That way, turnaround times will be much shorter.

@Fernthedev
Copy link
Contributor Author

Hi so I attempted to add the unit test as requested, though unfortunately I'm not very familiar with the test setup and rather confused with everything. I tried using the generated repositories for the test and git branches -a shows multiple branches. However, the test fails and it seems to only recognize main.

I apologize though I'll mention that I did allow maintainers to modify the branch in the case you'd like to do the finish touches. Thanks!

git branch -a 

 a
  b
  c
  d
  e
  f
  g
  h
  i
  j
* main

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does look like a struggle, and I am happy to finish the PR.

Before that, could you solve the linting failure and maybe setup tooling to auto-format code?

@Fernthedev
Copy link
Contributor Author

I formatted the files and it CI seems to be ok now (other than the broken test)

Appreciate your patience

@Byron Byron self-assigned this Jun 20, 2024
@Byron Byron force-pushed the feat/checkout-other-refs branch from abbcef1 to eba1204 Compare June 20, 2024 07:11
@Byron
Copy link
Member

Byron commented Jun 20, 2024

I have started working on this and plan to force-push a couple of times. It's probably best to not interact with this PR with more than comments until it is merged.

@Byron Byron force-pushed the feat/checkout-other-refs branch 5 times, most recently from 7c37302 to 9d0e01d Compare June 20, 2024 17:16
Byron added 4 commits June 20, 2024 19:36
`--ref` is similar to `--branch`, but was renamed as it also supports
tags for example.
Now it's clear why it does what it does with the internal
repository, even though admittedly it could also
be made so that it can be called multiple times (despite
making no sense).
@Byron Byron force-pushed the feat/checkout-other-refs branch from b252b15 to f36b9bd Compare June 20, 2024 17:37
@Byron Byron marked this pull request as ready for review June 20, 2024 17:37
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This certainly looks a tiny bit different than how it started out, and was quite hard even for me as I only had vague memory on the 'prior art' within the codebase. Fortunately, everything turned out to be easy once I rediscovered the facilities that were already available.

@Byron Byron merged commit ecfde07 into GitoxideLabs:main Jun 20, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants