Skip to content
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

Fetch submodules pointing to orphaned but still reachable commits #284

Merged
merged 1 commit into from Apr 6, 2021
Merged

Fetch submodules pointing to orphaned but still reachable commits #284

merged 1 commit into from Apr 6, 2021

Conversation

edigaryev
Copy link
Contributor

Sometimes submodules are not updated for long periods of time and the tree that holds the commit they point to gets removed at some point, which results in commits that are orphaned, but still reachable using allow-reachable-sha1-in-want Git's wire protocol capability.

Here's a real case: cirruslabs/cirrus-ci-docs#830 (comment).

This change makes sure go-git will behave similarly to Git in such cases, which does a subsequent fetch using a commit hash.

@fkorotkov
Copy link
Contributor

@mcuadros, gentle ping. I'll appreciate if you could take a look. This fix will help FreeBSD folks test FreeBSD on Cirrus CI faster.

@mcuadros
Copy link
Member

mcuadros commented Apr 6, 2021

any chance to do a test?

@edigaryev
Copy link
Contributor Author

any chance to do a test?

Sure, if you don't mind accepting a PR with a single commit first and then dropping the tree it belongs to.

I assume I'll need to create a PR in https://github.com/git-fixtures/basic?

@mcuadros
Copy link
Member

mcuadros commented Apr 6, 2021

Yup, there is a submodule fixture but maybe is not enough.

@mcuadros mcuadros merged commit 2f7c4ae into go-git:master Apr 6, 2021
@edigaryev edigaryev deleted the fetch-reachable-submodules branch April 6, 2021 21:57
@edigaryev
Copy link
Contributor Author

Yup, there is a submodule fixture but maybe is not enough.

Well, OK 🤷

It seems that the TestResolveRevision is failing on master, any idea if this might be related to this PR?

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.

None yet

3 participants