diff --git a/go.mod b/go.mod index 31feb127f49..035214dd0bf 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/Masterminds/semver/v3 v3.5.0 github.com/bmatcuk/doublestar/v4 v4.10.3 github.com/briandowns/spinner v1.23.2 + github.com/creack/pty v1.1.24 github.com/denisbrodbeck/machineid v1.0.1 github.com/f1bonacc1/process-compose v1.122.0 github.com/fatih/color v1.19.0 @@ -34,6 +35,7 @@ require ( golang.org/x/exp v0.0.0-20261014143641-f0251eb4aa3e golang.org/x/mod v0.42.0 golang.org/x/sync v0.24.0 + golang.org/x/sys v0.49.0 golang.org/x/tools v0.52.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -101,7 +103,6 @@ require ( github.com/clipperhouse/uax29/v2 v2.7.0 // indirect github.com/codeclysm/extract/v4 v4.0.0 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect - github.com/creack/pty v1.1.24 // indirect github.com/curioswitch/go-reassign v0.3.0 // indirect github.com/daixiang0/gci v0.13.7 // indirect github.com/dave/dst v0.28.0 // indirect @@ -274,7 +275,6 @@ require ( go4.org v0.0.0-20230225012048-214862532bf5 // indirect golang.org/x/exp/typeparams v0.0.0-20260811152304-ee035b5b010f // indirect golang.org/x/oauth2 v0.33.0 // indirect - golang.org/x/sys v0.49.0 // indirect golang.org/x/term v0.47.0 // indirect golang.org/x/text v0.43.0 // indirect google.golang.org/protobuf v1.36.12 // indirect diff --git a/internal/boxcli/midcobra/debug.go b/internal/boxcli/midcobra/debug.go index c6c9b7ca98b..673349366ee 100644 --- a/internal/boxcli/midcobra/debug.go +++ b/internal/boxcli/midcobra/debug.go @@ -52,6 +52,12 @@ func (d *DebugMiddleware) postRun(cmd *cobra.Command, args []string, runErr erro if runErr == nil { return } + // The user pressed Ctrl-C and the command has exited. The exit code (130) + // is all there is to report. + var execErr *usererr.ExitError + if errors.As(runErr, &execErr) && execErr.Interrupted() { + return + } if userErr, hasUserErr := usererr.Extract(runErr); hasUserErr { if usererr.IsWarning(userErr) { ux.Fwarning(cmd.ErrOrStderr(), runErr.Error()) diff --git a/internal/boxcli/usererr/exiterr.go b/internal/boxcli/usererr/exiterr.go index 2f7b0d25363..c7edaffbc9c 100644 --- a/internal/boxcli/usererr/exiterr.go +++ b/internal/boxcli/usererr/exiterr.go @@ -6,6 +6,7 @@ package usererr import ( "errors" "os/exec" + "syscall" ) // ExitError is an ExitError for a command run on behalf of a user @@ -27,3 +28,19 @@ func NewExecError(source error) error { } return &ExitError{ExitError: exitErr} } + +// ExitCode returns the command's exit code or, if a signal killed it, 128 plus +// the signal number, the same as a shell reports it (e.g. 130 for Ctrl-C). +func (e *ExitError) ExitCode() int { + if status, ok := e.Sys().(syscall.WaitStatus); ok && status.Signaled() { + return 128 + int(status.Signal()) + } + return e.ExitError.ExitCode() +} + +// Interrupted reports whether the command was killed by SIGINT, which usually +// means the user pressed Ctrl-C. +func (e *ExitError) Interrupted() bool { + status, ok := e.Sys().(syscall.WaitStatus) + return ok && status.Signaled() && status.Signal() == syscall.SIGINT +} diff --git a/internal/boxcli/usererr/exiterr_test.go b/internal/boxcli/usererr/exiterr_test.go new file mode 100644 index 00000000000..0a475d79f8c --- /dev/null +++ b/internal/boxcli/usererr/exiterr_test.go @@ -0,0 +1,37 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package usererr + +import ( + "errors" + "os/exec" + "testing" +) + +func TestExitErrorExitCode(t *testing.T) { + tests := map[string]struct { + script string + wantCode int + wantInterrupted bool + }{ + "exit status": {script: "exit 3", wantCode: 3}, + "killed by SIGINT": {script: "kill -INT $$", wantCode: 130, wantInterrupted: true}, + "killed by SIGTERM": {script: "kill -TERM $$", wantCode: 143}, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + err := NewExecError(exec.Command("sh", "-c", test.script).Run()) + var exitErr *ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("got error %v (%T), want *ExitError", err, err) + } + if got := exitErr.ExitCode(); got != test.wantCode { + t.Errorf("got exit code %d, want %d", got, test.wantCode) + } + if got := exitErr.Interrupted(); got != test.wantInterrupted { + t.Errorf("got Interrupted() = %t, want %t", got, test.wantInterrupted) + } + }) + } +} diff --git a/internal/nix/run.go b/internal/nix/run.go index 2625cddc6f1..4bde070e52e 100644 --- a/internal/nix/run.go +++ b/internal/nix/run.go @@ -9,6 +9,10 @@ import ( "log/slog" "os" "os/exec" + "os/signal" + "syscall" + + "golang.org/x/sys/unix" "go.jetify.com/devbox/internal/boxcli/usererr" "go.jetify.com/devbox/internal/cmdutil" @@ -35,5 +39,68 @@ func RunScript(projectDir, cmdWithArgs string, env map[string]string) error { slog.Debug("executing script", "cmd", cmd.Args) // Report error as exec error when executing scripts. - return usererr.NewExecError(cmd.Run()) + return usererr.NewExecError(runWaitingOnInterrupt(cmd)) +} + +// runWaitingOnInterrupt runs cmd. If the user presses Ctrl-C (or Ctrl-\), +// devbox stays alive until cmd exits, so that the shell prompt only comes back +// once the command has finished shutting down, the same as running the command +// directly or with `npm run`. +func runWaitingOnInterrupt(cmd *exec.Cmd) error { + // Subscribe before starting the command so that devbox can't be killed + // between starting it and waiting for it. Leave ignored signals alone: + // Notify would stop ignoring them, in devbox and in the command (e.g. in a + // background job of a non-interactive shell). + signals := make(chan os.Signal, 1) + for _, sig := range []os.Signal{syscall.SIGINT, syscall.SIGQUIT} { + if !signal.Ignored(sig) { + signal.Notify(signals, sig) + } + } + defer signal.Stop(signals) + + if err := cmd.Start(); err != nil { + return err + } + + done := make(chan struct{}) + defer close(done) + go func() { + for { + select { + case sig := <-signals: + handleInterrupt(sig) + case <-done: + return + } + } + }() + return cmd.Wait() +} + +func handleInterrupt(sig os.Signal) { + // The terminal sends Ctrl-C and Ctrl-\ to its whole foreground process + // group, which includes the command, so it's enough to wait for it. + if inForegroundProcessGroup() { + slog.Debug("waiting for command to exit after interrupt", "signal", sig) + return + } + // The signal came from somewhere else (e.g. kill -INT ), and + // the command didn't get it. Exit the way devbox did before it handled + // interrupts. + signal.Reset(sig) + _ = syscall.Kill(os.Getpid(), sig.(syscall.Signal)) +} + +// inForegroundProcessGroup reports whether devbox (and so the command it +// started, which shares its process group) is in the foreground process group +// of its controlling terminal. +func inForegroundProcessGroup() bool { + tty, err := os.Open("/dev/tty") + if err != nil { + return false // no controlling terminal + } + defer tty.Close() + pgrp, err := unix.IoctlGetInt(int(tty.Fd()), unix.TIOCGPGRP) + return err == nil && pgrp == unix.Getpgrp() } diff --git a/internal/nix/run_test.go b/internal/nix/run_test.go new file mode 100644 index 00000000000..979a96c7699 --- /dev/null +++ b/internal/nix/run_test.go @@ -0,0 +1,105 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package nix + +import ( + "bytes" + "errors" + "fmt" + "io" + "os" + "os/exec" + "strings" + "sync" + "testing" + "time" + + "github.com/creack/pty" + + "go.jetify.com/devbox/internal/boxcli/usererr" +) + +const runScriptHelperEnv = "DEVBOX_TEST_RUN_SCRIPT_HELPER" + +// TestRunScriptHelper plays the part of `devbox run` for +// TestRunScriptWaitsForCommandOnCtrlC, which runs it in a terminal. +func TestRunScriptHelper(t *testing.T) { + if os.Getenv(runScriptHelperEnv) == "" { + t.Skip("helper process for TestRunScriptWaitsForCommandOnCtrlC") + } + script := `trap 'echo command: shutting down; sleep 0.5; echo command: shutdown complete; exit 3' INT +echo command: started +while true; do sleep 0.1; done` + err := RunScript(t.TempDir(), script, map[string]string{"PATH": os.Getenv("PATH")}) + var exitErr *usererr.ExitError + if errors.As(err, &exitErr) { + fmt.Printf("RunScript returned exit code %d\n", exitErr.ExitCode()) + } else { + fmt.Printf("RunScript returned %v\n", err) + } + os.Exit(0) +} + +// TestRunScriptWaitsForCommandOnCtrlC checks that pressing Ctrl-C doesn't kill +// devbox while the command is still shutting down (#3026). +func TestRunScriptWaitsForCommandOnCtrlC(t *testing.T) { + cmd := exec.Command(os.Args[0], "-test.run=^TestRunScriptHelper$") + cmd.Env = append(os.Environ(), runScriptHelperEnv+"=1") + terminal, err := pty.Start(cmd) // the helper's controlling terminal + if err != nil { + t.Fatal(err) + } + defer terminal.Close() + + var outMu sync.Mutex + var out bytes.Buffer + go func() { + buf := make([]byte, 1024) + for { + n, err := terminal.Read(buf) + outMu.Lock() + out.Write(buf[:n]) + outMu.Unlock() + if err != nil { + return + } + } + }() + output := func() string { + outMu.Lock() + defer outMu.Unlock() + return out.String() + } + + deadline := time.Now().Add(10 * time.Second) + for !strings.Contains(output(), "command: started") { + if time.Now().After(deadline) { + t.Fatalf("timed out waiting for the command to start, output:\n%s", output()) + } + time.Sleep(10 * time.Millisecond) + } + if _, err := io.WriteString(terminal, "\x03"); err != nil { // Ctrl-C + t.Fatal(err) + } + + waitErr := make(chan error, 1) + go func() { waitErr <- cmd.Wait() }() + select { + case err := <-waitErr: + if err != nil { + t.Fatalf("devbox didn't exit cleanly: %v, output:\n%s", err, output()) + } + case <-time.After(10 * time.Second): + t.Fatalf("timed out waiting for devbox to exit, output:\n%s", output()) + } + + // Give the reader a moment to drain what's left in the terminal. + time.Sleep(100 * time.Millisecond) + got := output() + complete := strings.Index(got, "command: shutdown complete") + returned := strings.Index(got, "RunScript returned exit code 3") + if complete == -1 || returned == -1 || returned < complete { + t.Errorf("RunScript should return the command's exit code after it shuts down, output:\n%s", got) + } +}