Shallow Clone #7486
Shallow Clone #7486
Conversation
All existing tests pass
Get golang tests working again
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Also some todo clean up
This comment was marked as outdated.
This comment was marked as outdated.
|
@macneale4 DOLT
|
| @@ -142,7 +184,15 @@ func GetCommitAncestor(ctx context.Context, cm1, cm2 *Commit) (*Commit, error) { | |||
| return nil, err | |||
| } | |||
|
|
|||
| return NewCommit(ctx, cm1.vrw, cm1.ns, targetCommit) | |||
| if targetCommit.IsGhost() { | |||
There was a problem hiding this comment.
Are we expecting this to return ErrNoCommonAncestor in the case where we don't find the common ancestor because it's a ghost commit? Or does this actually work because the common ancestor logic is against the commit closure, which we have?
There was a problem hiding this comment.
Returning the ErrNoCommonAncestor would result in the wrong error. The way it is now (and we have a merge-base test for this), we message to the user that their shallow clone is incomplete and they should do a full clone.
| return nil | ||
| } | ||
|
|
||
| func (g *GhostBlockStore) PersistGhostHashes(ctx context.Context, hashes hash.HashSet) error { |
There was a problem hiding this comment.
No locking around the shared state needed here?
Should this method error if skippedRefs is not empty?
Would it be slightly more robust to write the file to a different, temporary filename and only rename it to g.ghostObjectsFile if the file contents successfully land on disk?
There was a problem hiding this comment.
the updates aren't racy. Did add a check to ensure we never write an empty set.
When fetching, verify tag commits aren't ghost commits
|
@macneale4 DOLT
|
Add the
--depthflag to the dolt_clone stored procedure and the dolt cliThis is possibly an MVP, as it's kind of impossible to test everything this touches. The guiding principle was to not break the code for any fully cloned repository.
There currently remain 7 problems to address in this code. They are each marked with "NM4"