[M] Change in coreboot[main]: util/autoport: Streamline external program invocation

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82403?usp=email )

Change subject: util/autoport: Streamline external program invocation
......................................................................

util/autoport: Streamline external program invocation

The original approach to call external programs was rather convoluted
and would fall back to running executables inside the current working
directory if running them from the location specified in the code did
not succeed, swallowing any errors from the first invocation.

Rewrite the system around the `LogMakingProgram` concept, a struct to
represent a program. Each program has a name, prefixes to try running
it from and the arguments to pass to it (if any). Plus, collect error
information from failed executions, but only show it when none of the
prefixes resulted in a successful invocation.

In addition, look for programs in PATH instead of CWD: it is unlikely
that all utils will be in the CWD, but utils can be in the PATH after
one installs them (`sudo make install`). For coreboot utils, look for
them in the utils folder first as the installed versions might not be
up-to-date.

Furthermore, print out the command about to be executed, as there are
some commands (e.g. `ectool` on boards without an EC) that can take a
very long time to complete.

Change-Id: I144bdf609e0aebd8f6ddebc0eb1216bedebfa313
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/82403
Reviewed-by: Nico Huber 
Tested-by: build bot (Jenkins) 
---
M util/autoport/log_maker.go
1 file changed, 72 insertions(+), 25 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/util/autoport/log_maker.go b/util/autoport/log_maker.go
index 5ca8284..ed157c7 100644
--- a/util/autoport/log_maker.go
+++ b/util/autoport/log_maker.go
@@ -12,14 +12,20 @@
    "bytes"
 )

-func TryRunAndSave(output string, name string, arg []string) error {
-	cmd := exec.Command(name, arg...)
+type LogMakingProgram struct {
+	name string
+	prefixes []string
+	args []string
+}

+func (prog LogMakingProgram) TryRunAndSave(output string, prefix string) error {
    f, err := os.Create(output)
    if err != nil {
    	log.Fatal(err)
    }
+	defer f.Close()

+	cmd := exec.Command(prefix+prog.name, prog.args...)
    cmd.Stdout = f
    cmd.Stderr = f

@@ -27,25 +33,35 @@
    if err != nil {
    	return err
    }
-	cmd.Wait()
-	return nil
+	return cmd.Wait()
 }

-func RunAndSave(output string, name string, arg ...string) {
-	err := TryRunAndSave(output, name, arg)
-	if err == nil {
-		return
+func (prog LogMakingProgram) RunAndSave(outDir string) {
+	output := fmt.Sprintf("%s/%s.log", outDir, prog.name)
+	cmdline := strings.Join(append([]string{prog.name}, prog.args...), " ")
+
+	fmt.Println("Running: "+cmdline)
+
+	var sb strings.Builder
+	for _, prefix := range prog.prefixes {
+		err := prog.TryRunAndSave(output, prefix)
+		if err == nil {
+			return
+		}
+		sb.WriteString("\nError running '"+prefix+cmdline+"': "+err.Error()+"\n")
+		data, ferr := os.ReadFile(output)
+		if ferr != nil {
+			sb.WriteString("<failed to open log>\n")
+		} else {
+			if len(data) > 0 {
+				sb.WriteString("Program output:\n\n")
+				sb.WriteString(string(data))
+			}
+		}
    }
-	idx := strings.LastIndex(name, "/")
-	relname := name
-	if idx >= 0 {
-		relname = name[idx+1:]
-	}
-	relname = "./" + relname
-	err = TryRunAndSave(output, relname, arg)
-	if err != nil {
-		log.Fatal(err)
-	}
+
+	fmt.Println("\nCould not run program: '"+cmdline+"'")
+	log.Fatal(sb.String())
 }

const MAXPROMPTRETRY = 3
@@ -140,11 +156,8 @@

func MakeLogs(outDir string) {
    os.MkdirAll(outDir, 0700)
-	RunAndSave(outDir+"/lspci.log", "lspci", "-nnvvvxxxx")
-	RunAndSave(outDir+"/dmidecode.log", "dmidecode")
-	RunAndSave(outDir+"/acpidump.log", "acpidump")

-	probeGFX := PromptUserBool("WARNING: The following tool MAY cause your system to hang when it attempts "+
+	probeGFX := PromptUserBool("WARNING: Running inteltool MAY cause your system to hang when it attempts "+
    	"to probe for graphics registers.  Having the graphics registers will help create a better port. "+
    	"Should autoport probe these registers?",
    	true)
@@ -154,9 +167,43 @@
    	inteltoolArgs += "f"
    }

-	RunAndSave(outDir+"/inteltool.log", "../inteltool/inteltool", inteltoolArgs)
-	RunAndSave(outDir+"/ectool.log", "../ectool/ectool", "-pd")
-	RunAndSave(outDir+"/superiotool.log", "../superiotool/superiotool", "-ade")
+	var programs = []LogMakingProgram {
+		LogMakingProgram {
+			name: "lspci",
+			prefixes: []string{""},
+			args: []string{"-nnvvvxxxx"},
+		},
+		LogMakingProgram {
+			name: "dmidecode",
+			prefixes: []string{""},
+			args: []string{},
+		},
+		LogMakingProgram {
+			name: "acpidump",
+			prefixes: []string{""},
+			args: []string{},
+		},
+		LogMakingProgram {
+			name: "inteltool",
+			prefixes: []string{"../inteltool/", ""},
+			args: []string{inteltoolArgs},
+		},
+		LogMakingProgram {
+			name: "ectool",
+			prefixes: []string{"../ectool/", ""},
+			args: []string{"-pd"},
+		},
+		LogMakingProgram {
+			name: "superiotool",
+			prefixes: []string{"../superiotool/", ""},
+			args: []string{"-ade"},
+		},
+	}
+
+	fmt.Println("Making logs...")
+	for _, prog := range programs {
+		prog.RunAndSave(outDir)
+	}

SysSound := "/sys/class/sound/"
    card := ""