From ff5088adc87bc05650dc158871fd4119f15f3a16 Mon Sep 17 00:00:00 2001
From: Fabian Holler <fabian.holler@simplesurance.de>
Date: Tue, 16 Jan 2024 18:07:37 +0100
Subject: [PATCH] add --require-clean-git-worktree flag to run and status
 commands

Add a new flag to "baur run" and "baur status" called
--require-clean-git-worktree.
If passed, the commands fail if the baur repository contains untracked or
modified tracked git files, this includes files that are in .gitignore.

"baur run", checks the state on start and before executing a task, to catch
cases were multiple tasks are run and of them modifies or creates new files in
the git repository.

This helps to ensure that tasks are executed in an clean reproducible
environment.
---
 internal/command/helpers.go         | 30 ++++++++++++++++++++
 internal/command/run.go             | 43 +++++++++++++++++++++++------
 internal/command/run_test.go        | 20 ++++++++++++++
 internal/command/status.go          | 23 +++++++++------
 internal/command/status_test.go     | 20 ++++++++++++++
 internal/command/term/streams.go    | 11 ++++++--
 internal/prettyprint/prettyprint.go | 12 ++++++++
 internal/vcs/git/git.go             |  4 +--
 internal/vcs/git/repository.go      | 12 ++++++++
 internal/vcs/novcsstate.go          |  8 ++++++
 internal/vcs/state.go               |  2 ++
 pkg/baur/taskrunner.go              | 25 +++++++++++++----
 pkg/baur/taskrunner_test.go         | 17 ++++++++++++
 13 files changed, 200 insertions(+), 27 deletions(-)
 create mode 100644 pkg/baur/taskrunner_test.go

