main: fix definition and implementation of relative GOHACK (#49)

rogpeppe has a far sharper brain than me and pointed out a major issue
with the previous implementation: that we hadn't actually implemented
a relative value of GOHACK relative to the main module.

Author: myitcv

cmdget.go

@@ -61,10 +61,6 @@ func runGet1(args []string) error {
 		// Perhaps we should be more resilient in that case?
 		return errors.Notef(err, nil, "cannot get module info")
 	}
-	modf, err := goModInfo()
-	if err != nil {
-		return errors.Notef(err, nil, "cannot get local module info")
-	}
 	for _, mpath := range args {
 		m := mods[mpath]
 		if m == nil {
@@ -74,7 +70,7 @@ func runGet1(args []string) error {
 		// Early check that we can replace the module, so we don't
 		// do all the work to check it out only to find we can't
 		// add the replace directive.
-		if err := checkCanReplace(modf, mpath); err != nil {
+		if err := checkCanReplace(mainModFile, mpath); err != nil {
 			errorf("%v", err)
 			continue
 		}
@@ -99,9 +95,8 @@ func runGet1(args []string) error {
 			}
 			repl = repl1
 		}
-		// Automatically generate a go.mod file if one doesn't
-		// already exist, because otherwise the directory cannot be
-		// used as a module.
+		// Automatically generate a go.mod file if one doesn't already exist,
+		// because otherwise the directory cannot be used as a module.
 		if err := ensureGoModFile(repl.modulePath, repl.dir); err != nil {
 			errorf("%v", err)
 		}
@@ -110,14 +105,14 @@ func runGet1(args []string) error {
 	if len(repls) == 0 {
 		return errors.New("all modules failed; not replacing anything")
 	}
-	if err := replace(modf, repls); err != nil {
+	if err := replace(mainModFile, repls); err != nil {
 		return errors.Notef(err, nil, "cannot replace")
 	}
-	if err := writeModFile(modf); err != nil {
+	if err := writeModFile(mainModFile); err != nil {
 		return errors.Wrap(err)
 	}
 	for _, info := range repls {
-		fmt.Printf("%s => %s\n", info.modulePath, info.dir)
+		fmt.Printf("%s => %s\n", info.modulePath, info.replDir)
 	}
 	return nil
 }
@@ -130,14 +125,18 @@ func updateFromLocalDir(m *listModule) (*modReplace, error) {
 	if err != nil {
 		return nil, errors.Notef(err, nil, "cannot hash %q", m.Dir)
 	}
-	destDir := moduleDir(m.Path)
+	destDir, replDir, err := moduleDir(m.Path)
+	if err != nil {
+		return nil, errors.Notef(err, nil, "failed to determine target directory for %v", m.Path)
+	}
 	_, err = os.Stat(destDir)
 	if err != nil && !os.IsNotExist(err) {
 		return nil, errors.Wrap(err)
 	}
 	repl := &modReplace{
 		modulePath: m.Path,
 		dir:        destDir,
+		replDir:    replDir,
 	}
 	if err != nil {
 		// Destination doesn't exist. Copy the entire directory.
@@ -234,18 +233,22 @@ func updateVCSDir(m *listModule) (*modReplace, error) {
 	return &modReplace{
 		modulePath: m.Path,
 		dir:        info.dir,
+		replDir:    info.replDir,
 	}, nil
 }
 
 type modReplace struct {
+	// modulePath is the module path
 	modulePath string
-	dir        string
+	// dir holds the absolute path to the replacement directory.
+	dir string
+	// replDir holds the path to use for the module in the go.mod replace directive.
+	replDir string
 }
 
 func replace(f *modfile.File, repls []*modReplace) error {
 	for _, repl := range repls {
-		// TODO should we use relative path here?
-		if err := replaceModule(f, repl.modulePath, repl.dir); err != nil {
+		if err := replaceModule(f, repl.modulePath, repl.replDir); err != nil {
 			return errors.Wrap(err)
 		}
 	}

cmdstatus.go

@@ -2,8 +2,6 @@ package main
 
 import (
 	"fmt"
-
-	"gopkg.in/errgo.v2/fmt/errors"
 )
 
 var statusCommand = &Command{
@@ -30,11 +28,7 @@ func cmdStatus(_ *Command, args []string) int {
 }
 
 func printReplacementInfo() error {
-	m, err := goModInfo()
-	if err != nil {
-		return errors.Wrap(err)
-	}
-	for _, r := range m.Replace {
+	for _, r := range mainModFile.Replace {
 		if r.Old.Version == "" && r.New.Version == "" {
 			fmt.Printf("%s => %s\n", r.Old.Path, r.New.Path)
 		}

cmdundo.go

@@ -39,10 +39,6 @@ func cmdUndo(_ *Command, args []string) int {
 }
 
 func cmdUndo1(modules []string) error {
-	modFile, err := goModInfo()
-	if err != nil {
-		return errors.Wrap(err)
-	}
 	modMap := make(map[string]bool)
 	if len(modules) > 0 {
 		for _, m := range modules {
@@ -51,15 +47,15 @@ func cmdUndo1(modules []string) error {
 	} else {
 		// With no modules specified, we un-gohack all modules
 		// we can find with local directory info in the go.mod file.
-		for _, r := range modFile.Replace {
+		for _, r := range mainModFile.Replace {
 			if r.Old.Version == "" && r.New.Version == "" {
 				modMap[r.Old.Path] = true
 				modules = append(modules, r.Old.Path)
 			}
 		}
 	}
 	drop := make(map[string]bool)
-	for _, r := range modFile.Replace {
+	for _, r := range mainModFile.Replace {
 		if !modMap[r.Old.Path] || r.Old.Version != "" || r.New.Version != "" {
 			continue
 		}
@@ -93,7 +89,7 @@ func cmdUndo1(modules []string) error {
 		delete(modMap, r.Old.Path)
 	}
 	for m := range drop {
-		if err := modFile.DropReplace(m, ""); err != nil {
+		if err := mainModFile.DropReplace(m, ""); err != nil {
 			return errors.Notef(err, nil, "cannot drop replacement for %v", m)
 		}
 	}
@@ -105,7 +101,7 @@ func cmdUndo1(modules []string) error {
 	for _, m := range failed {
 		errorf("%s not currently replaced; cannot drop", m)
 	}
-	if err := writeModFile(modFile); err != nil {
+	if err := writeModFile(mainModFile); err != nil {
 		return errors.Wrap(err)
 	}
 	for _, m := range modules {

main.go

@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/rogpeppe/go-internal/modfile"
 	"gopkg.in/errgo.v2/fmt/errors"
 )
 
@@ -52,6 +53,8 @@ var (
 var (
 	exitCode = 0
 	cwd      = "."
+
+	mainModFile *modfile.File
 )
 
 var commands = []*Command{
@@ -68,7 +71,7 @@ func main1() int {
 	if dir, err := os.Getwd(); err == nil {
 		cwd = dir
 	} else {
-		errorf("cannot get current working directory: %v", err)
+		return errorf("cannot get current working directory: %v", err)
 	}
 	flag.Usage = func() {
 		mainUsage(os.Stderr)
@@ -83,22 +86,40 @@ func main1() int {
 	if cmdName == "help" {
 		return runHelp(args)
 	}
-	for _, cmd := range commands {
-		if cmd.Name() != cmdName {
-			continue
+	var cmd *Command
+	for _, c := range commands {
+		if c.Name() == cmdName {
+			cmd = c
+			break
+		}
+	}
+	if cmd == nil {
+		errorf("gohack %s: unknown command\nRun 'gohack help' for usage\n", cmdName)
+		return 2
+	}
+
+	cmd.Flag.Usage = func() { cmd.Usage() }
+
+	if err := cmd.Flag.Parse(args); err != nil {
+		if err != flag.ErrHelp {
+			errorf(err.Error())
 		}
-		cmd.Flag.Usage = func() { cmd.Usage() }
-		cmd.Flag.Parse(args)
-		rcode := cmd.Run(cmd, cmd.Flag.Args())
-		return max(exitCode, rcode)
+		return 2
+	}
+
+	if mf, err := goModInfo(); err == nil {
+		mainModFile = mf
+	} else {
+		return errorf("cannot determine main module: %v", err)
 	}
-	errorf("gohack %s: unknown command\nRun 'gohack help' for usage\n", cmdName)
-	return 2
+
+	rcode := cmd.Run(cmd, cmd.Flag.Args())
+	return max(exitCode, rcode)
 }
 
 const debug = false
 
-func errorf(f string, a ...interface{}) {
+func errorf(f string, a ...interface{}) int {
 	fmt.Fprintln(os.Stderr, fmt.Sprintf(f, a...))
 	if debug {
 		for _, arg := range a {
@@ -108,6 +129,7 @@ func errorf(f string, a ...interface{}) {
 		}
 	}
 	exitCode = 1
+	return exitCode
 }
 
 func max(a, b int) int {

mod.go

@@ -4,6 +4,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/rogpeppe/go-internal/dirhash"
 	"golang.org/x/tools/go/vcs"
@@ -45,8 +46,10 @@ type moduleVCSInfo struct {
 	module *listModule
 	// alreadyExists holds whether the replacement directory already exists.
 	alreadyExists bool
-	// dir holds the path to the replacement directory.
+	// dir holds the absolute path to the replacement directory.
 	dir string
+	// replDir holds the path to use for the module in the go.mod replace directive.
+	replDir string
 	// root holds information on the VCS root of the module.
 	root *vcs.RepoRoot
 	// vcs holds the implementation of the VCS used by the module.
@@ -71,7 +74,10 @@ func getVCSInfoForModule(m *listModule) (*moduleVCSInfo, error) {
 	if !ok {
 		return nil, errors.Newf("unknown VCS kind %q", root.VCS.Cmd)
 	}
-	dir := moduleDir(m.Path)
+	dir, replDir, err := moduleDir(m.Path)
+	if err != nil {
+		return nil, errors.Notef(err, nil, "failed to determine target directory for %v", m.Path)
+	}
 	dirInfo, err := os.Stat(dir)
 	if err != nil && !os.IsNotExist(err) {
 		return nil, errors.Wrap(err)
@@ -84,6 +90,7 @@ func getVCSInfoForModule(m *listModule) (*moduleVCSInfo, error) {
 		root:          root,
 		alreadyExists: err == nil,
 		dir:           dir,
+		replDir:       replDir,
 		vcs:           v,
 	}
 	if !info.alreadyExists {
@@ -108,22 +115,38 @@ func getVCSInfoForModule(m *listModule) (*moduleVCSInfo, error) {
 	return info, nil
 }
 
-// moduleDir returns the path to the directory to be
-// used for storing the module with the given path.
-func moduleDir(module string) string {
-	// TODO decide what color this bikeshed should be.
-	d := filepath.FromSlash(os.Getenv("GOHACK"))
+// moduleDir returns the path to the directory to be used for storing the
+// module with the given path, as well as the filepath to be used in a replace
+// directive. If $GOHACK is set then it will be used. A relative $GOHACK will
+// be interpreted relative to main module directory.
+func moduleDir(module string) (path string, replPath string, err error) {
+	modfp := filepath.FromSlash(module)
+	d := os.Getenv("GOHACK")
 	if d == "" {
-		d = filepath.Join(os.Getenv("HOME"), "gohack")
+		uhd, err := UserHomeDir()
+		if err != nil {
+			return "", "", errors.Notef(err, nil, "failed to determine user home dir")
+		}
+		path = filepath.Join(uhd, "gohack", modfp)
+		return path, path, nil
 	}
 
-	return join(d, filepath.FromSlash(module))
-}
+	if filepath.IsAbs(d) {
+		path = filepath.Join(d, modfp)
+		return path, path, nil
+	}
 
-func join(ps ...string) string {
-	res := filepath.Join(ps...)
-	if !filepath.IsAbs(res) && res[0] != '.' {
-		res = "." + string(os.PathSeparator) + res
+	replPath = filepath.Join(d, modfp)
+	if !strings.HasPrefix(replPath, ".."+string(os.PathSeparator)) {
+		// We know replPath is relative, but filepath.Join strips any leading
+		// "./" prefix, and we need that in the replace directive because
+		// otherwise the path will be treated as a module path rather than a
+		// relative file path, so add it back.
+		replPath = "." + string(os.PathSeparator) + replPath
 	}
-	return res
+
+	mainModDir := filepath.Dir(mainModFile.Syntax.Name)
+	path = filepath.Join(mainModDir, replPath)
+
+	return path, replPath, err
 }

os_1.11.go

@@ -0,0 +1,37 @@
+// +build !go1.12
+
+package main
+
+import (
+	"errors"
+	"os"
+	"runtime"
+)
+
+// UserHomeDir was introduced in Go 1.12. When we drop support for Go 1.11, we can
+// lose this file.
+
+// UserHomeDir returns the current user's home directory.
+//
+// On Unix, including macOS, it returns the $HOME environment variable.
+// On Windows, it returns %USERPROFILE%.
+// On Plan 9, it returns the $home environment variable.
+func UserHomeDir() (string, error) {
+	env, enverr := "HOME", "$HOME"
+	switch runtime.GOOS {
+	case "windows":
+		env, enverr = "USERPROFILE", "%userprofile%"
+	case "plan9":
+		env, enverr = "home", "$home"
+	case "nacl", "android":
+		return "/", nil
+	case "darwin":
+		if runtime.GOARCH == "arm" || runtime.GOARCH == "arm64" {
+			return "/", nil
+		}
+	}
+	if v := os.Getenv(env); v != "" {
+		return v, nil
+	}
+	return "", errors.New(enverr + " is not defined")
+}

os_1.12.go

@@ -0,0 +1,12 @@
+// +build go1.12
+
+package main
+
+import "os"
+
+// UserHomeDir was introduced in Go 1.12. When we drop support for Go 1.11, we can
+// lose this file.
+
+func UserHomeDir() (string, error) {
+	return os.UserHomeDir()
+}