-
Notifications
You must be signed in to change notification settings - Fork 305
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
Pull depth fixes #2267
Pull depth fixes #2267
Conversation
This was already being done on the local depth pull test, so this just adds the matching logic to the remote depth pull test.
Ready for review again. I can squash the history if this looks good. |
Yeah agreed, there's no clean way to do this. I think I personally prefer what you had before, so let's drop that commit? |
Also, feel free to squash in your commits. Our merge bot doesn't autosquash anymore. |
Sure, I'll just leave in the cross referencing comments. |
When pulling with depth, missing parent commits are ignored. However, the check was applying to any commit, which means that it would succeed even if the requested commit was missing. This might happen on a corrupted remote repo or when using ref data from a stale summary. To achieve this, the semantics of the `commit_to_depth` hash table is changed slightly to only ever includes parent commits. This makes it easy to detect when a parent commit is being referenced (although there is a minor bug there when multiple refs are being pulled) while keeping references to commits that need their `commitpartial` files cleaned up. It also means that the table is only populated on depth pulls, which saves some memory and processing in the common depth=0 case. Fixes: ostreedev#2265
The local pull path was erroring on any missing commit, but that prevents a depth pull where the source repo has truncated history. As in the remote case, this also tries to pull in a tombstone commit if the source repo supports it. Fixes: ostreedev#2266
f42c7d0
to
d7f2955
Compare
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.
Thanks a lot!
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dbnicholson, jlebon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Here are a couple fixes I found when trying to do a
pull-local --depth=-1
. My use case is that I'm trying to merge 2 repos together locally but the repos have truncated histories due to pruning.