Cleanup RPC now uses proper prefix for worktree clean up

3 files changed, 22 insertions, 16 deletions

diff --git a/changelogs/unreleased/po-fix-worktree-cleanup-naive.yml b/changelogs/unreleased/po-fix-worktree-cleanup-naive.yml
new file mode 100644
index 000000000..69ac3ff23
--- /dev/null
+++ b/changelogs/unreleased/po-fix-worktree-cleanup-naive.yml
@@ -0,0 +1,5 @@
+---
+title: Cleanup RPC now uses proper prefix for worktree clean up
+merge_request: 1325
+author:
+type: fixed
diff --git a/internal/service/repository/cleanup.go b/internal/service/repository/cleanup.go
index 35e96f4bb..8d2bff957 100644
--- a/internal/service/repository/cleanup.go
+++ b/internal/service/repository/cleanup.go
@@ -102,7 +102,7 @@ func cleanPackedRefsLock(repoPath string, threshold time.Time) error {
 }
 
 func cleanStaleWorktrees(repoPath string, threshold time.Time) error {
-	worktreePath := filepath.Join(repoPath, "worktrees")
+	worktreePath := filepath.Join(repoPath, worktreePrefix)
 
 	dirInfo, err := os.Stat(worktreePath)
 	if err != nil {
diff --git a/internal/service/repository/cleanup_test.go b/internal/service/repository/cleanup_test.go
index cec59c4ec..e34b0a049 100644
--- a/internal/service/repository/cleanup_test.go
+++ b/internal/service/repository/cleanup_test.go
@@ -124,6 +124,8 @@ func TestCleanupDeletesPackedRefsLock(t *testing.T) {
 	}
 }
 
+// TODO: replace emulated rebase RPC with actual
+// https://gitlab.com/gitlab-org/gitaly/issues/1750
 func TestCleanupDeletesStaleWorktrees(t *testing.T) {
 	server, serverSocketPath := runRepoServer(t)
 	defer server.Stop()
@@ -160,11 +162,12 @@ func TestCleanupDeletesStaleWorktrees(t *testing.T) {
 
 			req := &gitalypb.CleanupRequest{Repository: testRepo}
 
-			testhelper.AddWorktree(t, testRepoPath, "test-worktree")
+			worktreeCheckoutPath := filepath.Join(testRepoPath, worktreePrefix, "test-worktree")
+			testhelper.AddWorktree(t, testRepoPath, worktreeCheckoutPath)
 			basePath := filepath.Join(testRepoPath, "worktrees")
 			worktreePath := filepath.Join(basePath, "test-worktree")
 
-			require.NoError(t, os.Chtimes(worktreePath, tc.worktreeTime, tc.worktreeTime))
+			require.NoError(t, os.Chtimes(worktreeCheckoutPath, tc.worktreeTime, tc.worktreeTime))
 
 			ctx, cancel := testhelper.Context()
 			defer cancel()
@@ -175,31 +178,27 @@ func TestCleanupDeletesStaleWorktrees(t *testing.T) {
 			assert.FileExists(t, filepath.Join(testRepoPath, "HEAD")) // For good measure
 
 			if tc.shouldExist {
-				assert.DirExists(t, basePath)
+				assert.DirExists(t, worktreeCheckoutPath)
 				assert.DirExists(t, worktreePath)
 			} else {
 				assert.NoError(t, err)
 				assert.NotNil(t, c)
 
+				testhelper.AssertFileNotExists(t, worktreeCheckoutPath)
 				testhelper.AssertFileNotExists(t, worktreePath)
 			}
 		})
 	}
 }
 
+// TODO: replace emulated rebase RPC with actual
+// https://gitlab.com/gitlab-org/gitaly/issues/1750
 func TestCleanupDisconnectedWorktrees(t *testing.T) {
 	const (
 		worktreeName     = "test-worktree"
 		worktreeAdminDir = "worktrees"
 	)
 
-	addWorkTree := func(repoPath, worktree string) error {
-		return exec.Command(
-			"git",
-			testhelper.AddWorktreeArgs(repoPath, worktree)...,
-		).Run()
-	}
-
 	server, serverSocketPath := runRepoServer(t)
 	defer server.Stop()
 
@@ -209,14 +208,14 @@ func TestCleanupDisconnectedWorktrees(t *testing.T) {
 	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
 	defer cleanupFn()
 
-	worktreePath := filepath.Join(testRepoPath, worktreeName)
+	worktreePath := filepath.Join(testRepoPath, worktreePrefix, worktreeName)
 	worktreeAdminPath := filepath.Join(
 		testRepoPath, worktreeAdminDir, filepath.Base(worktreeName),
 	)
 
 	req := &gitalypb.CleanupRequest{Repository: testRepo}
 
-	testhelper.AddWorktree(t, testRepoPath, worktreeName)
+	testhelper.AddWorktree(t, testRepoPath, worktreePath)
 
 	ctx, cancel := testhelper.Context()
 	defer cancel()
@@ -239,7 +238,10 @@ func TestCleanupDisconnectedWorktrees(t *testing.T) {
 	require.NoError(t, err)
 
 	if !pre2_20_0 {
-		err = addWorkTree(testRepoPath, worktreeName)
+		err := exec.Command(
+			"git",
+			testhelper.AddWorktreeArgs(testRepoPath, worktreePath)...,
+		).Run()
 		require.Error(t, err,
 			"creating a new work tree at the same path as a disconnected work tree should fail",
 		)
@@ -252,8 +254,7 @@ func TestCleanupDisconnectedWorktrees(t *testing.T) {
 
 	// if the worktree administrative files are pruned, then we should be able
 	// to checkout another worktree at the same path
-	err = addWorkTree(testRepoPath, worktreeName)
-	require.NoError(t, err)
+	testhelper.AddWorktree(t, testRepoPath, worktreePath)
 }
 
 func TestCleanupFileLocks(t *testing.T) {