From a4b7cedc404614f8a1f96a109033be3f187445d8 Mon Sep 17 00:00:00 2001 From: Ryan Gang Date: Wed, 15 May 2024 14:43:50 +0530 Subject: [PATCH 1/6] feat: make stdout, stderr buffers public --- executable/executable.go | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/executable/executable.go b/executable/executable.go index 25ae3f5..23634e8 100644 --- a/executable/executable.go +++ b/executable/executable.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "io/ioutil" "os" "path/filepath" "time" @@ -25,7 +24,9 @@ type Executable struct { // WorkingDir can be set before calling Start or Run to customize the working directory of the executable. WorkingDir string - StdinPipe io.WriteCloser + StdinPipe io.WriteCloser + StdoutBuffer *bytes.Buffer + StderrBuffer *bytes.Buffer // These are set & removed together atleastOneReadDone bool @@ -34,8 +35,6 @@ type Executable struct { stderrPipe io.ReadCloser stdoutBytes []byte stderrBytes []byte - stdoutBuffer *bytes.Buffer - stderrBuffer *bytes.Buffer stdoutLineWriter *linewriter.LineWriter stderrLineWriter *linewriter.LineWriter readDone chan bool @@ -64,7 +63,6 @@ func (w *loggerWriter) Write(bytes []byte) (n int, err error) { } func nullLogger(msg string) { - return } func (e *Executable) Clone() *Executable { @@ -91,7 +89,7 @@ func (e *Executable) isRunning() bool { } func (e *Executable) HasExited() bool { - return e.atleastOneReadDone == true + return e.atleastOneReadDone } // Start starts the specified command but does not wait for it to complete. @@ -138,7 +136,7 @@ func (e *Executable) Start(args ...string) error { return err } e.stdoutBytes = []byte{} - e.stdoutBuffer = bytes.NewBuffer(e.stdoutBytes) + e.StdoutBuffer = bytes.NewBuffer(e.stdoutBytes) e.stdoutLineWriter = linewriter.New(newLoggerWriter(e.loggerFunc), 500*time.Millisecond) // Setup stderr relay @@ -147,7 +145,7 @@ func (e *Executable) Start(args ...string) error { return err } e.stderrBytes = []byte{} - e.stderrBuffer = bytes.NewBuffer(e.stderrBytes) + e.StderrBuffer = bytes.NewBuffer(e.stderrBytes) e.stderrLineWriter = linewriter.New(newLoggerWriter(e.loggerFunc), 500*time.Millisecond) e.StdinPipe, err = cmd.StdinPipe() @@ -161,8 +159,8 @@ func (e *Executable) Start(args ...string) error { // At this point, it is safe to set e.cmd as cmd, if any of the above steps fail, we don't want to leave e.cmd in an inconsistent state e.cmd = cmd - e.setupIORelay(e.stdoutPipe, e.stdoutBuffer, e.stdoutLineWriter) - e.setupIORelay(e.stderrPipe, e.stderrBuffer, e.stderrLineWriter) + e.setupIORelay(e.stdoutPipe, e.StdoutBuffer, e.stdoutLineWriter) + e.setupIORelay(e.stderrPipe, e.StderrBuffer, e.stderrLineWriter) return nil } @@ -181,7 +179,7 @@ func (e *Executable) setupIORelay(source io.Reader, destination1 io.Writer, dest e.atleastOneReadDone = true e.readDone <- true - io.Copy(ioutil.Discard, source) // Let's drain the pipe in case any content is leftover + io.Copy(io.Discard, source) // Let's drain the pipe in case any content is leftover }() } @@ -218,8 +216,8 @@ func (e *Executable) Wait() (ExecutableResult, error) { e.cmd = nil e.stdoutPipe = nil e.stderrPipe = nil - e.stdoutBuffer = nil - e.stderrBuffer = nil + e.StdoutBuffer = nil + e.StderrBuffer = nil e.stdoutBytes = nil e.stderrBytes = nil e.stdoutLineWriter = nil @@ -245,8 +243,8 @@ func (e *Executable) Wait() (ExecutableResult, error) { e.stdoutLineWriter.Flush() e.stderrLineWriter.Flush() - stdout := e.stdoutBuffer.Bytes() - stderr := e.stderrBuffer.Bytes() + stdout := e.StdoutBuffer.Bytes() + stderr := e.StderrBuffer.Bytes() return ExecutableResult{ Stdout: stdout, From 2af007394f4617c88fe3985d43770e00d8490b59 Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Mon, 20 May 2024 19:22:48 +0100 Subject: [PATCH 2/6] Refactor Executable struct field names for consistency and clarity --- executable/executable.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/executable/executable.go b/executable/executable.go index 23634e8..f14c60b 100644 --- a/executable/executable.go +++ b/executable/executable.go @@ -17,7 +17,7 @@ import ( // Executable represents a program that can be executed type Executable struct { - path string + Path string timeoutInSecs int loggerFunc func(string) @@ -67,7 +67,7 @@ func nullLogger(msg string) { func (e *Executable) Clone() *Executable { return &Executable{ - path: e.path, + Path: e.Path, timeoutInSecs: e.timeoutInSecs, loggerFunc: e.loggerFunc, WorkingDir: e.WorkingDir, @@ -76,12 +76,12 @@ func (e *Executable) Clone() *Executable { // NewExecutable returns an Executable func NewExecutable(path string) *Executable { - return &Executable{path: path, timeoutInSecs: 10, loggerFunc: nullLogger} + return &Executable{Path: path, timeoutInSecs: 10, loggerFunc: nullLogger} } // NewVerboseExecutable returns an Executable struct with a logger configured func NewVerboseExecutable(path string, loggerFunc func(string)) *Executable { - return &Executable{path: path, timeoutInSecs: 10, loggerFunc: loggerFunc} + return &Executable{Path: path, timeoutInSecs: 10, loggerFunc: loggerFunc} } func (e *Executable) isRunning() bool { @@ -104,27 +104,27 @@ func (e *Executable) Start(args ...string) error { // While passing executables present on PATH, filepath.Abs is unable to resolve their absolute path. // In those cases we use the path returned by LookPath. - resolvedPath, err = exec.LookPath(e.path) + resolvedPath, err = exec.LookPath(e.Path) if err == nil { absolutePath = resolvedPath } else { - absolutePath, err = filepath.Abs(e.path) + absolutePath, err = filepath.Abs(e.Path) if err != nil { - return fmt.Errorf("%s not found", e.path) + return fmt.Errorf("%s not found", e.Path) } } fileInfo, err := os.Stat(absolutePath) if err != nil { - return fmt.Errorf("%s not found", e.path) + return fmt.Errorf("%s not found", e.Path) } // Check executable permission if fileInfo.Mode().Perm()&0111 == 0 || fileInfo.IsDir() { - return fmt.Errorf("%s is not an executable file", e.path) + return fmt.Errorf("%s is not an executable file", e.Path) } // TODO: Use timeout! - cmd := exec.Command(e.path, args...) + cmd := exec.Command(e.Path, args...) cmd.Dir = e.WorkingDir cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} e.readDone = make(chan bool) From 39db270b2cb00ef1244d8f30c5f4359fd72cc33b Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Mon, 20 May 2024 20:26:20 +0100 Subject: [PATCH 3/6] Add Plainf method to Logger for formatted logging. --- logger/logger.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/logger/logger.go b/logger/logger.go index 1387cad..966a18f 100644 --- a/logger/logger.go +++ b/logger/logger.go @@ -180,6 +180,10 @@ func (l *Logger) Debugln(msg string) { } } +func (l *Logger) Plainf(fstring string, args ...interface{}) { + l.logger.Printf(fstring, args...) +} + func (l *Logger) Plainln(msg string) { lines := strings.Split(msg, "\n") From 36319adace59e3ded60dd8164145d5da8f4fdf62 Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Tue, 21 May 2024 01:38:54 +0100 Subject: [PATCH 4/6] Refactor Plainf method in logger.go to print each line separately --- logger/logger.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/logger/logger.go b/logger/logger.go index 966a18f..b2c461c 100644 --- a/logger/logger.go +++ b/logger/logger.go @@ -181,7 +181,11 @@ func (l *Logger) Debugln(msg string) { } func (l *Logger) Plainf(fstring string, args ...interface{}) { - l.logger.Printf(fstring, args...) + formattedString := fmt.Sprintf(fstring, args...) + + for _, line := range strings.Split(formattedString, "\n") { + l.logger.Println(line) + } } func (l *Logger) Plainln(msg string) { From 0c06d912611c3131bbc1b63005c06445cd3d2142 Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Tue, 21 May 2024 01:40:31 +0100 Subject: [PATCH 5/6] minimize changes to executable.go --- executable/executable.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/executable/executable.go b/executable/executable.go index f14c60b..68117f7 100644 --- a/executable/executable.go +++ b/executable/executable.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "fmt" + "io/ioutil" "os" "path/filepath" "time" @@ -24,9 +25,7 @@ type Executable struct { // WorkingDir can be set before calling Start or Run to customize the working directory of the executable. WorkingDir string - StdinPipe io.WriteCloser - StdoutBuffer *bytes.Buffer - StderrBuffer *bytes.Buffer + StdinPipe io.WriteCloser // These are set & removed together atleastOneReadDone bool @@ -35,6 +34,8 @@ type Executable struct { stderrPipe io.ReadCloser stdoutBytes []byte stderrBytes []byte + stdoutBuffer *bytes.Buffer + stderrBuffer *bytes.Buffer stdoutLineWriter *linewriter.LineWriter stderrLineWriter *linewriter.LineWriter readDone chan bool @@ -63,6 +64,7 @@ func (w *loggerWriter) Write(bytes []byte) (n int, err error) { } func nullLogger(msg string) { + return } func (e *Executable) Clone() *Executable { @@ -89,7 +91,7 @@ func (e *Executable) isRunning() bool { } func (e *Executable) HasExited() bool { - return e.atleastOneReadDone + return e.atleastOneReadDone == true } // Start starts the specified command but does not wait for it to complete. @@ -136,7 +138,7 @@ func (e *Executable) Start(args ...string) error { return err } e.stdoutBytes = []byte{} - e.StdoutBuffer = bytes.NewBuffer(e.stdoutBytes) + e.stdoutBuffer = bytes.NewBuffer(e.stdoutBytes) e.stdoutLineWriter = linewriter.New(newLoggerWriter(e.loggerFunc), 500*time.Millisecond) // Setup stderr relay @@ -145,7 +147,7 @@ func (e *Executable) Start(args ...string) error { return err } e.stderrBytes = []byte{} - e.StderrBuffer = bytes.NewBuffer(e.stderrBytes) + e.stderrBuffer = bytes.NewBuffer(e.stderrBytes) e.stderrLineWriter = linewriter.New(newLoggerWriter(e.loggerFunc), 500*time.Millisecond) e.StdinPipe, err = cmd.StdinPipe() @@ -159,8 +161,8 @@ func (e *Executable) Start(args ...string) error { // At this point, it is safe to set e.cmd as cmd, if any of the above steps fail, we don't want to leave e.cmd in an inconsistent state e.cmd = cmd - e.setupIORelay(e.stdoutPipe, e.StdoutBuffer, e.stdoutLineWriter) - e.setupIORelay(e.stderrPipe, e.StderrBuffer, e.stderrLineWriter) + e.setupIORelay(e.stdoutPipe, e.stdoutBuffer, e.stdoutLineWriter) + e.setupIORelay(e.stderrPipe, e.stderrBuffer, e.stderrLineWriter) return nil } @@ -179,7 +181,7 @@ func (e *Executable) setupIORelay(source io.Reader, destination1 io.Writer, dest e.atleastOneReadDone = true e.readDone <- true - io.Copy(io.Discard, source) // Let's drain the pipe in case any content is leftover + io.Copy(ioutil.Discard, source) // Let's drain the pipe in case any content is leftover }() } @@ -216,8 +218,8 @@ func (e *Executable) Wait() (ExecutableResult, error) { e.cmd = nil e.stdoutPipe = nil e.stderrPipe = nil - e.StdoutBuffer = nil - e.StderrBuffer = nil + e.stdoutBuffer = nil + e.stderrBuffer = nil e.stdoutBytes = nil e.stderrBytes = nil e.stdoutLineWriter = nil @@ -243,8 +245,8 @@ func (e *Executable) Wait() (ExecutableResult, error) { e.stdoutLineWriter.Flush() e.stderrLineWriter.Flush() - stdout := e.StdoutBuffer.Bytes() - stderr := e.StderrBuffer.Bytes() + stdout := e.stdoutBuffer.Bytes() + stderr := e.stderrBuffer.Bytes() return ExecutableResult{ Stdout: stdout, From ed87d7896810375dd96ce28ad07f1dd8870ddee8 Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Tue, 21 May 2024 01:40:48 +0100 Subject: [PATCH 6/6] Refactor executable.go: Remove unused import and update io.Copy call --- executable/executable.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/executable/executable.go b/executable/executable.go index 68117f7..a7b21e1 100644 --- a/executable/executable.go +++ b/executable/executable.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "io/ioutil" "os" "path/filepath" "time" @@ -181,7 +180,7 @@ func (e *Executable) setupIORelay(source io.Reader, destination1 io.Writer, dest e.atleastOneReadDone = true e.readDone <- true - io.Copy(ioutil.Discard, source) // Let's drain the pipe in case any content is leftover + io.Copy(io.Discard, source) // Let's drain the pipe in case any content is leftover }() }