Merge pull request #2431 from dolthub/james/fix-2336-conflicts

Fix issue with not clearing conflicts correctly

go/cmd/dolt/commands/cnfcmds/cat.go

@@ -154,11 +154,17 @@ func printConflicts(ctx context.Context, root *doltdb.RootValue, tblNames []stri
 				return errhand.BuildDError("error: unable to read database").AddCause(err).Build()
 			}
 
+			has, err := root.HasConflicts(ctx)
+			if err != nil {
+				return errhand.BuildDError("failed to read conflicts").AddCause(err).Build()
+			}
+			if !has {
+				return nil
+			}
+
 			cnfRd, err := merge.NewConflictReader(ctx, tbl)
 
-			if err == doltdb.ErrNoConflicts {
-				return nil
-			} else if err != nil {
+			if err != nil {
 				return errhand.BuildDError("failed to read conflicts").AddCause(err).Build()
 			}
 

go/cmd/dolt/commands/cnfcmds/resolve.go

@@ -26,7 +26,6 @@ import (
 	"github.com/dolthub/dolt/go/cmd/dolt/cli"
 	"github.com/dolthub/dolt/go/cmd/dolt/commands"
 	"github.com/dolthub/dolt/go/cmd/dolt/errhand"
-	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env/actions"
 	"github.com/dolthub/dolt/go/libraries/doltcore/merge"
@@ -138,11 +137,6 @@ func autoResolve(ctx context.Context, apr *argparser.ArgParseResults, dEnv *env.
 	}
 
 	if err != nil {
-		if err == doltdb.ErrNoConflicts {
-			cli.Println("no conflicts to resolve.")
-			return nil
-		}
-
 		return errhand.BuildDError("error: failed to resolve").AddCause(err).Build()
 	}
 

go/libraries/doltcore/doltdb/errors.go

@@ -42,7 +42,6 @@ var ErrAlreadyOnWorkspace = errors.New("Already on workspace")
 
 var ErrNomsIO = errors.New("error reading from or writing to noms")
 
-var ErrNoConflicts = errors.New("no conflicts")
 var ErrUpToDate = errors.New("up to date")
 var ErrIsAhead = errors.New("current fast forward from a to b. a is ahead of b already")
 var ErrIsBehind = errors.New("cannot reverse from b to a. b is a is behind a already")

go/libraries/doltcore/doltdb/table.go

@@ -155,13 +155,15 @@ func (t *Table) SetConflicts(ctx context.Context, schemas Conflict, conflictData
 	return &Table{t.vrw, updatedSt}, nil
 }
 
+// GetConflicts returns a map built from ValueReadWriter when there are no conflicts in table
 func (t *Table) GetConflicts(ctx context.Context) (Conflict, types.Map, error) {
 	schemasVal, ok, err := t.tableStruct.MaybeGet(conflictSchemasKey)
 	if err != nil {
 		return Conflict{}, types.EmptyMap, err
 	}
 	if !ok {
-		return Conflict{}, types.EmptyMap, ErrNoConflicts
+		confMap, _ := types.NewMap(ctx, t.ValueReadWriter())
+		return Conflict{}, confMap, nil
 	}
 
 	schemas, err := ConflictFromTuple(schemasVal.(types.Tuple))
@@ -271,7 +273,7 @@ func (t *Table) GetConflictSchemas(ctx context.Context) (base, sch, mergeSch sch
 
 		return baseSch, sch, mergeSch, err
 	}
-	return nil, nil, nil, ErrNoConflicts
+	return nil, nil, nil, nil
 }
 
 // GetConstraintViolationsSchema returns the schema for the dolt_constraint_violations system table belonging to this

go/libraries/doltcore/merge/conflict_source.go

@@ -47,7 +47,10 @@ type ConflictReader struct {
 // NewConflictReader returns a new conflict reader for a given table
 func NewConflictReader(ctx context.Context, tbl *doltdb.Table) (*ConflictReader, error) {
 	base, sch, mergeSch, err := tbl.GetConflictSchemas(ctx)
-	if err == doltdb.ErrNoConflicts {
+	if err != nil {
+		return nil, err
+	}
+	if base == nil || sch == nil || mergeSch == nil {
 		base, err = tbl.GetSchema(ctx)
 		sch, mergeSch = base, base
 	}
@@ -71,9 +74,6 @@ func NewConflictReader(ctx context.Context, tbl *doltdb.Table) (*ConflictReader,
 	}
 
 	_, confData, err := tbl.GetConflicts(ctx)
-	if err == doltdb.ErrNoConflicts {
-		confData, err = types.NewMap(ctx, tbl.ValueReadWriter())
-	}
 	if err != nil {
 		return nil, err
 	}

go/libraries/doltcore/merge/resolve.go

@@ -18,9 +18,8 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/dolthub/dolt/go/libraries/doltcore/env"
-
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
+	"github.com/dolthub/dolt/go/libraries/doltcore/env"
 	"github.com/dolthub/dolt/go/libraries/doltcore/row"
 	"github.com/dolthub/dolt/go/libraries/doltcore/schema"
 	"github.com/dolthub/dolt/go/libraries/doltcore/table"
@@ -42,7 +41,7 @@ func ResolveTable(ctx context.Context, vrw types.ValueReadWriter, tblName string
 	if has, err := tbl.HasConflicts(); err != nil {
 		return err
 	} else if !has {
-		return doltdb.ErrNoConflicts
+		return nil
 	}
 
 	tblSch, err := tbl.GetSchema(ctx)
@@ -75,6 +74,18 @@ func ResolveTable(ctx context.Context, vrw types.ValueReadWriter, tblName string
 			return nil, err
 		}
 
+		numRowsInConflict, err := tbl.NumRowsInConflict(ctx)
+		if err != nil {
+			return nil, err
+		}
+
+		if numRowsInConflict == 0 {
+			tbl, err = tbl.ClearConflicts()
+			if err != nil {
+				return nil, err
+			}
+		}
+
 		return root.PutTable(ctx, tblName, tbl)
 	})
 }

integration-tests/bats/conflict-detection-2.bats

@@ -502,7 +502,6 @@ SQL
 @test "conflict-detection-2: conflicts table properly cleared on dolt conflicts resolve" {
     dolt sql -q "create table test(pk int, c1 int, primary key(pk))"
 
-    skip "This should be empty now but it's not"
     run dolt conflicts cat test
     [ $status -eq 0 ]
     [ "$output" = "" ]
@@ -522,13 +521,11 @@ SQL
     dolt merge branch1
     dolt conflicts resolve --ours test
 
-    skip "This should be empty now but it's not"
     run dolt conflicts cat test
     [ $status -eq 0 ]
     [ "$output" = "" ]
     ! [[ "$output" =~ "pk" ]] || false
 
-    skip "Should not get a warning updating working set after resolve"
     run dolt sql -q "update test set c1=1"
     [ $status -eq 0 ]
     ! [[ "$output" =~ "unresolved conflicts from the merge" ]] || false
@@ -536,7 +533,6 @@ SQL
     dolt add .
     dolt commit -m "Committing active merge"
 
-    skip "A commit should definitely clear the conflicts table"
     run dolt conflicts cat test
     [ $status -eq 0 ]
     [ "$output" = "" ]

integration-tests/bats/docs.bats

@@ -797,8 +797,7 @@ SQL
     run cat README.md
     [[ ! $output =~ "test-b branch" ]] || false
     [[ $output =~ "test-a branch" ]] || false
-    # Only allow `dolt add dolt_docs` when dolt_docs is in conflict
-    dolt add dolt_docs
+    # No need for `dolt add dolt_docs` as table is already added
     dolt commit -m "Resolved docs conflict with --ours"
 
     # If the conflicts are resolved with --theirs, the working root and the docs on the filesystem are updated.

integration-tests/bats/sql-conflicts.bats

@@ -87,9 +87,9 @@ SQL
   [ "$status" -eq 0 ]
   [[ "$output" =~ "$EXPECTED" ]] || false
 
-  # delete an already resolved conflict a 2nd time is an error
+  # delete an already resolved conflict a 2nd time is fine
   run dolt sql -q "DELETE from dolt_conflicts_one_pk WHERE our_pk1 = 0"
-  [ "$status" -eq 1 ]
+  [ "$status" -eq 0 ]
 }