Skip to content

Commit 452bcbb

Browse files
committed
fix(git): log warning when .git cleanup fails after clone error
If billyutil.RemoveAll fails to remove the orphaned .git directory, log a warning so the operator knows the submodule directory may be in a broken state.
1 parent 0ed9ca7 commit 452bcbb

2 files changed

Lines changed: 81 additions & 1 deletion

File tree

git/git.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,9 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr
766766
NoCheckout: true,
767767
})
768768
if err != nil {
769-
_ = billyutil.RemoveAll(subFS, ".git")
769+
if rmErr := billyutil.RemoveAll(subFS, ".git"); rmErr != nil {
770+
logf(" ⚠ Failed to clean up .git after clone failure: %v", rmErr)
771+
}
770772
return fmt.Errorf("clone submodule repository: %w", err)
771773
}
772774

git/git_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,84 @@ func TestCloneOptionsFromOptions_Submodules(t *testing.T) {
924924
require.Equal(t, 10, cloneOpts.SubmoduleDepth)
925925
}
926926

927+
// Regression: git.Open must find a repo that CloneRepo previously
928+
// created. Before the fix, the storer and worktree arguments were
929+
// swapped, so Open looked for HEAD at the worktree root instead of
930+
// .git/ and returned ErrRepositoryNotExists on every restart.
931+
func TestCloneRepo_RestartFindsExistingRepo(t *testing.T) {
932+
t.Parallel()
933+
934+
srvFS := memfs.New()
935+
_ = gittest.NewRepo(t, srvFS, gittest.Commit(t, "README.md", "hello", "init"))
936+
srv := httptest.NewServer(mwtest.BasicAuthMW("", "")(gittest.NewServer(srvFS)))
937+
t.Cleanup(srv.Close)
938+
939+
clientFS := memfs.New()
940+
cloned, err := git.CloneRepo(context.Background(), t.Logf, git.CloneRepoOptions{
941+
Path: "/workspace",
942+
RepoURL: srv.URL,
943+
Storage: clientFS,
944+
})
945+
require.NoError(t, err)
946+
require.True(t, cloned)
947+
948+
// Second call simulates a workspace restart. The repo exists on
949+
// disk; CloneRepo must detect it and return (false, nil).
950+
cloned, err = git.CloneRepo(context.Background(), t.Logf, git.CloneRepoOptions{
951+
Path: "/workspace",
952+
RepoURL: srv.URL,
953+
Storage: clientFS,
954+
})
955+
require.NoError(t, err)
956+
require.False(t, cloned, "second CloneRepo call must detect the existing repo")
957+
}
958+
959+
// Regression: a failed submodule clone must not leave behind a .git
960+
// directory that blocks all future retries. Before the fix, MkdirAll
961+
// created .git before CloneContext, and a clone failure left it in
962+
// place. The next start's Stat(".git") found it, entered the
963+
// already-cloned path, and failed permanently.
964+
func TestCloneRepo_SubmoduleCloneFailureAllowsRetry(t *testing.T) {
965+
t.Parallel()
966+
967+
// Submodule server that rejects all requests.
968+
subSrv := httptest.NewServer(mwtest.BasicAuthMW("secret", "secret")(gittest.NewServer(memfs.New())))
969+
t.Cleanup(subSrv.Close)
970+
971+
// Parent server with a submodule pointing at the rejecting server.
972+
subFS := memfs.New()
973+
subRepo := gittest.NewRepo(t, subFS, gittest.Commit(t, "f.txt", "x", "init"))
974+
subHead, err := subRepo.Head()
975+
require.NoError(t, err)
976+
977+
parentFS := memfs.New()
978+
_ = gittest.NewRepo(t, parentFS,
979+
gittest.Commit(t, "README.md", "hello", "init"),
980+
gittest.CommitSubmodule(t, "sub", subSrv.URL, subHead.Hash()),
981+
)
982+
parentSrv := httptest.NewServer(mwtest.BasicAuthMW("", "")(gittest.NewServer(parentFS)))
983+
t.Cleanup(parentSrv.Close)
984+
985+
clientFS := memfs.New()
986+
987+
// First attempt: clone succeeds but submodule init fails (auth).
988+
_, err = git.CloneRepo(context.Background(), t.Logf, git.CloneRepoOptions{
989+
Path: "/workspace",
990+
RepoURL: parentSrv.URL,
991+
Storage: clientFS,
992+
SubmoduleDepth: 1,
993+
})
994+
require.Error(t, err, "submodule clone should fail")
995+
996+
// The submodule's .git must have been cleaned up.
997+
workFS, err := clientFS.Chroot("/workspace")
998+
require.NoError(t, err)
999+
subModFS, err := workFS.Chroot("sub")
1000+
require.NoError(t, err)
1001+
_, statErr := subModFS.Stat(".git")
1002+
require.Error(t, statErr, ".git must not exist after failed clone")
1003+
}
1004+
9271005
// generates a random ed25519 private key
9281006
func randKeygen(t *testing.T) gossh.Signer {
9291007
t.Helper()

0 commit comments

Comments
 (0)