fix(summer): give delegated commands the terminal and keep their stderr apart
- appStreams hands the app binary summer's own stdout and stderr on a terminal, so it asks the production question, prints styled summaries and animates its widgets - off a terminal the binary's stdout goes through summer's Output and its stderr to summer's stderr, so --json and -q output carry no log lines - newAppCmd (unexported) takes the two writers; docs say the streams stay apart (CR-03)
This commit is contained in:
@@ -141,7 +141,22 @@ func runAppBinary(ctx context.Context, argv []string, out bonfire.Output) error
|
||||
return fmt.Errorf("summer: refusing to recurse into %s", bin)
|
||||
}
|
||||
}
|
||||
return runAppCmd(newAppCmd(ctx, bin, appDir, argv, out), argv)
|
||||
stdout, stderr := appStreams(out, bonfire.IsTerminal(out))
|
||||
return runAppCmd(newAppCmd(ctx, bin, appDir, argv, stdout, stderr), argv)
|
||||
}
|
||||
|
||||
// appStreams picks the app binary's stdout and stderr. On a terminal
|
||||
// (terminal is bonfire.IsTerminal(out)) the binary gets summer's own
|
||||
// streams, so it sees the terminal: it asks the production question, prints
|
||||
// styled summaries and animates its widgets. Otherwise its stdout goes
|
||||
// through out (tests and bonfire.Call roots capture it) and its stderr to
|
||||
// summer's stderr, so log lines and error blocks never mix into data such
|
||||
// as --json output.
|
||||
func appStreams(out bonfire.Output, terminal bool) (stdout, stderr io.Writer) {
|
||||
if terminal {
|
||||
return os.Stdout, os.Stderr
|
||||
}
|
||||
return out, os.Stderr
|
||||
}
|
||||
|
||||
// runAppCmd runs cmd, the app binary started with argv, forwarding SIGINT
|
||||
@@ -170,15 +185,16 @@ func runAppCmd(cmd *exec.Cmd, argv []string) error {
|
||||
// timeout of serve and the workers, so they finish a graceful shutdown.
|
||||
const appShutdownGrace = 15 * time.Second
|
||||
|
||||
// newAppCmd builds the command that runs the app binary bin with argv in dir.
|
||||
// The child inherits summer's stdin, so the hidden password prompts of
|
||||
// admin:create and admin:reset-password read from the terminal. Cancelling
|
||||
// ctx sends SIGTERM, and the child is killed only after appShutdownGrace.
|
||||
func newAppCmd(ctx context.Context, bin, dir string, argv []string, out io.Writer) *exec.Cmd {
|
||||
// newAppCmd builds the command that runs the app binary bin with argv in
|
||||
// dir, writing to stdout and stderr (see appStreams). The child inherits
|
||||
// summer's stdin, so the hidden password prompts of admin:create and
|
||||
// admin:reset-password read from the terminal. Cancelling ctx sends
|
||||
// SIGTERM, and the child is killed only after appShutdownGrace.
|
||||
func newAppCmd(ctx context.Context, bin, dir string, argv []string, stdout, stderr io.Writer) *exec.Cmd {
|
||||
cmd := exec.CommandContext(ctx, bin, argv...)
|
||||
cmd.Dir = dir
|
||||
cmd.Stdout = out
|
||||
cmd.Stderr = out
|
||||
cmd.Stdout = stdout
|
||||
cmd.Stderr = stderr
|
||||
cmd.Stdin = os.Stdin
|
||||
cmd.Env = os.Environ()
|
||||
cmd.Cancel = func() error {
|
||||
|
||||
@@ -5,9 +5,11 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"os/signal"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"slices"
|
||||
"strings"
|
||||
@@ -189,7 +191,7 @@ func TestRunChildForwardsSignals(t *testing.T) {
|
||||
for _, sig := range []os.Signal{os.Interrupt, syscall.SIGTERM} {
|
||||
t.Run(sig.String(), func(t *testing.T) {
|
||||
var out syncBuffer
|
||||
cmd := newAppCmd(context.Background(), os.Args[0], ".", helperArgv(), &out)
|
||||
cmd := newAppCmd(context.Background(), os.Args[0], ".", helperArgv(), &out, &out)
|
||||
helperEnv(cmd)
|
||||
sigs := make(chan os.Signal, 1)
|
||||
done := make(chan error, 1)
|
||||
@@ -218,7 +220,7 @@ func TestNewAppCmdCancelSendsSIGTERM(t *testing.T) {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
var out syncBuffer
|
||||
cmd := newAppCmd(ctx, os.Args[0], ".", helperArgv(), &out)
|
||||
cmd := newAppCmd(ctx, os.Args[0], ".", helperArgv(), &out, &out)
|
||||
helperEnv(cmd)
|
||||
done := make(chan error, 1)
|
||||
go func() { done <- runChild(cmd, nil) }()
|
||||
@@ -311,7 +313,8 @@ func exitHelperRunner(t *testing.T, mode string) {
|
||||
t.Helper()
|
||||
prev := appRunner
|
||||
appRunner = func(ctx context.Context, argv []string, out bonfire.Output) error {
|
||||
cmd := newAppCmd(ctx, os.Args[0], ".", []string{"-test.run=^TestExitHelperProcess$"}, out)
|
||||
stdout, stderr := appStreams(out, bonfire.IsTerminal(out))
|
||||
cmd := newAppCmd(ctx, os.Args[0], ".", []string{"-test.run=^TestExitHelperProcess$"}, stdout, stderr)
|
||||
cmd.Env = append(os.Environ(), "SUMMER_EXIT_HELPER="+mode)
|
||||
return runAppCmd(cmd, argv)
|
||||
}
|
||||
@@ -360,3 +363,53 @@ func TestDelegateExitCodePassthrough(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestDelegateStreams (CR-03): off a terminal the app binary's stdout goes
|
||||
// through summer's Output and its stderr to summer's stderr, never into
|
||||
// summer's stdout.
|
||||
func TestDelegateStreams(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("the fake app binary is a shell script")
|
||||
}
|
||||
app := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(app, "summer.yaml"), []byte("module: example.com/acme\nbinary: acme\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Join(app, "bin"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
script := "#!/bin/sh\necho data-line\necho log-line >&2\n"
|
||||
if err := os.WriteFile(filepath.Join(app, "bin", "acme"), []byte(script), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Chdir(app)
|
||||
out, err := runSummer(t, "route:list", "--json")
|
||||
if err != nil {
|
||||
t.Fatalf("route:list: %v\n%s", err, out)
|
||||
}
|
||||
if !strings.Contains(out, "data-line") {
|
||||
t.Errorf("the binary's stdout did not reach summer's Output: %q", out)
|
||||
}
|
||||
if strings.Contains(out, "log-line") {
|
||||
t.Errorf("the binary's stderr landed in summer's stdout: %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAppStreams (CR-03): on a terminal the app binary gets summer's own
|
||||
// stdout and stderr; off a terminal its stdout goes through the Output and
|
||||
// its stderr to summer's stderr.
|
||||
func TestAppStreams(t *testing.T) {
|
||||
var buf bytes.Buffer
|
||||
out := bonfire.NewOutput(strings.NewReader(""), &buf, &buf)
|
||||
if bonfire.IsTerminal(out) {
|
||||
t.Fatal("a buffer console reports a terminal")
|
||||
}
|
||||
stdout, stderr := appStreams(out, true)
|
||||
if stdout != os.Stdout || stderr != os.Stderr {
|
||||
t.Errorf("terminal: streams %v, %v; want os.Stdout, os.Stderr", stdout, stderr)
|
||||
}
|
||||
stdout, stderr = appStreams(out, false)
|
||||
if stdout != io.Writer(out) || stderr != os.Stderr {
|
||||
t.Errorf("not a terminal: streams %v, %v; want the Output, os.Stderr", stdout, stderr)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -141,6 +141,8 @@ summer -v migrate --pretend
|
||||
summer admin:token:create jane --scope journal.posts --json
|
||||
```
|
||||
|
||||
The binary writes to your terminal directly, so it asks the production question, prints its styled startup summary and animates its spinners as if you had run it yourself. Its error stream stays separate from its output: `summer route:list --json > routes.json` captures only the JSON, while warnings and log lines still reach the terminal on stderr.
|
||||
|
||||
When the binary fails, `summer` exits with the binary's exit code (a usage error stays `2`) and prints nothing more: the binary has already printed its error. A binary killed by a signal gets an error block from `summer` and exit code `1`.
|
||||
|
||||
When `summer` receives SIGINT or SIGTERM while the binary runs, it passes the signal on and waits for the binary to shut down, so stopping `summer serve` with Ctrl+C or `kill` frees its port.
|
||||
|
||||
@@ -43,7 +43,7 @@ Applications pin exact versions, so a new release never reaches an application u
|
||||
- **Exit codes.** A failed command exits 1 and a usage error (an unknown command or flag, a wrong number of arguments) exits 2. Errors print as a block on stderr with a hint where the framework knows the fix, and `-v` shows the wrapped errors.
|
||||
- **`-q` keeps data.** `-q` hides information and success lines, spinners and progress bars, but still prints what a command exists to produce: the token of `admin:token:create`, the key of `key:generate` and every `--json` output.
|
||||
- **Production confirmation.** `migrate`, `migrate:rollback`, `deferred:purge` and `queue:clear` ask before they run when the environment is `production` and they run on an interactive terminal; `--force` skips the question. Off a terminal, as in deploy scripts over SSH, nothing changes.
|
||||
- **`summer` delegates keep the binary's exit code.** Since 0.4.2 `summer migrate`, `summer serve` and the other delegated commands exit with the app binary's exit code and print only the binary's error block; 0.4.0 and 0.4.1 printed a second block and exited 1.
|
||||
- **`summer` delegates keep the binary's exit code.** Since 0.4.2 `summer migrate`, `summer serve` and the other delegated commands exit with the app binary's exit code and print only the binary's error block; 0.4.0 and 0.4.1 printed a second block and exited 1. The binary also gets the terminal again, so the production question, styled summaries and spinners work through `summer`, and its stderr no longer lands in `summer`'s stdout: a script that captured `summer route:list --json` together with log lines now gets only the JSON.
|
||||
- **`summer dev` runs `serve`.** It used to start the binary without a command, which printed the help and stopped; it now runs `bin/<app> serve` with the global options you give `summer dev`.
|
||||
- **Result lines.** `summer build` ends with `Built bin/<app> in <time>`, the `make:` commands print `Created <path>` per file, `migrate` prints `Applied N migrations` or `Nothing to migrate`, and `queue:clear` prints `Cleared N jobs from <queue>`. Adjust scripts that matched the old lower-case lines.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user