diff --git a/cmd/cachew/git_test.go b/cmd/cachew/git_test.go index 4f02e804..ec8b8dbd 100644 --- a/cmd/cachew/git_test.go +++ b/cmd/cachew/git_test.go @@ -19,6 +19,18 @@ import ( "github.com/block/cachew/client" ) +// TestMain drops the GIT_* variables git exports under hooks so test git +// commands target their temp dirs, not the ambient repository. +func TestMain(m *testing.M) { + for _, v := range []string{ + "GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", + "GIT_COMMON_DIR", "GIT_PREFIX", "GIT_NAMESPACE", + } { + _ = os.Unsetenv(v) + } + os.Exit(m.Run()) +} + func initGitRepo(t *testing.T, dir string, files map[string]string) { t.Helper() run := func(args ...string) { diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index 90a2bb32..b3b89793 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -27,6 +27,12 @@ const ( StateReady // Ready to use ) +// cloneTempPrefix is the prefix for the temporary directory created during a +// clone before it is atomically renamed into place. Interrupted clones (e.g. a +// killed process) can leave these behind, so they are skipped and removed by +// DiscoverExisting on startup. +const cloneTempPrefix = ".clone-" + func (s State) String() string { switch s { case StateEmpty: @@ -228,6 +234,14 @@ func (m *Manager) DiscoverExisting(ctx context.Context) ([]*Repository, error) { return nil } + if strings.HasPrefix(info.Name(), cloneTempPrefix) { + logging.FromContext(ctx).InfoContext(ctx, "Removing leftover clone temp dir", "path", path) + if rmErr := os.RemoveAll(path); rmErr != nil { + return errors.Wrapf(rmErr, "remove leftover clone temp dir %s", path) + } + return fs.SkipDir + } + headPath := filepath.Join(path, "HEAD") if _, statErr := os.Stat(headPath); statErr != nil { if errors.Is(statErr, os.ErrNotExist) { @@ -499,7 +513,7 @@ func (r *Repository) executeClone(ctx context.Context) error { return errors.Wrap(err, "create clone directory") } - tmpDir, err := os.MkdirTemp(parentDir, ".clone-*") + tmpDir, err := os.MkdirTemp(parentDir, cloneTempPrefix+"*") if err != nil { return errors.Wrap(err, "create temp clone directory") } diff --git a/internal/gitclone/manager_test.go b/internal/gitclone/manager_test.go index 40f8b21e..c5895d37 100644 --- a/internal/gitclone/manager_test.go +++ b/internal/gitclone/manager_test.go @@ -16,6 +16,18 @@ import ( "github.com/block/cachew/internal/logging" ) +// TestMain drops the GIT_* variables git exports under hooks so test git +// commands target their temp dirs, not the ambient repository. +func TestMain(m *testing.M) { + for _, v := range []string{ + "GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", + "GIT_COMMON_DIR", "GIT_PREFIX", "GIT_NAMESPACE", + } { + _ = os.Unsetenv(v) + } + os.Exit(m.Run()) +} + // testRepoConfig returns a Config with timeouts populated, suitable for // constructing Repository values directly in tests that bypass NewManager. func testRepoConfig() Config { @@ -209,6 +221,39 @@ func TestManager_DiscoverExisting(t *testing.T) { } } +func TestManager_DiscoverExisting_SkipsAndRemovesLeftoverCloneTempDirs(t *testing.T) { + _, ctx := logging.Configure(t.Context(), logging.Config{Level: slog.LevelError}) + tmpDir := t.TempDir() + config := Config{ + MirrorRoot: tmpDir, + FetchInterval: 15 * time.Minute, + RefCheckInterval: 10 * time.Second, + } + + manager, err := NewManager(ctx, config, nil) + assert.NoError(t, err) + + upstreamPath := createBareRepo(t, t.TempDir()) + + realRepo := filepath.Join(tmpDir, "github.com", "user1", "repo1") + assert.NoError(t, os.MkdirAll(filepath.Dir(realRepo), 0o755)) + assert.NoError(t, exec.Command("git", "clone", "--bare", upstreamPath, realRepo).Run()) + + // Simulate an interrupted clone: a leftover .clone-* temp dir containing a + // mirror clone at its "repo" subdirectory, alongside the real repo. + leftover := filepath.Join(tmpDir, "github.com", "user1", cloneTempPrefix+"123456") + assert.NoError(t, os.MkdirAll(leftover, 0o755)) + assert.NoError(t, exec.Command("git", "clone", "--bare", upstreamPath, filepath.Join(leftover, "repo")).Run()) + + discovered, err := manager.DiscoverExisting(ctx) + assert.NoError(t, err) + assert.Equal(t, 1, len(discovered)) + assert.Equal(t, "https://github.com/user1/repo1", discovered[0].UpstreamURL()) + + _, statErr := os.Stat(leftover) + assert.True(t, os.IsNotExist(statErr), "leftover clone temp dir should be removed") +} + func TestRepository_StateTransitions(t *testing.T) { repo := &Repository{ state: StateEmpty, diff --git a/internal/strategy/git/main_test.go b/internal/strategy/git/main_test.go new file mode 100644 index 00000000..b07488b8 --- /dev/null +++ b/internal/strategy/git/main_test.go @@ -0,0 +1,18 @@ +package git_test + +import ( + "os" + "testing" +) + +// TestMain drops the GIT_* variables git exports under hooks so test git +// commands target their temp dirs, not the ambient repository. +func TestMain(m *testing.M) { + for _, v := range []string{ + "GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", + "GIT_COMMON_DIR", "GIT_PREFIX", "GIT_NAMESPACE", + } { + _ = os.Unsetenv(v) + } + os.Exit(m.Run()) +}