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

object not found when trying to pull a repository cloned with Depth: 1 #589

Closed
wants to merge 1 commit into from

Conversation

rotty3000
Copy link

@rotty3000 rotty3000 commented Sep 29, 2022

fixes #305

In fact setting Depth to any value which results in the clone being partial will lead to the same error.

The iterator used in the fastforward check wasn't prepared for the case when a commit might have a missing parent (as in shallow clones). Handling that case solved the problem of pulling on shallow clones.

Signed-off-by: Raymond Augé raymond.auge@liferay.com

@rotty3000
Copy link
Author

I think I've actually found a better solution to this problem. It seems that the concept of haves v.s wants is not balanced in that shallowness is only checked in the wants side of the checks but not in the haves side.

This iteration should check if c.Hash is a shallow reference, something like this:

@@ -691,6 +691,15 @@ func getHavesFromRef(
 	toVisit := maxHavesToVisitPerRef
 	return walker.ForEach(func(c *object.Commit) error {
 		haves[c.Hash] = true
+
+		if s, _ := s.Shallow(); len(s) > 0 {
+			for _, sh := range s {
+				if sh == c.Hash {
+					return storer.ErrStop
+				}
+			}
+		}
+
 		toVisit--
 		// If toVisit starts out at 0 (indicating there is no
 		// max), then it will be negative here and we won't stop

@yarinm
Copy link

yarinm commented Jan 1, 2023

Who can review this and merge? We're dealing with similar issues and we believe this can solve our issues.

@rotty3000

@pjbgf
Copy link
Member

pjbgf commented Mar 6, 2023

Do you mind adding a test to make it explicit what issue is being resolved?
I tested this against the example given on #305 and it did not seem to fix the issue.

fixes go-git#305

Signed-off-by: Raymond Augé <raymond.auge@liferay.com>
@rotty3000
Copy link
Author

FYI, as I mentioned long ago, I feel the actual solution should be this one:

diff --git a/vendor/github.com/go-git/go-git/v5/remote.go b/vendor/github.com/go-git/go-git/v5/remote.go
index 418cb29..e213447 100644
--- a/vendor/github.com/go-git/go-git/v5/remote.go
+++ b/vendor/github.com/go-git/go-git/v5/remote.go
@@ -691,6 +691,15 @@ func getHavesFromRef(
 	toVisit := maxHavesToVisitPerRef
 	return walker.ForEach(func(c *object.Commit) error {
 		haves[c.Hash] = true
+
+		if s, _ := s.Shallow(); len(s) > 0 {
+			for _, sh := range s {
+				if sh == c.Hash {
+					return storer.ErrStop
+				}
+			}
+		}
+
 		toVisit--
 		// If toVisit starts out at 0 (indicating there is no
 		// max), then it will be negative here and we won't stop

@rotty3000 rotty3000 closed this Mar 7, 2023
@rotty3000 rotty3000 reopened this Mar 7, 2023
@rotty3000
Copy link
Author

sorry, accidentally closed.. Please see the updated solution which is the one we've been using for the past few months.

If someone wants to contribute a test to the PR in order to make it complete solution I wouldn't mind.

@AriehSchneier
Copy link
Contributor

Fixed by PR #778

@pjbgf
Copy link
Member

pjbgf commented Jul 1, 2023

Closing this in favour of the already merged #778.

@pjbgf pjbgf closed this Jul 1, 2023
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.

object not found when trying to pull a repository cloned with Depth: 1
4 participants