fix consecutive remotesapi pushes failing due to working set not being updated on fast-forward

Author: elianddb

go/libraries/doltcore/doltdb/doltdb.go

@@ -885,10 +885,12 @@ func (ddb *DoltDB) Commit(ctx context.Context, valHash hash.Hash, dref ref.DoltR
 	return ddb.CommitWithParentSpecs(ctx, valHash, dref, nil, cm)
 }
 
-// FastForwardWithWorkspaceCheck will perform a fast forward update of the branch given to the commit given, but only
-// if the working set is in sync with the head of the branch given. This is used in the course of pushing to a remote.
-// If the target doesn't currently have the working set ref, then no working set change will be made.
-func (ddb *DoltDB) FastForwardWithWorkspaceCheck(ctx context.Context, branch ref.DoltRef, commit *Commit) error {
+// FastForwardWithWorkspaceCheck performs a fast-forward of |branch| to |commit| only if the remote
+// working set is in sync with its HEAD, then updates the working set to match the new HEAD.
+// This is used when pushing to a remote. If the remote does not maintain a working set, no working
+// set update is performed.
+// When |allowDirtyWorking| is true, the working set may have unstaged changes, such as ignored tables.
+func (ddb *DoltDB) FastForwardWithWorkspaceCheck(ctx context.Context, branch ref.DoltRef, commit *Commit, allowDirtyWorking bool) error {
 	ds, err := ddb.db.GetDataset(ctx, branch.String())
 	if err != nil {
 		return err
@@ -909,7 +911,7 @@ func (ddb *DoltDB) FastForwardWithWorkspaceCheck(ctx context.Context, branch ref
 		ws = wsRef.String()
 	}
 
-	_, err = ddb.db.FastForward(ctx, ds, addr, ws)
+	_, err = ddb.db.FastForward(ctx, ds, addr, ws, allowDirtyWorking)
 
 	return err
 }
@@ -931,7 +933,7 @@ func (ddb *DoltDB) FastForwardToHash(ctx context.Context, branch ref.DoltRef, ha
 		return err
 	}
 
-	_, err = ddb.db.FastForward(ctx, ds, hash, "")
+	_, err = ddb.db.FastForward(ctx, ds, hash, "", false)
 
 	return err
 }

go/libraries/doltcore/doltdb/hooksdatabase.go

@@ -164,8 +164,8 @@ func (db hooksDatabase) SetHead(ctx context.Context, ds datas.Dataset, newHeadAd
 	return ds, err
 }
 
-func (db hooksDatabase) FastForward(ctx context.Context, ds datas.Dataset, newHeadAddr hash.Hash, workingSetPath string) (datas.Dataset, error) {
-	ds, err := db.Database.FastForward(ctx, ds, newHeadAddr, workingSetPath)
+func (db hooksDatabase) FastForward(ctx context.Context, ds datas.Dataset, newHeadAddr hash.Hash, workingSetPath string, allowDirtyWorking bool) (datas.Dataset, error) {
+	ds, err := db.Database.FastForward(ctx, ds, newHeadAddr, workingSetPath, allowDirtyWorking)
 	if err == nil {
 		db.ExecuteCommitHooks(ctx, ds, false, false)
 	}

go/libraries/doltcore/env/actions/remotes.go

@@ -86,18 +86,13 @@ func Push(ctx context.Context, tempTableDir string, mode ref.UpdateMode, destRef
 		}
 		err = srcDB.SetHeadToCommit(ctx, remoteRef, commit)
 	case ref.FastForwardOnly:
-		// ResolveBranchRoots fails for remotes that don't maintain a working set (e.g. file://),
-		// in which case FastForwardWithWorkspaceCheck handles it correctly without the ignored-table check.
+		// Ignored tables create a staged/working mismatch that blocks fast-forward pushes, so we detect and permit that case.
 		onlyIgnored := false
 		roots, err := destDB.ResolveBranchRoots(ctx, destRef)
 		if err == nil {
 			onlyIgnored, _ = diff.WorkingSetContainsOnlyIgnoredTables(ctx, roots)
 		}
-		if onlyIgnored {
-			err = destDB.FastForward(ctx, destRef, commit)
-		} else {
-			err = destDB.FastForwardWithWorkspaceCheck(ctx, destRef, commit)
-		}
+		err = destDB.FastForwardWithWorkspaceCheck(ctx, destRef, commit, onlyIgnored)
 		if err != nil {
 			return err
 		}

go/store/datas/database.go

@@ -149,14 +149,12 @@ type Database interface {
 	// is not provided, no working set update will be performed.
 	SetHead(ctx context.Context, ds Dataset, newHeadAddr hash.Hash, workingSetPath string) (Dataset, error)
 
-	// FastForward takes a types.Ref to a Commit object and makes it the new
-	// Head of ds iff it is a descendant of the current Head. Intended to be
-	// used e.g. after a call to Pull(). If the update cannot be performed,
-	// e.g., because another process moved the current Head out from under
-	// you, err will be non-nil.
-	// The workingSetPath is the path to the working set that should be used for updating the working set. If one
-	// is not provided, no working set update will be performed.
-	FastForward(ctx context.Context, ds Dataset, newHeadAddr hash.Hash, workingSetPath string) (Dataset, error)
+	// FastForward sets the head of |ds| to the commit at |newHeadAddr| if it is a descendant of
+	// the current head. If the update cannot be performed, for example because another writer has
+	// moved the head concurrently, err will be non-nil.
+	// If |workingSetPath| is non-empty, the working set at that path is updated to match the new head.
+	// When |allowDirtyWorking| is true, the working set may have unstaged changes, such as ignored tables.
+	FastForward(ctx context.Context, ds Dataset, newHeadAddr hash.Hash, workingSetPath string, allowDirtyWorking bool) (Dataset, error)
 
 	// Stats may return some kind of struct that reports statistics about the
 	// ChunkStore that backs this Database instance. The type is

go/store/datas/database_common.go

@@ -337,13 +337,13 @@ func (db *database) doSetHead(ctx context.Context, ds Dataset, addr hash.Hash, w
 	})
 }
 
-func (db *database) FastForward(ctx context.Context, ds Dataset, newHeadAddr hash.Hash, wsPath string) (Dataset, error) {
+func (db *database) FastForward(ctx context.Context, ds Dataset, newHeadAddr hash.Hash, wsPath string, allowDirtyWorking bool) (Dataset, error) {
 	return db.doHeadUpdate(ctx, ds, func(ds Dataset) error {
-		return db.doFastForward(ctx, ds, newHeadAddr, wsPath)
+		return db.doFastForward(ctx, ds, newHeadAddr, wsPath, allowDirtyWorking)
 	})
 }
 
-func (db *database) doFastForward(ctx context.Context, ds Dataset, newHeadAddr hash.Hash, workingSetPath string) error {
+func (db *database) doFastForward(ctx context.Context, ds Dataset, newHeadAddr hash.Hash, workingSetPath string, allowDirtyWorking bool) error {
 	newHead, err := db.readHead(ctx, newHeadAddr)
 	if err != nil {
 		return err
@@ -434,7 +434,7 @@ func (db *database) doFastForward(ctx context.Context, ds Dataset, newHeadAddr h
 
 					stagedHash := hash.New(msg.StagedRootAddrBytes())
 					workingSetHash := hash.New(msg.WorkingRootAddrBytes())
-					if stagedHash != workingSetHash {
+					if !allowDirtyWorking && stagedHash != workingSetHash {
 						return prolly.AddressMap{}, ErrDirtyWorkspace
 					}
 

go/store/datas/database_test.go

@@ -442,7 +442,7 @@ func (suite *DatabaseSuite) TestFastForward() {
 	cCommitAddr := mustHeadAddr(ds) // To use in FastForward() below.
 
 	// FastForward should disallow this, as |a| is not a descendant of |c|
-	_, err = suite.db.FastForward(context.Background(), ds, aCommitAddr, "")
+	_, err = suite.db.FastForward(context.Background(), ds, aCommitAddr, "", false)
 	suite.Error(err)
 
 	// Move Head back to something earlier in the lineage, so we can test FastForward
@@ -451,7 +451,7 @@ func (suite *DatabaseSuite) TestFastForward() {
 	suite.True(mustHeadValue(ds).Equals(a))
 
 	// This should succeed, because while |a| is not a direct parent of |c|, it is an ancestor.
-	ds, err = suite.db.FastForward(context.Background(), ds, cCommitAddr, "")
+	ds, err = suite.db.FastForward(context.Background(), ds, cCommitAddr, "", false)
 	suite.Require().NoError(err)
 	suite.True(mustHeadValue(ds).Equals(c))
 }

go/store/datas/pull/puller_test.go

@@ -412,7 +412,7 @@ func testPuller(t *testing.T, makeDB datasFactory) {
 
 			sinkDS, err := sinkdb.GetDataset(ctx, "ds")
 			require.NoError(t, err)
-			sinkDS, err = sinkdb.FastForward(ctx, sinkDS, rootAddr, "")
+			sinkDS, err = sinkdb.FastForward(ctx, sinkDS, rootAddr, "", false)
 			require.NoError(t, err)
 
 			require.NoError(t, err)