-
-
Notifications
You must be signed in to change notification settings - Fork 398
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
Conversation
|
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. |
|
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 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 |
Byron
left a comment
There was a problem hiding this 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?
|
I formatted the files and it CI seems to be ok now (other than the broken test) Appreciate your patience |
abbcef1 to
eba1204
Compare
|
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. |
7c37302 to
9d0e01d
Compare
`--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).
b252b15 to
f36b9bd
Compare
Byron
left a comment
There was a problem hiding this 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.
This PR adds support for checking non-head references such as commits, tags, branches etc. using
repo.find_reference()inPrepareCheckout.While I preferred using
Referenceas a parameter rather than&BStr, I kept running into lifetime issues related to borrows withself.repoin 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 statusis still set to track the HEAD reference it seems. However, the files are correctly cloned.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--branchdefinitely assures anything with that name (if the protocol supports filtering).With
--single-branchalso specified, we get:So it's the same, but what's different is the list of wants, which now is very short, matching
954bab9f16839002a02ac809c7ca3184e8588261 refs/heads/statusLastly, without
--branch --single-branchthe ls-refs call looks like this: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.
Conclusion
Tasks
--refto thegixCLIPostponed: 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-refto thegixCLI to make this feature more broadly available (and verifiable by hand)