From 547626a83caf68f8a3e1cde26ea7b40aa7543245 Mon Sep 17 00:00:00 2001 From: Marius van Niekerk Date: Thu, 21 May 2026 10:50:40 -0400 Subject: [PATCH] fix: keep built binary after app shutdown (#911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Air could delete build.entrypoint while stopping the previous app process when stop_on_error was enabled. After builds were changed to keep the old app alive until a replacement build succeeds, that cleanup could remove the freshly rebuilt binary before it was started, causing issue #910. Failed builds should not delete the last executable either. Keeping the binary in place makes stop_on_error stop the process without destroying the artifact a later rebuild or manual restart may need. Windows needs different sequencing because running executables are locked. Stop the old process before build.cmd on Windows, while keeping the retained-app behavior on Unix platforms. Validation: go test ./runner -run 'TestAddPlatformOverridesForInit|TestPlatformBuildOverridesSelection|TestShouldStopBinBeforeBuild|TestStopBinBeforeBuildIfNeeded|TestBuildRunKeepsIssue910GoBuildEntrypoint' -count=1 -v; go test ./runner -run 'TestBuildRunKeepsIssue910GoBuildEntrypoint|TestShouldStopBinBeforeBuild|TestBuildRunKeeps.*Binary.*StopOnError|TestBuildRunStopsExistingBinWhenBuildFailsWithStopOnError|TestBuildRunStopsExistingBinAfterSuccessfulBuild|TestRebuild$' -count=1 -v; go test ./...; make check 🤖 Generated with [OpenAI Codex](https://openai.com/codex) Co-authored-by: Marius van Niekerk Co-authored-by: OpenAI Codex --- runner/common.go | 6 +- runner/config.go | 8 +- runner/config_test.go | 28 +++++- runner/engine.go | 29 ++++--- runner/engine_test.go | 196 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 245 insertions(+), 22 deletions(-) diff --git a/runner/common.go b/runner/common.go index 8930e71..1e57f6e 100644 --- a/runner/common.go +++ b/runner/common.go @@ -1,6 +1,10 @@ package runner const ( - //PlatformWindows const for windows + // PlatformWindows is the GOOS value for Windows. PlatformWindows = "windows" + // PlatformDarwin is the GOOS value for macOS. + PlatformDarwin = "darwin" + // PlatformLinux is the GOOS value for Linux. + PlatformLinux = "linux" ) diff --git a/runner/config.go b/runner/config.go index 6d03fbc..1b6902a 100644 --- a/runner/config.go +++ b/runner/config.go @@ -425,9 +425,9 @@ func platformBuildOverrides(build *cfgBuild, goos string) *cfgBuildOverrides { switch goos { case PlatformWindows: return build.Windows - case "darwin": + case PlatformDarwin: return build.Darwin - case "linux": + case PlatformLinux: return build.Linux default: return nil @@ -483,9 +483,9 @@ func addPlatformOverridesForInit(cfg *Config, goos string) { switch goos { case PlatformWindows: cfg.Build.Windows = override - case "darwin": + case PlatformDarwin: cfg.Build.Darwin = override - case "linux": + case PlatformLinux: cfg.Build.Linux = override } } diff --git a/runner/config_test.go b/runner/config_test.go index 267e592..9f726c7 100644 --- a/runner/config_test.go +++ b/runner/config_test.go @@ -369,6 +369,28 @@ func TestAddPlatformOverridesForInit(t *testing.T) { if !reflect.DeepEqual(cfg.Build.Windows.Entrypoint, entrypoint{`tmp\main.exe`}) { t.Fatalf("windows entrypoint mismatch: got %v", cfg.Build.Windows.Entrypoint) } + + cfg = defaultConfigBase() + cfg.Build.Cmd = "darwin-cmd" + setEntrypointFromBin(&cfg) + addPlatformOverridesForInit(&cfg, PlatformDarwin) + if cfg.Build.Darwin == nil { + t.Fatal("expected darwin overrides to be set") + } + if cfg.Build.Darwin.Cmd != "go build -o ./tmp/main ." { + t.Fatalf("darwin cmd mismatch: got %s", cfg.Build.Darwin.Cmd) + } + + cfg = defaultConfigBase() + cfg.Build.Cmd = "linux-cmd" + setEntrypointFromBin(&cfg) + addPlatformOverridesForInit(&cfg, PlatformLinux) + if cfg.Build.Linux == nil { + t.Fatal("expected linux overrides to be set") + } + if cfg.Build.Linux.Cmd != "go build -o ./tmp/main ." { + t.Fatalf("linux cmd mismatch: got %s", cfg.Build.Linux.Cmd) + } } func TestDefaultConfigForOS(t *testing.T) { @@ -382,7 +404,7 @@ func TestDefaultConfigForOS(t *testing.T) { t.Fatalf("windows bin mismatch: got %q", winCfg.Build.Bin) } - linuxCfg := defaultConfigForOS("linux") + linuxCfg := defaultConfigForOS(PlatformLinux) if linuxCfg.Build.Cmd != "go build -o ./tmp/main ." { t.Fatalf("linux cmd mismatch: got %q", linuxCfg.Build.Cmd) } @@ -511,10 +533,10 @@ func TestPlatformBuildOverridesSelection(t *testing.T) { if got := platformBuildOverrides(build, PlatformWindows); got != win { t.Fatalf("windows override mismatch: got %v", got) } - if got := platformBuildOverrides(build, "darwin"); got != darwin { + if got := platformBuildOverrides(build, PlatformDarwin); got != darwin { t.Fatalf("darwin override mismatch: got %v", got) } - if got := platformBuildOverrides(build, "linux"); got != linux { + if got := platformBuildOverrides(build, PlatformLinux); got != linux { t.Fatalf("linux override mismatch: got %v", got) } if got := platformBuildOverrides(build, "freebsd"); got != nil { diff --git a/runner/engine.go b/runner/engine.go index 28f213f..f8de529 100644 --- a/runner/engine.go +++ b/runner/engine.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "sync" "sync/atomic" @@ -540,6 +541,8 @@ func (e *Engine) buildRun() { default: } + e.stopBinBeforeBuildIfNeeded(runtime.GOOS) + e.loadEnvFile() var err error @@ -586,6 +589,18 @@ func (e *Engine) buildRun() { } } +func shouldStopBinBeforeBuild(goos string) bool { + return goos == PlatformWindows +} + +func (e *Engine) stopBinBeforeBuildIfNeeded(goos string) { + // Windows locks running executables, so direct builds to entrypoint need + // the old process stopped before build.cmd can overwrite the binary. + if shouldStopBinBeforeBuild(goos) { + e.stopBin() + } +} + func (e *Engine) flushEvents() { for { select { @@ -717,20 +732,6 @@ func (e *Engine) runBin() error { } else { e.mainDebug("cmd killed, pid: %d", pid) } - - if e.config.Build.StopOnError { - relBinPath := e.config.rel(e.config.binPath()) - if relBinPath == "" || strings.HasPrefix(relBinPath, "..") { - return - } - cmdBinPath := cmdPath(relBinPath) - if _, err = os.Stat(cmdBinPath); os.IsNotExist(err) { - return - } - if err = os.Remove(cmdBinPath); err != nil { - e.mainLog("failed to remove %s, error: %s", relBinPath, err) - } - } }() return shutdown diff --git a/runner/engine_test.go b/runner/engine_test.go index 4fe7307..3b73627 100644 --- a/runner/engine_test.go +++ b/runner/engine_test.go @@ -302,6 +302,112 @@ func TestBuildRunStopsExistingBinAfterSuccessfulBuild(t *testing.T) { } } +func TestBuildRunKeepsFreshBinaryAfterSuccessfulBuildWithStopOnError(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("uses POSIX shell commands") + } + + tmpDir := t.TempDir() + chdir(t, tmpDir) + + require.NoError(t, os.Mkdir("tmp", 0o755)) + binPath := filepath.Join("tmp", "dev") + initialScript := []byte("#!/bin/sh\ntrap 'exit 0' INT TERM\nwhile :; do sleep 1; done\n") + require.NoError(t, os.WriteFile(binPath, initialScript, 0o755)) + require.NoError(t, os.WriteFile("next.sh", initialScript, 0o755)) + + engine, err := NewEngine("", nil, true) + require.NoError(t, err) + engine.config.Log.Silent = true + engine.config.Build.Cmd = "cp ./next.sh ./tmp/dev && chmod +x ./tmp/dev" + engine.config.Build.Entrypoint = entrypoint{"./tmp/dev"} + engine.config.Build.StopOnError = true + engine.config.Build.SendInterrupt = true + engine.config.Build.KillDelay = 100 * time.Millisecond + + require.NoError(t, engine.runBin()) + defer engine.stopBin() + + err = waitForCondition(t, time.Second, func() bool { + started := false + engine.withLock(func() { + started = engine.binStopCh != nil + }) + return started + }, "initial binary process to start") + require.NoError(t, err) + + engine.buildRun() + + err = waitForCondition(t, time.Second, func() bool { + started := false + engine.withLock(func() { + started = engine.binStopCh != nil + }) + return started + }, "rebuilt binary process to start") + require.NoError(t, err) + require.FileExists(t, binPath) +} + +func TestBuildRunKeepsIssue910GoBuildEntrypoint(t *testing.T) { + if runtime.GOOS == PlatformWindows { + t.Skip("issue #910 was reported on Linux; Windows stops before build because running executables are locked") + } + + tmpDir := t.TempDir() + chdir(t, tmpDir) + + require.NoError(t, os.Mkdir("tmp", 0o755)) + require.NoError(t, os.WriteFile("go.mod", []byte("module issue910.test\n\ngo 1.17\n"), 0o644)) + require.NoError(t, os.WriteFile("main.go", []byte(`package main + +import "time" + +func main() { + for { + time.Sleep(time.Second) + } +} +`), 0o644)) + + binPath := filepath.Join("tmp", "dev") + require.NoError(t, exec.Command("go", "build", "-o", binPath, ".").Run()) + + engine, err := NewEngine("", nil, true) + require.NoError(t, err) + engine.config.Log.Silent = true + engine.config.Build.Cmd = "go build -o ./tmp/dev .; echo 'build ok'; sleep 1" + engine.config.Build.Entrypoint = entrypoint{"./tmp/dev"} + engine.config.Build.StopOnError = true + engine.config.Build.SendInterrupt = true + engine.config.Build.KillDelay = 500 * time.Millisecond + + require.NoError(t, engine.runBin()) + defer engine.stopBin() + + err = waitForCondition(t, time.Second, func() bool { + started := false + engine.withLock(func() { + started = engine.binStopCh != nil + }) + return started + }, "initial issue #910 binary process to start") + require.NoError(t, err) + + engine.buildRun() + + err = waitForCondition(t, time.Second, func() bool { + started := false + engine.withLock(func() { + started = engine.binStopCh != nil + }) + return started + }, "rebuilt issue #910 binary process to start") + require.NoError(t, err) + require.FileExists(t, binPath) +} + func TestBuildRunStopsExistingBinWhenBuildFailsWithStopOnError(t *testing.T) { engine, err := NewEngine("", nil, true) require.NoError(t, err) @@ -328,6 +434,96 @@ func TestBuildRunStopsExistingBinWhenBuildFailsWithStopOnError(t *testing.T) { } } +func TestBuildRunKeepsBinaryAfterFailedBuildWithStopOnError(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("uses POSIX shell commands") + } + + tmpDir := t.TempDir() + chdir(t, tmpDir) + + require.NoError(t, os.Mkdir("tmp", 0o755)) + binPath := filepath.Join("tmp", "dev") + script := []byte("#!/bin/sh\ntrap 'exit 0' INT TERM\nwhile :; do sleep 1; done\n") + require.NoError(t, os.WriteFile(binPath, script, 0o755)) + + engine, err := NewEngine("", nil, true) + require.NoError(t, err) + engine.config.Log.Silent = true + engine.config.Build.Cmd = "false" + engine.config.Build.Entrypoint = entrypoint{"./tmp/dev"} + engine.config.Build.StopOnError = true + engine.config.Build.SendInterrupt = true + engine.config.Build.KillDelay = 100 * time.Millisecond + + require.NoError(t, engine.runBin()) + defer engine.stopBin() + + err = waitForCondition(t, time.Second, func() bool { + started := false + engine.withLock(func() { + started = engine.binStopCh != nil + }) + return started + }, "initial binary process to start") + require.NoError(t, err) + + engine.buildRun() + + require.FileExists(t, binPath) +} + +func TestShouldStopBinBeforeBuild(t *testing.T) { + tests := []struct { + goos string + want bool + }{ + {goos: PlatformWindows, want: true}, + {goos: PlatformDarwin, want: false}, + {goos: PlatformLinux, want: false}, + } + + for _, tt := range tests { + t.Run(tt.goos, func(t *testing.T) { + assert.Equal(t, tt.want, shouldStopBinBeforeBuild(tt.goos)) + }) + } +} + +func TestStopBinBeforeBuildIfNeeded(t *testing.T) { + engine, err := NewEngine("", nil, true) + require.NoError(t, err) + engine.config.Log.Silent = true + + stopped := make(chan struct{}) + shutdown := make(chan chan int, 1) + engine.binStopCh = shutdown + go func() { + closer := <-shutdown + close(closer) + close(stopped) + }() + + engine.stopBinBeforeBuildIfNeeded(PlatformWindows) + + select { + case <-stopped: + case <-time.After(time.Second): + t.Fatal("expected Windows pre-build stop to stop the existing binary") + } + + shutdown = make(chan chan int, 1) + engine.binStopCh = shutdown + + engine.stopBinBeforeBuildIfNeeded(PlatformLinux) + + select { + case <-shutdown: + t.Fatal("non-Windows pre-build step should keep the existing binary running") + default: + } +} + func GetPort() (int, func()) { l, err := net.Listen("tcp", "127.0.0.1:0") if err != nil {