From 41d1bf3d109a002b7dbb45cb27dd3cdfac2f38cf Mon Sep 17 00:00:00 2001 From: stffabi Date: Thu, 30 Jun 2022 12:13:06 +0200 Subject: [PATCH] [v2] Improvements to the dev command (#1510) * [v2, darwin] Fix nullreference exception when dev watcher failed to start and the user stopped wails dev with ctrl-c * [v2] Improve starting of dev watcher command Wails dev now also fails if the dev watcher could not be started. Since the dev watcher is used to start e.g. vite to serve the frontend, it does not make much sense to let Wails dev be running if the dev watcher failed to start. * [v2] Improve killing and cleanup of debug binary - Make sure to kill the debug process in all cases even if an error occured. - Make sure to kill the last started binary process --- v2/cmd/wails/internal/commands/dev/dev.go | 106 +++++++++++------- .../wails/internal/commands/dev/dev_other.go | 8 +- 2 files changed, 70 insertions(+), 44 deletions(-) diff --git a/v2/cmd/wails/internal/commands/dev/dev.go b/v2/cmd/wails/internal/commands/dev/dev.go index 4204f20a5..2ae004d3d 100644 --- a/v2/cmd/wails/internal/commands/dev/dev.go +++ b/v2/cmd/wails/internal/commands/dev/dev.go @@ -2,6 +2,7 @@ package dev import ( "context" + "errors" "fmt" "io" "net" @@ -168,8 +169,6 @@ func AddSubcommand(app *clir.Cli, w io.Writer) error { buildOptions.Logger = logger buildOptions.UserTags = internal.ParseUserTags(flags.tags) - var debugBinaryProcess *process.Process = nil - // Setup signal handler quitChannel := make(chan os.Signal, 1) signal.Notify(quitChannel, os.Interrupt, os.Kill, syscall.SIGTERM) @@ -177,19 +176,23 @@ func AddSubcommand(app *clir.Cli, w io.Writer) error { // Do initial build logger.Println("Building application for development...") - newProcess, appBinary, err := restartApp(buildOptions, debugBinaryProcess, flags, exitCodeChannel) + debugBinaryProcess, appBinary, err := restartApp(buildOptions, nil, flags, exitCodeChannel) if err != nil { return err } - if newProcess != nil { - debugBinaryProcess = newProcess - } + defer func() { + if err := killProcessAndCleanupBinary(debugBinaryProcess, appBinary); err != nil { + LogDarkYellow("Unable to kill process and cleanup binary: %s", err) + } + }() // frontend:dev:watcher command. if command := projectConfig.DevWatcherCommand; command != "" { - var devCommandWaitGroup sync.WaitGroup - closer := runFrontendDevWatcherCommand(cwd, command, &devCommandWaitGroup) - defer closer(&devCommandWaitGroup) + closer, err := runFrontendDevWatcherCommand(cwd, command) + if err != nil { + return err + } + defer closer() } // open browser @@ -221,22 +224,17 @@ func AddSubcommand(app *clir.Cli, w io.Writer) error { LogGreen("Using reload debounce setting of %d milliseconds", flags.debounceMS) // Watch for changes and trigger restartApp() - doWatcherLoop(buildOptions, debugBinaryProcess, flags, watcher, exitCodeChannel, quitChannel, devServerURL) + debugBinaryProcess = doWatcherLoop(buildOptions, debugBinaryProcess, flags, watcher, exitCodeChannel, quitChannel, devServerURL) - // Kill the current program if running - if debugBinaryProcess != nil { - err := debugBinaryProcess.Kill() - if err != nil { - return err - } - } - - // Remove dev binary - err = os.Remove(appBinary) - if err != nil { + // Kill the current program if running and remove dev binary + if err := killProcessAndCleanupBinary(debugBinaryProcess, appBinary); err != nil { return err } + // Reset the process and the binary so the defer knows about it and is a nop. + debugBinaryProcess = nil + appBinary = "" + LogGreen("Development mode exited") return nil @@ -244,6 +242,22 @@ func AddSubcommand(app *clir.Cli, w io.Writer) error { return nil } +func killProcessAndCleanupBinary(process *process.Process, binary string) error { + if process != nil && process.Running { + if err := process.Kill(); err != nil { + return err + } + } + + if binary != "" { + err := os.Remove(binary) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + } + return nil +} + func syncGoModVersion(cwd string) error { gomodFilename := filepath.Join(cwd, "go.mod") gomodData, err := os.ReadFile(gomodFilename) @@ -388,35 +402,39 @@ func loadAndMergeProjectConfig(cwd string, flags *devFlags) (*project.Project, e } // runFrontendDevWatcherCommand will run the `frontend:dev:watcher` command if it was given, ex- `npm run dev` -func runFrontendDevWatcherCommand(cwd string, devCommand string, wg *sync.WaitGroup) func(group *sync.WaitGroup) { - LogGreen("Running frontend dev watcher command: '%s'", devCommand) +func runFrontendDevWatcherCommand(cwd string, devCommand string) (func(), error) { ctx, cancel := context.WithCancel(context.Background()) dir := filepath.Join(cwd, "frontend") cmdSlice := strings.Split(devCommand, " ") - wg.Add(1) cmd := exec.CommandContext(ctx, cmdSlice[0], cmdSlice[1:]...) cmd.Stderr = os.Stderr cmd.Stdout = os.Stdout cmd.Dir = dir - setParentGID(cmd) - go func(ctx context.Context, devCommand string, cwd string, wg *sync.WaitGroup) { - err := cmd.Run() - if err != nil { + if err := cmd.Start(); err != nil { + cancel() + return nil, fmt.Errorf("Unable to start frontend DevWatcher: %w", err) + } + + LogGreen("Running frontend DevWatcher command: '%s'", devCommand) + var wg sync.WaitGroup + wg.Add(1) + go func() { + if err := cmd.Wait(); err != nil { if err.Error() != "exit status 1" { - LogRed("Error from '%s': %s", devCommand, err.Error()) + LogRed("Error from DevWatcher '%s': %s", devCommand, err.Error()) } } - LogGreen("Dev command exited!") + LogGreen("DevWatcher command exited!") wg.Done() - }(ctx, devCommand, cwd, wg) + }() - return func(wg *sync.WaitGroup) { + return func() { killProc(cmd, devCommand) - LogGreen("Dev command killed!") + LogGreen("DevWatcher command killed!") cancel() wg.Wait() - } + }, nil } // initialiseWatcher creates the project directory watcher that will trigger recompile @@ -463,8 +481,13 @@ func restartApp(buildOptions *build.Options, debugBinaryProcess *process.Process appBinary, err := build.Build(buildOptions) println() if err != nil { - LogRed("Build error - continuing to run current version") - LogDarkYellow(err.Error()) + LogRed("Build error - " + err.Error()) + + msg := "Continuing to run current version" + if debugBinaryProcess == nil { + msg = "No version running, build will be retriggered as soon as changes have been detected" + } + LogDarkYellow(msg) return nil, "", nil } @@ -510,12 +533,8 @@ func restartApp(buildOptions *build.Options, debugBinaryProcess *process.Process } // doWatcherLoop is the main watch loop that runs while dev is active -func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Process, flags devFlags, watcher *fsnotify.Watcher, exitCodeChannel chan int, quitChannel chan os.Signal, devServerURL *url.URL) { +func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Process, flags devFlags, watcher *fsnotify.Watcher, exitCodeChannel chan int, quitChannel chan os.Signal, devServerURL *url.URL) *process.Process { // Main Loop - var ( - err error - newBinaryProcess *process.Process - ) var extensionsThatTriggerARebuild = sliceToMap(strings.Split(flags.extensions, ",")) var dirsThatTriggerAReload []string for _, dir := range strings.Split(flags.reloadDirs, ",") { @@ -601,7 +620,7 @@ func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Proc rebuild = false LogGreen("[Rebuild triggered] files updated") // Try and build the app - newBinaryProcess, _, err = restartApp(buildOptions, debugBinaryProcess, flags, exitCodeChannel) + newBinaryProcess, _, err := restartApp(buildOptions, debugBinaryProcess, flags, exitCodeChannel) if err != nil { LogRed("Error during build: %s", err.Error()) continue @@ -647,7 +666,7 @@ func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Proc } if reload { reload = false - _, err = http.Get(reloadURL) + _, err := http.Get(reloadURL) if err != nil { LogRed("Error during refresh: %s", err.Error()) } @@ -657,6 +676,7 @@ func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Proc quit = true } } + return debugBinaryProcess } func joinPath(url *url.URL, subPath string) string { diff --git a/v2/cmd/wails/internal/commands/dev/dev_other.go b/v2/cmd/wails/internal/commands/dev/dev_other.go index 15267ceb2..c93aecfe4 100644 --- a/v2/cmd/wails/internal/commands/dev/dev_other.go +++ b/v2/cmd/wails/internal/commands/dev/dev_other.go @@ -6,6 +6,8 @@ package dev import ( "os/exec" "syscall" + + "golang.org/x/sys/unix" ) func setParentGID(cmd *exec.Cmd) { @@ -15,6 +17,10 @@ func setParentGID(cmd *exec.Cmd) { } func killProc(cmd *exec.Cmd, devCommand string) { + if cmd == nil || cmd.Process == nil { + return + } + // Experiencing the same issue on macOS BigSur // I'm using Vite, but I would imagine this could be an issue with Node (npm) in general // Also, after several edit/rebuild cycles any abnormal shutdown (crash or CTRL-C) may still leave Node running @@ -22,7 +28,7 @@ func killProc(cmd *exec.Cmd, devCommand string) { // Not tested on *nix pgid, err := syscall.Getpgid(cmd.Process.Pid) if err == nil { - err := syscall.Kill(-pgid, 15) // note the minus sign + err := syscall.Kill(-pgid, unix.SIGTERM) // note the minus sign if err != nil { LogRed("Error from '%s' when attempting to kill the process: %s", devCommand, err.Error()) }