cmd/gomobile: consolidate os/exec Run calls

Tested:
	go test golang.org/x/mobile/bind/java
	gomobile bind -target={ios,android} github.com/hyangah/ivy
	gobuild build -target={ios,android} golang.org/x/mobile/example/basic

	(With various takes on -x and -v.)

Change-Id: I15c8f605490381feb6fefb482110f2a1c210529d
Reviewed-on: https://go-review.googlesource.com/12411
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>

cmd/gomobile/bind.go

@@ -167,14 +167,20 @@ func copyFile(dst, src string) error {
 	if buildX {
 		printcmd("cp %s %s", src, dst)
 	}
-	f, err := os.Open(src)
-	if err != nil {
-		return err
-	}
-	defer f.Close()
 	return writeFile(dst, func(w io.Writer) error {
-		_, err := io.Copy(w, f)
+		if buildN {
-		return err
+			return nil
+		}
+		f, err := os.Open(src)
+		if err != nil {
+			return err
+		}
+		defer f.Close()
+
+		if _, err := io.Copy(w, f); err != nil {
+			return fmt.Errorf("cp %s %s failed: %v", src, dst, err)
+		}
+		return nil
 	})
 }
 

cmd/gomobile/bind_androidapp.go

@@ -6,7 +6,6 @@ package main
 
 import (
 	"archive/zip"
-	"bytes"
 	"fmt"
 	"go/build"
 	"io"
@@ -281,24 +280,10 @@ func buildJar(w io.Writer, srcDir string) error {
 	}
 	args = append(args, srcFiles...)
 
-	buf := new(bytes.Buffer)
 	javac := exec.Command("javac", args...)
 	javac.Dir = srcDir
-	if buildV {
+	if err := runCmd(javac); err != nil {
-		javac.Stdout = os.Stdout
+		return err
-		javac.Stderr = os.Stderr
-	} else {
-		javac.Stdout = buf
-		javac.Stderr = buf
-	}
-	if buildX {
-		printcmd("%s", strings.Join(javac.Args, " "))
-	}
-	if !buildN {
-		if err := javac.Run(); err != nil {
-			buf.WriteTo(xout)
-			return err
-		}
 	}
 
 	if buildX {
@@ -307,7 +292,6 @@ func buildJar(w io.Writer, srcDir string) error {
 	if buildN {
 		return nil
 	}
-
 	jarw := zip.NewWriter(w)
 	jarwcreate := func(name string) (io.Writer, error) {
 		if buildV {

cmd/gomobile/bind_iosapp.go

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"go/build"
 	"io"
-	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
@@ -56,15 +55,8 @@ func goIOSBind(pkg *build.Package) error {
 	}
 
 	cmd.Args = append(cmd.Args, "-o", buildO)
-	if buildX {
+	if err := runCmd(cmd); err != nil {
-		printcmd(strings.Join(cmd.Args, " "))
+		return err
-	}
-	if !buildN {
-		cmd.Stderr = os.Stderr
-		cmd.Stdout = os.Stdout
-		if err := cmd.Run(); err != nil {
-			return err
-		}
 	}
 
 	// Copy header file next to output archive.
@@ -92,33 +84,16 @@ func goIOSBindArchive(name, path string, env []string) (string, error) {
 	)
 	cmd.Args = append(cmd.Args, strings.Split(getenv(env, "CGO_CFLAGS"), " ")...)
 	cmd.Dir = filepath.Join(tmpdir, "objc")
-	cmd.Env = env
+	cmd.Env = append([]string{}, env...)
-	if buildX {
+	if err := runCmd(cmd); err != nil {
-		printcmd("PWD=" + cmd.Dir + " " + strings.Join(cmd.Env, " ") + strings.Join(cmd.Args, " "))
+		return "", err
-	}
-	if !buildN {
-		cmd.Stderr = os.Stderr // nominally silent
-		cmd.Stdout = os.Stdout
-		if err := cmd.Run(); err != nil {
-			return "", err
-		}
 	}
 
 	cmd = exec.Command("ar", "-q", "-s", archive, obj)
 	cmd.Dir = filepath.Join(tmpdir, "objc")
-	if buildX {
+	if err := runCmd(cmd); err != nil {
-		printcmd("PWD=" + cmd.Dir + " " + strings.Join(cmd.Args, " "))
+		return "", err
 	}
-	if !buildN {
-		out, err := cmd.CombinedOutput()
-		if buildV {
-			os.Stderr.Write(out)
-		}
-		if err != nil {
-			return "", fmt.Errorf("ar: %v\n%s", err, out)
-		}
-	}
-
 	return archive, nil
 }
 

cmd/gomobile/bind_test.go

@@ -61,7 +61,7 @@ mkdir -p $WORK/android/src/main/java/go/asset
 mkdir -p $WORK/android/src/main/java/go
 rm $WORK/android/src/main/java/go/Seq.java
 ln -s $GOPATH/src/golang.org/x/mobile/bind/java/Seq.java $WORK/android/src/main/java/go/Seq.java
-javac -d $WORK/javac-output -source 1.7 -target 1.7 -bootclasspath $ANDROID_HOME/platforms/android-22/android.jar *.java
+PWD=$WORK/android/src/main/java javac -d $WORK/javac-output -source 1.7 -target 1.7 -bootclasspath $ANDROID_HOME/platforms/android-22/android.jar *.java
 jar c -C $WORK/javac-output .
 rm -r -f "$WORK/javac-output"
 `))

cmd/gomobile/build.go

@@ -7,7 +7,6 @@
 package main
 
 import (
-	"bytes"
 	"fmt"
 	"go/build"
 	"io"
@@ -228,24 +227,5 @@ func goBuild(src string, env []string, args ...string) error {
 	cmd.Args = append(cmd.Args, args...)
 	cmd.Args = append(cmd.Args, src)
 	cmd.Env = append([]string{}, env...)
-	buf := new(bytes.Buffer)
+	return runCmd(cmd)
-	buf.WriteByte('\n')
-	if buildV {
-		cmd.Stdout = os.Stdout
-		cmd.Stderr = os.Stderr
-	} else {
-		cmd.Stdout = buf
-		cmd.Stderr = buf
-	}
-
-	if buildX {
-		printcmd("%s", strings.Join(cmd.Env, " ")+" "+strings.Join(cmd.Args, " "))
-	}
-	if !buildN {
-		cmd.Env = environ(cmd.Env)
-		if err := cmd.Run(); err != nil {
-			return fmt.Errorf("go build failed: %v%s", err, buf)
-		}
-	}
-	return nil
 }

cmd/gomobile/build_iosapp.go

@@ -5,10 +5,8 @@
 package main
 
 import (
-	"bytes"
 	"fmt"
 	"go/build"
-	"io"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -30,13 +28,13 @@ func goIOSBuild(pkg *build.Package) error {
 	}
 
 	for dst, v := range layout {
+		if err := mkdir(filepath.Dir(dst)); err != nil {
+			return err
+		}
 		if buildX {
 			printcmd("echo \"%s\" > %s", v, dst)
 		}
 		if !buildN {
-			if err := os.MkdirAll(filepath.Dir(dst), 0775|os.ModeDir); err != nil {
-				return err
-			}
 			if err := ioutil.WriteFile(dst, v, 0644); err != nil {
 				return err
 			}
@@ -57,19 +55,13 @@ func goIOSBuild(pkg *build.Package) error {
 	// We are using lipo tool to build multiarchitecture binaries.
 	// TODO(jbd): Investigate the new announcements about iO9's fat binary
 	// size limitations are breaking this feature.
-	if buildX {
+	cmd := exec.Command(
-		printcmd("xcrun lipo -create %s %s -o %s", armPath, arm64Path, filepath.Join(tmpdir, "main/main"))
+		"xcrun", "lipo",
-	}
+		"-create", armPath, arm64Path,
-	if !buildN {
+		"-o", filepath.Join(tmpdir, "main/main"),
-		cmd := exec.Command(
+	)
-			"xcrun", "lipo",
+	if err := runCmd(cmd); err != nil {
-			"-create", armPath, arm64Path,
+		return err
-			"-o", filepath.Join(tmpdir, "main/main"),
-		)
-		cmd.Stderr = os.Stderr
-		if err := cmd.Run(); err != nil {
-			return err
-		}
 	}
 
 	// TODO(jbd): Set the launcher icon.
@@ -78,30 +70,13 @@ func goIOSBuild(pkg *build.Package) error {
 	}
 
 	// Build and move the release build to the output directory.
-	cmd := exec.Command(
+	cmd = exec.Command(
 		"xcrun", "xcodebuild",
 		"-configuration", "Release",
 		"-project", tmpdir+"/main.xcodeproj",
 	)
-
+	if err := runCmd(cmd); err != nil {
-	if buildX {
+		return err
-		printcmd("%s", strings.Join(cmd.Args, " "))
-	}
-
-	buf := new(bytes.Buffer)
-	buf.WriteByte('\n')
-	if buildV {
-		cmd.Stdout = os.Stdout
-		cmd.Stderr = os.Stderr
-	} else {
-		cmd.Stdout = buf
-		cmd.Stderr = buf
-	}
-
-	if !buildN {
-		if err := cmd.Run(); err != nil {
-			return fmt.Errorf("xcodebuild failed: %v%s", err, buf)
-		}
 	}
 
 	// TODO(jbd): Fallback to copying if renaming fails.
@@ -125,13 +100,8 @@ func goIOSBuild(pkg *build.Package) error {
 
 func iosCopyAssets(pkg *build.Package, xcodeProjDir string) error {
 	dstAssets := xcodeProjDir + "/main/assets"
-	if buildX {
+	if err := mkdir(dstAssets); err != nil {
-		printcmd("mkdir -p %s", dstAssets)
+		return err
-	}
-	if !buildN {
-		if err := os.MkdirAll(dstAssets, 0755); err != nil {
-			return err
-		}
 	}
 
 	srcAssets := filepath.Join(pkg.Dir, "assets")
@@ -148,13 +118,6 @@ func iosCopyAssets(pkg *build.Package, xcodeProjDir string) error {
 		return nil
 	}
 
-	if buildX {
-		printcmd("cp -R %s %s", filepath.Join(pkg.Dir, "assets"), dstAssets)
-	}
-	if buildN {
-		return nil
-	}
-
 	return filepath.Walk(srcAssets, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
@@ -163,20 +126,7 @@ func iosCopyAssets(pkg *build.Package, xcodeProjDir string) error {
 			return nil
 		}
 		dst := dstAssets + "/" + path[len(srcAssets)+1:]
-		if err := os.MkdirAll(filepath.Dir(dst), 0755); err != nil {
+		return copyFile(dst, path)
-			return err
-		}
-		f, err := os.Create(dst)
-		if err != nil {
-			return err
-		}
-		w, err := os.Open(path)
-		if err != nil {
-			return err
-		}
-		defer f.Close()
-		_, err = io.Copy(w, f)
-		return err
 	})
 }
 

cmd/gomobile/init.go

@@ -167,10 +167,6 @@ func installStd(env []string, args ...string) error {
 	if buildV {
 		fmt.Fprintf(os.Stderr, "\n# Building standard library for %s/%s.\n", tOS, tArch)
 	}
-	envpath := os.Getenv("PATH")
-	if buildN {
-		envpath = "$PATH"
-	}
 
 	cmd := exec.Command("go", "install", "-pkgdir="+pkgdir(env))
 	cmd.Args = append(cmd.Args, args...)
@@ -181,29 +177,8 @@ func installStd(env []string, args ...string) error {
 		cmd.Args = append(cmd.Args, "-x")
 	}
 	cmd.Args = append(cmd.Args, "std")
-	cmd.Env = []string{"PATH=" + envpath}
+	cmd.Env = append([]string{}, env...)
-	cmd.Env = append(cmd.Env, env...)
+	return runCmd(cmd)
-	if buildX {
-		printcmd("%s", strings.Join(cmd.Env, " ")+" "+strings.Join(cmd.Args, " "))
-	}
-
-	buf := new(bytes.Buffer)
-	buf.WriteByte('\n')
-	if buildV {
-		cmd.Stdout = os.Stdout
-		cmd.Stderr = os.Stderr
-	} else {
-		cmd.Stdout = buf
-		cmd.Stderr = buf
-	}
-
-	if !buildN {
-		cmd.Env = environ(cmd.Env)
-		if err := cmd.Run(); err != nil {
-			return fmt.Errorf("go install std for %s/%s failed: %v%s", tOS, tArch, err, buf)
-		}
-	}
-	return nil
 }
 
 func removeGomobilepkg() {
@@ -452,18 +427,7 @@ func fetchFullNDK() error {
 		inflate = exec.Command("7z.exe", "x", archive)
 	}
 	inflate.Dir = tmpdir
-	if buildX {
+	return runCmd(inflate)
-		printcmd("%s", archive)
-	}
-	if !buildN {
-		out, err := inflate.CombinedOutput()
-		if err != nil {
-			if buildV {
-				os.Stderr.Write(out)
-			}
-			return err
-		}
-	}
 	return nil
 }
 
@@ -603,3 +567,35 @@ func goEnv(name string) string {
 	}
 	return strings.TrimSpace(string(val))
 }
+
+func runCmd(cmd *exec.Cmd) error {
+	if buildX {
+		dir := ""
+		if cmd.Dir != "" {
+			dir = "PWD=" + cmd.Dir + " "
+		}
+		env := strings.Join(cmd.Env, " ")
+		if env != "" {
+			env += " "
+		}
+		printcmd("%s%s%s", dir, env, strings.Join(cmd.Args, " "))
+	}
+
+	buf := new(bytes.Buffer)
+	buf.WriteByte('\n')
+	if buildV {
+		cmd.Stdout = os.Stdout
+		cmd.Stderr = os.Stderr
+	} else {
+		cmd.Stdout = buf
+		cmd.Stderr = buf
+	}
+
+	if !buildN {
+		cmd.Env = environ(cmd.Env)
+		if err := cmd.Run(); err != nil {
+			return fmt.Errorf("%s failed: %v%s", strings.Join(cmd.Args, " "), err, buf)
+		}
+	}
+	return nil
+}

cmd/gomobile/init_test.go

@@ -108,10 +108,10 @@ tar xfz $GOMOBILE/dl/gomobile-openal-soft-1.16.0.1.tar.gz
 mv $WORK/openal/include/AL $GOMOBILE/android-{{.NDK}}/arm/sysroot/usr/include/AL
 mkdir -p $GOMOBILE/android-{{.NDK}}/openal
 mv $WORK/openal/lib $GOMOBILE/android-{{.NDK}}/openal/lib
-{{if eq .GOOS "darwin"}}PATH=$PATH GOOS=android GOARCH=arm GOARM=7 CC=$GOMOBILE/android-{{.NDK}}/arm/bin/arm-linux-androideabi-gcc{{.EXE}} CXX=$GOMOBILE/android-{{.NDK}}/arm/bin/arm-linux-androideabi-g++ CGO_ENABLED=1 go install -pkgdir=$GOMOBILE/pkg_android_arm -x std
+{{if eq .GOOS "darwin"}}GOOS=android GOARCH=arm GOARM=7 CC=$GOMOBILE/android-{{.NDK}}/arm/bin/arm-linux-androideabi-gcc{{.EXE}} CXX=$GOMOBILE/android-{{.NDK}}/arm/bin/arm-linux-androideabi-g++ CGO_ENABLED=1 go install -pkgdir=$GOMOBILE/pkg_android_arm -x std
-PATH=$PATH GOOS=darwin GOARCH=arm GOARM=7 CC=clang-iphoneos CXX=clang-iphoneos CGO_CFLAGS=-isysroot=iphoneos -arch armv7 CGO_LDFLAGS=-isysroot=iphoneos -arch armv7 CGO_ENABLED=1 go install -pkgdir=$GOMOBILE/pkg_darwin_arm -x std
+GOOS=darwin GOARCH=arm GOARM=7 CC=clang-iphoneos CXX=clang-iphoneos CGO_CFLAGS=-isysroot=iphoneos -arch armv7 CGO_LDFLAGS=-isysroot=iphoneos -arch armv7 CGO_ENABLED=1 go install -pkgdir=$GOMOBILE/pkg_darwin_arm -x std
-PATH=$PATH GOOS=darwin GOARCH=arm64 CC=clang-iphoneos CXX=clang-iphoneos CGO_CFLAGS=-isysroot=iphoneos -arch arm64 CGO_LDFLAGS=-isysroot=iphoneos -arch arm64 CGO_ENABLED=1 go install -pkgdir=$GOMOBILE/pkg_darwin_arm64 -x std
+GOOS=darwin GOARCH=arm64 CC=clang-iphoneos CXX=clang-iphoneos CGO_CFLAGS=-isysroot=iphoneos -arch arm64 CGO_LDFLAGS=-isysroot=iphoneos -arch arm64 CGO_ENABLED=1 go install -pkgdir=$GOMOBILE/pkg_darwin_arm64 -x std
-PATH=$PATH GOOS=darwin GOARCH=amd64 CC=clang-iphonesimulator CXX=clang-iphonesimulator CGO_CFLAGS=-isysroot=iphonesimulator -mios-simulator-version-min=6.1 -arch x86_64 CGO_LDFLAGS=-isysroot=iphonesimulator -mios-simulator-version-min=6.1 -arch x86_64 CGO_ENABLED=1 go install -pkgdir=$GOMOBILE/pkg_darwin_amd64 -tags=ios -x std
+GOOS=darwin GOARCH=amd64 CC=clang-iphonesimulator CXX=clang-iphonesimulator CGO_CFLAGS=-isysroot=iphonesimulator -mios-simulator-version-min=6.1 -arch x86_64 CGO_LDFLAGS=-isysroot=iphonesimulator -mios-simulator-version-min=6.1 -arch x86_64 CGO_ENABLED=1 go install -pkgdir=$GOMOBILE/pkg_darwin_amd64 -tags=ios -x std
 {{end}}go version > $GOMOBILE/version
 rm -r -f "$WORK"
 `))