gitserver: Simplify clone and update (#61860)

Those two used to have separate mechanisms for locking and for making sure the operation keeps going in the background.

This PR unifies those two where possible.

Test plan:

The clone test covers this pretty well, and I made a few adjustments there to cover the new behavior.
Also, E2E tests will cover clone failures.

Author: eseliger

cmd/gitserver/internal/integration_tests/clone_test.go

@@ -200,8 +200,8 @@ func TestClone_Fail(t *testing.T) {
 	require.Contains(t, err.Error(), "error cloning repo: repo github.com/test/repo not cloneable:")
 	require.Contains(t, err.Error(), "exit status 128")
 
-	// No lock should have been acquired.
-	mockassert.NotCalled(t, locker.TryAcquireFunc)
+	mockassert.CalledOnce(t, locker.TryAcquireFunc)
+	mockassert.CalledOnce(t, lock.ReleaseFunc)
 
 	// Check we reported an error.
 	// Check that it was called exactly once total.
@@ -237,13 +237,13 @@ func TestClone_Fail(t *testing.T) {
 	require.Contains(t, err.Error(), "failed to clone github.com/test/repo: clone failed. Output: Creating bare repo\nCreated bare repo at")
 
 	// Should have acquired a lock.
-	mockassert.CalledOnce(t, locker.TryAcquireFunc)
+	mockassert.CalledN(t, locker.TryAcquireFunc, 2)
 	// Should have reported status. 7 lines is the output git currently produces.
 	// This number might need to be adjusted over time, but before doing so please
 	// check that the calls actually use the args you would expect them to use.
 	mockassert.CalledN(t, lock.SetStatusFunc, 7)
 	// Should have released the lock.
-	mockassert.CalledOnce(t, lock.ReleaseFunc)
+	mockassert.CalledN(t, lock.ReleaseFunc, 2)
 
 	// Check it was set to cloning first, then uncloned again (since clone failed).
 	mockassert.CalledN(t, gsStore.SetCloneStatusFunc, 2)

cmd/gitserver/internal/lock.go

@@ -20,8 +20,7 @@ type RepositoryLock interface {
 // When a repository is locked, only the owner of the lock is allowed to perform
 // writing operations against it.
 //
-// The main use of RepositoryLocker is to prevent concurrent clones. However,
-// it is also used during maintenance tasks such as recloning/migrating/etc.
+// The main use of RepositoryLocker is to prevent concurrent fetches.
 type RepositoryLocker interface {
 	// TryAcquire acquires the lock for repo. If it is already held, ok is false
 	// and lock is nil. Otherwise a non-nil lock is returned and true. When

cmd/gitserver/internal/main_test.go

@@ -39,7 +39,7 @@ func runCmd(t *testing.T, dir string, cmd string, arg ...string) string {
 		"GIT_AUTHOR_NAME=a",
 		"[email protected]",
 	}
-	b, err := c.CombinedOutput()
+	b, err := c.Output()
 	if err != nil {
 		t.Fatalf("%s %s failed: %s\nOutput: %s", cmd, strings.Join(arg, " "), err, b)
 	}