[sg] share runner implementation between run and start (#61161)

* first pass at watchable docker commands

* extracted options to common struct

* added repo root as field in options

* added docker specific config and tests

* merged in config overrides for linux

* cleanup

* Update dev/sg/internal/run/command.go

Co-authored-by: William Bezuidenhout <[email protected]>

* addressed comments

* share runner implementation between sg run and sg start

* Update dev/sg/sg_start.go

Co-authored-by: Robert Lin <[email protected]>

* small changes

* fixed compile error

---------

Co-authored-by: William Bezuidenhout <[email protected]>
Co-authored-by: Robert Lin <[email protected]>

Author: 3 people

Diff for: dev/sg/BUILD.bazel

@@ -111,7 +111,6 @@ go_library(
         "@com_github_opsgenie_opsgenie_go_sdk_v2//alert",
         "@com_github_opsgenie_opsgenie_go_sdk_v2//client",
         "@com_github_slack_go_slack//:slack",
-        "@com_github_sourcegraph_conc//pool",
         "@com_github_sourcegraph_log//:log",
         "@com_github_sourcegraph_run//:run",
         "@com_github_urfave_cli_v2//:cli",

Diff for: dev/sg/internal/run/bazel_command.go

@@ -42,6 +42,14 @@ func (bc BazelCommand) GetConfig() SGConfigCommandOptions {
 	return bc.Config
 }
 
+func (bc BazelCommand) UpdateConfig(f func(*SGConfigCommandOptions)) SGConfigCommand {
+	f(&bc.Config)
+	return bc
+}
+
+func (bc BazelCommand) GetBazelTarget() string {
+	return bc.Target
+}
 func (bc BazelCommand) watchPaths() ([]string, error) {
 	// If no target is defined, there is nothing to be built and watched
 	if bc.Target == "" {

Diff for: dev/sg/internal/run/command.go

@@ -55,6 +55,11 @@ func (cmd Command) GetConfig() SGConfigCommandOptions {
 	return cmd.Config
 }
 
+func (cmd Command) UpdateConfig(f func(*SGConfigCommandOptions)) SGConfigCommand {
+	f(&cmd.Config)
+	return cmd
+}
+
 func (cmd Command) GetName() string {
 	return cmd.Config.Name
 }
@@ -66,6 +71,10 @@ func (cmd Command) GetBinaryLocation() (string, error) {
 	return "", noBinaryError{name: cmd.Config.Name}
 }
 
+func (cmd Command) GetBazelTarget() string {
+	return ""
+}
+
 func (cmd Command) GetExecCmd(ctx context.Context) (*exec.Cmd, error) {
 	return exec.CommandContext(ctx, "bash", "-c", cmd.Cmd), nil
 }

Diff for: dev/sg/internal/run/docker_commmand.go

@@ -83,6 +83,15 @@ func (dc DockerCommand) GetBinaryLocation() (string, error) {
 	return binaryLocation(dc.Target)
 }
 
+func (dc DockerCommand) GetBazelTarget() string {
+	return dc.Target
+}
+
+func (dc DockerCommand) UpdateConfig(f func(*SGConfigCommandOptions)) SGConfigCommand {
+	f(&dc.Config)
+	return dc
+}
+
 func (dc DockerCommand) StartWatch(ctx context.Context) (<-chan struct{}, error) {
 	if watchPaths, err := dc.watchPaths(); err != nil {
 		return nil, err

Diff for: dev/sg/internal/run/ibazel.go

@@ -34,13 +34,12 @@ type IBazel struct {
 	events  *iBazelEventHandler
 	logsDir string
 	logFile *os.File
-	dir     string
 	proc    *startedCmd
 	logs    chan<- output.FancyLine
 }
 
 // returns a runner to interact with ibazel.
-func NewIBazel(targets []string, dir string) (*IBazel, error) {
+func NewIBazel(targets []string) (*IBazel, error) {
 	logsDir, err := initLogsDir()
 	if err != nil {
 		return nil, err
@@ -56,7 +55,6 @@ func NewIBazel(targets []string, dir string) (*IBazel, error) {
 		events:  newIBazelEventHandler(profileEventsPath(logsDir)),
 		logsDir: logsDir,
 		logFile: logFile,
-		dir:     dir,
 	}, nil
 }
 
@@ -146,11 +144,15 @@ func (ib *IBazel) WaitForInitialBuild(ctx context.Context) error {
 	return nil
 }
 
-func (ib *IBazel) getCommandOptions(ctx context.Context) commandOptions {
+func (ib *IBazel) getCommandOptions(ctx context.Context) (commandOptions, error) {
+	dir, err := root.RepositoryRoot()
+	if err != nil {
+		return commandOptions{}, err
+	}
 	return commandOptions{
 		name: "iBazel",
 		exec: ib.GetExecCmd(ctx),
-		dir:  ib.dir,
+		dir:  dir,
 		// Don't output iBazel logs (which are all on stderr) until
 		// initial build is complete as it will break the progress bar
 		stderr: outputOptions{
@@ -159,13 +161,17 @@ func (ib *IBazel) getCommandOptions(ctx context.Context) commandOptions {
 				ib.logFile,
 				&patternMatcher{regex: watchErrorRegex, callback: ib.logWatchError},
 			}},
-	}
+	}, nil
 }
 
 // Build starts an ibazel process to build the targets provided in the constructor
 // It runs perpetually, watching for file changes
-func (ib *IBazel) build(ctx context.Context) (err error) {
-	ib.proc, err = startCmd(ctx, ib.getCommandOptions(ctx))
+func (ib *IBazel) build(ctx context.Context) error {
+	opts, err := ib.getCommandOptions(ctx)
+	if err != nil {
+		return err
+	}
+	ib.proc, err = startCmd(ctx, opts)
 	return err
 }
 

Diff for: dev/sg/internal/run/installer.go

@@ -50,7 +50,7 @@ type InstallManager struct {
 	stats               *installAnalytics
 }
 
-func Install(ctx context.Context, env map[string]string, verbose bool, cmds ...Installer) error {
+func Install(ctx context.Context, env map[string]string, verbose bool, cmds []Installer) error {
 	installer := newInstallManager(cmds, std.Out, env, verbose)
 
 	installer.start(ctx)

Diff for: dev/sg/internal/run/run.go

@@ -24,7 +24,7 @@ type cmdRunner struct {
 	verbose   bool
 }
 
-func Commands(ctx context.Context, parentEnv map[string]string, verbose bool, cmds ...SGConfigCommand) (err error) {
+func Commands(ctx context.Context, parentEnv map[string]string, verbose bool, cmds []SGConfigCommand) (err error) {
 	if len(cmds) == 0 {
 		// Exit early if there are no commands to run.
 		return nil

Diff for: dev/sg/internal/run/sgconfig_command.go

@@ -13,6 +13,10 @@ type SGConfigCommand interface {
 	GetConfig() SGConfigCommandOptions
 	GetBinaryLocation() (string, error)
 	GetExecCmd(context.Context) (*exec.Cmd, error)
+	UpdateConfig(func(*SGConfigCommandOptions)) SGConfigCommand
+
+	// Optionally returns a bazel target associated with this command
+	GetBazelTarget() string
 
 	// Start a file watcher on the relevant filesystem sub-tree for this command
 	StartWatch(context.Context) (<-chan struct{}, error)

Diff for: dev/sg/sg_run.go

@@ -2,24 +2,21 @@ package main
 
 import (
 	"context"
-	"flag"
 	"fmt"
 	"sort"
 	"strings"
 
 	"github.com/urfave/cli/v2"
 	"gopkg.in/yaml.v3"
 
-	"github.com/sourcegraph/conc/pool"
-
 	"github.com/sourcegraph/sourcegraph/dev/sg/internal/category"
-	"github.com/sourcegraph/sourcegraph/dev/sg/internal/run"
 	"github.com/sourcegraph/sourcegraph/dev/sg/internal/std"
 	"github.com/sourcegraph/sourcegraph/dev/sg/interrupt"
 	"github.com/sourcegraph/sourcegraph/lib/cliutil/completions"
-	"github.com/sourcegraph/sourcegraph/lib/output"
 )
 
+var deprecationNotice = "sg run is deprecated. Use 'sg start -cmd' instead.\n"
+
 func init() {
 	postInitHooks = append(postInitHooks,
 		func(cmd *cli.Context) {
@@ -39,9 +36,9 @@ func init() {
 
 var runCommand = &cli.Command{
 	Name:      "run",
-	Usage:     "Run the given commands",
+	Usage:     deprecationNotice,
 	ArgsUsage: "[command]",
-	UsageText: `
+	UsageText: deprecationNotice + `
 # Run specific commands
 sg run gitserver
 sg run frontend
@@ -56,17 +53,13 @@ sg run gitserver frontend repo-updater
 sg run -describe jaeger
 `,
 	Category: category.Dev,
+	Action:   runExec,
 	Flags: []cli.Flag{
 		&cli.BoolFlag{
 			Name:  "describe",
 			Usage: "Print details about selected run target",
 		},
-		&cli.BoolFlag{
-			Name:  "legacy",
-			Usage: "Force run to pick the non-bazel variant of the command",
-		},
 	},
-	Action: runExec,
 	BashComplete: completions.CompleteArgs(func() (options []string) {
 		config, _ := getConfig()
 		if config == nil {
@@ -84,28 +77,13 @@ func runExec(ctx *cli.Context) error {
 	if err != nil {
 		return err
 	}
-	legacy := ctx.Bool("legacy")
-
-	args := ctx.Args().Slice()
-	if len(args) == 0 {
-		std.Out.WriteLine(output.Styled(output.StyleWarning, "No command specified"))
-		return flag.ErrHelp
-	}
-
-	cmds := make([]run.SGConfigCommand, 0, len(args))
-	for _, arg := range args {
-		if bazelCmd, ok := config.BazelCommands[arg]; ok && !legacy {
-			cmds = append(cmds, bazelCmd)
-		} else if cmd, ok := config.Commands[arg]; ok {
-			cmds = append(cmds, cmd)
-		} else {
-			std.Out.WriteLine(output.Styledf(output.StyleWarning, "ERROR: command %q not found :(", arg))
-			return flag.ErrHelp
-		}
+	cmds, err := listToCommands(config, ctx.Args().Slice())
+	if err != nil {
+		return err
 	}
 
 	if ctx.Bool("describe") {
-		for _, cmd := range cmds {
+		for _, cmd := range cmds.commands {
 			out, err := yaml.Marshal(cmd)
 			if err != nil {
 				return err
@@ -118,17 +96,14 @@ func runExec(ctx *cli.Context) error {
 		return nil
 	}
 
-	p := pool.New().WithContext(ctx.Context).WithCancelOnError()
-	p.Go(func(ctx context.Context) error {
-		return run.Commands(ctx, config.Env, verbose, cmds...)
-	})
-
-	return p.Wait()
+	return cmds.start(ctx.Context)
 }
 
 func constructRunCmdLongHelp() string {
 	var out strings.Builder
 
+	fmt.Fprint(&out, deprecationNotice)
+
 	fmt.Fprintf(&out, "Runs the given command. If given a whitespace-separated list of commands it runs the set of commands.\n")
 
 	config, err := getConfig()