sg: do not restart binary if it didn't change (#21509)

This adds the `checkBinary` property to `sg.config.yaml` which allows `sg` to check whether the specified binary
actually changed after rebuilding a command.

In practice this means that multiple file modifications that result in the same binary do not lead to multiple restarts.

dev/sg/README.md

@@ -254,8 +254,6 @@ tests:
   - [ ] Implement the `sg generate` command
   - [ ] Implement `sg edit site-config` and `sg edit external-services`
   - [ ] Implement `sg tail-log`
-- [ ] Add a _simple_ way to define in the config file when a restart after a rebuild is not necessary
-  - Something like `check_binary: .bin/frontend` which would take a SHA256 before and after rebuild and only restart if SHA doesn't match
 - [ ] Add built-in support for "download binary" so that the `caddy` command, for example, would be 3 lines instead of 20. That would allow us to get rid of the bash code.
 
 ## Hacking

dev/sg/config.go

@@ -47,6 +47,7 @@ type Command struct {
 	Name             string
 	Cmd              string            `yaml:"cmd"`
 	Install          string            `yaml:"install"`
+	CheckBinary      string            `yaml:"checkBinary"`
 	Env              map[string]string `yaml:"env"`
 	Watch            []string          `yaml:"watch"`
 	InstallDocDarwin string            `yaml:"installDoc.darwin"`

dev/sg/run.go

@@ -3,10 +3,12 @@ package main
 import (
 	"bytes"
 	"context"
+	"crypto/md5"
 	"fmt"
 	"io"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -159,6 +161,20 @@ func printCmdError(out *output.Output, cmdName string, err error) {
 func runWatch(ctx context.Context, cmd Command, root string, reload <-chan struct{}) error {
 	startedOnce := false
 
+	var (
+		md5hash    string
+		md5changed bool
+	)
+
+	var wg sync.WaitGroup
+	var cancelFuncs []context.CancelFunc
+
+	errs := make(chan error, 1)
+	defer func() {
+		wg.Wait()
+		close(errs)
+	}()
+
 	for {
 		// Build it
 		if cmd.Install != "" {
@@ -189,85 +205,135 @@ func runWatch(ctx context.Context, cmd Command, root string, reload <-chan struc
 			}
 
 			out.WriteLine(output.Linef("", output.StyleSuccess, "%sSuccessfully installed %s%s", output.StyleBold, cmd.Name, output.StyleReset))
-		}
 
-		// Run it
-		out.WriteLine(output.Linef("", output.StylePending, "Running %s...", cmd.Name))
-
-		commandCtx, cancel := context.WithCancel(ctx)
-		defer cancel()
-
-		c := exec.CommandContext(commandCtx, "bash", "-c", cmd.Cmd)
-		c.Dir = root
-		c.Env = makeEnv(conf.Env, cmd.Env)
-
-		var (
-			stdoutBuf = &prefixSuffixSaver{N: 32 << 10}
-			stderrBuf = &prefixSuffixSaver{N: 32 << 10}
-		)
-
-		logger := newCmdLogger(cmd.Name, out)
-		if cmd.IgnoreStdout {
-			out.WriteLine(output.Linef("", output.StyleSuggestion, "Ignoring stdout of %s", cmd.Name))
-		} else {
-			c.Stdout = io.MultiWriter(logger, stdoutBuf)
-		}
-		if cmd.IgnoreStderr {
-			out.WriteLine(output.Linef("", output.StyleSuggestion, "Ignoring stderr of %s", cmd.Name))
-		} else {
-			c.Stderr = io.MultiWriter(logger, stderrBuf)
-		}
-
-		if err := c.Start(); err != nil {
-			return err
-		}
-
-		errs := make(chan error, 1)
-		go func() {
-			defer close(errs)
-
-			errs <- (func() error {
-				if err := c.Wait(); err != nil {
-					if exitErr, ok := err.(*exec.ExitError); ok {
-						return runErr{
-							cmdName:  cmd.Name,
-							exitCode: exitErr.ExitCode(),
-							stderr:   string(stderrBuf.Bytes()),
-							stdout:   string(stdoutBuf.Bytes()),
-						}
-					}
-
-					return err
+			if cmd.CheckBinary != "" {
+				newHash, err := md5HashFile(filepath.Join(root, cmd.CheckBinary))
+				if err != nil {
+					return installErr{cmdName: cmd.Name, output: string(cmdOut)}
 				}
 
-				return nil
-			})()
-		}()
-
-		// TODO: We should probably only set this after N seconds (or when
-		// we're sure that the command has booted up -- maybe healthchecks?)
-		startedOnce = true
-	outer:
-		for {
-			select {
-			case <-reload:
-				out.WriteLine(output.Linef("", output.StylePending, "Change detected. Reloading %s...", cmd.Name))
-
-				cancel()    // Stop command
-				<-errs      // Wait for exit
-				break outer // Reinstall
-
-			case err := <-errs:
-				// Exited on its own or errored
-				if err == nil {
-					out.WriteLine(output.Linef("", output.StyleSuccess, "%s%s exited without error%s", output.StyleBold, cmd.Name, output.StyleReset))
-				}
-				return err
+				md5changed = md5hash != newHash
+				md5hash = newHash
 			}
 		}
+
+		if cmd.CheckBinary == "" || md5changed {
+			for _, cancel := range cancelFuncs {
+				cancel() // Stop command
+				<-errs   // Wait for exit
+			}
+			cancelFuncs = nil
+
+			// Run it
+			out.WriteLine(output.Linef("", output.StylePending, "Running %s...", cmd.Name))
+
+			sc, err := startCmd(ctx, root, cmd)
+			defer sc.cancel()
+
+			if err != nil {
+				return err
+			}
+
+			cancelFuncs = append(cancelFuncs, sc.cancel)
+
+			wg.Add(1)
+			go func() {
+				defer wg.Done()
+
+				err := sc.Wait()
+
+				if exitErr, ok := err.(*exec.ExitError); ok {
+					err = runErr{
+						cmdName:  cmd.Name,
+						exitCode: exitErr.ExitCode(),
+						stderr:   sc.CapturedStderr(),
+						stdout:   sc.CapturedStdout(),
+					}
+				}
+
+				errs <- err
+			}()
+
+			// TODO: We should probably only set this after N seconds (or when
+			// we're sure that the command has booted up -- maybe healthchecks?)
+			startedOnce = true
+		} else {
+			out.WriteLine(output.Linef("", output.StylePending, "Binary did not change. Not restarting."))
+		}
+
+		select {
+		case <-reload:
+			out.WriteLine(output.Linef("", output.StylePending, "Change detected. Reloading %s...", cmd.Name))
+
+			continue // Reinstall
+
+		case err := <-errs:
+			// Exited on its own or errored
+			if err == nil {
+				out.WriteLine(output.Linef("", output.StyleSuccess, "%s%s exited without error%s", output.StyleBold, cmd.Name, output.StyleReset))
+			}
+			return err
+		}
 	}
 }
 
+type startedCmd struct {
+	*exec.Cmd
+
+	cancel func()
+
+	stdoutBuf *prefixSuffixSaver
+	stderrBuf *prefixSuffixSaver
+}
+
+func (sc *startedCmd) CapturedStdout() string {
+	if sc.stdoutBuf == nil {
+		return ""
+	}
+
+	return string(sc.stdoutBuf.Bytes())
+}
+
+func (sc *startedCmd) CapturedStderr() string {
+	if sc.stderrBuf == nil {
+		return ""
+	}
+
+	return string(sc.stderrBuf.Bytes())
+}
+
+func startCmd(ctx context.Context, dir string, cmd Command) (*startedCmd, error) {
+	sc := &startedCmd{
+		stdoutBuf: &prefixSuffixSaver{N: 32 << 10},
+		stderrBuf: &prefixSuffixSaver{N: 32 << 10},
+	}
+
+	commandCtx, cancel := context.WithCancel(ctx)
+	sc.cancel = cancel
+
+	sc.Cmd = exec.CommandContext(commandCtx, "bash", "-c", cmd.Cmd)
+	sc.Cmd.Dir = dir
+	sc.Cmd.Env = makeEnv(conf.Env, cmd.Env)
+
+	logger := newCmdLogger(cmd.Name, out)
+	if cmd.IgnoreStdout {
+		out.WriteLine(output.Linef("", output.StyleSuggestion, "Ignoring stdout of %s", cmd.Name))
+	} else {
+		sc.Cmd.Stdout = io.MultiWriter(logger, sc.stdoutBuf)
+	}
+	if cmd.IgnoreStderr {
+		out.WriteLine(output.Linef("", output.StyleSuggestion, "Ignoring stderr of %s", cmd.Name))
+	} else {
+		sc.Cmd.Stderr = io.MultiWriter(logger, sc.stderrBuf)
+	}
+
+	if err := sc.Start(); err != nil {
+		return sc, err
+	}
+
+	return sc, nil
+}
+
 func makeEnv(envs ...map[string]string) []string {
 	combined := os.Environ()
 
@@ -299,6 +365,21 @@ func makeEnv(envs ...map[string]string) []string {
 	return combined
 }
 
+func md5HashFile(filename string) (string, error) {
+	f, err := os.Open(filename)
+	if err != nil {
+		return "", err
+	}
+	defer f.Close()
+
+	h := md5.New()
+	if _, err := io.Copy(h, f); err != nil {
+		return "", err
+	}
+
+	return string(h.Sum(nil)), nil
+}
+
 //
 //
 

sg.config.yaml

@@ -83,6 +83,7 @@ commands:
   frontend:
     cmd: ulimit -n 10000 && .bin/frontend
     install: go build -o .bin/frontend github.com/sourcegraph/sourcegraph/cmd/frontend
+    checkBinary: .bin/frontend
     env:
       CONFIGURATION_MODE: server
       USE_ENHANCED_LANGUAGE_DETECTION: false
@@ -100,6 +101,7 @@ commands:
       export SOURCEGRAPH_LICENSE_GENERATION_KEY=$(cat ../dev-private/enterprise/dev/test-license-generation-key.pem)
       .bin/enterprise-frontend
     install: go build -o .bin/enterprise-frontend github.com/sourcegraph/sourcegraph/enterprise/cmd/frontend
+    checkBinary: .bin/enterprise-frontend
     env:
       CONFIGURATION_MODE: server
       USE_ENHANCED_LANGUAGE_DETECTION: false
@@ -118,6 +120,7 @@ commands:
   gitserver:
     cmd: .bin/gitserver
     install: go build -o .bin/gitserver github.com/sourcegraph/sourcegraph/cmd/gitserver
+    checkBinary: .bin/gitserver
     env:
       HOSTNAME: 127.0.0.1:3178
     watch:
@@ -128,6 +131,7 @@ commands:
   github-proxy:
     cmd: .bin/github-proxy
     install: go build -o .bin/github-proxy github.com/sourcegraph/sourcegraph/cmd/github-proxy
+    checkBinary: .bin/github-proxy
     env:
       HOSTNAME: 127.0.0.1:3178
     watch:
@@ -138,14 +142,16 @@ commands:
   repo-updater:
     cmd: .bin/repo-updater
     install: go build -o .bin/repo-updater github.com/sourcegraph/sourcegraph/cmd/repo-updater
+    checkBinary: .bin/repo-updater
     watch:
       - lib
       - internal
       - cmd/repo-updater
 
   enterprise-repo-updater:
-    cmd: .bin/repo-updater
-    install: go build -o .bin/repo-updater github.com/sourcegraph/sourcegraph/enterprise/cmd/repo-updater
+    cmd: .bin/enterprise-repo-updater
+    install: go build -o .bin/enterprise-repo-updater github.com/sourcegraph/sourcegraph/enterprise/cmd/repo-updater
+    checkBinary: .bin/enterprise-repo-updater
     env:
       HOSTNAME: $SRC_GIT_SERVER_1
       ENTERPRISE: 1
@@ -159,6 +165,7 @@ commands:
   query-runner:
     cmd: .bin/query-runner
     install: go build -o .bin/query-runner github.com/sourcegraph/sourcegraph/cmd/query-runner
+    checkBinary: .bin/query-runner
     watch:
       - lib
       - internal
@@ -170,6 +177,7 @@ commands:
       ./dev/libsqlite3-pcre/build.sh &&
       ./cmd/symbols/build-ctags.sh &&
       go build -o .bin/symbols github.com/sourcegraph/sourcegraph/cmd/symbols
+    checkBinary: .bin/symbols
     env:
       LIBSQLITE3_PCRE: ./dev/libsqlite3-pcre/build.sh libpath
       CTAGS_COMMAND: cmd/symbols/universal-ctags-dev
@@ -182,6 +190,7 @@ commands:
   searcher:
     cmd: .bin/searcher
     install: go build -o .bin/searcher github.com/sourcegraph/sourcegraph/cmd/searcher
+    checkBinary: .bin/searcher
     watch:
       - lib
       - internal
@@ -292,6 +301,7 @@ commands:
       go install github.com/google/zoekt/cmd/zoekt-archive-index
       go install github.com/google/zoekt/cmd/zoekt-git-index
       go install github.com/google/zoekt/cmd/zoekt-sourcegraph-indexserver
+    checkBinary: .bin/zoekt-sourcegraph-indexserver
     env: &zoektenv
       GOGC: 50
       CTAGS_COMMAND: cmd/symbols/universal-ctags-dev
@@ -317,6 +327,7 @@ commands:
     install: |
       mkdir -p .bin
       env GOBIN="${PWD}/.bin" GO111MODULE=on go install github.com/google/zoekt/cmd/zoekt-webserver
+    checkBinary: .bin/zoekt-webserver
     env:
       JAEGER_DISABLED: false
       GOGC: 50
@@ -334,6 +345,7 @@ commands:
     cmd: .bin/precise-code-intel-worker
     install: |
       go build -o .bin/precise-code-intel-worker github.com/sourcegraph/sourcegraph/enterprise/cmd/precise-code-intel-worker
+    checkBinary: .bin/precise-code-intel-worker
     watch:
       - lib
       - internal
@@ -345,6 +357,7 @@ commands:
     cmd: .bin/executor-queue
     install: |
       go build -o .bin/executor-queue github.com/sourcegraph/sourcegraph/enterprise/cmd/executor-queue
+    checkBinary: .bin/executor-queue
     watch:
       - lib
       - internal
@@ -355,6 +368,7 @@ commands:
     cmd: .bin/executor
     install: |
       go build -o .bin/executor github.com/sourcegraph/sourcegraph/enterprise/cmd/executor
+    checkBinary: .bin/executor
     env:
       EXECUTOR_QUEUE_NAME: codeintel
       TMPDIR: $HOME/.sourcegraph/indexer-temp