Jib builder should not reuse commands

Fix #2206

Signed-off-by: David Gageot <david@gageot.net>

pkg/skaffold/build/local/jib_gradle.go

@@ -60,7 +60,7 @@ func (b *Builder) runGradleCommand(ctx context.Context, out io.Writer, workspace
 	cmd.Stderr = out
 
 	logrus.Infof("Building %s: %s, %v", workspace, cmd.Path, cmd.Args)
-	if err := util.RunCmd(cmd); err != nil {
+	if err := util.RunCmd(&cmd); err != nil {
 		return errors.Wrap(err, "gradle build failed")
 	}
 

pkg/skaffold/build/local/jib_maven.go

@@ -78,7 +78,7 @@ func verifyJibPackageGoal(ctx context.Context, requiredGoal string, workspace st
 
 	cmd := jib.MavenCommand.CreateCommand(ctx, workspace, command)
 	logrus.Debugf("Looking for jib bound package goals for %s: %s, %v", workspace, cmd.Path, cmd.Args)
-	stdout, err := util.RunCmdOut(cmd)
+	stdout, err := util.RunCmdOut(&cmd)
 	if err != nil {
 		return errors.Wrap(err, "could not obtain jib package goals")
 	}
@@ -100,7 +100,7 @@ func (b *Builder) runMavenCommand(ctx context.Context, out io.Writer, workspace
 	cmd.Stderr = out
 
 	logrus.Infof("Building %s: %s, %v", workspace, cmd.Path, cmd.Args)
-	if err := util.RunCmd(cmd); err != nil {
+	if err := util.RunCmd(&cmd); err != nil {
 		return errors.Wrap(err, "maven build failed")
 	}
 

pkg/skaffold/jib/jib.go

@@ -56,7 +56,7 @@ type filesLists struct {
 var watchedFiles = map[string]filesLists{}
 
 // getDependencies returns a list of files to watch for changes to rebuild
-func getDependencies(workspace string, cmd *exec.Cmd, projectName string) ([]string, error) {
+func getDependencies(workspace string, cmd exec.Cmd, projectName string) ([]string, error) {
 	var dependencyList []string
 	files, ok := watchedFiles[projectName]
 	if !ok {
@@ -107,8 +107,8 @@ func getDependencies(workspace string, cmd *exec.Cmd, projectName string) ([]str
 }
 
 // refreshDependencyList calls out to Jib to update files with the latest list of files/directories to watch.
-func refreshDependencyList(files *filesLists, cmd *exec.Cmd) error {
-	stdout, err := util.RunCmdOut(cmd)
+func refreshDependencyList(files *filesLists, cmd exec.Cmd) error {
+	stdout, err := util.RunCmdOut(&cmd)
 	if err != nil {
 		return errors.Wrap(err, "failed to get Jib dependencies; it's possible you are using an old version of Jib (Skaffold requires Jib v1.0.2+)")
 	}

pkg/skaffold/jib/jib_gradle.go

@@ -42,7 +42,7 @@ func GetDependenciesGradle(ctx context.Context, workspace string, a *latest.JibG
 	return deps, nil
 }
 
-func getCommandGradle(ctx context.Context, workspace string, a *latest.JibGradleArtifact) *exec.Cmd {
+func getCommandGradle(ctx context.Context, workspace string, a *latest.JibGradleArtifact) exec.Cmd {
 	args := []string{gradleCommand(a, "_jibSkaffoldFilesV2"), "-q"}
 	return GradleCommand.CreateCommand(ctx, workspace, args)
 }

pkg/skaffold/jib/jib_gradle_test.go

@@ -111,37 +111,37 @@ func TestGetCommandGradle(t *testing.T) {
 		description       string
 		jibGradleArtifact latest.JibGradleArtifact
 		filesInWorkspace  []string
-		expectedCmd       func(workspace string) *exec.Cmd
+		expectedCmd       func(workspace string) exec.Cmd
 	}{
 		{
 			description:       "gradle default",
 			jibGradleArtifact: latest.JibGradleArtifact{},
 			filesInWorkspace:  []string{},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return GradleCommand.CreateCommand(ctx, workspace, []string{":_jibSkaffoldFilesV2", "-q"})
 			},
 		},
 		{
 			description:       "gradle default with project",
 			jibGradleArtifact: latest.JibGradleArtifact{Project: "project"},
 			filesInWorkspace:  []string{},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return GradleCommand.CreateCommand(ctx, workspace, []string{":project:_jibSkaffoldFilesV2", "-q"})
 			},
 		},
 		{
 			description:       "gradle with wrapper",
 			jibGradleArtifact: latest.JibGradleArtifact{},
 			filesInWorkspace:  []string{"gradlew", "gradlew.cmd"},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return GradleCommand.CreateCommand(ctx, workspace, []string{":_jibSkaffoldFilesV2", "-q"})
 			},
 		},
 		{
 			description:       "gradle with wrapper and project",
 			jibGradleArtifact: latest.JibGradleArtifact{Project: "project"},
 			filesInWorkspace:  []string{"gradlew", "gradlew.cmd"},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return GradleCommand.CreateCommand(ctx, workspace, []string{":project:_jibSkaffoldFilesV2", "-q"})
 			},
 		},

pkg/skaffold/jib/jib_maven.go

@@ -40,7 +40,7 @@ func GetDependenciesMaven(ctx context.Context, workspace string, a *latest.JibMa
 	return deps, nil
 }
 
-func getCommandMaven(ctx context.Context, workspace string, a *latest.JibMavenArtifact) *exec.Cmd {
+func getCommandMaven(ctx context.Context, workspace string, a *latest.JibMavenArtifact) exec.Cmd {
 	args := mavenArgs(a)
 	args = append(args, "jib:_skaffold-files-v2", "--quiet")
 

pkg/skaffold/jib/jib_maven_test.go

@@ -110,13 +110,13 @@ func TestGetCommandMaven(t *testing.T) {
 		description      string
 		jibMavenArtifact latest.JibMavenArtifact
 		filesInWorkspace []string
-		expectedCmd      func(workspace string) *exec.Cmd
+		expectedCmd      func(workspace string) exec.Cmd
 	}{
 		{
 			description:      "maven no profile",
 			jibMavenArtifact: latest.JibMavenArtifact{},
 			filesInWorkspace: []string{},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return MavenCommand.CreateCommand(ctx, workspace, []string{"--non-recursive", "jib:_skaffold-files-v2", "--quiet"})
 			},
 		},
@@ -126,47 +126,47 @@ func TestGetCommandMaven(t *testing.T) {
 				Flags: []string{"-DskipTests", "-x"},
 			},
 			filesInWorkspace: []string{},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return MavenCommand.CreateCommand(ctx, workspace, []string{"-DskipTests", "-x", "--non-recursive", "jib:_skaffold-files-v2", "--quiet"})
 			},
 		},
 		{
 			description:      "maven with profile",
 			jibMavenArtifact: latest.JibMavenArtifact{Profile: "profile"},
 			filesInWorkspace: []string{},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return MavenCommand.CreateCommand(ctx, workspace, []string{"--activate-profiles", "profile", "--non-recursive", "jib:_skaffold-files-v2", "--quiet"})
 			},
 		},
 		{
 			description:      "maven with wrapper no profile",
 			jibMavenArtifact: latest.JibMavenArtifact{},
 			filesInWorkspace: []string{"mvnw", "mvnw.bat"},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return MavenCommand.CreateCommand(ctx, workspace, []string{"--non-recursive", "jib:_skaffold-files-v2", "--quiet"})
 			},
 		},
 		{
 			description:      "maven with wrapper no profile",
 			jibMavenArtifact: latest.JibMavenArtifact{},
 			filesInWorkspace: []string{"mvnw", "mvnw.cmd"},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return MavenCommand.CreateCommand(ctx, workspace, []string{"--non-recursive", "jib:_skaffold-files-v2", "--quiet"})
 			},
 		},
 		{
 			description:      "maven with wrapper and profile",
 			jibMavenArtifact: latest.JibMavenArtifact{Profile: "profile"},
 			filesInWorkspace: []string{"mvnw", "mvnw.bat"},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return MavenCommand.CreateCommand(ctx, workspace, []string{"--activate-profiles", "profile", "--non-recursive", "jib:_skaffold-files-v2", "--quiet"})
 			},
 		},
 		{
 			description:      "maven with multi-modules",
 			jibMavenArtifact: latest.JibMavenArtifact{Module: "module"},
 			filesInWorkspace: []string{"mvnw", "mvnw.bat"},
-			expectedCmd: func(workspace string) *exec.Cmd {
+			expectedCmd: func(workspace string) exec.Cmd {
 				return MavenCommand.CreateCommand(ctx, workspace, []string{"--projects", "module", "--also-make", "jib:_skaffold-files-v2", "--quiet"})
 			},
 		},

pkg/skaffold/jib/jib_test.go

@@ -83,18 +83,41 @@ func TestGetDependencies(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		// Reset map between each test to ensure stdout is read each time
-		watchedFiles = map[string]filesLists{}
-
 		testutil.Run(t, "", func(t *testutil.T) {
-			t.Override(&util.DefaultExecCommand, t.FakeRunOut(
-				"ignored",
-				test.stdout,
-			))
+			t.Override(&util.DefaultExecCommand, t.FakeRunOut("ignored", test.stdout))
 
-			results, err := getDependencies(tmpDir.Root(), &exec.Cmd{Args: []string{"ignored"}, Dir: tmpDir.Root()}, "test")
+			results, err := getDependencies(tmpDir.Root(), exec.Cmd{Args: []string{"ignored"}, Dir: tmpDir.Root()}, util.RandomID())
 
 			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedDeps, results)
 		})
 	}
 }
+
+func TestGetUpdatedDependencies(t *testing.T) {
+	testutil.Run(t, "Both build definitions are created at the same time", func(t *testutil.T) {
+		tmpDir := t.NewTempDir()
+
+		stdout := fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\",\"%s\"],\"inputs\":[],\"ignore\":[]}\n", tmpDir.Path("build.gradle"), tmpDir.Path("settings.gradle"))
+		t.Override(&util.DefaultExecCommand, t.
+			FakeRunOut("ignored", stdout).
+			WithRunOut("ignored", stdout).
+			WithRunOut("ignored", stdout),
+		)
+
+		listCmd := exec.Cmd{Args: []string{"ignored"}, Dir: tmpDir.Root()}
+		projectID := util.RandomID()
+
+		// List dependencies
+		_, err := getDependencies(tmpDir.Root(), listCmd, projectID)
+		t.CheckNoError(err)
+
+		// Create new build definition files
+		tmpDir.
+			Write("build.gradle", "").
+			Write("settings.gradle", "")
+
+		// Update dependencies
+		_, err = getDependencies(tmpDir.Root(), listCmd, projectID)
+		t.CheckNoError(err)
+	})
+}

pkg/skaffold/util/wrapper_unix.go

@@ -28,7 +28,7 @@ import (
 // CreateCommand creates an `exec.Cmd` that is configured to call the
 // executable (possibly using a wrapper in `workingDir`, when found) with the given arguments,
 // with working directory set to `workingDir`.
-func (cw CommandWrapper) CreateCommand(ctx context.Context, workingDir string, args []string) *exec.Cmd {
+func (cw CommandWrapper) CreateCommand(ctx context.Context, workingDir string, args []string) exec.Cmd {
 	executable := cw.Executable
 
 	if cw.Wrapper != "" && !SkipWrapperCheck {
@@ -40,5 +40,5 @@ func (cw CommandWrapper) CreateCommand(ctx context.Context, workingDir string, a
 
 	cmd := exec.CommandContext(ctx, executable, args...)
 	cmd.Dir = workingDir
-	return cmd
+	return *cmd
 }

pkg/skaffold/util/wrapper_windows.go

@@ -26,7 +26,7 @@ import (
 // CreateCommand creates an `exec.Cmd` that is configured to call the
 // executable (possibly using a wrapper in `workingDir`, when found) with the given arguments,
 // with working directory set to `workingDir`.
-func (cw CommandWrapper) CreateCommand(ctx context.Context, workingDir string, args []string) *exec.Cmd {
+func (cw CommandWrapper) CreateCommand(ctx context.Context, workingDir string, args []string) exec.Cmd {
 	executable := cw.Executable
 
 	if cw.Wrapper != "" && !SkipWrapperCheck {
@@ -43,5 +43,5 @@ func (cw CommandWrapper) CreateCommand(ctx context.Context, workingDir string, a
 
 	cmd := exec.CommandContext(ctx, executable, args...)
 	cmd.Dir = workingDir
-	return cmd
+	return *cmd
 }