diff --git a/Dockerfile b/Dockerfile index a42ed15..e3ac2ed 100644 --- a/Dockerfile +++ b/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/ diff --git a/barney.yaml b/barney.yaml index 06ec457..a4e7fe6 100644 --- a/barney.yaml +++ b/barney.yaml @@ -219,6 +219,7 @@ images: - 'main.go' - 'cmd/*.go' - 'dnfconfig/*.go' + - 'executor/*.go' - 'impl/*.go' - 'manifest/*.go' - 'srcconfig/*.go' diff --git a/cmd/build.go b/cmd/build.go index 1f617da..9a2536d 100644 --- a/cmd/build.go +++ b/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()) }, } diff --git a/cmd/create_srpm.go b/cmd/create_srpm.go index a7624d2..718ce37 100644 --- a/cmd/create_srpm.go +++ b/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 }, } diff --git a/cmd/mock.go b/cmd/mock.go index 0de61a5..b7cf9c6 100644 --- a/cmd/mock.go +++ b/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()) }, } diff --git a/cmd/root.go b/cmd/root.go index 66f4f22..62bdaf9 100644 --- a/cmd/root.go +++ b/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} + } +} diff --git a/executor/dryrun_executor.go b/executor/dryrun_executor.go new file mode 100644 index 0000000..46a851c --- /dev/null +++ b/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 { + preamble := "#!/usr/bin/env sh\n" + return strings.Join(append([]string{preamble}, ex.shellScript...), "\n") +} + +func (ex *DryRunExecutor) GenerateDescription() string { + return strings.Join(ex.invocations, "\n") +} diff --git a/executor/dryrun_executor_test.go b/executor/dryrun_executor_test.go new file mode 100644 index 0000000..713c405 --- /dev/null +++ b/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) + } +} diff --git a/executor/executor.go b/executor/executor.go new file mode 100644 index 0000000..56ad82c --- /dev/null +++ b/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) +} diff --git a/executor/os_executor.go b/executor/os_executor.go new file mode 100644 index 0000000..b467889 --- /dev/null +++ b/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 simply 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, " ") +} diff --git a/executor/os_executor_test.go b/executor/os_executor_test.go new file mode 100644 index 0000000..f4283ac --- /dev/null +++ b/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) + } + } +} diff --git a/impl/build.go b/impl/build.go index a503199..f66ebc4 100644 --- a/impl/build.go +++ b/impl/build.go @@ -5,17 +5,19 @@ package impl import ( "log" + + "code.arista.io/eos/tools/eext/executor" ) // Build calls CreateSrpm and Mock in sequence func Build(repo string, pkg string, arch string, extraCreateSrpmArgs CreateSrpmExtraCmdlineArgs, - extraMockArgs MockExtraCmdlineArgs) error { - if err := CreateSrpm(repo, pkg, extraCreateSrpmArgs); err != nil { + extraMockArgs MockExtraCmdlineArgs, executor executor.Executor) error { + if err := CreateSrpm(repo, pkg, extraCreateSrpmArgs, executor); err != nil { return err } - if err := Mock(repo, pkg, arch, extraMockArgs); err != nil { + if err := Mock(repo, pkg, arch, extraMockArgs, executor); err != nil { return err } log.Println("SUCCESS: Build") diff --git a/impl/common.go b/impl/common.go index 880f65d..c1fe0c3 100644 --- a/impl/common.go +++ b/impl/common.go @@ -15,6 +15,7 @@ import ( "github.com/spf13/viper" + "code.arista.io/eos/tools/eext/executor" "code.arista.io/eos/tools/eext/manifest" "code.arista.io/eos/tools/eext/util" ) @@ -273,13 +274,13 @@ func filterAndCopy(pathMap map[string]string, errPrefix util.ErrPrefix) error { var gpgKeysLoaded = false -func loadGpgKeys() error { +func loadGpgKeys(executor executor.Executor) error { if gpgKeysLoaded { return nil } // Remove any stale keys from rpmdb - if _, err := util.CheckOutput("rpm", "-e", "gpg-pubkey", "--allmatches"); err != nil { + if _, err := executor.Output("rpm", "-e", "gpg-pubkey", "--allmatches"); err != nil { // Ignore error if no keys installed. if !strings.Contains(err.Error(), "package gpg-pubkey is not installed") { return fmt.Errorf("Error '%s' clearing gpg-pubkey from rpmdb", err) @@ -289,7 +290,7 @@ func loadGpgKeys() error { // Now add the keys pubKeys, _ := filepath.Glob(filepath.Join(getRpmKeysDir(), "*.pem")) for _, pubKey := range pubKeys { - if err := util.RunSystemCmd("rpm", "--import", pubKey); err != nil { + if err := executor.Exec("rpm", "--import", pubKey); err != nil { return fmt.Errorf("Error '%s' importing %s to rpmdb", err, pubKey) } } @@ -350,11 +351,11 @@ func getEextSignature(errPrefix util.ErrPrefix) ( return combineSrcEnv(false, ",", errPrefix) } -func setup() error { +func setup(executor executor.Executor) error { if err := CheckEnv(); err != nil { return err } - if err := loadGpgKeys(); err != nil { + if err := loadGpgKeys(executor); err != nil { return err } return nil diff --git a/impl/create_srpm.go b/impl/create_srpm.go index ebf4ee0..dabbd37 100644 --- a/impl/create_srpm.go +++ b/impl/create_srpm.go @@ -13,6 +13,7 @@ import ( "golang.org/x/exp/slices" + "code.arista.io/eos/tools/eext/executor" "code.arista.io/eos/tools/eext/manifest" "code.arista.io/eos/tools/eext/srcconfig" "code.arista.io/eos/tools/eext/util" @@ -34,6 +35,7 @@ type srpmBuilder struct { errPrefix util.ErrPrefix upstreamSrc []upstreamSrcSpec srcConfig *srcconfig.SrcConfig + executor executor.Executor } // CreateSrpmExtraCmdlineArgs is a bundle of extra args for impl.CreateSrpm @@ -140,7 +142,7 @@ func (bldr *srpmBuilder) verifyUpstreamSrpm() error { } // Check if downloaded file is a valid rpm - err := util.RunSystemCmd("rpm", "-q", "-p", upstreamSrpmFilePath) + err := bldr.executor.Exec("rpm", "-q", "-p", upstreamSrpmFilePath) if err != nil { return fmt.Errorf("%sDownloaded SRPM file is not a valid rpm: %s", bldr.errPrefix, err) @@ -212,7 +214,7 @@ func (bldr *srpmBuilder) setupRpmbuildTreeSrpm() error { "-i", upstreamSrpmFilePath, } - if err := util.RunSystemCmd("rpm", rpmInstArgs...); err != nil { + if err := bldr.executor.Exec("rpm", rpmInstArgs...); err != nil { return fmt.Errorf("%sError '%s' installing upstream SRPM file %s", bldr.errPrefix, err, upstreamSrpmFilePath) } @@ -284,7 +286,7 @@ func (bldr *srpmBuilder) patchUpstreamSpecFileWithEextRelease() error { // Backup original spec file origSpecFile := specFile + ".orig" - if err := util.RunSystemCmd("cp", specFile, origSpecFile); err != nil { + if err := bldr.executor.Exec("cp", specFile, origSpecFile); err != nil { return fmt.Errorf("%scopying %s to %s errored out with '%s'", bldr.errPrefix, specFile, origSpecFile, err) } @@ -295,7 +297,7 @@ func (bldr *srpmBuilder) patchUpstreamSpecFileWithEextRelease() error { // Validate regex match on upstream spec file grepCmdOptions := []string{"-c", releaseRegex, specFile} - if grepOutput, grepErr := util.CheckOutput("egrep", grepCmdOptions...); grepErr != nil { + if grepOutput, grepErr := bldr.executor.Output("egrep", grepCmdOptions...); grepErr != nil { return fmt.Errorf("%s%s", bldr.errPrefix, grepErr) } else { numMatchingLinesStr := strings.TrimRight(grepOutput, "\n") @@ -321,7 +323,7 @@ func (bldr *srpmBuilder) patchUpstreamSpecFileWithEextRelease() error { // Run sed to patch the spec file sedScript := fmt.Sprintf("s/%s/%s/", sedMatchPattern, sedReplacePattern) sedCmdOptions := []string{"-i", "-e", sedScript, specFile} - if err := util.RunSystemCmd("sed", sedCmdOptions...); err != nil { + if err := bldr.executor.Exec("sed", sedCmdOptions...); err != nil { return fmt.Errorf("%s%s", bldr.errPrefix, err) } @@ -443,7 +445,7 @@ func (bldr *srpmBuilder) build(prep bool) error { rpmbuildArgs = append(rpmbuildArgs, specFile) - if err := util.RunSystemCmd("rpmbuild", rpmbuildArgs...); err != nil { + if err := bldr.executor.Exec("rpmbuild", rpmbuildArgs...); err != nil { return fmt.Errorf("%sfailed", bldr.errPrefix) } bldr.log("succesful") @@ -550,8 +552,8 @@ func (bldr *srpmBuilder) runStages() error { // The packages(SRPMs) are specified in the manifest. // If a pkg is specified, only it is built. Otherwise, we walk over all the packages // in the manifest and build them. -func CreateSrpm(repo string, pkg string, extraArgs CreateSrpmExtraCmdlineArgs) error { - if err := setup(); err != nil { +func CreateSrpm(repo string, pkg string, extraArgs CreateSrpmExtraCmdlineArgs, executor executor.Executor) error { + if err := setup(executor); err != nil { return err } @@ -589,6 +591,7 @@ func CreateSrpm(repo string, pkg string, extraArgs CreateSrpmExtraCmdlineArgs) e skipBuildPrep: extraArgs.SkipBuildPrep, errPrefixBase: errPrefixBase, srcConfig: srcConfig, + executor: executor, } bldr.setupStageErrPrefix("") diff --git a/impl/create_srpm_from_unmodified_srpm_test.go b/impl/create_srpm_from_unmodified_srpm_test.go index 0f5c378..256f7be 100644 --- a/impl/create_srpm_from_unmodified_srpm_test.go +++ b/impl/create_srpm_from_unmodified_srpm_test.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/viper" "github.com/stretchr/testify/require" + "code.arista.io/eos/tools/eext/executor" "code.arista.io/eos/tools/eext/testutil" ) @@ -75,7 +76,7 @@ func testCreateSrpmFromUnmodifiedSrpm(t *testing.T, testutil.SetupSrcEnv(sources) defer testutil.CleanupSrcEnv(sources) - createSrpmErr := CreateSrpm(pkg, pkg, CreateSrpmExtraCmdlineArgs{}) + createSrpmErr := CreateSrpm(pkg, pkg, CreateSrpmExtraCmdlineArgs{}, &executor.OsExecutor{}) require.NoError(t, createSrpmErr) srpmsResultDir := filepath.Join(destDir, "SRPMS", pkg) diff --git a/impl/mock.go b/impl/mock.go index 654cca0..d67c470 100644 --- a/impl/mock.go +++ b/impl/mock.go @@ -7,7 +7,6 @@ import ( "fmt" "log" "os" - "os/exec" "path/filepath" "strings" @@ -16,6 +15,7 @@ import ( "golang.org/x/exp/slices" "code.arista.io/eos/tools/eext/dnfconfig" + "code.arista.io/eos/tools/eext/executor" "code.arista.io/eos/tools/eext/manifest" "code.arista.io/eos/tools/eext/util" ) @@ -141,7 +141,7 @@ func (bldr *mockBuilder) setupDeps() error { if copyErr := filterAndCopy(pathMap, bldr.errPrefix); copyErr != nil { return copyErr } - createRepoErr := util.RunSystemCmd("createrepo", mockDepsDir) + createRepoErr := bldr.executor.Exec("createrepo", mockDepsDir) if createRepoErr != nil { return fmt.Errorf("%screaterepo %s errored out with %s", bldr.errPrefix, mockDepsDir, createRepoErr) @@ -194,24 +194,19 @@ func (bldr *mockBuilder) mockArgs(extraArgs []string) []string { func (bldr *mockBuilder) printLogFile(filename string) { resultdir := getMockResultsDir(bldr.pkg, bldr.arch) logPath := filepath.Join(resultdir, filename) - if util.CheckPath(logPath, false, false) == nil { - bldr.log("--- start of mock %s ---", filename) - dumpLogCmd := exec.Command("cat", logPath) - dumpLogCmd.Stderr = os.Stderr - dumpLogCmd.Stdout = os.Stdout - if dumpLogCmd.Run() != nil { - bldr.log("Dumping logfile failed") - } - bldr.log("--- end of %s ---", filename) - } else { - bldr.log("No %s found", filename) + contents, err := os.ReadFile(logPath) + if err != nil { + bldr.log("Dumping logfile %s failed. Error: %s", logPath, err) } + bldr.log("--- start of mock %s ---", filename) + bldr.log(string(contents)) + bldr.log("--- end of %s ---", filename) } func (bldr *mockBuilder) runMockCmd(extraArgs []string) error { mockArgs := bldr.mockArgs(extraArgs) bldr.log("Running mock %s", strings.Join(mockArgs, " ")) - mockErr := util.RunSystemCmd("mock", mockArgs...) + mockErr := bldr.executor.Exec("mock", mockArgs...) if mockErr != nil { bldr.printLogFile("root.log") bldr.printLogFile("build.log") @@ -336,8 +331,8 @@ func (bldr *mockBuilder) runStages() error { // from the already built SRPMs and places the results in // /RPMS/// // 'arch' cannot be empty, needs to be a valid architecture. -func Mock(repo string, pkg string, arch string, extraArgs MockExtraCmdlineArgs) error { - if err := setup(); err != nil { +func Mock(repo string, pkg string, arch string, extraArgs MockExtraCmdlineArgs, executor executor.Executor) error { + if err := setup(executor); err != nil { return err } @@ -414,6 +409,7 @@ func Mock(repo string, pkg string, arch string, extraArgs MockExtraCmdlineArgs) errPrefix: errPrefix, dependencyList: dependencyList, enableNetwork: pkgSpec.Build.EnableNetwork, + executor: executor, }, onlyCreateCfg: extraArgs.OnlyCreateCfg, noCheck: extraArgs.NoCheck, diff --git a/impl/mock_cfg.go b/impl/mock_cfg.go index addf6c6..b398970 100644 --- a/impl/mock_cfg.go +++ b/impl/mock_cfg.go @@ -12,6 +12,7 @@ import ( "golang.org/x/exp/maps" "code.arista.io/eos/tools/eext/dnfconfig" + "code.arista.io/eos/tools/eext/executor" "code.arista.io/eos/tools/eext/manifest" "code.arista.io/eos/tools/eext/util" ) @@ -37,6 +38,7 @@ type builderCommon struct { errPrefix util.ErrPrefix dependencyList []string enableNetwork bool + executor executor.Executor } type mockCfgBuilder struct { diff --git a/impl/setupDeps_test.go b/impl/setupDeps_test.go index ffc7b0c..e90218b 100644 --- a/impl/setupDeps_test.go +++ b/impl/setupDeps_test.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/viper" "github.com/stretchr/testify/require" + "code.arista.io/eos/tools/eext/executor" "code.arista.io/eos/tools/eext/manifest" "code.arista.io/eos/tools/eext/testutil" ) @@ -88,6 +89,7 @@ func TestSetupDeps(t *testing.T) { arch: targetArch, buildSpec: &manifestObj.Package[0].Build, dependencyList: dependencyList, + executor: &executor.OsExecutor{}, }, }