Revert recent sg changes that cause high load and instability in dev (#60362)

* Revert "Made all file watches in `sg start` recursive (#60285)"

This reverts commit 4b8a403.

* Revert "SG start bazel enhancements (#59718)"

This reverts commit 00185f9.

Author: eseliger

.bazel_fix_commands.json

@@ -1,17 +1 @@
-[
-  {
-    "regex": "^Check that imports in Go sources match importpath attributes in deps.$",
-    "command": "./dev/bazel_configure_accept_changes.sh",
-    "args": []
-  },
-  {
-    "regex": "missing input file",
-    "command": "./dev/bazel_configure_accept_changes.sh",
-    "args": []
-  },
-  {
-    "regex": ": undefined:",
-    "command": "./dev/bazel_configure_accept_changes.sh",
-    "args": []
-  }
-]
+[]

deps.bzl

@@ -4201,8 +4201,8 @@ def go_dependencies():
         name = "com_github_nxadm_tail",
         build_file_proto_mode = "disable_global",
         importpath = "github.com/nxadm/tail",
-        sum = "h1:8feyoE3OzPrcshW5/MJ4sGESc5cqmGkGCWlco4l0bqY=",
-        version = "v1.4.11",
+        sum = "h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=",
+        version = "v1.4.8",
     )
     go_repository(
         name = "com_github_nytimes_gziphandler",

dev/bazel_configure_accept_changes.sh

@@ -1,19 +1,19 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
 load("//dev:go_defs.bzl", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
 
 go_library(
     name = "run",
     srcs = [
+        "bazel_build.go",
         "bazel_command.go",
         "command.go",
         "helpers.go",
         "ibazel.go",
-        "installer.go",
         "logger.go",
         "pid.go",
         "prefix_suffix_saver.go",
         "run.go",
-        "sgconfig_command.go",
+        "run_bazel.go",
     ],
     importpath = "github.com/sourcegraph/sourcegraph/dev/sg/internal/run",
     visibility = ["//dev/sg:__subpackages__"],
@@ -28,9 +28,9 @@ go_library(
         "//lib/output",
         "//lib/process",
         "@com_github_grafana_regexp//:regexp",
-        "@com_github_nxadm_tail//:tail",
         "@com_github_rjeczalik_notify//:notify",
         "@com_github_sourcegraph_conc//pool",
+        "@org_golang_x_sync//semaphore",
     ],
 )
 

dev/sg/internal/run/BUILD.bazel

@@ -0,0 +1,64 @@
+package run
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"os/exec"
+
+	"github.com/sourcegraph/sourcegraph/dev/sg/internal/std"
+	"github.com/sourcegraph/sourcegraph/dev/sg/root"
+	"github.com/sourcegraph/sourcegraph/lib/output"
+	"github.com/sourcegraph/sourcegraph/lib/process"
+)
+
+// BazelBuild peforms a bazel build command with all the given targets and blocks until an
+// error is returned or the build is completed.
+func BazelBuild(ctx context.Context, cmds ...BazelCommand) error {
+	if len(cmds) == 0 {
+		// no Bazel commands so we return
+		return nil
+	}
+	std.Out.WriteLine(output.Styled(output.StylePending, fmt.Sprintf("Detected %d bazel targets, running bazel build before anything else", len(cmds))))
+
+	repoRoot, err := root.RepositoryRoot()
+	if err != nil {
+		return err
+	}
+
+	targets := make([]string, 0, len(cmds))
+	for _, cmd := range cmds {
+		targets = append(targets, cmd.Target)
+	}
+
+	var cancel func()
+	ctx, cancel = context.WithCancel(ctx)
+
+	args := append([]string{"build"}, targets...)
+	cmd := exec.CommandContext(ctx, "bazel", args...)
+
+	sc := &startedCmd{
+		stdoutBuf: &prefixSuffixSaver{N: 32 << 10},
+		stderrBuf: &prefixSuffixSaver{N: 32 << 10},
+	}
+
+	sc.cancel = cancel
+	sc.Cmd = cmd
+	sc.Cmd.Dir = repoRoot
+
+	var stdoutWriter, stderrWriter io.Writer
+	logger := newCmdLogger(ctx, "bazel", std.Out.Output)
+	stdoutWriter = io.MultiWriter(logger, sc.stdoutBuf)
+	stderrWriter = io.MultiWriter(logger, sc.stderrBuf)
+	eg, err := process.PipeOutputUnbuffered(ctx, sc.Cmd, stdoutWriter, stderrWriter)
+	if err != nil {
+		return err
+	}
+	sc.outEg = eg
+
+	// Bazel out directory should exist here before returning
+	if err := sc.Start(); err != nil {
+		return err
+	}
+	return sc.Wait()
+}

dev/sg/internal/run/bazel_build.go

@@ -3,123 +3,150 @@ package run
 import (
 	"context"
 	"fmt"
+	"io"
 	"os/exec"
-	"strings"
 
 	"github.com/rjeczalik/notify"
-
 	"github.com/sourcegraph/sourcegraph/dev/sg/internal/secrets"
+	"github.com/sourcegraph/sourcegraph/dev/sg/internal/std"
+	"github.com/sourcegraph/sourcegraph/lib/errors"
+	"github.com/sourcegraph/sourcegraph/lib/output"
+	"github.com/sourcegraph/sourcegraph/lib/process"
 )
 
 // A BazelCommand is a command definition for sg run/start that uses
 // bazel under the hood. It will handle restarting itself autonomously,
 // as long as iBazel is running and watch that specific target.
 type BazelCommand struct {
-	Name                string
-	Description         string            `yaml:"description"`
-	Target              string            `yaml:"target"`
-	Args                string            `yaml:"args"`
-	PreCmd              string            `yaml:"precmd"`
-	Env                 map[string]string `yaml:"env"`
-	IgnoreStdout        bool              `yaml:"ignoreStdout"`
-	IgnoreStderr        bool              `yaml:"ignoreStderr"`
-	ContinueWatchOnExit bool              `yaml:"continueWatchOnExit"`
-	// Preamble is a short and visible message, displayed when the command is launched.
-	Preamble        string                            `yaml:"preamble"`
+	Name            string
+	Description     string                            `yaml:"description"`
+	Target          string                            `yaml:"target"`
+	Args            string                            `yaml:"args"`
+	PreCmd          string                            `yaml:"precmd"`
+	Env             map[string]string                 `yaml:"env"`
+	IgnoreStdout    bool                              `yaml:"ignoreStdout"`
+	IgnoreStderr    bool                              `yaml:"ignoreStderr"`
 	ExternalSecrets map[string]secrets.ExternalSecret `yaml:"external_secrets"`
-
-	// RunTarget specifies a target that should be run via `bazel run $RunTarget` instead of directly executing the binary.
-	RunTarget string `yaml:"runTarget"`
 }
 
-func (bc BazelCommand) GetName() string {
-	return bc.Name
+func (bc *BazelCommand) BinLocation() (string, error) {
+	return binLocation(bc.Target)
 }
 
-func (bc BazelCommand) GetContinueWatchOnExit() bool {
-	return bc.ContinueWatchOnExit
-}
+func (bc *BazelCommand) watch(ctx context.Context) (<-chan struct{}, error) {
+	// Grab the location of the binary in bazel-out.
+	binLocation, err := bc.BinLocation()
+	if err != nil {
+		return nil, err
+	}
 
-func (bc BazelCommand) GetEnv() map[string]string {
-	return bc.Env
-}
+	// Set up the watcher.
+	restart := make(chan struct{})
+	events := make(chan notify.EventInfo, 1)
+	if err := notify.Watch(binLocation, events, notify.All); err != nil {
+		return nil, err
+	}
 
-func (bc BazelCommand) GetIgnoreStdout() bool {
-	return bc.IgnoreStdout
-}
+	// Start watching for a freshly compiled version of the binary.
+	go func() {
+		defer close(events)
+		defer notify.Stop(events)
+
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			case e := <-events:
+				if e.Event() != notify.Remove {
+					restart <- struct{}{}
+				}
+			}
 
-func (bc BazelCommand) GetIgnoreStderr() bool {
-	return bc.IgnoreStderr
-}
+		}
+	}()
 
-func (bc BazelCommand) GetPreamble() string {
-	return bc.Preamble
+	return restart, nil
 }
 
-func (bc BazelCommand) GetBinaryLocation() (string, error) {
-	baseOutput, err := outputPath()
+func (bc *BazelCommand) Start(ctx context.Context, dir string, parentEnv map[string]string) error {
+	std.Out.WriteLine(output.Styledf(output.StylePending, "Running %s...", bc.Name))
+
+	// Run the binary for the first time.
+	cancel, err := bc.start(ctx, dir, parentEnv)
 	if err != nil {
-		return "", err
+		return errors.Wrapf(err, "failed to start Bazel command %q", bc.Name)
 	}
-	// Trim "bazel-out" because the next bazel query will include it.
-	outputPath := strings.TrimSuffix(strings.TrimSpace(string(baseOutput)), "bazel-out")
 
-	// Get the binary from the specific target.
-	cmd := exec.Command("bazel", "cquery", bc.Target, "--output=files")
-	baseOutput, err = cmd.Output()
+	// Restart when the binary change.
+	wantRestart, err := bc.watch(ctx)
 	if err != nil {
-		return "", err
+		return err
 	}
-	binPath := strings.TrimSpace(string(baseOutput))
 
-	return fmt.Sprintf("%s%s", outputPath, binPath), nil
+	// Wait forever until we're asked to stop or that restarting returns an error.
+	for {
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-wantRestart:
+			std.Out.WriteLine(output.Styledf(output.StylePending, "Restarting %s...", bc.Name))
+			cancel()
+			cancel, err = bc.start(ctx, dir, parentEnv)
+			if err != nil {
+				return err
+			}
+		}
+	}
 }
 
-func (bc BazelCommand) GetExternalSecrets() map[string]secrets.ExternalSecret {
-	return bc.ExternalSecrets
-}
+func (bc *BazelCommand) start(ctx context.Context, dir string, parentEnv map[string]string) (func(), error) {
+	binLocation, err := bc.BinLocation()
+	if err != nil {
+		return nil, err
+	}
 
-func (bc BazelCommand) watchPaths() ([]string, error) {
-	// If no target is defined, there is nothing to be built and watched
-	if bc.Target == "" {
-		return nil, nil
+	sc := &startedCmd{
+		stdoutBuf: &prefixSuffixSaver{N: 32 << 10},
+		stderrBuf: &prefixSuffixSaver{N: 32 << 10},
 	}
-	// Grab the location of the binary in bazel-out.
-	binLocation, err := bc.GetBinaryLocation()
+
+	commandCtx, cancel := context.WithCancel(ctx)
+	sc.cancel = cancel
+	sc.Cmd = exec.CommandContext(commandCtx, "bash", "-c", fmt.Sprintf("%s\n%s", bc.PreCmd, binLocation))
+	sc.Cmd.Dir = dir
+
+	secretsEnv, err := getSecrets(ctx, bc.Name, bc.ExternalSecrets)
 	if err != nil {
-		return nil, err
+		std.Out.WriteLine(output.Styledf(output.StyleWarning, "[%s] %s %s",
+			bc.Name, output.EmojiFailure, err.Error()))
 	}
-	return []string{binLocation}, nil
 
-}
+	sc.Cmd.Env = makeEnv(parentEnv, secretsEnv, bc.Env)
 
-func (bc BazelCommand) StartWatch(ctx context.Context) (<-chan struct{}, error) {
-	if watchPaths, err := bc.watchPaths(); err != nil {
-		return nil, err
+	var stdoutWriter, stderrWriter io.Writer
+	logger := newCmdLogger(commandCtx, bc.Name, std.Out.Output)
+	if bc.IgnoreStdout {
+		std.Out.WriteLine(output.Styledf(output.StyleSuggestion, "Ignoring stdout of %s", bc.Name))
+		stdoutWriter = sc.stdoutBuf
 	} else {
-		// skip remove events as we don't care about files being removed, we only
-		// want to know when the binary has been rebuilt
-		return WatchPaths(ctx, watchPaths, notify.Remove)
+		stdoutWriter = io.MultiWriter(logger, sc.stdoutBuf)
 	}
-}
-
-func (bc BazelCommand) GetExecCmd(ctx context.Context) (*exec.Cmd, error) {
-	var cmd string
-	var err error
-	if bc.RunTarget != "" {
-		cmd = "bazel run " + bc.RunTarget
+	if bc.IgnoreStderr {
+		std.Out.WriteLine(output.Styledf(output.StyleSuggestion, "Ignoring stderr of %s", bc.Name))
+		stderrWriter = sc.stderrBuf
 	} else {
-		if cmd, err = bc.GetBinaryLocation(); err != nil {
-			return nil, err
-		}
+		stderrWriter = io.MultiWriter(logger, sc.stderrBuf)
 	}
 
-	return exec.CommandContext(ctx, "bash", "-c", fmt.Sprintf("%s\n%s", bc.PreCmd, cmd)), nil
-}
+	eg, err := process.PipeOutputUnbuffered(ctx, sc.Cmd, stdoutWriter, stderrWriter)
+	if err != nil {
+		return nil, err
+	}
+	sc.outEg = eg
+
+	if err := sc.Start(); err != nil {
+		return nil, err
+	}
 
-func outputPath() ([]byte, error) {
-	// Get the output directory from Bazel, which varies depending on which OS
-	// we're running against.
-	cmd := exec.Command("bazel", "info", "output_path")
-	return cmd.Output()
+	return cancel, nil
 }