cmd/go: fix relative imports again

I tried before to make relative imports work by simply
invoking the compiler in the right directory, so that
an import of ./foo could be resolved by ./foo.a.
This required creating a separate tree of package binaries
that included the full path to the source directory, so that
/home/gopher/bar.go would be compiled in
tmpdir/work/local/home/gopher and perhaps find
a ./foo.a in that directory.

This model breaks on Windows because : appears in path
names but cannot be used in subdirectory names, and I
missed one or two places where it needed to be removed.

The model breaks more fundamentally when compiling
a test of a package that lives outside the Go path, because
we effectively use a ./ import in the generated testmain,
but there we want to be able to resolve the ./ import
of the test package to one directory and all the other ./
imports to a different directory.  Piggybacking on the compiler's
current working directory is then no longer possible.

Instead, introduce a new compiler option -D prefix that
makes the compiler turn a ./ import into prefix+that,
so that import "./foo" with -D a/b/c turns into import
"a/b/c/foo".  Then we can invent a package hierarchy
"_/" with subdirectories named for file system paths:
import "./foo" in the directory /home/gopher becomes
import "_/home/gopher/foo", and since that final path
is just an ordinary import now, all the ordinary processing
works, without special cases.

We will have to change the name of the hierarchy if we
ever decide to introduce a standard package with import
path "_", but that seems unlikely, and the detail is known
only in temporary packages that get thrown away at the
end of a build.

Fixes #3169.

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/5732045

src/cmd/gc/doc.go

@@ -38,6 +38,8 @@ Flags:
 	-p path
 		assume that path is the eventual import path for this code,
 		and diagnose any attempt to import a package that depends on it.
+	-D path
+		treat a relative import as relative to path
 	-L
 		show entire file path when printing line numbers in errors
 	-I dir1 -I dir2

src/cmd/gc/go.h

@@ -772,6 +772,7 @@ extern	char*	runtimeimport;
 extern	char*	unsafeimport;
 EXTERN	char*	myimportpath;
 EXTERN	Idir*	idirs;
+EXTERN	char*	localimport;
 
 EXTERN	Type*	types[NTYPE];
 EXTERN	Type*	idealstring;

src/cmd/gc/lex.c

@@ -148,6 +148,7 @@ usage(void)
 	// -y print declarations in cannedimports (used with -d)
 	// -% print non-static initializers
 	// -+ indicate that the runtime is being compiled
+	print("  -D PATH interpret local imports relative to this import path\n");
 	print("  -I DIR search for packages in DIR\n");
 	print("  -L show full path in file:line prints\n");
 	print("  -N disable optimizations\n");
@@ -238,14 +239,18 @@ main(int argc, char *argv[])
 		myimportpath = EARGF(usage());
 		break;
 
-	case 'I':
-		addidir(EARGF(usage()));
-		break;
-	
 	case 'u':
 		safemode = 1;
 		break;
 
+	case 'D':
+		localimport = EARGF(usage());
+		break;
+
+	case 'I':
+		addidir(EARGF(usage()));
+		break;
+	
 	case 'V':
 		p = expstring();
 		if(strcmp(p, "X:none") == 0)
@@ -516,8 +521,12 @@ islocalname(Strlit *name)
 	   	return 1;
 	if(name->len >= 2 && strncmp(name->s, "./", 2) == 0)
 		return 1;
+	if(name->len == 1 && strncmp(name->s, ".", 1) == 0)
+		return 1;
 	if(name->len >= 3 && strncmp(name->s, "../", 3) == 0)
 		return 1;
+	if(name->len == 2 && strncmp(name->s, "..", 2) == 0)
+		return 1;
 	return 0;
 }
 
@@ -588,7 +597,7 @@ importfile(Val *f, int line)
 	int32 c;
 	int len;
 	Strlit *path;
-	char *cleanbuf;
+	char *cleanbuf, *prefix;
 
 	USED(line);
 
@@ -642,8 +651,11 @@ importfile(Val *f, int line)
 			fakeimport();
 			return;
 		}
-		cleanbuf = mal(strlen(pathname) + strlen(path->s) + 2);
-		strcpy(cleanbuf, pathname);
+		prefix = pathname;
+		if(localimport != nil)
+			prefix = localimport;
+		cleanbuf = mal(strlen(prefix) + strlen(path->s) + 2);
+		strcpy(cleanbuf, prefix);
 		strcat(cleanbuf, "/");
 		strcat(cleanbuf, path->s);
 		cleanname(cleanbuf);

src/cmd/go/pkg.go

