fix and re-enable "garble test" (#268)

With the many refactors building up to v0.1.0, we broke "garble test" as
we no longer dealt with test packages well.

Luckily, now that we can depend on TOOLEXEC_IMPORTPATH, we can support
the test command again, as we can always figure out what package we're
currently compiling, without having to track a "main" package.

Note that one major pitfall there is test packages, where
TOOLEXEC_IMPORTPATH does not agree with ImportPath from "go list -json".
However, we can still work around that with a bit of glue code, which is
also copiously documented.

The second change necessary is to consider test packages private
depending on whether their non-test package is private or not. This can
be done via the ForTest field in "go list -json".

The third change is to obfuscate "_testmain.go" files, which are the
code-generated main functions which actually run tests. We used to not
need to obfuscate them, since test function names are never obfuscated
and we used to not obfuscate import paths at compilation time. Now we do
rewrite import paths, so we must do that for "_testmain.go" too.

The fourth change is to re-enable test.txt, and expand it with more
sanity checks and edge cases.

Finally, document "garble test" again.

Fixes #241.

main.go

@@ -68,6 +68,8 @@ Usage:
 
 	garble [flags] build [build flags] [packages]
 
+Aside from "build", the "test" command mirroring "go test" is also supported.
+
 garble accepts the following flags:
 
 `[1:])
@@ -245,7 +247,7 @@ func mainErr(args []string) error {
 		return nil
 	case "reverse":
 		return commandReverse(args)
-	case "build":
+	case "build", "test":
 		cmd, err := toolexecCmd(command, args)
 		if err != nil {
 			return err
@@ -278,6 +280,32 @@ func mainErr(args []string) error {
 	}
 
 	toolexecImportPath := os.Getenv("TOOLEXEC_IMPORTPATH")
+
+	// Unfortunately, TOOLEXEC_IMPORTPATH is just "foo/bar" for the package
+	// whose ImportPath in "go list -json" is "foo/bar [foo/bar.test]".
+	// The ImportPath "foo/bar" also exists in "go list -json", so we can't
+	// possibly differentiate between the two versions of a package.
+	// The same happens with "foo/bar_test", whose ImportPath is actually
+	// "foo/bar_test [foo/bar.test]".
+	// We'll likely file this as an upstream bug to fix in Go 1.17.
+	//
+	// Until then, here's our workaround: since this edge case only happens
+	// for the compiler, check if any "_test.go" files are being compiled.
+	// If so, we are compiling a test package, so we add the missing extra.
+	if tool == "compile" {
+		isTestPkg := false
+		_, paths := splitFlagsFromFiles(args, ".go")
+		for _, path := range paths {
+			if strings.HasSuffix(path, "_test.go") {
+				isTestPkg = true
+				break
+			}
+		}
+		if isTestPkg {
+			forPkg := strings.TrimSuffix(toolexecImportPath, "_test")
+			toolexecImportPath = fmt.Sprintf("%s [%s.test]", toolexecImportPath, forPkg)
+		}
+	}
 	curPkg = cache.ListedPackages[toolexecImportPath]
 	if curPkg == nil {
 		return fmt.Errorf("TOOLEXEC_IMPORTPATH not found in listed packages: %s", toolexecImportPath)
@@ -435,9 +463,6 @@ func transformCompile(args []string) ([]string, error) {
 			break
 		}
 	}
-	if len(paths) == 1 && filepath.Base(paths[0]) == "_testmain.go" {
-		return append(flags, paths...), nil
-	}
 
 	// If the value of -trimpath doesn't contain the separator ';', the 'go
 	// build' command is most likely not using '-trimpath'.
@@ -599,9 +624,11 @@ func transformCompile(args []string) ([]string, error) {
 		}
 
 		// Uncomment for some quick debugging. Do not delete.
-		// fmt.Fprintf(os.Stderr, "\n-- %s/%s --\n", curPkgPath, origName)
+		// if curPkg.Private {
-		// if err := printConfig.Fprint(os.Stderr, fset, file); err != nil {
+		// 	fmt.Fprintf(os.Stderr, "\n-- %s/%s --\n", curPkg.ImportPath, origName)
-		// 	return nil, err
+		// 	if err := printConfig.Fprint(os.Stderr, fset, file); err != nil {
+		// 		return nil, err
+		// 	}
 		// }
 
 		tempFile, err := ioutil.TempFile(sharedTempDir, name+".*.go")
@@ -802,14 +829,14 @@ var runtimeRelated = map[string]bool{
 	"crypto":        true,
 }
 
-// isPrivate checks if GOPRIVATE matches path.
+// isPrivate checks if a package import path should be considered private,
-//
+// meaning that it should be obfuscated.
-// To allow using garble without GOPRIVATE for standalone main packages, it will
-// default to not matching standard library packages.
 func isPrivate(path string) bool {
+	// We don't support obfuscating these yet.
 	if runtimeRelated[path] {
 		return false
 	}
+	// These are main packages, so we must always obfuscate them.
 	if path == "command-line-arguments" || strings.HasPrefix(path, "plugin/unnamed") {
 		return true
 	}
@@ -904,6 +931,11 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) {
 		}
 		fmt.Fprintf(newCfg, "packagefile %s=%s\n", impPath, pkg.packagefile)
 	}
+
+	// Uncomment to debug the transformed importcfg. Do not delete.
+	// newCfg.Seek(0, 0)
+	// io.Copy(os.Stderr, newCfg)
+
 	if err := newCfg.Close(); err != nil {
 		return "", err
 	}

shared.go

@@ -156,6 +156,7 @@ func setFlagOptions() error {
 type listedPackage struct {
 	Name       string
 	ImportPath string
+	ForTest    string
 	Export     string
 	BuildID    string
 	Deps       []string
@@ -179,6 +180,7 @@ func (p *listedPackage) obfuscatedImportPath() string {
 		return p.ImportPath
 	}
 	newPath := hashWith(p.GarbleActionID, p.ImportPath)
+	// log.Printf("%q hashed with %x to %q", p.ImportPath, p.GarbleActionID, newPath)
 	return newPath
 }
 
@@ -232,7 +234,12 @@ func setListedPackages(patterns []string) error {
 
 	anyPrivate := false
 	for path, pkg := range cache.ListedPackages {
-		if isPrivate(path) {
+		// If "GOPRIVATE=foo/bar", "foo/bar_test" is also private.
+		if pkg.ForTest != "" {
+			path = pkg.ForTest
+		}
+		// Test main packages like "foo/bar.test" are always private.
+		if (pkg.Name == "main" && strings.HasSuffix(path, ".test")) || isPrivate(path) {
 			pkg.Private = true
 			anyPrivate = true
 		}

testdata/scripts/test.txt

@@ -1,23 +1,36 @@
-skip # see https://github.com/burrowers/garble/issues/241
-
-# Note that we need bar_test too.
-env GOPRIVATE=test/bar,test/bar_test
-
 # build the test binary
-garble test -c
+garble test -c -a
 binsubstr bar.test$exe 'TestFoo' 'TestSeparateFoo'
-! binsubstr bar.test$exe 'ImportedVar'
+! binsubstr bar.test$exe 'LocalFoo|ImportedVar|OriginalFuncName'
 
-# run the tests
+# run the tests; OriginalFuncName should be obfuscated
 exec ./bar.test -test.v
 stdout 'PASS.*TestFoo'
 stdout 'PASS.*TestSeparateFoo'
+stdout 'package bar, func name:'
+stdout 'package bar_test, func name:'
+! stdout 'OriginalFuncName'
 
 [short] stop # no need to verify this with -short
 
-# verify with regular cmd/go
+# also check that many packages test fine, including a main package and multiple
+# test packages
+garble test -v . ./somemain ./sometest
+stdout 'ok\s+test/bar\s'
+stdout 'PASS.*TestFoo'
+stdout 'PASS.*TestSeparateFoo'
+stdout 'package bar, func name:'
+stdout 'package bar_test, func name:'
+! stdout 'OriginalFuncName'
+stdout '\?\s+test/bar/somemain\s'
+stdout 'ok\s+test/bar/sometest\s'
+
+# verify with regular cmd/go; OriginalFuncName should appear
 go test -v
 stdout 'PASS.*TestFoo'
+stdout 'PASS.*TestSeparateFoo'
+stdout 'package bar, func name: test/bar\.OriginalFuncName'
+stdout 'package bar_test, func name: test/bar\.OriginalFuncName'
 
 -- go.mod --
 module test/bar
@@ -26,9 +39,18 @@ go 1.16
 -- bar.go --
 package bar
 
-func Foo() string { return "Foo" }
+import "runtime"
+
+func LocalFoo() string { return "Foo" }
 
 var ImportedVar = "imported var value"
+
+// OriginalFuncName returns its own func name.
+func OriginalFuncName() string {
+	pc, _, _, _ := runtime.Caller(0)
+	fn := runtime.FuncForPC(pc)
+	return fn.Name()
+}
 -- bar_test.go --
 package bar
 
@@ -36,9 +58,10 @@ import "testing"
 
 func TestFoo(t *testing.T) {
 	t.Log(ImportedVar)
-	if Foo() != "Foo" {
+	if LocalFoo() != "Foo" {
 		t.FailNow()
 	}
+	t.Logf("package bar, func name: %s", OriginalFuncName())
 }
 -- bar_separate_test.go --
 package bar_test
@@ -51,9 +74,10 @@ import (
 
 func TestSeparateFoo(t *testing.T) {
 	t.Log(bar.ImportedVar)
-	if bar.Foo() != "Foo" {
+	if bar.LocalFoo() != "Foo" {
 		t.FailNow()
 	}
+	t.Logf("package bar_test, func name: %s", bar.OriginalFuncName())
 }
 -- main_test.go --
 package bar
@@ -66,3 +90,13 @@ import (
 func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }
+-- somemain/main.go --
+package main
+
+func main() {}
+-- sometest/foo_test.go --
+package sometest
+
+import "testing"
+
+func TestFoo(t *testing.T) {}