From 6a909bc179f9abff6a93effe7792dd236d6d789f Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Mon, 14 Sep 2020 12:43:33 -0700 Subject: [PATCH 1/5] go/store/datas: Update FindCommonAncestor to work when two commits are from different ValueReadWriters. --- go/libraries/doltcore/doltdb/commit.go | 6 ++--- go/store/cmd/noms/noms_merge.go | 2 +- go/store/datas/commit.go | 33 +++++++++++++++++--------- go/store/datas/commit_test.go | 4 ++-- go/store/datas/database_common.go | 2 +- 5 files changed, 29 insertions(+), 18 deletions(-) diff --git a/go/libraries/doltcore/doltdb/commit.go b/go/libraries/doltcore/doltdb/commit.go index bc96ee8e5c..8698aa06dc 100644 --- a/go/libraries/doltcore/doltdb/commit.go +++ b/go/libraries/doltcore/doltdb/commit.go @@ -199,7 +199,7 @@ func GetCommitAncestor(ctx context.Context, cm1, cm2 *Commit) (*Commit, error) { return nil, err } - ref, err := getCommitAncestorRef(ctx, ref1, ref2, cm1.vrw) + ref, err := getCommitAncestorRef(ctx, ref1, ref2, cm1.vrw, cm2.vrw) if err != nil { return nil, err @@ -220,8 +220,8 @@ func GetCommitAncestor(ctx context.Context, cm1, cm2 *Commit) (*Commit, error) { return NewCommit(cm1.vrw, ancestorSt), nil } -func getCommitAncestorRef(ctx context.Context, ref1, ref2 types.Ref, vrw types.ValueReadWriter) (types.Ref, error) { - ancestorRef, ok, err := datas.FindCommonAncestor(ctx, ref1, ref2, vrw) +func getCommitAncestorRef(ctx context.Context, ref1, ref2 types.Ref, vrw1, vrw2 types.ValueReadWriter) (types.Ref, error) { + ancestorRef, ok, err := datas.FindCommonAncestor(ctx, ref1, ref2, vrw1, vrw2) if err != nil { return types.Ref{}, err diff --git a/go/store/cmd/noms/noms_merge.go b/go/store/cmd/noms/noms_merge.go index 7cca1954aa..071211cd89 100644 --- a/go/store/cmd/noms/noms_merge.go +++ b/go/store/cmd/noms/noms_merge.go @@ -192,7 +192,7 @@ func getMergeCandidates(ctx context.Context, db datas.Database, leftDS, rightDS } func getCommonAncestor(ctx context.Context, r1, r2 types.Ref, vr types.ValueReader) (a types.Struct, found bool) { - aRef, found, err := datas.FindCommonAncestor(ctx, r1, r2, vr) + aRef, found, err := datas.FindCommonAncestor(ctx, r1, r2, vr, vr) d.PanicIfError(err) if !found { return diff --git a/go/store/datas/commit.go b/go/store/datas/commit.go index a6988a3cdc..63da61ecaa 100644 --- a/go/store/datas/commit.go +++ b/go/store/datas/commit.go @@ -76,8 +76,9 @@ func NewCommit(ctx context.Context, value types.Value, parentsList types.List, m // FindCommonAncestor returns the most recent common ancestor of c1 and c2, if // one exists, setting ok to true. If there is no common ancestor, ok is set -// to false. -func FindCommonAncestor(ctx context.Context, c1, c2 types.Ref, vr types.ValueReader) (a types.Ref, ok bool, err error) { +// to false. Refs of |c1| are dereferenced through |vr1|, while refs of |c2| +// are dereference through |vr2|. +func FindCommonAncestor(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (a types.Ref, ok bool, err error) { t1, err := types.TypeOf(c1) if err != nil { @@ -107,12 +108,12 @@ func FindCommonAncestor(ctx context.Context, c1, c2 types.Ref, vr types.ValueRea if common, ok := findCommonRef(c1Parents, c2Parents); ok { return common, true, nil } - parentsToQueue(ctx, c1Parents, c1Q, vr) - parentsToQueue(ctx, c2Parents, c2Q, vr) + parentsToQueue(ctx, c1Parents, c1Q, vr1) + parentsToQueue(ctx, c2Parents, c2Q, vr2) } else if c1Ht > c2Ht { - parentsToQueue(ctx, c1Q.PopRefsOfHeight(c1Ht), c1Q, vr) + parentsToQueue(ctx, c1Q.PopRefsOfHeight(c1Ht), c1Q, vr1) } else { - parentsToQueue(ctx, c2Q.PopRefsOfHeight(c2Ht), c2Q, vr) + parentsToQueue(ctx, c2Q.PopRefsOfHeight(c2Ht), c2Q, vr2) } } @@ -134,18 +135,28 @@ func parentsToQueue(ctx context.Context, refs types.RefSlice, q *types.RefByHeig } c := v.(types.Struct) - ps, ok, err := c.MaybeGet(ParentsField) - + ps, ok, err := c.MaybeGet(ParentsListField) if err != nil { return err } - if ok { - p := ps.(types.Set) - err = p.IterAll(ctx, func(v types.Value) error { + p := ps.(types.List) + err = p.IterAll(ctx, func(v types.Value, _ uint64) error { q.PushBack(v.(types.Ref)) return nil }) + } else { + ps, ok, err := c.MaybeGet(ParentsField) + if err != nil { + return err + } + if ok { + p := ps.(types.Set) + err = p.IterAll(ctx, func(v types.Value) error { + q.PushBack(v.(types.Ref)) + return nil + }) + } } } diff --git a/go/store/datas/commit_test.go b/go/store/datas/commit_test.go index 18a04ba348..9def0ba563 100644 --- a/go/store/datas/commit_test.go +++ b/go/store/datas/commit_test.go @@ -258,7 +258,7 @@ func TestFindCommonAncestor(t *testing.T) { // Assert that c is the common ancestor of a and b assertCommonAncestor := func(expected, a, b types.Struct) { - found, ok, err := FindCommonAncestor(context.Background(), mustRef(types.NewRef(a, types.Format_7_18)), mustRef(types.NewRef(b, types.Format_7_18)), db) + found, ok, err := FindCommonAncestor(context.Background(), mustRef(types.NewRef(a, types.Format_7_18)), mustRef(types.NewRef(b, types.Format_7_18)), db, db) assert.NoError(err) if assert.True(ok) { @@ -317,7 +317,7 @@ func TestFindCommonAncestor(t *testing.T) { assertCommonAncestor(a1, a6, c3) // Traversing multiple parents on both sides // No common ancestor - found, ok, err := FindCommonAncestor(context.Background(), mustRef(types.NewRef(d2, types.Format_7_18)), mustRef(types.NewRef(a6, types.Format_7_18)), db) + found, ok, err := FindCommonAncestor(context.Background(), mustRef(types.NewRef(d2, types.Format_7_18)), mustRef(types.NewRef(a6, types.Format_7_18)), db, db) assert.NoError(err) if !assert.False(ok) { diff --git a/go/store/datas/database_common.go b/go/store/datas/database_common.go index 0c4ca0c47b..4719668606 100644 --- a/go/store/datas/database_common.go +++ b/go/store/datas/database_common.go @@ -361,7 +361,7 @@ func (db *database) doCommit(ctx context.Context, datasetID string, commit types return err } - ancestorRef, found, err := FindCommonAncestor(ctx, commitRef, currentHeadRef, db) + ancestorRef, found, err := FindCommonAncestor(ctx, commitRef, currentHeadRef, db, db) if err != nil { return err From 8e97c61d73be9999ccfd1ac0e3d53a5e5f8f6ccc Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Mon, 14 Sep 2020 13:07:32 -0700 Subject: [PATCH 2/5] go/libraries/doltcore/env/actions/commitwalk: Update GetDotDotRevisions to work across two DoltDBs as sources for commits. --- .../env/actions/commitwalk/commitwalk.go | 45 +++++++++---------- .../env/actions/commitwalk/commitwalk_test.go | 8 ++-- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go b/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go index 4a046abad0..ed748a29bd 100644 --- a/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go +++ b/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go @@ -23,6 +23,7 @@ import ( ) type c struct { + ddb *doltdb.DoltDB commit *doltdb.Commit hash hash.Hash height uint64 @@ -34,8 +35,6 @@ type q struct { pending []*c numVisiblePending int loaded map[hash.Hash]*c - - ddb *doltdb.DoltDB } func (q *q) NumVisiblePending() int { @@ -51,8 +50,8 @@ func (q *q) PopPending() *c { return c } -func (q *q) AddPendingIfUnseen(ctx context.Context, id hash.Hash) error { - c, err := q.Get(ctx, id) +func (q *q) AddPendingIfUnseen(ctx context.Context, ddb *doltdb.DoltDB, id hash.Hash) error { + c, err := q.Get(ctx, ddb, id) if err != nil { return err } @@ -90,8 +89,8 @@ func (q *q) AddPendingIfUnseen(ctx context.Context, id hash.Hash) error { return nil } -func (q *q) SetInvisible(ctx context.Context, id hash.Hash) error { - c, err := q.Get(ctx, id) +func (q *q) SetInvisible(ctx context.Context, ddb *doltdb.DoltDB, id hash.Hash) error { + c, err := q.Get(ctx, ddb, id) if err != nil { return err } @@ -104,24 +103,24 @@ func (q *q) SetInvisible(ctx context.Context, id hash.Hash) error { return nil } -func (q *q) load(ctx context.Context, h hash.Hash) (*doltdb.Commit, error) { +func load(ctx context.Context, ddb *doltdb.DoltDB, h hash.Hash) (*doltdb.Commit, error) { cs, err := doltdb.NewCommitSpec(h.String()) if err != nil { return nil, err } - c, err := q.ddb.Resolve(ctx, cs, nil) + c, err := ddb.Resolve(ctx, cs, nil) if err != nil { return nil, err } return c, nil } -func (q *q) Get(ctx context.Context, id hash.Hash) (*c, error) { +func (q *q) Get(ctx context.Context, ddb *doltdb.DoltDB, id hash.Hash) (*c, error) { if l, ok := q.loaded[id]; ok { return l, nil } - l, err := q.load(ctx, id) + l, err := load(ctx, ddb, id) if err != nil { return nil, err } @@ -130,13 +129,13 @@ func (q *q) Get(ctx context.Context, id hash.Hash) (*c, error) { return nil, err } - c := &c{commit: l, height: h, hash: id} + c := &c{ddb: ddb, commit: l, height: h, hash: id} q.loaded[id] = c return c, nil } -func newQueue(ddb *doltdb.DoltDB) *q { - return &q{ddb: ddb, loaded: make(map[hash.Hash]*c)} +func newQueue() *q { + return &q{loaded: make(map[hash.Hash]*c)} } // GetDotDotRevisions returns the commits reachable from commit at hash @@ -148,16 +147,16 @@ func newQueue(ddb *doltdb.DoltDB) *q { // ties are broken by timestamp; newer commits appear first. // // Roughly mimics `git log master..feature`. -func GetDotDotRevisions(ctx context.Context, ddb *doltdb.DoltDB, includedHead hash.Hash, excludedHead hash.Hash, num int) ([]*doltdb.Commit, error) { +func GetDotDotRevisions(ctx context.Context, includedDB *doltdb.DoltDB, includedHead hash.Hash, excludedDB *doltdb.DoltDB, excludedHead hash.Hash, num int) ([]*doltdb.Commit, error) { commitList := make([]*doltdb.Commit, 0, num) - q := newQueue(ddb) - if err := q.SetInvisible(ctx, excludedHead); err != nil { + q := newQueue() + if err := q.SetInvisible(ctx, excludedDB, excludedHead); err != nil { return nil, err } - if err := q.AddPendingIfUnseen(ctx, excludedHead); err != nil { + if err := q.AddPendingIfUnseen(ctx, excludedDB, excludedHead); err != nil { return nil, err } - if err := q.AddPendingIfUnseen(ctx, includedHead); err != nil { + if err := q.AddPendingIfUnseen(ctx, includedDB, includedHead); err != nil { return nil, err } for q.NumVisiblePending() > 0 { @@ -168,11 +167,11 @@ func GetDotDotRevisions(ctx context.Context, ddb *doltdb.DoltDB, includedHead ha } for _, parentID := range parents { if nextC.invisible { - if err := q.SetInvisible(ctx, parentID); err != nil { + if err := q.SetInvisible(ctx, nextC.ddb, parentID); err != nil { return nil, err } } - if err := q.AddPendingIfUnseen(ctx, parentID); err != nil { + if err := q.AddPendingIfUnseen(ctx, nextC.ddb, parentID); err != nil { return nil, err } } @@ -231,7 +230,7 @@ func (i *commiterator) Next(ctx context.Context) (hash.Hash, *doltdb.Commit, err } for _, parentID := range parents { - if err := i.q.AddPendingIfUnseen(ctx, parentID); err != nil { + if err := i.q.AddPendingIfUnseen(ctx, nextC.ddb, parentID); err != nil { return hash.Hash{}, nil, err } } @@ -244,8 +243,8 @@ func (i *commiterator) Next(ctx context.Context) (hash.Hash, *doltdb.Commit, err // Reset implements doltdb.CommitItr func (i *commiterator) Reset(ctx context.Context) error { - i.q = newQueue(i.ddb) - if err := i.q.AddPendingIfUnseen(ctx, i.startCommitHash); err != nil { + i.q = newQueue() + if err := i.q.AddPendingIfUnseen(ctx, i.ddb, i.startCommitHash); err != nil { return err } return nil diff --git a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go index 4fd6018d7b..01e4991b6c 100644 --- a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go +++ b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go @@ -105,7 +105,7 @@ func TestGetDotDotRevisions(t *testing.T) { masterHash := mustGetHash(t, masterCommits[6]) featurePreMergeHash := mustGetHash(t, featureCommits[3]) - res, err := GetDotDotRevisions(context.Background(), env.DoltDB, featureHash, masterHash, 100) + res, err := GetDotDotRevisions(context.Background(), env.DoltDB, featureHash, env.DoltDB, masterHash, 100) require.NoError(t, err) assert.Len(t, res, 7) assert.Equal(t, featureCommits[7], res[0]) @@ -116,18 +116,18 @@ func TestGetDotDotRevisions(t *testing.T) { assert.Equal(t, featureCommits[2], res[5]) assert.Equal(t, featureCommits[1], res[6]) - res, err = GetDotDotRevisions(context.Background(), env.DoltDB, masterHash, featureHash, 100) + res, err = GetDotDotRevisions(context.Background(), env.DoltDB, masterHash, env.DoltDB, featureHash, 100) require.NoError(t, err) assert.Len(t, res, 0) - res, err = GetDotDotRevisions(context.Background(), env.DoltDB, featureHash, masterHash, 3) + res, err = GetDotDotRevisions(context.Background(), env.DoltDB, featureHash, env.DoltDB, masterHash, 3) require.NoError(t, err) assert.Len(t, res, 3) assert.Equal(t, featureCommits[7], res[0]) assert.Equal(t, featureCommits[6], res[1]) assert.Equal(t, featureCommits[5], res[2]) - res, err = GetDotDotRevisions(context.Background(), env.DoltDB, featurePreMergeHash, masterHash, 3) + res, err = GetDotDotRevisions(context.Background(), env.DoltDB, featurePreMergeHash, env.DoltDB, masterHash, 3) require.NoError(t, err) assert.Len(t, res, 3) assert.Equal(t, featureCommits[3], res[0]) From 871b7d59a397e9bd6daec3623925856193d6ffe7 Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Mon, 14 Sep 2020 14:03:35 -0700 Subject: [PATCH 3/5] go: commitwalk: Add test for GetDotDotRevisions across repositories. --- .../env/actions/commitwalk/commitwalk_test.go | 107 +++++++++++++++--- 1 file changed, 94 insertions(+), 13 deletions(-) diff --git a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go index 01e4991b6c..d9ab9b68bb 100644 --- a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go +++ b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go @@ -25,6 +25,7 @@ import ( "github.com/liquidata-inc/dolt/go/libraries/doltcore/env" "github.com/liquidata-inc/dolt/go/libraries/doltcore/ref" "github.com/liquidata-inc/dolt/go/libraries/utils/filesys" + "github.com/liquidata-inc/dolt/go/store/datas" "github.com/liquidata-inc/dolt/go/store/hash" "github.com/liquidata-inc/dolt/go/store/types" ) @@ -108,13 +109,13 @@ func TestGetDotDotRevisions(t *testing.T) { res, err := GetDotDotRevisions(context.Background(), env.DoltDB, featureHash, env.DoltDB, masterHash, 100) require.NoError(t, err) assert.Len(t, res, 7) - assert.Equal(t, featureCommits[7], res[0]) - assert.Equal(t, featureCommits[6], res[1]) - assert.Equal(t, featureCommits[5], res[2]) - assert.Equal(t, featureCommits[4], res[3]) - assert.Equal(t, featureCommits[3], res[4]) - assert.Equal(t, featureCommits[2], res[5]) - assert.Equal(t, featureCommits[1], res[6]) + assertEqualHashes(t, featureCommits[7], res[0]) + assertEqualHashes(t, featureCommits[6], res[1]) + assertEqualHashes(t, featureCommits[5], res[2]) + assertEqualHashes(t, featureCommits[4], res[3]) + assertEqualHashes(t, featureCommits[3], res[4]) + assertEqualHashes(t, featureCommits[2], res[5]) + assertEqualHashes(t, featureCommits[1], res[6]) res, err = GetDotDotRevisions(context.Background(), env.DoltDB, masterHash, env.DoltDB, featureHash, 100) require.NoError(t, err) @@ -123,16 +124,79 @@ func TestGetDotDotRevisions(t *testing.T) { res, err = GetDotDotRevisions(context.Background(), env.DoltDB, featureHash, env.DoltDB, masterHash, 3) require.NoError(t, err) assert.Len(t, res, 3) - assert.Equal(t, featureCommits[7], res[0]) - assert.Equal(t, featureCommits[6], res[1]) - assert.Equal(t, featureCommits[5], res[2]) + assertEqualHashes(t, featureCommits[7], res[0]) + assertEqualHashes(t, featureCommits[6], res[1]) + assertEqualHashes(t, featureCommits[5], res[2]) res, err = GetDotDotRevisions(context.Background(), env.DoltDB, featurePreMergeHash, env.DoltDB, masterHash, 3) require.NoError(t, err) assert.Len(t, res, 3) - assert.Equal(t, featureCommits[3], res[0]) - assert.Equal(t, featureCommits[2], res[1]) - assert.Equal(t, featureCommits[1], res[2]) + assertEqualHashes(t, featureCommits[3], res[0]) + assertEqualHashes(t, featureCommits[2], res[1]) + assertEqualHashes(t, featureCommits[1], res[2]) + + res, err = GetDotDotRevisions(context.Background(), env.DoltDB, featurePreMergeHash, env.DoltDB, masterHash, 3) + require.NoError(t, err) + assert.Len(t, res, 3) + assertEqualHashes(t, featureCommits[3], res[0]) + assertEqualHashes(t, featureCommits[2], res[1]) + assertEqualHashes(t, featureCommits[1], res[2]) + + // Create a similar branch to "feature" on a forked repository and GetDotDotRevisions using that as well. + forkEnv := mustForkDB(t, env.DoltDB, "feature", featureCommits[4]) + + // Create 3 commits on feature branch. + for i := 5; i < 8; i++ { + featureCommits[i] = mustCreateCommit(t, forkEnv.DoltDB, "feature", rvh, featureCommits[i-1]) + } + + featureHash = mustGetHash(t, featureCommits[7]) + masterHash = mustGetHash(t, masterCommits[6]) + featurePreMergeHash = mustGetHash(t, featureCommits[3]) + + res, err = GetDotDotRevisions(context.Background(), env.DoltDB, featureHash, env.DoltDB, masterHash, 100) + require.Error(t, err) + res, err = GetDotDotRevisions(context.Background(), forkEnv.DoltDB, featureHash, env.DoltDB, masterHash, 100) + require.NoError(t, err) + assert.Len(t, res, 7) + assertEqualHashes(t, featureCommits[7], res[0]) + assertEqualHashes(t, featureCommits[6], res[1]) + assertEqualHashes(t, featureCommits[5], res[2]) + assertEqualHashes(t, featureCommits[4], res[3]) + assertEqualHashes(t, featureCommits[3], res[4]) + assertEqualHashes(t, featureCommits[2], res[5]) + assertEqualHashes(t, featureCommits[1], res[6]) + + res, err = GetDotDotRevisions(context.Background(), env.DoltDB, masterHash, env.DoltDB, featureHash, 100) + require.Error(t, err) + res, err = GetDotDotRevisions(context.Background(), env.DoltDB, masterHash, forkEnv.DoltDB, featureHash, 100) + require.NoError(t, err) + assert.Len(t, res, 0) + + res, err = GetDotDotRevisions(context.Background(), forkEnv.DoltDB, featureHash, env.DoltDB, masterHash, 3) + require.NoError(t, err) + assert.Len(t, res, 3) + assertEqualHashes(t, featureCommits[7], res[0]) + assertEqualHashes(t, featureCommits[6], res[1]) + assertEqualHashes(t, featureCommits[5], res[2]) + + res, err = GetDotDotRevisions(context.Background(), env.DoltDB, featurePreMergeHash, env.DoltDB, masterHash, 3) + require.NoError(t, err) + assert.Len(t, res, 3) + assertEqualHashes(t, featureCommits[3], res[0]) + assertEqualHashes(t, featureCommits[2], res[1]) + assertEqualHashes(t, featureCommits[1], res[2]) + + res, err = GetDotDotRevisions(context.Background(), forkEnv.DoltDB, featurePreMergeHash, env.DoltDB, masterHash, 3) + require.NoError(t, err) + assert.Len(t, res, 3) + assertEqualHashes(t, featureCommits[3], res[0]) + assertEqualHashes(t, featureCommits[2], res[1]) + assertEqualHashes(t, featureCommits[1], res[2]) +} + +func assertEqualHashes(t *testing.T, lc, rc *doltdb.Commit) { + assert.Equal(t, mustGetHash(t, lc), mustGetHash(t, rc)) } func mustCreateCommit(t *testing.T, ddb *doltdb.DoltDB, bn string, rvh hash.Hash, parents ...*doltdb.Commit) *doltdb.Commit { @@ -150,6 +214,23 @@ func mustCreateCommit(t *testing.T, ddb *doltdb.DoltDB, bn string, rvh hash.Hash return commit } +func mustForkDB(t *testing.T, fromDB *doltdb.DoltDB, bn string, cm *doltdb.Commit) *env.DoltEnv { + stref, err := cm.GetStRef() + require.NoError(t, err) + forkEnv := createUninitializedEnv() + err = forkEnv.InitRepo(context.Background(), types.Format_LD_1, "Bill Billerson", "bill@billerson.com") + require.NoError(t, err) + p1 := make(chan datas.PullProgress) + p2 := make(chan datas.PullerEvent) + go func() { for range p1{ } }() + go func() { for range p2{ } }() + err = forkEnv.DoltDB.PullChunks(context.Background(), "", fromDB, stref, p1, p2) + require.NoError(t, err) + err = forkEnv.DoltDB.SetHead(context.Background(), ref.NewBranchRef(bn), stref) + require.NoError(t, err) + return forkEnv +} + func mustGetHash(t *testing.T, c *doltdb.Commit) hash.Hash { h, err := c.HashOf() require.NoError(t, err) From 8235268e8a7fb0a5c5a744fa824a4ca940c06876 Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Mon, 14 Sep 2020 14:28:48 -0700 Subject: [PATCH 4/5] go: store/datas: Write a test for FindCommonAncestor cross-Database behavior. --- go/store/datas/commit.go | 30 +++++++-- go/store/datas/commit_test.go | 112 ++++++++++++++++++++++++++-------- 2 files changed, 111 insertions(+), 31 deletions(-) diff --git a/go/store/datas/commit.go b/go/store/datas/commit.go index 63da61ecaa..0c78438ec7 100644 --- a/go/store/datas/commit.go +++ b/go/store/datas/commit.go @@ -24,6 +24,7 @@ package datas import ( "context" "sort" + "fmt" "github.com/liquidata-inc/dolt/go/store/d" "github.com/liquidata-inc/dolt/go/store/hash" @@ -108,12 +109,24 @@ func FindCommonAncestor(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.Va if common, ok := findCommonRef(c1Parents, c2Parents); ok { return common, true, nil } - parentsToQueue(ctx, c1Parents, c1Q, vr1) - parentsToQueue(ctx, c2Parents, c2Q, vr2) + err = parentsToQueue(ctx, c1Parents, c1Q, vr1) + if err != nil { + return types.Ref{}, false, err + } + err = parentsToQueue(ctx, c2Parents, c2Q, vr2) + if err != nil { + return types.Ref{}, false, err + } } else if c1Ht > c2Ht { - parentsToQueue(ctx, c1Q.PopRefsOfHeight(c1Ht), c1Q, vr1) + err = parentsToQueue(ctx, c1Q.PopRefsOfHeight(c1Ht), c1Q, vr1) + if err != nil { + return types.Ref{}, false, err + } } else { - parentsToQueue(ctx, c2Q.PopRefsOfHeight(c2Ht), c2Q, vr2) + err = parentsToQueue(ctx, c2Q.PopRefsOfHeight(c2Ht), c2Q, vr2) + if err != nil { + return types.Ref{}, false, err + } } } @@ -129,12 +142,17 @@ func parentsToQueue(ctx context.Context, refs types.RefSlice, q *types.RefByHeig seen[r.TargetHash()] = true v, err := r.TargetValue(ctx, vr) - if err != nil { return err } + if v == nil { + return fmt.Errorf("target not found: %v", r.TargetHash()) + } - c := v.(types.Struct) + c, ok := v.(types.Struct) + if !ok { + return fmt.Errorf("target ref is not struct: %v", v) + } ps, ok, err := c.MaybeGet(ParentsListField) if err != nil { return err diff --git a/go/store/datas/commit_test.go b/go/store/datas/commit_test.go index 9def0ba563..823650b615 100644 --- a/go/store/datas/commit_test.go +++ b/go/store/datas/commit_test.go @@ -243,12 +243,9 @@ func toRefList(vrw types.ValueReadWriter, commits ...types.Struct) (types.List, func TestFindCommonAncestor(t *testing.T) { assert := assert.New(t) - storage := &chunks.TestStorage{} - db := NewDatabase(storage.NewView()) - defer db.Close() // Add a commit and return it - addCommit := func(datasetID string, val string, parents ...types.Struct) types.Struct { + addCommit := func(db Database, datasetID string, val string, parents ...types.Struct) types.Struct { ds, err := db.GetDataset(context.Background(), datasetID) assert.NoError(err) ds, err = db.Commit(context.Background(), ds, types.String(val), CommitOptions{ParentsList: mustList(toRefList(db, parents...))}) @@ -257,12 +254,12 @@ func TestFindCommonAncestor(t *testing.T) { } // Assert that c is the common ancestor of a and b - assertCommonAncestor := func(expected, a, b types.Struct) { - found, ok, err := FindCommonAncestor(context.Background(), mustRef(types.NewRef(a, types.Format_7_18)), mustRef(types.NewRef(b, types.Format_7_18)), db, db) + assertCommonAncestor := func(expected, a, b types.Struct, ldb, rdb Database) { + found, ok, err := FindCommonAncestor(context.Background(), mustRef(types.NewRef(a, types.Format_7_18)), mustRef(types.NewRef(b, types.Format_7_18)), ldb, rdb) assert.NoError(err) if assert.True(ok) { - tv, err := found.TargetValue(context.Background(), db) + tv, err := found.TargetValue(context.Background(), ldb) assert.NoError(err) ancestor := tv.(types.Struct) expV, _, _ := expected.MaybeGet(ValueField) @@ -280,6 +277,9 @@ func TestFindCommonAncestor(t *testing.T) { } } + storage := &chunks.TestStorage{} + db := NewDatabase(storage.NewView()) + // Build commit DAG // // ds-a: a1<-a2<-a3<-a4<-a5<-a6 @@ -296,25 +296,25 @@ func TestFindCommonAncestor(t *testing.T) { // ds-d: d1<-d2 // a, b, c, d := "ds-a", "ds-b", "ds-c", "ds-d" - a1 := addCommit(a, "a1") - d1 := addCommit(d, "d1") - a2 := addCommit(a, "a2", a1) - c2 := addCommit(c, "c2", a1) - d2 := addCommit(d, "d2", d1) - a3 := addCommit(a, "a3", a2) - b3 := addCommit(b, "b3", a2) - c3 := addCommit(c, "c3", c2, d2) - a4 := addCommit(a, "a4", a3) - b4 := addCommit(b, "b4", b3) - a5 := addCommit(a, "a5", a4) - b5 := addCommit(b, "b5", b4, a3) - a6 := addCommit(a, "a6", a5, b5) + a1 := addCommit(db, a, "a1") + d1 := addCommit(db, d, "d1") + a2 := addCommit(db, a, "a2", a1) + c2 := addCommit(db, c, "c2", a1) + d2 := addCommit(db, d, "d2", d1) + a3 := addCommit(db, a, "a3", a2) + b3 := addCommit(db, b, "b3", a2) + c3 := addCommit(db, c, "c3", c2, d2) + a4 := addCommit(db, a, "a4", a3) + b4 := addCommit(db, b, "b4", b3) + a5 := addCommit(db, a, "a5", a4) + b5 := addCommit(db, b, "b5", b4, a3) + a6 := addCommit(db, a, "a6", a5, b5) - assertCommonAncestor(a1, a1, a1) // All self - assertCommonAncestor(a1, a1, a2) // One side self - assertCommonAncestor(a2, a3, b3) // Common parent - assertCommonAncestor(a2, a4, b4) // Common grandparent - assertCommonAncestor(a1, a6, c3) // Traversing multiple parents on both sides + assertCommonAncestor(a1, a1, a1, db, db) // All self + assertCommonAncestor(a1, a1, a2, db, db) // One side self + assertCommonAncestor(a2, a3, b3, db, db) // Common parent + assertCommonAncestor(a2, a4, b4, db, db) // Common grandparent + assertCommonAncestor(a1, a6, c3, db, db) // Traversing multiple parents on both sides // No common ancestor found, ok, err := FindCommonAncestor(context.Background(), mustRef(types.NewRef(d2, types.Format_7_18)), mustRef(types.NewRef(a6, types.Format_7_18)), db, db) @@ -334,6 +334,68 @@ func TestFindCommonAncestor(t *testing.T) { fV, ) } + + assert.NoError(db.Close()) + + storage = &chunks.TestStorage{} + db = NewDatabase(storage.NewView()) + defer db.Close() + + rstorage := &chunks.TestStorage{} + rdb := NewDatabase(rstorage.NewView()) + defer rdb.Close() + + // Rerun the tests when using two difference Databases for left and + // right commits. Both databases have all the previous commits. + a, b, c, d = "ds-a", "ds-b", "ds-c", "ds-d" + a1 = addCommit(db, a, "a1") + d1 = addCommit(db, d, "d1") + a2 = addCommit(db, a, "a2", a1) + c2 = addCommit(db, c, "c2", a1) + d2 = addCommit(db, d, "d2", d1) + a3 = addCommit(db, a, "a3", a2) + b3 = addCommit(db, b, "b3", a2) + c3 = addCommit(db, c, "c3", c2, d2) + a4 = addCommit(db, a, "a4", a3) + b4 = addCommit(db, b, "b4", b3) + a5 = addCommit(db, a, "a5", a4) + b5 = addCommit(db, b, "b5", b4, a3) + a6 = addCommit(db, a, "a6", a5, b5) + + addCommit(rdb, a, "a1") + addCommit(rdb, d, "d1") + addCommit(rdb, a, "a2", a1) + addCommit(rdb, c, "c2", a1) + addCommit(rdb, d, "d2", d1) + addCommit(rdb, a, "a3", a2) + addCommit(rdb, b, "b3", a2) + addCommit(rdb, c, "c3", c2, d2) + addCommit(rdb, a, "a4", a3) + addCommit(rdb, b, "b4", b3) + addCommit(rdb, a, "a5", a4) + addCommit(rdb, b, "b5", b4, a3) + addCommit(rdb, a, "a6", a5, b5) + + // Additionally, |db| has a6<-a7<-a8<-a9. + // |rdb| has a6<-ra7<-ra8<-ra9. + a7 := addCommit(db, a, "a7", a6) + a8 := addCommit(db, a, "a8", a7) + a9 := addCommit(db, a, "a9", a8) + + ra7 := addCommit(rdb, a, "ra7", a6) + ra8 := addCommit(rdb, a, "ra8", ra7) + ra9 := addCommit(rdb, a, "ra9", ra8) + + assertCommonAncestor(a1, a1, a1, db, rdb) // All self + assertCommonAncestor(a1, a1, a2, db, rdb) // One side self + assertCommonAncestor(a2, a3, b3, db, rdb) // Common parent + assertCommonAncestor(a2, a4, b4, db, rdb) // Common grandparent + assertCommonAncestor(a1, a6, c3, db, rdb) // Traversing multiple parents on both sides + + assertCommonAncestor(a6, a9, ra9, db, rdb) // Common third parent + + _, _, err = FindCommonAncestor(context.Background(), mustRef(types.NewRef(a9, types.Format_7_18)), mustRef(types.NewRef(ra9, types.Format_7_18)), rdb, db) + assert.Error(err) } func TestNewCommitRegressionTest(t *testing.T) { From 2b2f1b0f0221049f0b5184491d2bca71ff63b5c1 Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Mon, 14 Sep 2020 14:29:19 -0700 Subject: [PATCH 5/5] gofmt. --- .../doltcore/env/actions/commitwalk/commitwalk_test.go | 10 ++++++++-- go/store/datas/commit.go | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go index d9ab9b68bb..e66242d4d1 100644 --- a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go +++ b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go @@ -222,8 +222,14 @@ func mustForkDB(t *testing.T, fromDB *doltdb.DoltDB, bn string, cm *doltdb.Commi require.NoError(t, err) p1 := make(chan datas.PullProgress) p2 := make(chan datas.PullerEvent) - go func() { for range p1{ } }() - go func() { for range p2{ } }() + go func() { + for range p1 { + } + }() + go func() { + for range p2 { + } + }() err = forkEnv.DoltDB.PullChunks(context.Background(), "", fromDB, stref, p1, p2) require.NoError(t, err) err = forkEnv.DoltDB.SetHead(context.Background(), ref.NewBranchRef(bn), stref) diff --git a/go/store/datas/commit.go b/go/store/datas/commit.go index 0c78438ec7..6ccfb08e77 100644 --- a/go/store/datas/commit.go +++ b/go/store/datas/commit.go @@ -23,8 +23,8 @@ package datas import ( "context" - "sort" "fmt" + "sort" "github.com/liquidata-inc/dolt/go/store/d" "github.com/liquidata-inc/dolt/go/store/hash"