fix: always prune on git fetch (argoproj#10664)

This should clean up any old branches and save on disk space, and fix any errors around bad branch names. Signed-off-by: Daniel Hix <danieljacobhix@gmail.com> Signed-off-by: Daniel Hix <danieljacobhix@gmail.com>
ashutosh16 · Oct 7, 2022 · f1c710f · f1c710f
1 parent 0b4a038
commit f1c710f
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 96 deletions.
diff --git a/applicationset/services/repo_service_test.go b/applicationset/services/repo_service_test.go
@@ -163,7 +163,7 @@ func TestGetFiles(t *testing.T) {
 			revision:            "this-tag-does-not-exist",
 			pattern:             "*",
 			expectSubsetOfPaths: []string{},
-			expectedError:       fmt.Errorf("Error during fetching repo: `git fetch origin this-tag-does-not-exist --tags --force` failed exit status 128: fatal: couldn't find remote ref this-tag-does-not-exist"),
+			expectedError:       fmt.Errorf("Error during fetching repo: `git fetch origin this-tag-does-not-exist --tags --force --prune` failed exit status 128: fatal: couldn't find remote ref this-tag-does-not-exist"),
 		},
 		{
 			name: "pull a specific revision of example apps, and use a ** pattern",

diff --git a/util/git/client.go b/util/git/client.go
@@ -320,9 +320,9 @@ func (m *nativeGitClient) IsLFSEnabled() bool {
 func (m *nativeGitClient) fetch(revision string) error {
 	var err error
 	if revision != "" {
-		err = m.runCredentialedCmd("git", "fetch", "origin", revision, "--tags", "--force")
+		err = m.runCredentialedCmd("git", "fetch", "origin", revision, "--tags", "--force", "--prune")
 	} else {
-		err = m.runCredentialedCmd("git", "fetch", "origin", "--tags", "--force")
+		err = m.runCredentialedCmd("git", "fetch", "origin", "--tags", "--force", "--prune")
 	}
 	return err
 }
@@ -334,19 +334,7 @@ func (m *nativeGitClient) Fetch(revision string) error {
 		defer done()
 	}
 
-	var err error
-
-	err = m.fetch(revision)
-	if err != nil {
-		errMsg := strings.ReplaceAll(err.Error(), "\n", "")
-		if strings.Contains(errMsg, "try running 'git remote prune origin'") {
-			// Prune any deleted refs, then try fetching again
-			if err := m.runCredentialedCmd("git", "remote", "prune", "origin"); err != nil {
-				return err
-			}
-			err = m.fetch(revision)
-		}
-	}
+	err := m.fetch(revision)
 
 	// When we have LFS support enabled, check for large files and fetch them too.
 	if err == nil && m.IsLFSEnabled() {

diff --git a/util/git/client_test.go b/util/git/client_test.go
@@ -11,22 +11,22 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func runCmd(workingDir string, name string, args ...string) error {
+	cmd := exec.Command(name, args...)
+	cmd.Dir = workingDir
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+	return cmd.Run()
+}
+
 func Test_nativeGitClient_Fetch(t *testing.T) {
 	tempDir, err := os.MkdirTemp("", "")
 	require.NoError(t, err)
 
-	cmd := exec.Command("git", "init")
-	cmd.Dir = tempDir
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(tempDir, "git", "init")
 	require.NoError(t, err)
 
-	cmd = exec.Command("git", "commit", "-m", "Initial commit", "--allow-empty")
-	cmd.Dir = tempDir
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(tempDir, "git", "commit", "-m", "Initial commit", "--allow-empty")
 	require.NoError(t, err)
 
 	client, err := NewClient(fmt.Sprintf("file://%s", tempDir), NopCreds{}, true, false, "")
@@ -43,18 +43,10 @@ func Test_nativeGitClient_Fetch_Prune(t *testing.T) {
 	tempDir, err := os.MkdirTemp("", "")
 	require.NoError(t, err)
 
-	cmd := exec.Command("git", "init")
-	cmd.Dir = tempDir
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(tempDir, "git", "init")
 	require.NoError(t, err)
 
-	cmd = exec.Command("git", "commit", "-m", "Initial commit", "--allow-empty")
-	cmd.Dir = tempDir
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(tempDir, "git", "commit", "-m", "Initial commit", "--allow-empty")
 	require.NoError(t, err)
 
 	client, err := NewClient(fmt.Sprintf("file://%s", tempDir), NopCreds{}, true, false, "")
@@ -63,31 +55,15 @@ func Test_nativeGitClient_Fetch_Prune(t *testing.T) {
 	err = client.Init()
 	require.NoError(t, err)
 
-	// Create branch
-	cmd = exec.Command("git", "branch", "test/foo")
-	cmd.Dir = tempDir
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(tempDir, "git", "branch", "test/foo")
 	require.NoError(t, err)
 
 	err = client.Fetch("")
 	assert.NoError(t, err)
 
-	// Delete branch
-	cmd = exec.Command("git", "branch", "-d", "test/foo")
-	cmd.Dir = tempDir
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(tempDir, "git", "branch", "-d", "test/foo")
 	require.NoError(t, err)
-
-	// Create branch that conflicts with previous branch name
-	cmd = exec.Command("git", "branch", "test/foo/bar")
-	cmd.Dir = tempDir
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(tempDir, "git", "branch", "test/foo/bar")
 	require.NoError(t, err)
 
 	err = client.Fetch("")
@@ -102,55 +78,31 @@ func Test_nativeGitClient_Submodule(t *testing.T) {
 	err = os.Mkdir(foo, 0755)
 	require.NoError(t, err)
 
-	cmd := exec.Command("git", "init")
-	cmd.Dir = foo
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(foo, "git", "init")
 	require.NoError(t, err)
 
 	bar := filepath.Join(tempDir, "bar")
 	err = os.Mkdir(bar, 0755)
 	require.NoError(t, err)
 
-	cmd = exec.Command("git", "init")
-	cmd.Dir = bar
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(bar, "git", "init")
 	require.NoError(t, err)
 
-	cmd = exec.Command("git", "commit", "-m", "Initial commit", "--allow-empty")
-	cmd.Dir = bar
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(bar, "git", "commit", "-m", "Initial commit", "--allow-empty")
 	require.NoError(t, err)
 
 	// Embed repository bar into repository foo
-	cmd = exec.Command("git", "submodule", "add", bar)
-	cmd.Dir = foo
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(foo, "git", "submodule", "add", bar)
 	require.NoError(t, err)
 
-	cmd = exec.Command("git", "commit", "-m", "Initial commit")
-	cmd.Dir = foo
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(foo, "git", "commit", "-m", "Initial commit")
 	require.NoError(t, err)
 
 	tempDir, err = os.MkdirTemp("", "")
 	require.NoError(t, err)
 
 	// Clone foo
-	cmd = exec.Command("git", "clone", foo)
-	cmd.Dir = tempDir
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(tempDir, "git", "clone", foo)
 	require.NoError(t, err)
 
 	client, err := NewClient(fmt.Sprintf("file://%s", foo), NopCreds{}, true, false, "")
@@ -170,30 +122,22 @@ func Test_nativeGitClient_Submodule(t *testing.T) {
 	assert.NoError(t, err)
 
 	// Check if submodule url does not exist in .git/config
-	cmd = exec.Command("git", "config", "submodule.bar.url")
-	cmd.Dir = client.Root()
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(client.Root(), "git", "config", "submodule.bar.url")
 	assert.Error(t, err)
 
 	// Call Submodule() via Checkout() with submoduleEnabled=true.
 	err = client.Checkout(commitSHA, true)
 	assert.NoError(t, err)
 
 	// Check if the .gitmodule URL is reflected in .git/config
-	cmd = exec.Command("git", "config", "submodule.bar.url")
+	cmd := exec.Command("git", "config", "submodule.bar.url")
 	cmd.Dir = client.Root()
 	result, err := cmd.Output()
 	assert.NoError(t, err)
 	assert.Equal(t, bar+"\n", string(result))
 
 	// Change URL of submodule bar
-	cmd = exec.Command("git", "config", "--file=.gitmodules", "submodule.bar.url", bar+"baz")
-	cmd.Dir = client.Root()
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err = runCmd(client.Root(), "git", "config", "--file=.gitmodules", "submodule.bar.url", bar+"baz")
 	require.NoError(t, err)
 
 	// Call Submodule()