add mockable executor

Dockerfile

@@ -19,6 +19,7 @@ COPY ./*.go ./
 COPY ./cmd/ cmd/
 COPY ./configfiles/ configfiles/
 COPY ./dnfconfig/ dnfconfig/
+COPY ./executor/ executor/
 COPY ./go.mod ./
 COPY ./go.sum ./
 COPY ./impl/ impl/

barney.yaml

@@ -219,6 +219,7 @@ images:
               - 'main.go'
               - 'cmd/*.go'
               - 'dnfconfig/*.go'
+              - 'executor/*.go'
               - 'impl/*.go'
               - 'manifest/*.go'
               - 'srcconfig/*.go'

cmd/build.go

@@ -28,7 +28,7 @@ var buildCmd = &cobra.Command{
 		extraMockArgs := impl.MockExtraCmdlineArgs{
 			NoCheck: noCheck,
 		}
-		return impl.Build(repo, pkg, defaultArch, extraCreateSrpmArgs, extraMockArgs)
+		return impl.Build(repo, pkg, defaultArch, extraCreateSrpmArgs, extraMockArgs, selectExecutor())
 	},
 }
 

cmd/create_srpm.go

@@ -27,7 +27,7 @@ In situations where multiple SRPMs need to be built in dependency order, the man
 		extraArgs := impl.CreateSrpmExtraCmdlineArgs{
 			SkipBuildPrep: skipBuildPrep,
 		}
-		err := impl.CreateSrpm(repo, pkg, extraArgs)
+		err := impl.CreateSrpm(repo, pkg, extraArgs, selectExecutor())
 		return err
 	},
 }

cmd/mock.go

@@ -27,7 +27,7 @@ var mockCmd = &cobra.Command{
 			NoCheck:       noCheck,
 			OnlyCreateCfg: onlyCreateCfg,
 		}
-		return impl.Mock(repo, pkg, target, extraArgs)
+		return impl.Mock(repo, pkg, target, extraArgs, selectExecutor())
 	},
 }
 

cmd/root.go

@@ -10,6 +10,7 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 
+	"code.arista.io/eos/tools/eext/executor"
 	"code.arista.io/eos/tools/eext/util"
 )
 
@@ -46,6 +47,7 @@ func init() {
 
 	rootCmd.PersistentFlags().String("config", "", "config file (default is eext-viper.yaml in /etc or $HOME/.config)")
 	rootCmd.PersistentFlags().BoolVarP(&(util.GlobalVar.Quiet), "quiet", "q", false, "Quiet terminal output (default is false)")
+	rootCmd.PersistentFlags().BoolP("dry-run", "d", false, "Instead of running the commands, print what would be run")
 
 	// Cobra also supports local flags, which will only run
 	// when this action is called directly.
@@ -71,3 +73,15 @@ func initConfig() {
 		fmt.Fprintln(os.Stderr, "Using config file:", viper.ConfigFileUsed())
 	}
 }
+
+// Select appropriate strategy for running commands based on the dry-run flag
+func selectExecutor() executor.Executor {
+	// In the executors ever grow to become expensive on unweildy to be created
+	// multiple times in one program session, sync.Once could be used
+	if dryRun, _ := rootCmd.PersistentFlags().GetBool("dry-run"); dryRun {
+		return &executor.DryRunExecutor{}
+	} else {
+		suppress, _ := rootCmd.PersistentFlags().GetBool("quiet")
+		return &executor.OsExecutor{Suppress: suppress}
+	}
+}

executor/dryrun_executor.go

@@ -0,0 +1,63 @@
+package executor
+
+import (
+	"fmt"
+	"strings"
+)
+
+// An executor that only notes all the commands that would normally be executed.
+type DryRunExecutor struct {
+
+	// Note that instead of having multiple ledgers updated with each subsequent
+	// invocation, we could store an abstract invocation object, and then have a
+	// set of "export" functions, like "ExportShell" that would generate
+	// appropriate shell script, but with limited number of exporters those
+	// extra abstractions feel unnecessary.
+
+	// human friendly description of what an invocation would do
+	invocations []string
+
+	// a script that would be equivalent to the invocations requested
+	shellScript []string
+}
+
+func (ex *DryRunExecutor) Exec(name string, arg ...string) error {
+	escapedInvocation := shellEscape(append([]string{name}, arg...))
+	message := fmt.Sprintf("Would execute: %s", escapedInvocation)
+	fmt.Println(message)
+	ex.invocations = append(ex.invocations, message)
+	ex.shellScript = append(ex.shellScript, escapedInvocation)
+	return nil
+}
+
+func (ex *DryRunExecutor) ExecInDir(dir string, name string, arg ...string) error {
+	escapedInvocation := shellEscape(append([]string{name}, arg...))
+	message := fmt.Sprintf(
+		"In the directory '%s', would execute: %s", dir, escapedInvocation)
+
+	ex.invocations = append(ex.invocations, message)
+
+	// empty `dir` means run in the same directory, but if we just simply
+	// interpolate arg for `cd` with an empty string, the result will change the
+	// directory to the homedir. Let's prevent that, and replace it with PWD
+	if dir == "" {
+		dir = "$PWD"
+	}
+	full_invocation := fmt.Sprintf("(cd '%s' && %s)", dir, escapedInvocation)
+	ex.shellScript = append(ex.shellScript, full_invocation)
+	return nil
+}
+
+func (ex *DryRunExecutor) Output(name string, arg ...string) (string, error) {
+	ex.Exec(name, arg...)
+	return "", nil
+}
+
+func (ex *DryRunExecutor) GenerateShellScript() string {
+	preambule := "#!/usr/bin/env sh\n"
+	return strings.Join(append([]string{preambule}, ex.shellScript...), "\n")
+}
+
+func (ex *DryRunExecutor) GenerateDescription() string {
+	return strings.Join(ex.invocations, "\n")
+}

executor/dryrun_executor_test.go

@@ -0,0 +1,42 @@
+package executor
+
+import (
+	"testing"
+)
+
+func TestDryRunWithMixedCalls(t *testing.T) {
+	drex := DryRunExecutor{}
+	drex.Exec("true")
+	drex.Exec("with some spaces")
+	drex.Exec("cat", "space-y file")
+	drex.ExecInDir("/tmp", "pwd")
+	drex.ExecInDir("/tmp", "cat", "difficult 'quoted' arg")
+	drex.ExecInDir("", "true")
+	drex.Output("cat", "file")
+	shellExpected := `#!/usr/bin/env sh
+
+true
+'with some spaces'
+cat 'space-y file'
+(cd '/tmp' && pwd)
+(cd '/tmp' && cat 'difficult \'quoted\' arg')
+(cd '$PWD' && true)
+cat file`
+	shellActual := drex.GenerateShellScript()
+	if shellActual != shellExpected {
+		t.Fatalf("GenerateShellScript generated unexpected output. Expected:\n%s\n\nGot:\n%s\n",
+			shellExpected, shellActual)
+	}
+	descExpected := `Would execute: true
+Would execute: 'with some spaces'
+Would execute: cat 'space-y file'
+In the directory '/tmp', would execute: pwd
+In the directory '/tmp', would execute: cat 'difficult \'quoted\' arg'
+In the directory '', would execute: true
+Would execute: cat file`
+	descActual := drex.GenerateDescription()
+	if descActual != descExpected {
+		t.Fatalf("GenerateDescription generated unexpected output. Expected:\n%s\n\nGot:\n%s\n",
+			descExpected, descActual)
+	}
+}

executor/executor.go

@@ -0,0 +1,16 @@
+package executor
+
+type Executor interface {
+	// Execute a command in the current working directory.
+	Exec(name string, arg ...string) error
+
+	// Execute a command in a directory specified by `dir`.
+	// The current working directory is not changed after the call.
+	ExecInDir(dir string, name string, arg ...string) error
+
+	// Execute a command and capture its standard output.
+	// If the command returns 0, the output is returned.
+	// Should the command return a non-zero code, the standard error is embedded
+	// in the error object's error message.
+	Output(name string, arg ...string) (string, error)
+}

executor/os_executor.go

@@ -0,0 +1,61 @@
+package executor
+
+import (
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+	"strings"
+)
+
+// An executor that dispatches to os.Exec
+type OsExecutor struct {
+	Suppress bool // whether to suppress the subcmd output
+}
+
+func (ex *OsExecutor) ExecInDir(dir string, name string, arg ...string) error {
+	cmd := exec.Command(name, arg...)
+	cmd.Dir = dir
+	if ex.Suppress {
+		cmd.Stdout = io.Discard
+	} else {
+		cmd.Stdout = os.Stdout
+	}
+	return cmd.Run()
+
+}
+func (ex *OsExecutor) Exec(name string, arg ...string) error {
+	return ex.ExecInDir("", name, arg...)
+}
+
+func (ex *OsExecutor) Output(name string, arg ...string) (string, error) {
+	output, err := exec.Command(name, arg...).Output()
+	if err != nil {
+		escaped_args := shellEscape(append([]string{name}, arg...))
+		if exitErr, ok := err.(*exec.ExitError); ok {
+			return string(output),
+				fmt.Errorf("running `%s` exited with exit-code %d\nstderr:\n%s",
+					escaped_args, exitErr.ExitCode(), exitErr.Stderr)
+		}
+		return string(output),
+			fmt.Errorf("running `%s` failed with '%w'", escaped_args, err)
+	}
+	return string(output), nil
+}
+
+// Join strings in a way that preserves the shell token boundries. For instance
+// the args ["cat", "a file"] when simpy joined with strings. Join would result
+// in a string "cat a file" which has a different meaning to the original. This
+// function is a simple shell escaping join.
+func shellEscape(args []string) string {
+	var processedArgs []string
+	for _, arg := range args {
+		escaped := strings.ReplaceAll(arg, "'", "\\'")
+		if strings.Contains(arg, " ") {
+			processedArgs = append(processedArgs, "'"+escaped+"'")
+		} else {
+			processedArgs = append(processedArgs, escaped)
+		}
+	}
+	return strings.Join(processedArgs, " ")
+}

executor/os_executor_test.go

@@ -0,0 +1,134 @@
+package executor
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestTrueRunsClean(t *testing.T) {
+	ex := OsExecutor{}
+	err := ex.Exec("true")
+	if err != nil {
+		t.Fatalf("`true` returned error: %v", err)
+	}
+}
+
+func TestFalseReturnsError(t *testing.T) {
+	ex := OsExecutor{}
+	err := ex.Exec("false")
+	if err == nil {
+		t.Fatal("`false` did not return error")
+	}
+}
+
+func TestOutputEchoEchoes(t *testing.T) {
+	ex := OsExecutor{}
+	// the `-n` in echo makes it not print the newline at the end
+	// making the errors easier on the eyes when printed
+	if out, err := ex.Output("echo", "-n", "Hello!"); err != nil {
+		t.Fatalf("`echo Hello!` returned error: %v", err)
+	} else {
+		if out != "Hello!" {
+			t.Fatalf("`echo Hello!` returned '%s' instead of Hello!", out)
+		}
+	}
+}
+
+func TestOutputPreservesStdErrOnFail(t *testing.T) {
+	ex := OsExecutor{}
+	// The bash invocation below spawns a nested shell session, with the last arg
+	// being what to run in that sub-shell. This way the whole contents of this
+	// one arg becomes the "text" to run inside the shell, enabling magic like
+	// command chaining and redirecting to stderr
+	_, err := ex.Output("bash", "-c", "echo err_msg >&2; false")
+	if err != nil {
+		if !strings.Contains(err.Error(), "err_msg") {
+			t.Fatal("the error message was not forwarded")
+		}
+	} else {
+		t.Fatal("Output() should have returned an error")
+	}
+}
+
+func tempDirOrFatal(dir string, pattern string, t *testing.T) string {
+	dir, err := os.MkdirTemp(dir, pattern)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return dir
+}
+func TestExecInDirRunsThere(t *testing.T) {
+	dir := tempDirOrFatal("", "OsExecutorTest", t)
+	defer os.RemoveAll(dir)
+	// we cannot be in (as in: have PWD pointing at) the newly created temp dir
+	// so no need to check if startingCwd != dir
+	startingCwd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Cannot get the current working directory. Error: %s", err)
+	}
+	ex := OsExecutor{}
+
+	bash_line := "test `pwd` = " + dir
+	if err := ex.ExecInDir(dir, "bash", "-c", bash_line); err != nil {
+		t.Fatal("The test failed1", dir)
+	}
+	finalCwd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Cannot get the current working directory. Error: %s", err)
+	}
+	if startingCwd != finalCwd {
+		t.Fatal("The working directory is different after running Exec")
+	}
+
+}
+func TestExecInDirRunsThereCheckWithFile(t *testing.T) {
+	dir := tempDirOrFatal("", "OsExecutorTest", t)
+	defer os.RemoveAll(dir)
+	_, err := os.Create(filepath.Join(dir, "payload"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	ex := OsExecutor{}
+	if err := ex.ExecInDir(dir, "ls", "payload"); err != nil {
+		t.Fatalf("ls did not find the file that should be there. Error: %s", err)
+	}
+}
+
+type shellEscapeTest struct {
+	args     []string
+	expected string
+}
+
+var shellEscapeTests = []shellEscapeTest{
+	// Empty slice of args, empty string should be returned
+	{[]string{}, ""},
+	// Joins nothing - one thing, nothing should be changed
+	{[]string{"true"}, "true"},
+	// Joins nothing but escapes because of the space
+	{[]string{"foo bar"}, "'foo bar'"},
+	// Joins two things with no special escaping
+	{[]string{"cat", "and_the_dog"}, "cat and_the_dog"},
+	// Joins two things with one that has a space inside
+	{[]string{"cat", "and the_dog"}, "cat 'and the_dog'"},
+	// Joins three args with no spaces
+	{[]string{"foo", "bar", "baz"}, "foo bar baz"},
+	// Joins three args with spaces
+	{[]string{"f o", "b r", "b z"}, "'f o' 'b r' 'b z'"},
+	// Escapes an explicit quote mark
+	{[]string{"b'r"}, "b\\'r"},
+	// Escapes has multiple quote marks
+	{[]string{"foo'bar'baz"}, "foo\\'bar\\'baz"},
+	// Escapes spaces and quote marks
+	{[]string{"foo 'bar' baz"}, "'foo \\'bar\\' baz'"},
+}
+
+func TestShellEscape(t *testing.T) {
+	for _, tt := range shellEscapeTests {
+		actual := shellEscape(tt.args)
+		if actual != tt.expected {
+			t.Fatalf("Expected '%s', got '%s'", tt.expected, actual)
+		}
+	}
+}