@@ -10,9 +10,10 @@ import (
1010 "fmt"
1111 "io"
1212 "os"
13- "os/exec"
1413 "strings"
1514 "time"
15+
16+ "github.com/sourcegraph/run"
1617)
1718
1819// Command contains the name, arguments and environment variables of a command.
@@ -121,6 +122,45 @@ type RunInDirOptions struct {
121122 Stderr io.Writer
122123}
123124
125+ // newRunCmd builds a *run.Command from the Command, applying the directory,
126+ // environment variables and default timeout.
127+ func (c * Command ) newRunCmd (dir string ) * run.Command {
128+ ctx := c .ctx
129+ if ctx == nil {
130+ ctx = context .Background ()
131+ }
132+
133+ // Apply default timeout if the context doesn't already have a deadline.
134+ if _ , ok := ctx .Deadline (); ! ok {
135+ var cancel context.CancelFunc
136+ ctx , cancel = context .WithTimeout (ctx , DefaultTimeout )
137+ // We cannot defer cancel here because the command hasn't run yet.
138+ // The caller is responsible for the context lifecycle. In practice the
139+ // timeout context will be collected when it expires or when the parent
140+ // is cancelled. We attach the cancel func to the context so the caller
141+ // could cancel it, but for this fire-and-forget usage the GC handles it.
142+ _ = cancel
143+ }
144+
145+ // run.Cmd joins all parts into a single string and then shell-parses it.
146+ // We must quote each argument so that special characters (spaces, quotes,
147+ // angle brackets, etc.) are preserved correctly.
148+ parts := make ([]string , 0 , 1 + len (c .args ))
149+ parts = append (parts , c .name )
150+ for _ , arg := range c .args {
151+ parts = append (parts , run .Arg (arg ))
152+ }
153+
154+ cmd := run .Cmd (ctx , parts ... )
155+ if dir != "" {
156+ cmd = cmd .Dir (dir )
157+ }
158+ if len (c .envs ) > 0 {
159+ cmd = cmd .Environ (append (os .Environ (), c .envs ... ))
160+ }
161+ return cmd
162+ }
163+
124164// RunInDirWithOptions executes the command in given directory and options. It
125165// pipes stdin from supplied io.Reader, and pipes stdout and stderr to supplied
126166// io.Writer. If the command's context does not have a deadline, DefaultTimeout
@@ -133,10 +173,10 @@ func (c *Command) RunInDirWithOptions(dir string, opts ...RunInDirOptions) (err
133173 }
134174
135175 buf := new (bytes.Buffer )
136- w := opt .Stdout
176+ stdout := opt .Stdout
137177 if logOutput != nil {
138178 buf .Grow (512 )
139- w = & limitDualWriter {
179+ stdout = & limitDualWriter {
140180 W : buf ,
141181 N : int64 (buf .Cap ()),
142182 w : opt .Stdout ,
@@ -151,65 +191,88 @@ func (c *Command) RunInDirWithOptions(dir string, opts ...RunInDirOptions) (err
151191 }
152192 }()
153193
154- ctx := c .ctx
155- if ctx = =nil {
156- ctx = context . Background ( )
194+ cmd := c .newRunCmd ( dir )
195+ if opt . Stdin ! =nil {
196+ cmd = cmd . Input ( opt . Stdin )
157197 }
158198
159- // Apply default timeout if the context doesn't already have a deadline.
160- if _ , ok := ctx .Deadline (); ! ok {
161- var cancel context.CancelFunc
162- ctx , cancel = context .WithTimeout (ctx , DefaultTimeout )
163- defer cancel ()
164- }
199+ // Build a combined writer for the command output. We need to capture
200+ // both stdout and stderr separately. sourcegraph/run's default Output
201+ // is combined output, but we need to split them.
202+ //
203+ // We use StdOut() to get only stdout on the output stream and handle
204+ // stderr via a pipe.
205+ //
206+ // However, sourcegraph/run doesn't have a direct way to wire both stdout
207+ // and stderr to separate writers in a single Run call. Instead, we use
208+ // the approach of streaming stdout and collecting stderr from the error.
209+ //
210+ // For the streaming case, we stream stdout to the writer and if there's
211+ // an error, stderr is included in the error message by default.
165212
166- cmd := exec .CommandContext (ctx , c .name , c .args ... )
167- if len (c .envs ) > 0 {
168- cmd .Env = append (os .Environ (), c .envs ... )
169- }
170- cmd .Dir = dir
171- cmd .Stdin = opt .Stdin
172- cmd .Stdout = w
173- cmd .Stderr = opt .Stderr
174- if err = cmd .Start (); err != nil {
175- if ctx .Err () == context .DeadlineExceeded {
176- return ErrExecTimeout
177- } else if ctx .Err () != nil {
178- return ctx .Err ()
213+ if stdout != nil && opt .Stderr != nil {
214+ // When both stdout and stderr writers are provided, we need to stream
215+ // stdout and capture stderr. We use StdOut() to only get stdout.
216+ output := cmd .StdOut ().Run ()
217+ streamErr := output .Stream (stdout )
218+ if streamErr != nil {
219+ // Extract stderr from the error and write it to the stderr writer.
220+ // sourcegraph/run includes stderr in the error by default.
221+ _ , _ = fmt .Fprint (opt .Stderr , extractStderr (streamErr ))
222+ return mapContextError (streamErr , c .ctx )
179223 }
180- return err
224+ return nil
225+ } else if stdout != nil {
226+ // Only stdout writer provided
227+ output := cmd .StdOut ().Run ()
228+ streamErr := output .Stream (stdout )
229+ if streamErr != nil {
230+ return mapContextError (streamErr , c .ctx )
231+ }
232+ return nil
181233 }
182234
183- result := make (chan error )
184- go func () {
185- result <- cmd .Wait ()
186- }()
235+ // No writers - just wait for completion
236+ waitErr := cmd .Run ().Wait ()
237+ if waitErr != nil {
238+ return mapContextError (waitErr , c .ctx )
239+ }
240+ return nil
241+ }
187242
188- select {
189- case <- ctx .Done ():
190- // Kill the process before waiting so cancellation is enforced promptly.
191- if cmd .Process != nil {
192- _ = cmd .Process .Kill ()
243+ // mapContextError maps context errors to the appropriate sentinel errors used
244+ // by this package.
245+ func mapContextError (err error , ctx context.Context ) error {
246+ if ctx == nil {
247+ return err
248+ }
249+ if ctxErr := ctx .Err (); ctxErr != nil {
250+ if ctxErr == context .DeadlineExceeded {
251+ return ErrExecTimeout
193252 }
194- <- result
195-
253+ return ctxErr
254+ }
255+ // Also check if the error itself wraps a context error
256+ if strings .Contains (err .Error (), "signal: killed" ) || strings .Contains (err .Error (), context .DeadlineExceeded .Error ()) {
196257 if ctx .Err () == context .DeadlineExceeded {
197258 return ErrExecTimeout
198259 }
199- return ctx .Err ()
200- case err = <- result :
201- // Normalize errors when the context may have expired around the same time.
202- if err != nil {
203- if ctxErr := ctx .Err (); ctxErr != nil {
204- if ctxErr == context .DeadlineExceeded {
205- return ErrExecTimeout
206- }
207- return ctxErr
208- }
209- }
210- return err
211260 }
261+ return err
262+ }
212263
264+ // extractStderr attempts to extract the stderr portion from a sourcegraph/run
265+ // error. The error format is typically "exit status N: <stderr content>".
266+ func extractStderr (err error ) string {
267+ if err == nil {
268+ return ""
269+ }
270+ msg := err .Error ()
271+ // sourcegraph/run error format: "exit status N: <stderr>"
272+ if idx := strings .Index (msg , ": " ); idx >= 0 && strings .HasPrefix (msg , "exit status" ) {
273+ return msg [idx + 2 :]
274+ }
275+ return msg
213276}
214277
215278// RunInDirPipeline executes the command in given directory. It pipes stdout and
@@ -225,11 +288,42 @@ func (c *Command) RunInDirPipeline(stdout, stderr io.Writer, dir string) error {
225288// RunInDir executes the command in given directory. It returns stdout and error
226289// (combined with stderr).
227290func (c * Command ) RunInDir (dir string ) ([]byte , error ) {
291+ cmd := c .newRunCmd (dir )
292+
293+ logBuf := new (bytes.Buffer )
294+ if logOutput != nil {
295+ logBuf .Grow (512 )
296+ }
297+
298+ defer func () {
299+ if len (dir ) == 0 {
300+ log ("%s\n %s" , c , logBuf .Bytes ())
301+ } else {
302+ log ("%s: %s\n %s" , dir , c , logBuf .Bytes ())
303+ }
304+ }()
305+
306+ // Use Stream to a buffer to preserve raw bytes (including NUL bytes from
307+ // commands like "ls-tree -z"). The String/Lines methods process output
308+ // line-by-line which corrupts binary-ish output.
228309 stdout := new (bytes.Buffer )
229- stderr := new (bytes.Buffer )
230- if err := c .RunInDirPipeline (stdout , stderr , dir ); err != nil {
231- return nil , concatenateError (err , stderr .String ())
310+ err := cmd .StdOut ().Run ().Stream (stdout )
311+ if err != nil {
312+ return nil , mapContextError (err , c .ctx )
313+ }
314+
315+ if logOutput != nil {
316+ data := stdout .Bytes ()
317+ limit := len (data )
318+ if limit > 512 {
319+ limit = 512
320+ }
321+ logBuf .Write (data [:limit ])
322+ if len (data ) > 512 {
323+ logBuf .WriteString ("... (more omitted)" )
324+ }
232325 }
326+
233327 return stdout .Bytes (), nil
234328}
235329
0 commit comments