git: Fix fetching after shallow clone. Fixes #305 #766
Conversation
| @@ -864,6 +865,21 @@ func getHavesFromRef( | |||
| toVisit := maxHavesToVisitPerRef | |||
| return walker.ForEach(func(c *object.Commit) error { | |||
There was a problem hiding this comment.
Another option would be just to completely ignore any errors returned from this walker.ForEach call.
AriehSchneier
force-pushed
the
fix-fetch-after-shallow
branch
from
8336a5d
to
973deae
Compare
| @@ -864,6 +865,21 @@ func getHavesFromRef( | |||
| toVisit := maxHavesToVisitPerRef | |||
| return walker.ForEach(func(c *object.Commit) error { | |||
| haves[c.Hash] = true | |||
|
|
|||
| // initialise the shallows map | |||
| if *shallows == nil { | |||
There was a problem hiding this comment.
Deferencing a nil pointer leads to panic.
| if *shallows == nil { | |
| if shallows == nil { |
There was a problem hiding this comment.
This is a private function that we only call in 1 location, it will also contain a value (I would have passed by reference if go had that feature but it doesn't). We need to check whether the location that this variable is pointing to is nil or not. I could change this to if shallows != nil && *shallows != nil { but IMHO that would add an extra unnecessary check.
| @@ -837,6 +837,7 @@ func getHavesFromRef( | |||
| remoteRefs map[plumbing.Hash]bool, | |||
| s storage.Storer, | |||
| haves map[plumbing.Hash]bool, | |||
| shallows *map[plumbing.Hash]struct{}, | |||
There was a problem hiding this comment.
This will inevitably be passed "by reference".
| shallows *map[plumbing.Hash]struct{}, | |
| shallows map[plumbing.Hash]struct{}, |
There was a problem hiding this comment.
There was a problem hiding this comment.
PS if you test your suggested change you will find that nil will always be passed in and we will recalculate the map for every iteration of range localRefs
There was a problem hiding this comment.
You are absolutely right, thanks for sharing the link. 👍
| // initialise the shallows map | ||
| if *shallows == nil { | ||
| shallowList, _ := s.Shallow() | ||
| *shallows = make(map[plumbing.Hash]struct{}, len(shallowList)) |
There was a problem hiding this comment.
| *shallows = make(map[plumbing.Hash]struct{}, len(shallowList)) | |
| shallows = make(map[plumbing.Hash]struct{}, len(shallowList)) |
| shallowList, _ := s.Shallow() | ||
| *shallows = make(map[plumbing.Hash]struct{}, len(shallowList)) | ||
| for _, sh := range shallowList { | ||
| (*shallows)[sh] = struct{}{} |
There was a problem hiding this comment.
| (*shallows)[sh] = struct{}{} | |
| shallows[sh] = struct{}{} |
| } | ||
|
|
||
| // If this is a shallow reference don't try to iterate further | ||
| if _, ok := (*shallows)[c.Hash]; ok { |
There was a problem hiding this comment.
| if _, ok := (*shallows)[c.Hash]; ok { | |
| if _, ok := shallows[c.Hash]; ok { |
| @@ -899,7 +918,7 @@ func getHaves( | |||
| continue | |||
| } | |||
|
|
|||
| err = getHavesFromRef(ref, remoteRefs, s, haves) | |||
| err = getHavesFromRef(ref, remoteRefs, s, haves, &shallows) | |||
There was a problem hiding this comment.
| err = getHavesFromRef(ref, remoteRefs, s, haves, &shallows) | |
| err = getHavesFromRef(ref, remoteRefs, s, haves, shallows) |
AriehSchneier
force-pushed
the
fix-fetch-after-shallow
branch
from
973deae
to
254c9e4
Compare
Signed-off-by: Arieh Schneier <15041913+AriehSchneier@users.noreply.github.com>
AriehSchneier
force-pushed
the
fix-fetch-after-shallow
branch
from
254c9e4
to
05da5ef
Compare
|
Closing this in favour of #778 |
Also fixes #207