Elgato_dark/argo-cd
1
0
Fork 0
mirror of https://github.com/argoproj/argo-cd synced 2026-04-21 17:07:16 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 22

fix: skip unnecessary git state clean (#26714)

Browse source 
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
This commit is contained in:
Alexander Matyushentsev 2026-03-09 00:28:31 -07:00 committed by GitHub
parent 92e4b2c877
commit 984a29c921
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 118 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
reposerver/repository/lock.go
View file

@ -18,7 +18,9 @@ type repositoryLock struct {
}
// Lock acquires lock unless lock is already acquired with the same commit and allowConcurrent is set to true
func (r *repositoryLock) Lock(path string, revision string, allowConcurrent bool, init func() (io.Closer, error)) (io.Closer, error) {
// The init callback receives `clean` parameter which indicates if repo state must be cleaned after running non-concurrent operation.
// The first init always runs with `clean` set to true because we cannot be sure about initial repo state.
func (r *repositoryLock) Lock(path string, revision string, allowConcurrent bool, init func(clean bool) (io.Closer, error)) (io.Closer, error) {
r.lock.Lock()
state, ok := r.stateByKey[path]
if !ok {
@ -52,7 +54,7 @@ func (r *repositoryLock) Lock(path string, revision string, allowConcurrent bool
state.cond.L.Lock()
if state.revision == "" {
// no in progress operation for that repo. Go ahead.
initCloser, err := init()
initCloser, err := init(!state.allowConcurrent)
if err != nil {
state.cond.L.Unlock()
return nil, fmt.Errorf("failed to initialize repository resources: %w", err)

33
reposerver/repository/lock_test.go
View file

@ -26,8 +26,8 @@ func lockQuickly(action func() (io.Closer, error)) (io.Closer, bool) {
}
}
func numberOfInits(initializedTimes *int) func() (io.Closer, error) {
return func() (io.Closer, error) {
func numberOfInits(initializedTimes *int) func(_ bool) (io.Closer, error) {
return func(_ bool) (io.Closer, error) {
*initializedTimes++
return utilio.NopCloser, nil
}
@ -120,7 +120,7 @@ func TestLock_FailedInitialization(t *testing.T) {
lock := NewRepositoryLock()
closer1, done := lockQuickly(func() (io.Closer, error) {
return lock.Lock("myRepo", "1", true, func() (io.Closer, error) {
return lock.Lock("myRepo", "1", true, func(_ bool) (io.Closer, error) {
return utilio.NopCloser, errors.New("failed")
})
})
@ -132,7 +132,7 @@ func TestLock_FailedInitialization(t *testing.T) {
assert.Nil(t, closer1)
closer2, done := lockQuickly(func() (io.Closer, error) {
return lock.Lock("myRepo", "1", true, func() (io.Closer, error) {
return lock.Lock("myRepo", "1", true, func(_ bool) (io.Closer, error) {
return utilio.NopCloser, nil
})
})
@ -168,3 +168,28 @@ func TestLock_SameRevisionFirstNotConcurrent(t *testing.T) {
utilio.Close(closer1)
}
func TestLock_CleanForNonConcurrent(t *testing.T) {
lock := NewRepositoryLock()
initClean := false
init := func(clean bool) (io.Closer, error) {
initClean = clean
return utilio.NopCloser, nil
}
closer, done := lockQuickly(func() (io.Closer, error) {
return lock.Lock("myRepo", "1", true, init)
})
assert.True(t, done)
// first time always clean because we cannot be sure about the state of repository
assert.True(t, initClean)
utilio.Close(closer)
closer, done = lockQuickly(func() (io.Closer, error) {
return lock.Lock("myRepo", "1", true, init)
})
assert.True(t, done)
assert.False(t, initClean)
utilio.Close(closer)
}

38
reposerver/repository/repository.go
View file

@ -252,8 +252,8 @@ func (s *Service) ListApps(ctx context.Context, q *apiclient.ListAppsRequest) (*
s.metricsServer.IncPendingRepoRequest(q.Repo.Repo)
defer s.metricsServer.DecPendingRepoRequest(q.Repo.Repo)
closer, err := s.repoLock.Lock(gitClient.Root(), commitSHA, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, commitSHA, s.initConstants.SubmoduleEnabled, q.Repo.Depth)
closer, err := s.repoLock.Lock(gitClient.Root(), commitSHA, true, func(clean bool) (goio.Closer, error) {
return s.checkoutRevision(gitClient, commitSHA, s.initConstants.SubmoduleEnabled, q.Repo.Depth, clean)
})
if err != nil {
return nil, fmt.Errorf("error acquiring repository lock: %w", err)
@ -454,8 +454,8 @@ func (s *Service) runRepoOperation(
return &operationContext{chartPath, ""}, nil
})
}
closer, err := s.repoLock.Lock(gitClient.Root(), revision, settings.allowConcurrent, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, s.initConstants.SubmoduleEnabled, repo.Depth)
closer, err := s.repoLock.Lock(gitClient.Root(), revision, settings.allowConcurrent, func(clean bool) (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, s.initConstants.SubmoduleEnabled, repo.Depth, clean)
})
if err != nil {
return err
@ -839,8 +839,8 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
ch.errCh <- fmt.Errorf("cannot reference a different revision of the same repository (%s references %q which resolves to %q while the application references %q which resolves to %q)", refVar, refSourceMapping.TargetRevision, referencedCommitSHA, q.Revision, commitSHA)
return
}
closer, err := s.repoLock.Lock(gitClient.Root(), referencedCommitSHA, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, referencedCommitSHA, s.initConstants.SubmoduleEnabled, q.Repo.Depth)
closer, err := s.repoLock.Lock(gitClient.Root(), referencedCommitSHA, true, func(clean bool) (goio.Closer, error) {
return s.checkoutRevision(gitClient, referencedCommitSHA, s.initConstants.SubmoduleEnabled, q.Repo.Depth, clean)
})
if err != nil {
log.Errorf("failed to acquire lock for referenced source %s", normalizedRepoURL)
@ -2445,8 +2445,8 @@ func (s *Service) GetRevisionMetadata(_ context.Context, q *apiclient.RepoServer
s.metricsServer.IncPendingRepoRequest(q.Repo.Repo)
defer s.metricsServer.DecPendingRepoRequest(q.Repo.Repo)
closer, err := s.repoLock.Lock(gitClient.Root(), q.Revision, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, q.Revision, s.initConstants.SubmoduleEnabled, q.Repo.Depth)
closer, err := s.repoLock.Lock(gitClient.Root(), q.Revision, true, func(clean bool) (goio.Closer, error) {
return s.checkoutRevision(gitClient, q.Revision, s.initConstants.SubmoduleEnabled, q.Repo.Depth, clean)
})
if err != nil {
return nil, fmt.Errorf("error acquiring repo lock: %w", err)
@ -2672,9 +2672,9 @@ func directoryPermissionInitializer(rootPath string) goio.Closer {
// checkoutRevision is a convenience function to initialize a repo, fetch, and checkout a revision
// Returns the 40 character commit SHA after the checkout has been performed
func (s *Service) checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bool, depth int64) (goio.Closer, error) {
func (s *Service) checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bool, depth int64, clean bool) (goio.Closer, error) {
closer := s.gitRepoInitializer(gitClient.Root())
err := checkoutRevision(gitClient, revision, submoduleEnabled, depth)
err := checkoutRevision(gitClient, revision, submoduleEnabled, depth, clean)
if err != nil {
s.metricsServer.IncGitFetchFail(gitClient.Root(), revision)
}
@ -2725,7 +2725,7 @@ func fetch(gitClient git.Client, targetRevisions []string) error {
return nil
}
func checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bool, depth int64) error {
func checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bool, depth int64, cleanState bool) error {
err := gitClient.Init()
if err != nil {
return status.Errorf(codes.Internal, "Failed to initialize git repo: %v", err)
@ -2751,7 +2751,7 @@ func checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bo
}
}
_, err = gitClient.Checkout(revision, submoduleEnabled)
_, err = gitClient.Checkout(revision, submoduleEnabled, cleanState)
if err != nil {
// When fetching with no revision, only refs/heads/* and refs/remotes/origin/* are fetched. If checkout fails
// for the given revision, try explicitly fetching it.
@ -2762,7 +2762,7 @@ func checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bo
return status.Errorf(codes.Internal, "Failed to checkout revision %s: %v", revision, err)
}
_, err = gitClient.Checkout("FETCH_HEAD", submoduleEnabled)
_, err = gitClient.Checkout("FETCH_HEAD", submoduleEnabled, cleanState)
if err != nil {
return status.Errorf(codes.Internal, "Failed to checkout FETCH_HEAD: %v", err)
}
@ -2904,8 +2904,8 @@ func (s *Service) GetGitFiles(_ context.Context, request *apiclient.GitFilesRequ
defer s.metricsServer.DecPendingRepoRequest(repo.Repo)
// cache miss, generate the results
closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, request.GetSubmoduleEnabled(), repo.Depth)
closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func(clean bool) (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, request.GetSubmoduleEnabled(), repo.Depth, clean)
})
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to checkout git repo %s with revision %s pattern %s: %v", repo.Repo, revision, gitPath, err)
@ -2986,8 +2986,8 @@ func (s *Service) GetGitDirectories(_ context.Context, request *apiclient.GitDir
defer s.metricsServer.DecPendingRepoRequest(repo.Repo)
// cache miss, generate the results
closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, request.GetSubmoduleEnabled(), repo.Depth)
closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func(clean bool) (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, request.GetSubmoduleEnabled(), repo.Depth, clean)
})
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to checkout git repo %s with revision %s: %v", repo.Repo, revision, err)
@ -3067,8 +3067,8 @@ func (s *Service) gitSourceHasChanges(repo *v1alpha1.Repository, revision, synce
s.metricsServer.IncPendingRepoRequest(repo.Repo)
defer s.metricsServer.DecPendingRepoRequest(repo.Repo)
closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, false, 0)
closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func(clean bool) (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, false, 0, clean)
})
if err != nil {
return files, status.Errorf(codes.Internal, "unable to checkout git repo %s with revision %s: %v", repo.Repo, revision, err)

50
reposerver/repository/repository_test.go
View file

@ -112,7 +112,7 @@ func newServiceWithMocks(t *testing.T, root string, signed bool) (*Service, *git
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent(mock.Anything).Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().LsRemote(mock.Anything).Return(mock.Anything, nil)
gitClient.EXPECT().CommitSHA().Return(mock.Anything, nil)
gitClient.EXPECT().Root().Return(root)
@ -199,7 +199,7 @@ func newServiceWithCommitSHA(t *testing.T, root, revision string) *Service {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent(mock.Anything).Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().LsRemote(revision).Return(revision, revisionErr)
gitClient.EXPECT().CommitSHA().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.EXPECT().Root().Return(root)
@ -3173,10 +3173,10 @@ func TestCheckoutRevisionCanGetNonstandardRefs(t *testing.T) {
pullSha, err := gitClient.LsRemote("refs/pull/123/head")
require.NoError(t, err)
err = checkoutRevision(gitClient, "does-not-exist", false, 0)
err = checkoutRevision(gitClient, "does-not-exist", false, 0, true)
require.Error(t, err)
err = checkoutRevision(gitClient, pullSha, false, 0)
err = checkoutRevision(gitClient, pullSha, false, 0, true)
require.NoError(t, err)
}
@ -3186,9 +3186,9 @@ func TestCheckoutRevisionPresentSkipFetch(t *testing.T) {
gitClient := &gitmocks.Client{}
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent(revision).Return(true)
gitClient.EXPECT().Checkout(revision, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(revision, mock.Anything, mock.Anything).Return("", nil)
err := checkoutRevision(gitClient, revision, false, 0)
err := checkoutRevision(gitClient, revision, false, 0, true)
require.NoError(t, err)
}
@ -3199,9 +3199,9 @@ func TestCheckoutRevisionNotPresentCallFetch(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent(revision).Return(false)
gitClient.EXPECT().Fetch("", mock.Anything).Return(nil)
gitClient.EXPECT().Checkout(revision, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(revision, mock.Anything, mock.Anything).Return("", nil)
err := checkoutRevision(gitClient, revision, false, 0)
err := checkoutRevision(gitClient, revision, false, 0, true)
require.NoError(t, err)
}
@ -3607,7 +3607,7 @@ func TestErrorGetGitDirectories(t *testing.T) {
}, want: nil, wantErr: assert.Error},
{name: "InvalidResolveRevision", fields: fields{service: func() *Service {
s, _, _ := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().LsRemote(mock.Anything).Return("", errors.New("ah error"))
gitClient.EXPECT().Root().Return(root)
paths.EXPECT().GetPath(mock.Anything).Return(".", nil)
@ -3624,7 +3624,7 @@ func TestErrorGetGitDirectories(t *testing.T) {
}, want: nil, wantErr: assert.Error},
{name: "ErrorVerifyCommit", fields: fields{service: func() *Service {
s, _, _ := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().LsRemote(mock.Anything).Return("", errors.New("ah error"))
gitClient.EXPECT().VerifyCommitSignature(mock.Anything).Return("", fmt.Errorf("revision %s is not signed", "sadfsadf"))
gitClient.EXPECT().Root().Return(root)
@ -3661,7 +3661,7 @@ func TestGetGitDirectories(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent(mock.Anything).Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Once().Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Once().Return("", nil)
gitClient.EXPECT().LsRemote("HEAD").Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.EXPECT().Root().Return(root)
paths.EXPECT().GetPath(mock.Anything).Return(root, nil)
@ -3694,7 +3694,7 @@ func TestGetGitDirectoriesWithHiddenDirSupported(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent(mock.Anything).Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Once().Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Once().Return("", nil)
gitClient.EXPECT().LsRemote("HEAD").Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.EXPECT().Root().Return(root)
paths.EXPECT().GetPath(mock.Anything).Return(root, nil)
@ -3749,7 +3749,7 @@ func TestErrorGetGitFiles(t *testing.T) {
}, want: nil, wantErr: assert.Error},
{name: "InvalidResolveRevision", fields: fields{service: func() *Service {
s, _, _ := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().LsRemote(mock.Anything).Return("", errors.New("ah error"))
gitClient.EXPECT().Root().Return(root)
paths.EXPECT().GetPath(mock.Anything).Return(".", nil)
@ -3788,7 +3788,7 @@ func TestGetGitFiles(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent(mock.Anything).Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Once().Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Once().Return("", nil)
gitClient.EXPECT().LsRemote("HEAD").Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.EXPECT().Root().Return(root)
gitClient.EXPECT().LsFiles(mock.Anything, mock.Anything).Once().Return(files, nil)
@ -3853,7 +3853,7 @@ func TestErrorUpdateRevisionForPaths(t *testing.T) {
}, want: nil, wantErr: assert.Error},
{name: "InvalidResolveRevision", fields: fields{service: func() *Service {
s, _, _ := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().LsRemote(mock.Anything).Return("", errors.New("ah error"))
gitClient.EXPECT().Root().Return(root)
paths.EXPECT().GetPath(mock.Anything).Return(".", nil)
@ -3871,7 +3871,7 @@ func TestErrorUpdateRevisionForPaths(t *testing.T) {
}, want: nil, wantErr: assert.Error},
{name: "InvalidResolveSyncedRevision", fields: fields{service: func() *Service {
s, _, _ := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().LsRemote("HEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.EXPECT().LsRemote(mock.Anything).Return("", errors.New("ah error"))
gitClient.EXPECT().Root().Return(root)
@ -3925,7 +3925,7 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}{
{name: "NoPathAbort", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, _ *iomocks.TempPaths) {
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
}, ".")
return fields{
service: s,
@ -3942,7 +3942,7 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}, want: &apiclient.UpdateRevisionForPathsResponse{Changes: true}, wantErr: assert.NoError},
{name: "SameResolvedRevisionAbort", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().LsRemote("HEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.EXPECT().LsRemote("SYNCEDHEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
paths.EXPECT().GetPath(mock.Anything).Return(".", nil)
@ -3972,7 +3972,7 @@ func TestUpdateRevisionForPaths(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().IsRevisionPresent("632039659e542ed7de0c170a4fcc1c571b288fc0").Once().Return(false)
gitClient.EXPECT().Checkout("632039659e542ed7de0c170a4fcc1c571b288fc0", mock.Anything).Once().Return("", nil)
gitClient.EXPECT().Checkout("632039659e542ed7de0c170a4fcc1c571b288fc0", mock.Anything, mock.Anything).Once().Return("", nil)
// fetch
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
@ -4009,7 +4009,7 @@ func TestUpdateRevisionForPaths(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().IsRevisionPresent("632039659e542ed7de0c170a4fcc1c571b288fc0").Once().Return(false)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(false)
// fetch
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
@ -4055,7 +4055,7 @@ func TestUpdateRevisionForPaths(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent("632039659e542ed7de0c170a4fcc1c571b288fc0").Once().Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
// fetch
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(true)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
@ -4101,7 +4101,7 @@ func TestUpdateRevisionForPaths(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent("632039659e542ed7de0c170a4fcc1c571b288fc0").Once().Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
// fetch
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(true)
gitClient.EXPECT().IsRevisionPresent("732039659e542ed7de0c170a4fcc1c571b288fc1").Once().Return(true)
@ -4158,7 +4158,7 @@ func TestUpdateRevisionForPaths(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent("632039659e542ed7de0c170a4fcc1c571b288fc0").Once().Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
// fetch
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(true)
gitClient.EXPECT().IsRevisionPresent("732039659e542ed7de0c170a4fcc1c571b288fc1").Once().Return(true)
@ -4213,7 +4213,7 @@ func TestUpdateRevisionForPaths(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent("632039659e542ed7de0c170a4fcc1c571b288fc0").Once().Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
// fetch
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(true)
gitClient.EXPECT().IsRevisionPresent("732039659e542ed7de0c170a4fcc1c571b288fc1").Once().Return(true)
@ -4261,7 +4261,7 @@ func TestUpdateRevisionForPaths(t *testing.T) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().IsRevisionPresent("632039659e542ed7de0c170a4fcc1c571b288fc0").Once().Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
// fetch
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(true)
gitClient.EXPECT().IsRevisionPresent("732039659e542ed7de0c170a4fcc1c571b288fc1").Once().Return(true)

26
util/git/client.go
View file

@ -127,7 +127,7 @@ type Client interface {
Init() error
Fetch(revision string, depth int64) error
Submodule() error
Checkout(revision string, submoduleEnabled bool) (string, error)
Checkout(revision string, submoduleEnabled bool, cleanState bool) (string, error)
LsRefs() (*Refs, error)
LsRemote(revision string) (string, error)
LsFiles(path string, enableNewGitFileGlobbing bool) ([]string, error)
@ -581,7 +581,7 @@ func (m *nativeGitClient) Submodule() error {
}
// Checkout checks out the specified revision
func (m *nativeGitClient) Checkout(revision string, submoduleEnabled bool) (string, error) {
func (m *nativeGitClient) Checkout(revision string, submoduleEnabled bool, cleanState bool) (string, error) {
if revision == "" || revision == "HEAD" {
revision = "origin/HEAD"
}
@ -609,13 +609,15 @@ func (m *nativeGitClient) Checkout(revision string, submoduleEnabled bool) (stri
}
}
}
// NOTE
// The double “f” in the arguments is not a typo: the first “f” tells
// `git clean` to delete untracked files and directories, and the second “f”
// tells it to clean untracked nested Git repositories (for example a
// submodule which has since been removed).
if out, err := m.runCmd(ctx, "clean", "-ffdx"); err != nil {
return out, fmt.Errorf("failed to clean: %w", err)
if cleanState || submoduleEnabled {
// NOTE
// The double “f” in the arguments is not a typo: the first “f” tells
// `git clean` to delete untracked files and directories, and the second “f”
// tells it to clean untracked nested Git repositories (for example a
// submodule which has since been removed).
if out, err := m.runCmd(ctx, "clean", "-ffdx"); err != nil {
return out, fmt.Errorf("failed to clean: %w", err)
}
}
return "", nil
}
@ -1031,7 +1033,7 @@ func (m *nativeGitClient) SetAuthor(name, email string) (string, error) {
// CheckoutOrOrphan checks out the branch. If the branch does not exist, it creates an orphan branch.
func (m *nativeGitClient) CheckoutOrOrphan(branch string, submoduleEnabled bool) (string, error) {
out, err := m.Checkout(branch, submoduleEnabled)
out, err := m.Checkout(branch, submoduleEnabled, true)
if err != nil {
// If the branch doesn't exist, create it as an orphan branch.
if !strings.Contains(err.Error(), "did not match any file(s) known to git") {
@ -1061,14 +1063,14 @@ func (m *nativeGitClient) CheckoutOrOrphan(branch string, submoduleEnabled bool)
// CheckoutOrNew checks out the given branch. If the branch does not exist, it creates an empty branch based on
// the base branch.
func (m *nativeGitClient) CheckoutOrNew(branch, base string, submoduleEnabled bool) (string, error) {
out, err := m.Checkout(branch, submoduleEnabled)
out, err := m.Checkout(branch, submoduleEnabled, true)
if err != nil {
if !strings.Contains(err.Error(), "did not match any file(s) known to git") {
return out, fmt.Errorf("failed to checkout branch: %w", err)
}
// If the branch does not exist, create any empty branch based on the sync branch
// First, checkout the sync branch.
out, err = m.Checkout(base, submoduleEnabled)
out, err = m.Checkout(base, submoduleEnabled, true)
if err != nil {
return out, fmt.Errorf("failed to checkout sync branch: %w", err)
}

12
util/git/client_test.go
View file

@ -405,7 +405,7 @@ func Test_nativeGitClient_Submodule(t *testing.T) {
require.NoError(t, err)
// Call Checkout() with submoduleEnabled=false.
_, err = client.Checkout(commitSHA, false)
_, err = client.Checkout(commitSHA, false, true)
require.NoError(t, err)
// Check if submodule url does not exist in .git/config
@ -413,7 +413,7 @@ func Test_nativeGitClient_Submodule(t *testing.T) {
require.Error(t, err)
// Call Submodule() via Checkout() with submoduleEnabled=true.
_, err = client.Checkout(commitSHA, true)
_, err = client.Checkout(commitSHA, true, true)
require.NoError(t, err)
// Check if the .gitmodule URL is reflected in .git/config
@ -888,7 +888,7 @@ func Test_nativeGitClient_CommitAndPush(t *testing.T) {
err = client.Fetch(branch, 0)
require.NoError(t, err)
out, err = client.Checkout(branch, false)
out, err = client.Checkout(branch, false, true)
require.NoError(t, err, "error output: ", out)
// make a file then commit and push
@ -1307,7 +1307,7 @@ func Test_nativeGitClient_GetCommitNote(t *testing.T) {
err = client.Fetch(branch, 0)
require.NoError(t, err)
out, err = client.Checkout(branch, false)
out, err = client.Checkout(branch, false, true)
require.NoError(t, err, "error output: ", out)
// Create and commit a test file
@ -1365,7 +1365,7 @@ func Test_nativeGitClient_AddAndPushNote(t *testing.T) {
err = client.Fetch(branch, 0)
require.NoError(t, err)
out, err = client.Checkout(branch, false)
out, err = client.Checkout(branch, false, true)
require.NoError(t, err, "error output: ", out)
// Create and commit a test file
@ -1429,7 +1429,7 @@ func Test_nativeGitClient_HasFileChanged(t *testing.T) {
err = client.Fetch(branch, 0)
require.NoError(t, err)
out, err = client.Checkout(branch, false)
out, err = client.Checkout(branch, false, true)
require.NoError(t, err, "error output: ", out)
// Create the file inside repo root

6
util/git/git_test.go
View file

@ -328,7 +328,7 @@ func TestLFSClient(t *testing.T) {
err = client.Fetch("", 0)
require.NoError(t, err)
_, err = client.Checkout(commitSHA, true)
_, err = client.Checkout(commitSHA, true, true)
require.NoError(t, err)
largeFiles, err := client.LsLargeFiles()
@ -366,7 +366,7 @@ func TestVerifyCommitSignature(t *testing.T) {
commitSHA, err := client.LsRemote("HEAD")
require.NoError(t, err)
_, err = client.Checkout(commitSHA, true)
_, err = client.Checkout(commitSHA, true, true)
require.NoError(t, err)
// 28027897aad1262662096745f2ce2d4c74d02b7f is a commit that is signed in the repo
@ -423,7 +423,7 @@ func TestNewFactory(t *testing.T) {
err = client.Fetch("", 0)
require.NoError(t, err)
_, err = client.Checkout(commitSHA, true)
_, err = client.Checkout(commitSHA, true, true)
require.NoError(t, err)
revisionMetadata, err := client.RevisionMetadata(commitSHA)

30
util/git/mocks/Client.go generated
View file

@ -168,8 +168,8 @@ func (_c *Client_ChangedFiles_Call) RunAndReturn(run func(revision string, targe
}
// Checkout provides a mock function for the type Client
func (_mock *Client) Checkout(revision string, submoduleEnabled bool) (string, error) {
ret := _mock.Called(revision, submoduleEnabled)
func (_mock *Client) Checkout(revision string, submoduleEnabled bool, cleanState bool) (string, error) {
ret := _mock.Called(revision, submoduleEnabled, cleanState)
if len(ret) == 0 {
panic("no return value specified for Checkout")
@ -177,16 +177,16 @@ func (_mock *Client) Checkout(revision string, submoduleEnabled bool) (string, e
var r0 string
var r1 error
if returnFunc, ok := ret.Get(0).(func(string, bool) (string, error)); ok {
return returnFunc(revision, submoduleEnabled)
if returnFunc, ok := ret.Get(0).(func(string, bool, bool) (string, error)); ok {
return returnFunc(revision, submoduleEnabled, cleanState)
}
if returnFunc, ok := ret.Get(0).(func(string, bool) string); ok {
r0 = returnFunc(revision, submoduleEnabled)
if returnFunc, ok := ret.Get(0).(func(string, bool, bool) string); ok {
r0 = returnFunc(revision, submoduleEnabled, cleanState)
} else {
r0 = ret.Get(0).(string)
}
if returnFunc, ok := ret.Get(1).(func(string, bool) error); ok {
r1 = returnFunc(revision, submoduleEnabled)
if returnFunc, ok := ret.Get(1).(func(string, bool, bool) error); ok {
r1 = returnFunc(revision, submoduleEnabled, cleanState)
} else {
r1 = ret.Error(1)
}
@ -201,11 +201,12 @@ type Client_Checkout_Call struct {
// Checkout is a helper method to define mock.On call
// - revision string
// - submoduleEnabled bool
func (_e *Client_Expecter) Checkout(revision interface{}, submoduleEnabled interface{}) *Client_Checkout_Call {
return &Client_Checkout_Call{Call: _e.mock.On("Checkout", revision, submoduleEnabled)}
// - cleanState bool
func (_e *Client_Expecter) Checkout(revision interface{}, submoduleEnabled interface{}, cleanState interface{}) *Client_Checkout_Call {
return &Client_Checkout_Call{Call: _e.mock.On("Checkout", revision, submoduleEnabled, cleanState)}
}
func (_c *Client_Checkout_Call) Run(run func(revision string, submoduleEnabled bool)) *Client_Checkout_Call {
func (_c *Client_Checkout_Call) Run(run func(revision string, submoduleEnabled bool, cleanState bool)) *Client_Checkout_Call {
_c.Call.Run(func(args mock.Arguments) {
var arg0 string
if args[0] != nil {
@ -215,9 +216,14 @@ func (_c *Client_Checkout_Call) Run(run func(revision string, submoduleEnabled b
if args[1] != nil {
arg1 = args[1].(bool)
}
var arg2 bool
if args[2] != nil {
arg2 = args[2].(bool)
}
run(
arg0,
arg1,
arg2,
)
})
return _c
@ -228,7 +234,7 @@ func (_c *Client_Checkout_Call) Return(s string, err error) *Client_Checkout_Cal
return _c
}
func (_c *Client_Checkout_Call) RunAndReturn(run func(revision string, submoduleEnabled bool) (string, error)) *Client_Checkout_Call {
func (_c *Client_Checkout_Call) RunAndReturn(run func(revision string, submoduleEnabled bool, cleanState bool) (string, error)) *Client_Checkout_Call {
_c.Call.Return(run)
return _c
}