@@ -12,6 +12,7 @@ import (
 	"go/scanner"
 	"go/token"
 	"os"
+	pathpkg "path"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -60,15 +61,16 @@ type Package struct {
 	XTestImports []string `json:",omitempty"` // imports from XTestGoFiles
 
 	// Unexported fields are not part of the public API.
-	build      *build.Package
-	pkgdir     string // overrides build.PkgDir
-	imports    []*Package
-	deps       []*Package
-	gofiles    []string // GoFiles+CgoFiles+TestGoFiles+XTestGoFiles files, absolute paths
-	target     string   // installed file for this package (may be executable)
-	fake       bool     // synthesized package
-	forceBuild bool     // this package must be rebuilt
-	local      bool     // imported via local path (./ or ../)
+	build       *build.Package
+	pkgdir      string // overrides build.PkgDir
+	imports     []*Package
+	deps        []*Package
+	gofiles     []string // GoFiles+CgoFiles+TestGoFiles+XTestGoFiles files, absolute paths
+	target      string   // installed file for this package (may be executable)
+	fake        bool     // synthesized package
+	forceBuild  bool     // this package must be rebuilt
+	local       bool     // imported via local path (./ or ../)
+	localPrefix string   // interpret ./ and ../ imports relative to this prefix
 }
 
 func (p *Package) copyBuild(pp *build.Package) {
@@ -161,6 +163,17 @@ func reloadPackage(arg string, stk *importStack) *Package {
 	return loadPackage(arg, stk)
 }
 
+// dirToImportPath returns the pseudo-import path we use for a package
+// outside the Go path.  It begins with _/ and then contains the full path
+// to the directory.  If the package lives in c:\home\gopher\my\pkg then
+// the pseudo-import path is _/c_/home/gopher/my/pkg.
+// Using a pseudo-import path like this makes the ./ imports no longer
+// a special case, so that all the code to deal with ordinary imports works
+// automatically.
+func dirToImportPath(dir string) string {
+	return pathpkg.Join("_", strings.Replace(filepath.ToSlash(dir), ":", "_", -1))
+}
+
 // loadImport scans the directory named by path, which must be an import path,
 // but possibly a local import path (an absolute file system path or one beginning
 // with ./ or ../).  A local relative path is interpreted relative to srcDir.
@@ -170,24 +183,28 @@ func loadImport(path string, srcDir string, stk *importStack, importPos []token.
 	defer stk.pop()
 
 	// Determine canonical identifier for this package.
-	// For a local path (./ or ../) the identifier is the full
-	// directory name.  Otherwise it is the import path.
-	pkgid := path
+	// For a local import the identifier is the pseudo-import path
+	// we create from the full directory to the package.
+	// Otherwise it is the usual import path.
+	importPath := path
 	isLocal := build.IsLocalImport(path)
 	if isLocal {
-		pkgid = filepath.Join(srcDir, path)
+		importPath = dirToImportPath(filepath.Join(srcDir, path))
 	}
-	if p := packageCache[pkgid]; p != nil {
+	if p := packageCache[importPath]; p != nil {
 		return reusePackage(p, stk)
 	}
 
 	p := new(Package)
-	packageCache[pkgid] = p
+	p.local = isLocal
+	p.ImportPath = importPath
+	packageCache[importPath] = p
 
 	// Load package.
 	// Import always returns bp != nil, even if an error occurs,
 	// in order to return partial information.
 	bp, err := buildContext.Import(path, srcDir, build.AllowBinary)
+	bp.ImportPath = importPath
 	p.load(stk, bp, err)
 	if p.Error != nil && len(importPos) > 0 {
 		pos := importPos[0]
@@ -211,7 +228,6 @@ func reusePackage(p *Package, stk *importStack) *Package {
 				ImportStack: stk.copy(),
 				Err:         "import loop",
 			}
-			panic("loop")
 		}
 		p.Incomplete = true
 	}
@@ -258,14 +274,12 @@ func expandScanner(err error) error {
 // be the result of calling build.Context.Import.
 func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package {
 	p.copyBuild(bp)
-	p.local = build.IsLocalImport(p.ImportPath)
-	if p.local {
-		// The correct import for this package depends on which
-		// directory you are in.  Instead, record the full directory path.
-		// That can't be used as an import path at all, but at least
-		// it uniquely identifies the package.
-		p.ImportPath = p.Dir
-	}
+
+	// The localPrefix is the path we interpret ./ imports relative to.
+	// Now that we've fixed the import path, it's just the import path.
+	// Synthesized main packages sometimes override this.
+	p.localPrefix = p.ImportPath
+
 	if err != nil {
 		p.Incomplete = true
 		err = expandScanner(err)
@@ -326,7 +340,7 @@ func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package
 		}
 		p1 := loadImport(path, p.Dir, stk, p.build.ImportPos[path])
 		if p1.local {
-			path = p1.Dir
+			path = p1.ImportPath
 			importPaths[i] = path
 		}
 		deps[path] = true

src/cmd/go/run.go

@@ -45,6 +45,12 @@ func runRun(cmd *Command, args []string) {
 	}
 	files, cmdArgs := args[:i], args[i:]
 	p := goFilesPackage(files)
+	if p.Error != nil {
+		fatalf("%s", p.Error)
+	}
+	if p.Name != "main" {
+		fatalf("cannot run non-main package")
+	}
 	p.target = "" // must build - not up to date
 	a1 := b.action(modeBuild, modeBuild, p)
 	a := &action{f: (*builder).runProgram, args: cmdArgs, deps: []*action{a1}}

src/cmd/go/test.bash

@@ -30,6 +30,14 @@ if ! grep -q '^easysub\.Hello' hello.out; then
 	ok=false
 fi
 
+./testgo build -o hello testdata/local/easysub/main.go
+./hello >hello.out
+if ! grep -q '^easysub\.Hello' hello.out; then
+	echo "testdata/local/easysub/main.go did not generate expected output"
+	cat hello.out
+	ok=false
+fi
+
 ./testgo build -o hello testdata/local/hard.go
 ./hello >hello.out
 if ! grep -q '^sub\.Hello' hello.out || ! grep -q '^subsub\.Hello' hello.out ; then
@@ -46,6 +54,19 @@ if ./testgo install testdata/local/easy.go >/dev/null 2>&1; then
 	ok=false
 fi
 
+# Test tests with relative imports.
+if ! ./testgo test ./testdata/testimport; then
+	echo "go test ./testdata/testimport failed"
+	ok=false
+fi
+
+# Test tests with relative imports in packages synthesized
+# from Go files named on the command line.
+if ! ./testgo test ./testdata/testimport/*.go; then
+	echo "go test ./testdata/testimport/*.go failed"
+	ok=false
+fi
+
 if $ok; then
 	echo PASS
 else

src/cmd/go/test.go

@@ -351,7 +351,7 @@ func runTest(cmd *Command, args []string) {
 
 	warned := false
 	for _, a := range actionList(root) {
-		if a.p != nil && a.f != nil && !okBuild[a.p] && !a.p.fake {
+		if a.p != nil && a.f != nil && !okBuild[a.p] && !a.p.fake && !a.p.local {
 			okBuild[a.p] = true // don't warn again
 			if !warned {
 				fmt.Fprintf(os.Stderr, "warning: building out-of-date packages:\n")
@@ -474,11 +474,12 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
 	// External test package.
 	if len(p.XTestGoFiles) > 0 {
 		pxtest = &Package{
-			Name:       p.Name + "_test",
-			ImportPath: p.ImportPath + "_test",
-			Dir:        p.Dir,
-			GoFiles:    p.XTestGoFiles,
-			Imports:    p.XTestImports,
+			Name:        p.Name + "_test",
+			ImportPath:  p.ImportPath + "_test",
+			localPrefix: p.localPrefix,
+			Dir:         p.Dir,
+			GoFiles:     p.XTestGoFiles,
+			Imports:     p.XTestImports,
 			build: &build.Package{
 				ImportPos: p.build.XTestImportPos,
 			},

src/cmd/go/testdata/local/easysub/main.go

+// +build ignore
+
+package main
+
+import "."
+
+func main() {
+	easysub.Hello()
+}

src/cmd/go/testdata/testimport/p.go

+package p
+
+func F() int { return 1 }

src/cmd/go/testdata/testimport/p1/p1.go

+package p1
+
+func F() int { return 1 }

src/cmd/go/testdata/testimport/p2/p2.go

+package p2
+
+func F() int { return 1 }

src/cmd/go/testdata/testimport/p_test.go

+package p
+
+import (
+	"./p1"
+
+	"testing"
+)
+
+func TestF(t *testing.T) {
+	if F() != p1.F() {
+		t.Fatal(F())
+	}
+}

src/cmd/go/testdata/testimport/x_test.go

+package p_test
+
+import (
+	. "../testimport"
+
+	"./p2"
+
+	"testing"
+)
+
+func TestF1(t *testing.T) {
+	if F() != p2.F() {
+		t.Fatal(F())
+	}
+}