Merge pull request #8158 from dolthub/macneale4/no-op-merge-msg

Fix no-op merge msg in CLI

go/cmd/dolt/commands/merge.go

@@ -144,24 +144,34 @@ func (cmd MergeCmd) Exec(ctx context.Context, commandStr string, args []string,
 		cli.Println(err.Error())
 		return 1
 	}
-	// if merge is called with '--no-commit', we need to commit the sql transaction or the staged changes will be lost
-	_, _, err = queryist.Query(sqlCtx, "COMMIT")
+	rows, err := sql.RowIterToRows(sqlCtx, rowIter)
 	if err != nil {
 		cli.Println(err.Error())
+		return 0
+	}
+	if len(rows) != 1 {
+		cli.Println("Runtime error: merge operation returned unexpected number of rows: ", len(rows))
 		return 1
 	}
-	rows, err := sql.RowIterToRows(sqlCtx, rowIter)
+	mergeResultRow := rows[0]
+
+	upToDate, err := everythingUpToDate(mergeResultRow)
 	if err != nil {
-		cli.Println("merge finished, but failed to check for fast-forward")
 		cli.Println(err.Error())
+		return 1
+	}
+	if upToDate {
+		// dolt uses "Everything up-to-date" message, but Git CLI uses "Already up to date".
+		cli.Println(doltdb.ErrUpToDate.Error())
 		return 0
 	}
 
-	if len(rows) != 1 {
-		cli.Println("Runtime error: merge operation returned unexpected number of rows: ", len(rows))
+	// if merge is called with '--no-commit', we need to commit the sql transaction or the staged changes will be lost
+	_, _, err = queryist.Query(sqlCtx, "COMMIT")
+	if err != nil {
+		cli.Println(err.Error())
 		return 1
 	}
-	row := rows[0]
 
 	if !apr.Contains(cli.AbortParam) {
 		//todo: refs with the `remotes/` prefix will fail to get a hash
@@ -176,7 +186,7 @@ func (cmd MergeCmd) Exec(ctx context.Context, commandStr string, args []string,
 			cli.Println(mergeHashErr.Error())
 		}
 
-		fastFwd := getFastforward(row, dprocedures.MergeProcFFIndex)
+		fastFwd := getFastforward(mergeResultRow, dprocedures.MergeProcFFIndex)
 
 		if apr.Contains(cli.NoCommitFlag) {
 			return printMergeStats(fastFwd, apr, queryist, sqlCtx, usage, headHash, mergeHash, "HEAD", "STAGED")
@@ -706,3 +716,28 @@ func handleMergeErr(sqlCtx *sql.Context, queryist cli.Queryist, mergeErr error,
 
 	return 0
 }
+
+func everythingUpToDate(row sql.Row) (bool, error) {
+	if row == nil {
+		return false, fmt.Errorf("Runtime error: nil row returned from merge operation")
+	}
+
+	// We don't currently define these in a readily accessible way, so we'll just hard-code the column indexes.
+	// Confident we'll never change these.
+	hashColumn := 0
+	msgColumn := 3
+
+	if hash, ok := row[hashColumn].(string); ok {
+		if msg, ok := row[msgColumn].(string); ok {
+			if hash == "" && msg == doltdb.ErrUpToDate.Error() { // "Everything up-to-date" message.
+				return true, nil
+			}
+		} else {
+			return false, fmt.Errorf("Runtime error: merge operation returned unexpected message column type: %v", row[msgColumn])
+		}
+	} else {
+		return false, fmt.Errorf("Runtime error: merge operation returned unexpected hash column type: %v", row[hashColumn])
+	}
+
+	return false, nil
+}

go/libraries/doltcore/doltdb/errors.go

@@ -42,6 +42,7 @@ var ErrAlreadyOnWorkspace = errors.New("Already on workspace")
 
 var ErrNomsIO = errors.New("error reading from or writing to noms")
 
+// ErrUpToDate is returned when a merge is up-to-date. Not actually an error, and we do use this message in non-error contexts.
 var ErrUpToDate = errors.New("Everything up-to-date")
 var ErrIsAhead = errors.New("cannot 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")

integration-tests/bats/shallow-clone.bats

@@ -142,6 +142,76 @@ seed_and_start_serial_remote() {
     [ "$status" -eq 0 ]
     [[ "$output" =~ "15" ]] || false # 1+2+3+4+5 = 15.
 }
+
+@test "shallow-clone: fast forward merge" {
+    seed_local_remote
+    cd remote
+    dolt remote add origin file://../file-remote
+    dolt push origin main
+    cd ..
+
+    mkdir clones
+    cd clones
+    run dolt sql -q "call dolt_clone('--depth', '1','file://../file-remote')"
+    [ "$status" -eq 0 ]
+
+    cd file-remote
+    dolt checkout -b branch
+
+    dolt sql -q "insert into vals values (6, 'six')"
+    dolt commit -a -m "Added Val: 6 -> six"
+
+    dolt checkout main
+    dolt merge branch
+}
+
+@test "shallow-clone: simple merge" {
+    seed_local_remote
+    cd remote
+    dolt remote add origin file://../file-remote
+    dolt push origin main
+    cd ..
+
+    mkdir clones
+    cd clones
+    run dolt sql -q "call dolt_clone('--depth', '1','file://../file-remote')"
+    [ "$status" -eq 0 ]
+
+    cd file-remote
+    dolt checkout -b branch
+
+    dolt sql -q "insert into vals values (6, 'six')"
+    dolt commit -a -m "Added Val: 6 -> six"
+
+    dolt checkout main
+
+    dolt sql -q "insert into vals values (7, 'seven')"
+    dolt commit -a -m "Added Val: 7 -> seven"
+
+    dolt merge branch
+}
+
+@test "shallow-clone: no-op merge" {
+    seed_local_remote
+    cd remote
+    dolt remote add origin file://../file-remote
+    dolt push origin main
+    cd ..
+
+    mkdir clones
+    cd clones
+    run dolt sql -q "call dolt_clone('--depth', '1','file://../file-remote')"
+    [ "$status" -eq 0 ]
+
+    cd file-remote
+    dolt branch other HEAD
+
+    dolt checkout main
+    run dolt merge other
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "Everything up-to-date" ]] || false
+}
+
 @test "shallow-clone: push to a new remote should error" {
     seed_and_start_serial_remote
 
@@ -679,7 +749,6 @@ seed_and_start_complex_remote() {
 #   - Pull when there are remote changes on main
 # - Sensible error when branching/checking out a commit which they don't have.
 # - merge base errors
-# - GC works? or gives a decent error message?
 # - reset work to a commit we have, and errors when we don't have the commit.
 # - Sensible error when we attempt to use HEAD~51 or something.
 # - Don't serve from a shallow repository