feat: Flag to fetch the diff-base before diffing for if_changed

clicommand/pipeline_upload.go

@@ -90,6 +90,7 @@ type PipelineUploadConfig struct {
 	// Used for if_changed processing
 	ApplyIfChanged   bool   `cli:"apply-if-changed"`
 	GitDiffBase      string `cli:"git-diff-base"`
+	FetchDiffBase    bool   `cli:"fetch-diff-base"`
 	ChangedFilesPath string `cli:"changed-files-path"`
 
 	// Used for signing
@@ -146,6 +147,11 @@ var PipelineUploadCommand = cli.Command{
 			Usage:  "Provides the base from which to find the git diff when processing ′if_changed′, e.g. origin/main. If not provided, it uses the first valid value of {origin/$BUILDKITE_PULL_REQUEST_BASE_BRANCH, origin/$BUILDKITE_PIPELINE_DEFAULT_BRANCH, origin/main}.",
 			EnvVar: "BUILDKITE_GIT_DIFF_BASE",
 		},
+		cli.BoolTFlag{
+			Name:   "fetch-diff-base",
+			Usage:  "When enabled, the base for computing the git diff will be git-fetched prior to computing the diff (default: true)",
+			EnvVar: "BUILDKITE_FETCH_DIFF_BASE",
+		},
 		cli.StringFlag{
 			Name:   "changed-files-path",
 			Usage:  "Path to a file containing the list of changed files (newline-separated) to use for ′if_changed′ evaluation. When provided, the agent skips running git commands to determine changed files.",
@@ -306,6 +312,7 @@ var PipelineUploadCommand = cli.Command{
 				prependOriginIfNonempty("BUILDKITE_PIPELINE_DEFAULT_BRANCH"),
 				defaultGitDiffBase,
 			),
+			fetch:            cfg.FetchDiffBase,
 			changedFilesPath: cfg.ChangedFilesPath,
 		}
 
@@ -639,8 +646,8 @@ func readChangedFilesFromPath(l logger.Logger, path string) ([]string, error) {
 	return changedPaths, nil
 }
 
-// gatherChangedFiles determines changed files in this build.
-func gatherChangedFiles(l logger.Logger, diffBase string) (changedPaths []string, err error) {
+// computeGitDiff determines changed files in this build.
+func computeGitDiff(l logger.Logger, diffBase string) (changedPaths []string, err error) {
 	// Corporate needs you to find the differences between diffBase and HEAD.
 	diffBaseCommit, err := exec.Command("git", "rev-parse", diffBase).Output()
 	if err != nil {
@@ -750,6 +757,7 @@ type ifChangedApplicator struct {
 	enabled          bool // apply-if-changed is enabled
 	gathered         bool // the changed files have been computed?
 	diffBase         string
+	fetch            bool   // fetch diffBase before computing diff?
 	changedFilesPath string // path to a file containing newline-separated changed files
 	changedPaths     []string
 }
@@ -799,52 +807,12 @@ stepsLoop:
 
 		// If we don't know the changed paths yet, either read from file or call out to Git.
 		if !ica.gathered {
-			var cps []string
-			var err error
-
-			if ica.changedFilesPath != "" {
-				// Read changed files from the provided file path.
-				cps, err = readChangedFilesFromPath(l, ica.changedFilesPath)
-				if err != nil {
-					l.Error("Couldn't read changed files from %q, not skipping any pipeline steps: %v", ica.changedFilesPath, err)
-					ica.enabled = false
-					continue stepsLoop
-				}
-			} else {
-				// Determine changed files using git.
-				cps, err = gatherChangedFiles(l, ica.diffBase)
-				if err != nil {
-					l.Error("Couldn't determine git diff from upstream, not skipping any pipeline steps: %v", err)
-					var exitErr *exec.ExitError
-					if errors.As(err, &exitErr) && len(exitErr.Stderr) > 0 {
-						// stderr came from git, which is typically human readable
-						l.Error("git: %s", exitErr.Stderr)
-					}
-					switch err := err.(type) {
-					case gitRevParseError:
-						l.Error("This could be because %q might not be a commit in the repository.\n"+
-							"You may need to change the --git-diff-base flag or BUILDKITE_GIT_DIFF_BASE env var.",
-							err.arg,
-						)
-
-					case gitMergeBaseError:
-						l.Error("This could be because %q might not be a commit in the repository.\n"+
-							"You may need to change the --git-diff-base flag or BUILDKITE_GIT_DIFF_BASE env var.",
-							err.diffBase,
-						)
-
-					case gitDiffError:
-						l.Error("This could be because the merge-base that Git found, %q, might be invalid.\n"+
-							"You may need to change the --git-diff-base flag or BUILDKITE_GIT_DIFF_BASE env var.",
-							err.mergeBase,
-						)
-					}
-
-					// Because changed files couldn't be determined, we switch into
-					// disabled mode.
-					ica.enabled = false
-					continue stepsLoop
-				}
+			cps, err := ica.gatherChangedPaths(l)
+			if err != nil {
+				// Because changed files couldn't be determined, we switch into
+				// disabled mode.
+				ica.enabled = false
+				continue stepsLoop
 			}
 
 			// The changed files are now known.
@@ -916,6 +884,69 @@ stepsLoop:
 	}
 }
 
+func (ica *ifChangedApplicator) gatherChangedPaths(l logger.Logger) ([]string, error) {
+	if ica.changedFilesPath != "" {
+		// Read changed files from the provided file path.
+		cps, err := readChangedFilesFromPath(l, ica.changedFilesPath)
+		if err != nil {
+			l.Error("Couldn't read changed files from %q, not skipping any pipeline steps: %v", ica.changedFilesPath, err)
+			return nil, err
+		}
+		return cps, nil
+	}
+
+	if ica.fetch {
+		// First, fetch the remote refspec specified by diffBase.
+		remote, refspec, slash := strings.Cut(ica.diffBase, "/")
+		if !slash {
+			l.Warn("The diff-base %q was not in 'remote/refspec' form - continuing with the remote 'origin'", ica.diffBase)
+			remote = "origin"
+			refspec = ica.diffBase
+		}
+		if err := exec.Command("git", "fetch", "--", remote, refspec).Run(); err != nil {
+			l.Error("Couldn't fetch %q from origin: %v", err)
+			var exitErr *exec.ExitError
+			if errors.As(err, &exitErr) && len(exitErr.Stderr) > 0 {
+				// stderr came from git, which is typically human readable
+				l.Error("git: %s", exitErr.Stderr)
+			}
+			l.Info("if_changed will continue processing, but the diff may fail, or produce more paths than expected.")
+		}
+	}
+
+	// Determine changed files using git.
+	cps, err := computeGitDiff(l, ica.diffBase)
+	if err != nil {
+		l.Error("Couldn't determine git diff from upstream, not skipping any pipeline steps: %v", err)
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) && len(exitErr.Stderr) > 0 {
+			// stderr came from git, which is typically human readable
+			l.Error("git: %s", exitErr.Stderr)
+		}
+		switch err := err.(type) {
+		case gitRevParseError:
+			l.Error("This could be because %q might not be a commit in the repository.\n"+
+				"You may need to change the --git-diff-base flag or BUILDKITE_GIT_DIFF_BASE env var, or add --fetch-diff-base.",
+				err.arg,
+			)
+
+		case gitMergeBaseError:
+			l.Error("This could be because %q might not be a commit in the repository.\n"+
+				"You may need to change the --git-diff-base flag or BUILDKITE_GIT_DIFF_BASE env var, or add --fetch-diff-base.",
+				err.diffBase,
+			)
+
+		case gitDiffError:
+			l.Error("This could be because the merge-base that Git found, %q, might be invalid.\n"+
+				"You may need to change the --git-diff-base flag or BUILDKITE_GIT_DIFF_BASE env var, or add --fetch-diff-base.",
+				err.mergeBase,
+			)
+		}
+		return nil, err
+	}
+	return cps, nil
+}
+
 // ifChangedPatterns converts a string or list within `if_changed` into a slice
 // of parsed globs.
 func ifChangedPatterns(value any) ([]*zzglob.Pattern, error) {