lib: make gosh.BuildGoPkg a package-level helper
Also updates v23test's BuildGoPkg to be package-level.
MultiPart: 3/6
Change-Id: I53f94d47b08a84591c6223eea05bf2dbab035fda
diff --git a/gosh/.api b/gosh/.api
index fc7b9c5..10ee7c1 100644
--- a/gosh/.api
+++ b/gosh/.api
@@ -1,3 +1,4 @@
+pkg gosh, func BuildGoPkg(*Shell, string, string, ...string) string
pkg gosh, func InitChildMain()
pkg gosh, func InitMain()
pkg gosh, func NewPipeline(*Cmd, ...*Cmd) *Pipeline
@@ -36,7 +37,6 @@
pkg gosh, method (*Pipeline) Terminate(os.Signal)
pkg gosh, method (*Pipeline) Wait()
pkg gosh, method (*Shell) AddCleanupHandler(func())
-pkg gosh, method (*Shell) BuildGoPkg(string, ...string) string
pkg gosh, method (*Shell) Cleanup()
pkg gosh, method (*Shell) Cmd(string, ...string) *Cmd
pkg gosh, method (*Shell) FuncCmd(*Func, ...interface{}) *Cmd
@@ -62,7 +62,6 @@
pkg gosh, type Cmd struct, Vars map[string]string
pkg gosh, type Func struct
pkg gosh, type Opts struct
-pkg gosh, type Opts struct, BinDir string
pkg gosh, type Opts struct, ChildOutputDir string
pkg gosh, type Opts struct, Fatalf func(string, ...interface{})
pkg gosh, type Opts struct, Logf func(string, ...interface{})
diff --git a/gosh/internal/gosh_example/main.go b/gosh/internal/gosh_example/main.go
index bbe0eee..a3072ad 100644
--- a/gosh/internal/gosh_example/main.go
+++ b/gosh/internal/gosh_example/main.go
@@ -17,14 +17,15 @@
defer sh.Cleanup()
// Start server.
- binPath := sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_server")
+ binDir := sh.MakeTempDir()
+ binPath := gosh.BuildGoPkg(sh, binDir, "v.io/x/lib/gosh/internal/gosh_example_server")
c := sh.Cmd(binPath)
c.Start()
addr := c.AwaitVars("addr")["addr"]
fmt.Println(addr)
// Run client.
- binPath = sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_client")
+ binPath = gosh.BuildGoPkg(sh, binDir, "v.io/x/lib/gosh/internal/gosh_example_client")
c = sh.Cmd(binPath, "-addr="+addr)
fmt.Print(c.Stdout())
}
diff --git a/gosh/shell.go b/gosh/shell.go
index 5340b17..c318b57 100644
--- a/gosh/shell.go
+++ b/gosh/shell.go
@@ -30,7 +30,6 @@
)
const (
- envBinDir = "GOSH_BIN_DIR"
envChildOutputDir = "GOSH_CHILD_OUTPUT_DIR"
envExitAfter = "GOSH_EXIT_AFTER"
envInvocation = "GOSH_INVOCATION"
@@ -81,9 +80,6 @@
// files in this directory.
// If not specified, defaults to GOSH_CHILD_OUTPUT_DIR.
ChildOutputDir string
- // Directory where BuildGoPkg() writes compiled binaries.
- // If not specified, defaults to GOSH_BIN_DIR.
- BinDir string
}
// NewShell returns a new Shell.
@@ -134,17 +130,6 @@
sh.HandleError(sh.move(oldpath, newpath))
}
-// BuildGoPkg compiles a Go package using the "go build" command and writes the
-// resulting binary to sh.Opts.BinDir, or to the -o flag location if specified.
-// Returns the absolute path to the binary.
-func (sh *Shell) BuildGoPkg(pkg string, flags ...string) string {
- // TODO(sadovsky): Convert BuildGoPkg into a utility function.
- sh.Ok()
- res, err := sh.buildGoPkg(pkg, flags...)
- sh.HandleError(err)
- return res
-}
-
// MakeTempFile creates a new temporary file in os.TempDir, opens the file for
// reading and writing, and returns the resulting *os.File.
func (sh *Shell) MakeTempFile() *os.File {
@@ -238,7 +223,7 @@
}
// Filter out any gosh env vars coming from outside.
shVars := copyMap(osVars)
- for _, key := range []string{envBinDir, envChildOutputDir, envExitAfter, envInvocation, envWatchParent} {
+ for _, key := range []string{envChildOutputDir, envExitAfter, envInvocation, envWatchParent} {
delete(shVars, key)
}
sh := &Shell{
@@ -247,16 +232,6 @@
calledNewShell: true,
cleanupDone: make(chan struct{}),
}
- if sh.Opts.BinDir == "" {
- sh.Opts.BinDir = osVars[envBinDir]
- if sh.Opts.BinDir == "" {
- var err error
- if sh.Opts.BinDir, err = sh.makeTempDir(); err != nil {
- sh.cleanup()
- return sh, err
- }
- }
- }
sh.cleanupOnSignal()
return sh, nil
}
@@ -385,55 +360,6 @@
return os.Remove(oldpath)
}
-func extractOutputFlag(flags ...string) (string, []string) {
- for i, f := range flags {
- if f == "-o" && len(flags) > i {
- return flags[i+1], append(flags[:i], flags[i+2:]...)
- }
- }
- return "", flags
-}
-
-func (sh *Shell) buildGoPkg(pkg string, flags ...string) (string, error) {
- outputFlag, flags := extractOutputFlag(flags...)
- binPath := filepath.Join(sh.Opts.BinDir, path.Base(pkg))
- if outputFlag != "" {
- if filepath.IsAbs(outputFlag) {
- binPath = outputFlag
- } else {
- binPath = filepath.Join(sh.Opts.BinDir, outputFlag)
- }
- }
- // If this binary has already been built, don't rebuild it.
- if _, err := os.Stat(binPath); err == nil {
- return binPath, nil
- } else if !os.IsNotExist(err) {
- return "", err
- }
- // Build binary to tempBinPath (in a fresh temporary directory), then move it
- // to binPath.
- tempDir, err := ioutil.TempDir(sh.Opts.BinDir, "")
- if err != nil {
- return "", err
- }
- defer os.RemoveAll(tempDir)
- tempBinPath := filepath.Join(tempDir, path.Base(pkg))
- args := []string{"build", "-o", tempBinPath}
- args = append(args, flags...)
- args = append(args, pkg)
- c, err := sh.cmd(nil, "go", args...)
- if err != nil {
- return "", err
- }
- if err := c.run(); err != nil {
- return "", err
- }
- if err := sh.move(tempBinPath, binPath); err != nil {
- return "", err
- }
- return binPath, nil
-}
-
func (sh *Shell) makeTempFile() (*os.File, error) {
sh.cleanupMu.Lock()
defer sh.cleanupMu.Unlock()
@@ -610,3 +536,78 @@
}
os.Exit(0)
}
+
+// BuildGoPkg compiles a Go package using the "go build" command and writes the
+// resulting binary to the given binDir, or to the -o flag location if
+// specified. If -o is relative, it is interpreted as relative to binDir. If the
+// binary already exists at the target location, it is not rebuilt. Returns the
+// absolute path to the binary.
+func BuildGoPkg(sh *Shell, binDir, pkg string, flags ...string) string {
+ sh.Ok()
+ res, err := buildGoPkg(sh, binDir, pkg, flags...)
+ sh.HandleError(err)
+ return res
+}
+
+func extractOutputFlag(flags ...string) (outputFlag string, otherFlags []string, err error) {
+ for i := 0; i < len(flags); i++ {
+ v := flags[i]
+ if v == "-o" || v == "--o" {
+ i++
+ if i == len(flags) {
+ return "", nil, errors.New("gosh: passed -o without location")
+ }
+ outputFlag = flags[i]
+ } else {
+ otherFlags = append(otherFlags, v)
+ }
+ }
+ return
+}
+
+func buildGoPkg(sh *Shell, binDir, pkg string, flags ...string) (string, error) {
+ outputFlag, flags, err := extractOutputFlag(flags...)
+ if err != nil {
+ return "", err
+ }
+ var binPath string
+ if outputFlag == "" {
+ binPath = filepath.Join(binDir, path.Base(pkg))
+ } else if filepath.IsAbs(outputFlag) {
+ binPath = outputFlag
+ } else {
+ binPath = filepath.Join(binDir, outputFlag)
+ }
+ // If the binary already exists at the target location, don't rebuild it.
+ if _, err := os.Stat(binPath); err == nil {
+ return binPath, nil
+ } else if !os.IsNotExist(err) {
+ return "", err
+ }
+ // Build binary to tempBinPath (in a fresh temporary directory), then move it
+ // to binPath.
+ tempDir, err := ioutil.TempDir(binDir, "")
+ if err != nil {
+ return "", err
+ }
+ defer os.RemoveAll(tempDir)
+ tempBinPath := filepath.Join(tempDir, path.Base(pkg))
+ args := []string{"build", "-o", tempBinPath}
+ args = append(args, flags...)
+ args = append(args, pkg)
+ c, err := sh.cmd(nil, "go", args...)
+ if err != nil {
+ return "", err
+ }
+ if err := c.run(); err != nil {
+ return "", err
+ }
+ // Create target directory, if needed.
+ if err := os.MkdirAll(filepath.Dir(binPath), 0700); err != nil {
+ return "", err
+ }
+ if err := sh.move(tempBinPath, binPath); err != nil {
+ return "", err
+ }
+ return binPath, nil
+}
diff --git a/gosh/shell_test.go b/gosh/shell_test.go
index 9078c0e..1cb124f 100644
--- a/gosh/shell_test.go
+++ b/gosh/shell_test.go
@@ -7,7 +7,7 @@
// TODO(sadovsky): Add more tests:
// - effects of Shell.Cleanup
// - Shell.{Vars,Args,Rename,MakeTempFile,MakeTempDir}
-// - Shell.Opts.{PropagateChildOutput,ChildOutputDir,BinDir}
+// - Shell.Opts.{PropagateChildOutput,ChildOutputDir}
// - Cmd.Clone
// - Cmd.Opts.{IgnoreParentExit,ExitAfter,PropagateOutput}
@@ -204,22 +204,28 @@
eq(t, getwdEvalSymlinks(t), startDir)
}
+const (
+ helloWorldPkg = "v.io/x/lib/gosh/internal/hello_world"
+ helloWorldStr = "Hello, world!\n"
+)
+
// Mirrors ExampleCmd in internal/gosh_example/main.go.
func TestCmd(t *testing.T) {
sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
// Start server.
- binPath := sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_server")
+ binDir := sh.MakeTempDir()
+ binPath := gosh.BuildGoPkg(sh, binDir, "v.io/x/lib/gosh/internal/gosh_example_server")
c := sh.Cmd(binPath)
c.Start()
addr := c.AwaitVars("addr")["addr"]
neq(t, addr, "")
// Run client.
- binPath = sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_client")
+ binPath = gosh.BuildGoPkg(sh, binDir, "v.io/x/lib/gosh/internal/gosh_example_client")
c = sh.Cmd(binPath, "-addr="+addr)
- eq(t, c.Stdout(), "Hello, world!\n")
+ eq(t, c.Stdout(), helloWorldStr)
}
var (
@@ -240,27 +246,52 @@
// Run client.
c = sh.FuncCmd(getFunc, addr)
- eq(t, c.Stdout(), "Hello, world!\n")
+ eq(t, c.Stdout(), helloWorldStr)
}
-// Tests that BuildGoPkg works even when the -o flag is passed.
-// TODO(sadovsky): Add tests for (1) missing name after -o, (2) multiple
-// instances of -o, (3) using --o instead of -o, and perhaps other cases as
-// well.
+// Tests BuildGoPkg's handling of the -o flag.
func TestBuildGoPkg(t *testing.T) {
+ if testing.Short() {
+ t.Skip()
+ }
sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
- tempDir := sh.MakeTempDir()
- absName := filepath.Join(tempDir, "hw")
- sh.BuildGoPkg("v.io/x/lib/gosh/internal/hello_world", "-o", absName)
+ // Set -o to an absolute name.
+ relName := "hw"
+ absName := filepath.Join(sh.MakeTempDir(), relName)
+ eq(t, gosh.BuildGoPkg(sh, "", helloWorldPkg, "-o", absName), absName)
c := sh.Cmd(absName)
- eq(t, c.Stdout(), "Hello, world!\n")
+ eq(t, c.Stdout(), helloWorldStr)
- absName = filepath.Join(sh.Opts.BinDir, "hw")
- sh.BuildGoPkg("v.io/x/lib/gosh/internal/hello_world", "-o", "hw")
+ // Set -o to a relative name with no path separators.
+ binDir := sh.MakeTempDir()
+ absName = filepath.Join(binDir, relName)
+ eq(t, gosh.BuildGoPkg(sh, binDir, helloWorldPkg, "-o", relName), absName)
c = sh.Cmd(absName)
- eq(t, c.Stdout(), "Hello, world!\n")
+ eq(t, c.Stdout(), helloWorldStr)
+
+ // Set -o to a relative name that contains a path separator.
+ relNameWithSlash := filepath.Join("subdir", relName)
+ absName = filepath.Join(binDir, relNameWithSlash)
+ eq(t, gosh.BuildGoPkg(sh, binDir, helloWorldPkg, "-o", relNameWithSlash), absName)
+ c = sh.Cmd(absName)
+ eq(t, c.Stdout(), helloWorldStr)
+
+ // Missing location after -o.
+ setsErr(t, sh, func() { gosh.BuildGoPkg(sh, "", helloWorldPkg, "-o") })
+
+ // Multiple -o.
+ absName = filepath.Join(sh.MakeTempDir(), relName)
+ gosh.BuildGoPkg(sh, "", helloWorldPkg, "-o", relName, "-o", absName)
+ c = sh.Cmd(absName)
+ eq(t, c.Stdout(), helloWorldStr)
+
+ // Use --o instead of -o.
+ absName = filepath.Join(sh.MakeTempDir(), relName)
+ gosh.BuildGoPkg(sh, "", helloWorldPkg, "--o", absName)
+ c = sh.Cmd(absName)
+ eq(t, c.Stdout(), helloWorldStr)
}
// Tests that Shell.Cmd uses Shell.Vars["PATH"] to locate executables with
@@ -269,12 +300,13 @@
sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
- tempDir := sh.MakeTempDir()
- sh.Vars["PATH"] += ":" + tempDir
- absName, relName := filepath.Join(tempDir, "hw"), "hw"
- sh.BuildGoPkg("v.io/x/lib/gosh/internal/hello_world", "-o", absName)
+ binDir := sh.MakeTempDir()
+ sh.Vars["PATH"] = binDir + ":" + sh.Vars["PATH"]
+ relName := "hw"
+ absName := filepath.Join(binDir, relName)
+ gosh.BuildGoPkg(sh, "", helloWorldPkg, "-o", absName)
c := sh.Cmd(relName)
- eq(t, c.Stdout(), "Hello, world!\n")
+ eq(t, c.Stdout(), helloWorldStr)
// Test the case where we cannot find the executable.
sh.Vars["PATH"] = ""