diff --git a/internal/command/helpers.go b/internal/command/helpers.go
index f725bc9ca..4f2a718f9 100644
--- a/internal/command/helpers.go
+++ b/internal/command/helpers.go
@@ -9,7 +9,9 @@ import (
 	"github.com/simplesurance/baur/v3/internal/command/term"
 	"github.com/simplesurance/baur/v3/internal/format"
 	"github.com/simplesurance/baur/v3/internal/log"
+	"github.com/simplesurance/baur/v3/internal/prettyprint"
 	"github.com/simplesurance/baur/v3/internal/vcs"
+	"github.com/simplesurance/baur/v3/internal/vcs/git"
 	"github.com/simplesurance/baur/v3/pkg/baur"
 	"github.com/simplesurance/baur/v3/pkg/cfg"
 	"github.com/simplesurance/baur/v3/pkg/storage"
@@ -218,6 +220,11 @@ func exitOnErrf(err error, format string, v ...interface{}) {
 	exitFunc(1)
 }
 
+func fatal(msg ...interface{}) {
+	stderr.PrintErrln(msg...)
+	exitFunc(1)
+}
+
 func exitOnErr(err error, msg ...interface{}) {
 	if err == nil {
 		return
@@ -247,3 +254,26 @@ func subStr(input string, start int, length int) string {
 
 	return string(asRunes[start : start+length])
 }
+
+func mustUntrackedFilesNotExist(requireCleanGitWorktree bool, vcsState vcs.StateFetcher) {
+	if !requireCleanGitWorktree {
+		return
+	}
+
+	if vcsState.Name() != git.Name {
+		exitOnErr(
+			fmt.Errorf("--%s was specified but baur repository is not a git repository", flagNameRequireCleanGitWorktree),
+		)
+	}
+
+	untracked, err := vcsState.UntrackedFiles()
+	exitOnErr(err)
+	if len(untracked) != 0 {
+		fatal(untrackedFilesExistErrMsg(untracked))
+	}
+}
+
+func untrackedFilesExistErrMsg(untrackedFiles []string) string {
+	return fmt.Sprintf("%s was specified, expecting only tracked unmodified files but found the following untracked or modified files:\n%s",
+		term.Highlight("--"+flagNameRequireCleanGitWorktree), term.Highlight(prettyprint.TruncatedStrSlice(untrackedFiles, 10)))
+}
diff --git a/internal/command/run.go b/internal/command/run.go
index 69c9c7eea..827990cb4 100644
--- a/internal/command/run.go
+++ b/internal/command/run.go
@@ -19,6 +19,7 @@ import (
 	"github.com/simplesurance/baur/v3/internal/upload/filecopy"
 	"github.com/simplesurance/baur/v3/internal/upload/s3"
 	"github.com/simplesurance/baur/v3/internal/vcs"
+	"github.com/simplesurance/baur/v3/internal/vcs/git"
 	"github.com/simplesurance/baur/v3/pkg/baur"
 	"github.com/simplesurance/baur/v3/pkg/storage"
 )
@@ -30,6 +31,8 @@ baur run *.build			run all tasks named build of the all applications and upload
 baur run --force			run and upload all tasks of applications, independent of their status
 `
 
+const flagNameRequireCleanGitWorktree = "require-clean-git-worktree"
+
 var runLongHelp = fmt.Sprintf(`
 Execute tasks of applications.
 
@@ -80,12 +83,13 @@ type runCmd struct {
 	cobra.Command
 
 	// Cmdline parameters
-	skipUpload           bool
-	force                bool
-	inputStr             []string
-	lookupInputStr       string
-	taskRunnerGoRoutines uint
-	showOutput           bool
+	skipUpload              bool
+	force                   bool
+	inputStr                []string
+	lookupInputStr          string
+	taskRunnerGoRoutines    uint
+	showOutput              bool
+	requireCleanGitWorktree bool
 
 	// other fields
 	storage      storage.Storer
@@ -127,8 +131,10 @@ func newRunCmd() *runCmd {
 		"specifies the max. number of tasks to run in parallel")
 	cmd.Flags().BoolVarP(&cmd.showOutput, "show-task-output", "o", false,
 		"show the output of tasks, if disabled the output is only shown "+
-			"when task execution failed",
+			"when task execution fails",
 	)
+	cmd.Flags().BoolVarP(&cmd.requireCleanGitWorktree, flagNameRequireCleanGitWorktree, "c", false,
+		"fail if the git repository contains modified or untracked files")
 
 	return &cmd
 }
@@ -146,6 +152,10 @@ func (c *runCmd) run(_ *cobra.Command, args []string) {
 	repo := mustFindRepository()
 	c.repoRootPath = repo.Path
 
+	c.vcsState = mustGetRepoState(repo.Path)
+
+	mustUntrackedFilesNotExist(c.requireCleanGitWorktree, c.vcsState)
+
 	c.storage = mustNewCompatibleStorage(repo)
 	defer c.storage.Close()
 
@@ -159,6 +169,10 @@ func (c *runCmd) run(_ *cobra.Command, args []string) {
 		c.taskRunner.LogFn = stderr.Printf
 	}
 
+	if c.requireCleanGitWorktree {
+		c.taskRunner.GitUntrackedFilesFn = git.UntrackedFiles
+	}
+
 	c.dockerClient, err = docker.NewClient(log.StdLogger.Debugf)
 	exitOnErr(err)
 
@@ -166,8 +180,6 @@ func (c *runCmd) run(_ *cobra.Command, args []string) {
 	exitOnErr(err)
 	c.uploader = baur.NewUploader(c.dockerClient, s3Client, filecopy.New(log.Debugf))
 
-	c.vcsState = mustGetRepoState(repo.Path)
-
 	if c.skipUpload {
 		stdout.Printf("--skip-upload was passed, outputs won't be uploaded and task runs not recorded\n\n")
 	}
@@ -183,6 +195,13 @@ func (c *runCmd) run(_ *cobra.Command, args []string) {
 	pendingTasks, err := c.filterPendingTasks(tasks)
 	exitOnErr(err)
 
+	if c.requireCleanGitWorktree && len(pendingTasks) == 1 {
+		// if we only execute 1 task, the initial worktree check is
+		// sufficient, no other tasks ran in between that could have
+		// modified the worktree
+		c.taskRunner.GitUntrackedFilesFn = nil
+	}
+
 	stdout.PrintSep()
 
 	if c.force {
@@ -300,6 +319,12 @@ func (c *runCmd) runTask(task *baur.Task) (*baur.RunResult, error) {
 		return nil, err
 	}
 
+	var eUntracked *baur.ErrUntrackedGitFilesExist
+	if errors.As(err, &eUntracked) {
+		stderr.Println(untrackedFilesExistErrMsg(eUntracked.UntrackedFiles))
+		return nil, err
+	}
+
 	stderr.Printf("%s: executing %q %s: %s\n",
 		term.Highlight(task),
 		term.Highlight(result.Command),
diff --git a/internal/command/run_test.go b/internal/command/run_test.go
index efbe1c56c..ebfcfd355 100644
--- a/internal/command/run_test.go
+++ b/internal/command/run_test.go
@@ -14,6 +14,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/simplesurance/baur/v3/internal/testutils/fstest"
+	"github.com/simplesurance/baur/v3/internal/testutils/gittest"
 	"github.com/simplesurance/baur/v3/internal/testutils/repotest"
 	"github.com/simplesurance/baur/v3/pkg/cfg"
 )
@@ -350,3 +352,21 @@ func TestEnvVarInput_Optional(t *testing.T) {
 		})
 	})
 }
+
+func TestRunFailsWhenGitWorktreeIsDirty(t *testing.T) {
+	initTest(t)
+
+	r := repotest.CreateBaurRepository(t, repotest.WithNewDB())
+	gittest.CreateRepository(t, r.Dir)
+	r.CreateSimpleApp(t)
+	fname := "untrackedFile"
+	fstest.WriteToFile(t, []byte("hello"), filepath.Join(r.Dir, fname))
+
+	_, stderrBuf := interceptCmdOutput(t)
+	runCmd := newRunCmd()
+	runCmd.SetArgs([]string{"--" + flagNameRequireCleanGitWorktree})
+	require.Panics(t, func() { require.NoError(t, runCmd.Execute()) })
+
+	require.Contains(t, stderrBuf.String(), fname)
+	require.Contains(t, stderrBuf.String(), "expecting only tracked unmodified files")
+}
diff --git a/internal/command/status.go b/internal/command/status.go
index 523f99188..80352c5f1 100644
--- a/internal/command/status.go
+++ b/internal/command/status.go
@@ -44,13 +44,14 @@ func init() {
 type statusCmd struct {
 	cobra.Command
 
-	csv            bool
-	quiet          bool
-	absPaths       bool
-	inputStr       []string
-	lookupInputStr string
-	buildStatus    flag.TaskStatus
-	fields         *flag.Fields
+	csv                     bool
+	quiet                   bool
+	absPaths                bool
+	inputStr                []string
+	lookupInputStr          string
+	buildStatus             flag.TaskStatus
+	fields                  *flag.Fields
+	requireCleanGitWorktree bool
 }
 
 func newStatusCmd() *statusCmd {
@@ -104,6 +105,9 @@ func newStatusCmd() *statusCmd {
 	cmd.Flags().StringVar(&cmd.lookupInputStr, "lookup-input-str", "",
 		"if a run can not be found, try to find a run with this value as input-string")
 
+	cmd.Flags().BoolVarP(&cmd.requireCleanGitWorktree, flagNameRequireCleanGitWorktree, "c", false,
+		"fail if the git repository contains modified or untracked files")
+
 	return &cmd
 }
 
@@ -139,10 +143,13 @@ func (c *statusCmd) run(_ *cobra.Command, args []string) {
 	var storageClt storage.Storer
 
 	repo := mustFindRepository()
+	vcsState := mustGetRepoState(repo.Path)
+
+	mustUntrackedFilesNotExist(c.requireCleanGitWorktree, vcsState)
 
 	loader, err := baur.NewLoader(
 		repo.Cfg,
-		mustGetRepoState(repo.Path).CommitID,
+		vcsState.CommitID,
 		log.StdLogger,
 	)
 	exitOnErr(err)
diff --git a/internal/command/status_test.go b/internal/command/status_test.go
index e5a3006b2..293299ee6 100644
--- a/internal/command/status_test.go
+++ b/internal/command/status_test.go
@@ -14,6 +14,8 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/simplesurance/baur/v3/internal/testutils/dbtest"
+	"github.com/simplesurance/baur/v3/internal/testutils/fstest"
+	"github.com/simplesurance/baur/v3/internal/testutils/gittest"
 	"github.com/simplesurance/baur/v3/internal/testutils/repotest"
 )
 
@@ -226,3 +228,21 @@ func TestStatusCombininingFieldAndStatusParameters(t *testing.T) {
 
 	require.Contains(t, stdoutBuf.String(), app.Name)
 }
+
+func TestStatusFailsWhenGitWorktreeIsDirty(t *testing.T) {
+	initTest(t)
+
+	r := repotest.CreateBaurRepository(t, repotest.WithNewDB())
+	gittest.CreateRepository(t, r.Dir)
+	r.CreateSimpleApp(t)
+	fname := "untrackedFile"
+	fstest.WriteToFile(t, []byte("hello"), filepath.Join(r.Dir, fname))
+
+	_, stderrBuf := interceptCmdOutput(t)
+	statusCmd := newStatusCmd()
+	statusCmd.SetArgs([]string{"--" + flagNameRequireCleanGitWorktree})
+	require.Panics(t, func() { require.NoError(t, statusCmd.Execute()) })
+
+	require.Contains(t, stderrBuf.String(), fname)
+	require.Contains(t, stderrBuf.String(), "expecting only tracked unmodified files")
+}
diff --git a/internal/command/term/streams.go b/internal/command/term/streams.go
index ed10efac8..9417b268b 100644
--- a/internal/command/term/streams.go
+++ b/internal/command/term/streams.go
@@ -12,7 +12,7 @@ import (
 
 const separator = "------------------------------------------------------------------------------"
 
-var errorPrefix = color.New(color.FgRed).Sprint("ERROR:")
+var ErrorPrefix = color.New(color.FgRed).Sprint("ERROR:")
 
 // Stream is a concurrency-safe output for term.messages.
 type Stream struct {
@@ -49,12 +49,12 @@ func (s *Stream) TaskPrintf(task *baur.Task, format string, a ...interface{}) {
 // The method prints the error in the format: errorPrefix msg: err
 func (s *Stream) ErrPrintln(err error, msg ...interface{}) {
 	if len(msg) == 0 {
-		s.Println(errorPrefix, err)
+		s.Println(ErrorPrefix, err)
 		return
 	}
 
 	wholeMsg := fmt.Sprint(msg...)
-	s.Printf("%s %s: %s\n", errorPrefix, wholeMsg, err)
+	s.Printf("%s %s: %s\n", ErrorPrefix, wholeMsg, err)
 }
 
 // ErrPrintf prints an error with an optional printf-style message.
@@ -63,6 +63,11 @@ func (s *Stream) ErrPrintf(err error, format string, a ...interface{}) {
 	s.ErrPrintln(err, fmt.Sprintf(format, a...))
 }
 
+// PrintErrln prints as message that is prefixed with "ERROR: "
+func (s *Stream) PrintErrln(msg ...interface{}) {
+	s.Println(ErrorPrefix, fmt.Sprint(msg...))
+}
+
 // PrintSep prints a separator line
 func (s *Stream) PrintSep() {
 	fmt.Fprintln(s.stream, separator)
diff --git a/internal/prettyprint/prettyprint.go b/internal/prettyprint/prettyprint.go
index 3cdd27ee5..5beabf138 100644
--- a/internal/prettyprint/prettyprint.go
+++ b/internal/prettyprint/prettyprint.go
@@ -3,6 +3,7 @@ package prettyprint
 import (
 	"encoding/json"
 	"fmt"
+	"strings"
 )
 
 // AsString returns in as indented JSON
@@ -14,3 +15,14 @@ func AsString(in interface{}) string {
 
 	return string(res)
 }
+
+// TruncatedStrSlice returns sl as string, joined by ", ".
+// If sl has more then maxElems, only the first maxElems elements will be
+// returned and additional truncation marker.
+func TruncatedStrSlice(sl []string, maxElems int) string {
+	if len(sl) <= maxElems {
+		return strings.Join(sl, ", ")
+	}
+
+	return strings.Join(sl[:maxElems], ", ") + ", [...]"
+}
diff --git a/internal/vcs/git/git.go b/internal/vcs/git/git.go
index 4d74b2ec3..60a192426 100644
--- a/internal/vcs/git/git.go
+++ b/internal/vcs/git/git.go
@@ -93,8 +93,8 @@ func WorktreeIsDirty(dir string) (bool, error) {
 	return true, nil
 }
 
-// UntrackedFiles returns a list of untracked files in the repository found at dir.
-// The returned paths are relative to dir.
+// UntrackedFiles returns a list of untracked and modified files in the git repository.
+// Files that exist and are in a .gitignore file are included.
 func UntrackedFiles(dir string) ([]string, error) {
 	const untrackedFilePrefix = "?? "
 	const ignoredFilePrefix = "!! "
diff --git a/internal/vcs/git/repository.go b/internal/vcs/git/repository.go
index d8577efb3..477d551d0 100644
--- a/internal/vcs/git/repository.go
+++ b/internal/vcs/git/repository.go
@@ -5,6 +5,8 @@ import (
 	"sync"
 )
 
+const Name = "git"
+
 // Repository reads information from a Git repository.
 type Repository struct {
 	path string
@@ -114,3 +116,13 @@ func (g *Repository) initUntrackedFiles() error {
 
 	return nil
 }
+
+// UntrackedFiles returns a list of untracked and modified files in the git repository.
+// Files that exist and are in a .gitignore file are included.
+func (g *Repository) UntrackedFiles() ([]string, error) {
+	return UntrackedFiles(g.path)
+}
+
+func (g *Repository) Name() string {
+	return Name
+}
diff --git a/internal/vcs/novcsstate.go b/internal/vcs/novcsstate.go
index bff7435ba..dd4f7413d 100644
--- a/internal/vcs/novcsstate.go
+++ b/internal/vcs/novcsstate.go
@@ -22,3 +22,11 @@ func (*NoVCsState) WorktreeIsDirty() (bool, error) {
 func (NoVCsState) WithoutUntracked(_ ...string) ([]string, error) {
 	return nil, ErrVCSRepositoryNotExist
 }
+
+func (*NoVCsState) Name() string {
+	return "none"
+}
+
+func (*NoVCsState) UntrackedFiles() ([]string, error) {
+	return nil, ErrVCSRepositoryNotExist
+}
diff --git a/internal/vcs/state.go b/internal/vcs/state.go
index 413009308..0db8249a4 100644
--- a/internal/vcs/state.go
+++ b/internal/vcs/state.go
@@ -13,6 +13,8 @@ type StateFetcher interface {
 	CommitID() (string, error)
 	WorktreeIsDirty() (bool, error)
 	WithoutUntracked(paths ...string) ([]string, error)
+	Name() string
+	UntrackedFiles() ([]string, error)
 }
 
 var state = struct {
diff --git a/pkg/baur/taskrunner.go b/pkg/baur/taskrunner.go
index 970e70519..6db658b29 100644
--- a/pkg/baur/taskrunner.go
+++ b/pkg/baur/taskrunner.go
@@ -12,13 +12,22 @@ import (
 	"github.com/simplesurance/baur/v3/internal/exec"
 )
 
+type ErrUntrackedGitFilesExist struct {
+	UntrackedFiles []string
+}
+
+func (e *ErrUntrackedGitFilesExist) Error() string {
+	return "untracked or modified files exist in the git repository"
+}
+
 // ErrTaskRunSkipped is returned when a task run was skipped instead of executed.
 var ErrTaskRunSkipped = errors.New("task run skipped")
 
 // TaskRunner executes the command of a task.
 type TaskRunner struct {
-	skipEnabled uint32 // must be accessed via atomic operations
-	LogFn       exec.PrintfFn
+	skipEnabled         uint32 // must be accessed via atomic operations
+	LogFn               exec.PrintfFn
+	GitUntrackedFilesFn func(dir string) ([]string, error)
 }
 
 func NewTaskRunner() *TaskRunner {
@@ -37,12 +46,18 @@ type RunResult struct {
 // Run executes the command of a task and returns the execution result.
 // The output of the commands are logged with debug log level.
 func (t *TaskRunner) Run(task *Task) (*RunResult, error) {
-	startTime := time.Now()
+	if t.GitUntrackedFilesFn != nil {
+		untracked, err := t.GitUntrackedFilesFn(task.RepositoryRoot)
+		if err != nil {
+			return nil, err
+		}
 
-	if t.SkipRunsIsEnabled() {
-		return nil, ErrTaskRunSkipped
+		if len(untracked) != 0 {
+			return nil, &ErrUntrackedGitFilesExist{UntrackedFiles: untracked}
+		}
 	}
 
+	startTime := time.Now()
 	execResult, err := exec.Command(task.Command[0], task.Command[1:]...).
 		Directory(task.Directory).
 		LogPrefix(color.YellowString(fmt.Sprintf("%s: ", task))).
diff --git a/pkg/baur/taskrunner_test.go b/pkg/baur/taskrunner_test.go
new file mode 100644
index 000000000..6bb36e753
--- /dev/null
+++ b/pkg/baur/taskrunner_test.go
@@ -0,0 +1,17 @@
+package baur
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestRunningTaskFailsWhenGitWorktreeIsDirty(t *testing.T) {
+	tr := NewTaskRunner()
+	tr.GitUntrackedFilesFn = func(_ string) ([]string, error) {
+		return []string{"1"}, nil
+	}
+	_, err := tr.Run(&Task{})
+	var eu *ErrUntrackedGitFilesExist
+	require.ErrorAs(t, err, &eu)
+}