work around a build cache regression in the previous commit

The added comment in main.go explains the situation in detail.
The added test is a minimization of the scenario, which failed:

        > cd mod1
        > garble -seed=${SEED1} build -v gopkg.in/garbletest.v2
        > cd ../mod2
        > garble -seed=${SEED1} build -v
        [stderr]
        test/main/mod2
        # test/main/mod2
        cannot load garble export file for gopkg.in/garbletest.v2: open […]/go-build/ed/[…]-garble-ZV[…]-d: no such file or directory

To work around the problem, we'll always add each package's
GarbleActionID to its build artifact, even when not using -seed.
This will get us the previous behavior with regards to the build cache,
meaning that we fix the recent regression.
The added variable doesn't make it to the final binary either.

While here, improve the cached file loading error's context,
and add an extra sanity check for duplicates on ListedPackages.

main.go

@@ -735,6 +735,26 @@ func transformCompile(args []string) ([]string, error) {
 		if err != nil {
 			return nil, err
 		}
+		// It is possible to end up in an edge case where two instances of the
+		// same package have different Action IDs, but their obfuscation and
+		// builds produce exactly the same results.
+		// In such an edge case, Go's build cache is smart enough for the second
+		// instance to reuse the first's build artifact.
+		// However, garble's caching via garbleExportFile is not as smart,
+		// as we base the location of these files purely based on Action IDs.
+		// Thus, the incremental build can fail to find garble's cached file.
+		// To sidestep this bug entirely, ensure that different action IDs never
+		// produce the same cached output when building with garble.
+		// Note that this edge case tends to happen when a -seed is provided,
+		// as then a package's Action ID is not used as an obfuscation seed.
+		// TODO(mvdan): replace this workaround with an actual fix if we can.
+		// This workaround is presumably worse on the build cache,
+		// as we end up with extra near-duplicate cached artifacts.
+		if i == 0 {
+			src = append(src, fmt.Sprintf(
+				"\nvar garbleActionID = %q\n", hashToString(curPkg.GarbleActionID),
+			)...)
+		}
 
 		// Uncomment for some quick debugging. Do not delete.
 		// if curPkg.ToObfuscate {
@@ -1027,7 +1047,7 @@ func loadCachedOutputs() error {
 			return nil
 		}()
 		if err != nil {
-			return err
+			return fmt.Errorf("cannot load garble export file for %s: %w", path, err)
 		}
 		loaded++
 	}

shared.go

@@ -209,7 +209,10 @@ func appendListedPackages(packages []string, withDeps bool) error {
 		if err := dec.Decode(&pkg); err != nil {
 			return err
 		}
-		if pkg.Export != "" {
+		if cache.ListedPackages[pkg.ImportPath] != nil {
+			return fmt.Errorf("duplicate package: %q", pkg.ImportPath)
+		}
+		if pkg.BuildID != "" {
 			actionID := decodeHash(splitActionID(pkg.BuildID))
 			pkg.GarbleActionID = addGarbleToHash(actionID)
 		}

testdata/mod/gopkg.in_garbletest.v2_v2.999.0.txt

@@ -3,10 +3,22 @@ module gopkg.in/garbletest.v2@v2.999.0
 -- .mod --
 module gopkg.in/garbletest.v2
 
-go 1.15
+go 1.17
+
+require "rsc.io/sampler" v1.3.0
 -- .info --
 {"Version":"v2.999.0","Time":"2020-11-17T15:46:20Z"}
+-- go.mod --
+module gopkg.in/garbletest.v2
+
+go 1.17
+
+require "rsc.io/sampler" v1.3.0
 -- apic.go --
 package garbletest
 
+import "rsc.io/sampler/unchanged"
+
 func Test() { println("this is the dummy garbletest.v2 package") }
+
+var _ = unchanged.Foo

testdata/mod/rsc.io_sampler_v1.3.0.txt

@@ -200,3 +200,7 @@ func (t *text) find(prefs []language.Tag) string {
 	}
 	return s
 }
+-- unchanged/unchanged.go --
+package unchanged
+
+const Foo = 4

testdata/mod/rsc.io_sampler_v1.99.99.txt

@@ -138,3 +138,7 @@ func (t *text) find(prefs []language.Tag) string {
 	}
 	return s
 }
+-- unchanged/unchanged.go --
+package unchanged
+
+const Foo = 4

testdata/scripts/imports.txt

@@ -60,11 +60,11 @@ require (
 -- go.sum --
 golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:pvCbr/wm8HzDD3fVywevekufpn6tCGPY3spdHeZJEsw=
 golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
-gopkg.in/garbletest.v2 v2.999.0 h1:wiZfOKGiXX7DoYVgbNvnTaCjqElrpZQSvKg0HYouw/o=
-gopkg.in/garbletest.v2 v2.999.0/go.mod h1:MF1BPTBjmDdc9x86+9UMLL9pAH2eMFPHvltohOvlGEw=
+gopkg.in/garbletest.v2 v2.999.0 h1:XHlBQi3MAcJL2fjNiEPAPAilkzc7hAv4vyyjY5w+IUY=
+gopkg.in/garbletest.v2 v2.999.0/go.mod h1:MI9QqKJD8i8oL8mW/bR0qq19/VuezEdJbVvl2B8Pa40=
 rsc.io/quote v1.5.2 h1:3fEykkD9k7lYzXqCYrwGAf7iNhbk4yCjHmKBN9td4L0=
 rsc.io/quote v1.5.2/go.mod h1:LzX7hefJvL54yjefDEDHNONDjII0t9xZLPXsUe+TKr0=
-rsc.io/sampler v1.3.0 h1:HLGR/BgEtI3r0uymSP/nl2uPLsUnNJX8toRyhfpBTII=
+rsc.io/sampler v1.3.0 h1:+lXbM7nYGGOYhnMEiMtjCwcUfjn4sajeMm15HMT6SnU=
 rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
 rsc.io/testonly v1.0.0 h1:K/VWHdO+Jv7woUXG0GzVNx1czBXUt3Ib1deaMn+xk64=
 rsc.io/testonly v1.0.0/go.mod h1:OqmGbIFOcF+XrFReLOGZ6BhMM7uMBiQwZsyNmh74SzY=

testdata/scripts/seed-cache.txt

@@ -0,0 +1,74 @@
+# Regression test for a build cache edge case.
+# Both mod1 and mod2 build the same version of gopkg.in/garbletest.v2,
+# but with different versions of one of its deps, rsc.io/quote.
+# However, the change in quote's version does not actually change the build.
+# This results in mod2's build never recompiling garbletest.v2,
+# even though its cached build from mod1 has a different Action ID hash.
+# In the past, this threw off garble's extra cached gob files.
+
+env SEED1=OQg9kACEECQ
+env GOGARBLE=*
+
+# First, ensure that mod1's garbletest.v2 is in the cache.
+cd mod1
+garble -seed=${SEED1} build gopkg.in/garbletest.v2
+
+# We collect the Action IDs to ensure they're different.
+# This is one of the factors that confused garble.
+go list -trimpath -export -f '{{.BuildID}}' gopkg.in/garbletest.v2
+cp stdout ../buildid-mod1
+
+# Then, do the mod2 build, using the different-but-equal garbletest.v2.
+# Ensure that our workaround's inserted garbleActionID does not end up in the binary.
+cd ../mod2
+garble -seed=${SEED1} build
+! binsubstr mod2$exe 'garbleActionID'
+
+go list -trimpath -export -f '{{.BuildID}}' gopkg.in/garbletest.v2
+cp stdout ../buildid-mod2
+
+cd ..
+
+! bincmp buildid-mod1 buildid-mod2
+
+-- mod1/go.mod --
+module test/main/mod1
+
+go 1.17
+
+require gopkg.in/garbletest.v2 v2.999.0
+
+require rsc.io/sampler v1.3.0 // indirect
+-- mod1/go.sum --
+golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+gopkg.in/garbletest.v2 v2.999.0 h1:XHlBQi3MAcJL2fjNiEPAPAilkzc7hAv4vyyjY5w+IUY=
+gopkg.in/garbletest.v2 v2.999.0/go.mod h1:MI9QqKJD8i8oL8mW/bR0qq19/VuezEdJbVvl2B8Pa40=
+rsc.io/sampler v1.3.0 h1:+lXbM7nYGGOYhnMEiMtjCwcUfjn4sajeMm15HMT6SnU=
+rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
+-- mod1/pkg.go --
+package main
+
+import garbletest "gopkg.in/garbletest.v2"
+
+func main() { garbletest.Test() }
+-- mod2/go.mod --
+module test/main/mod2
+
+go 1.17
+
+require gopkg.in/garbletest.v2 v2.999.0
+
+require rsc.io/sampler v1.99.99 // indirect
+-- mod2/go.sum --
+golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+gopkg.in/garbletest.v2 v2.999.0 h1:XHlBQi3MAcJL2fjNiEPAPAilkzc7hAv4vyyjY5w+IUY=
+gopkg.in/garbletest.v2 v2.999.0/go.mod h1:MI9QqKJD8i8oL8mW/bR0qq19/VuezEdJbVvl2B8Pa40=
+rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
+rsc.io/sampler v1.99.99 h1:fz0uBgsEGkv94x3b3GDw3Tvhj6yint6lYdsQOnFXNuw=
+rsc.io/sampler v1.99.99/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
+-- mod2/pkg.go --
+package main
+
+import garbletest "gopkg.in/garbletest.v2"
+
+func main() { garbletest.Test() }

testdata/scripts/seed.txt

@@ -2,7 +2,11 @@ env GOGARBLE=test/main
 
 # Note that in this test we use "! bincmp" on plaintext output files,
 # as a workaround for "cmp" not supporting "! cmp".
+# TODO: now that obfuscation with -seed is deterministic,
+# can we just rely on the regular "cmp" with fixed output files?
 
+# TODO: consider setting these seeds globally,
+# so we can reuse them across tests and make better use of the shared build cache.
 env SEED1=OQg9kACEECQ
 env SEED2=NruiDmVz6/s
 
@@ -11,7 +15,7 @@ garble -seed=${SEED1} build
 exec ./main$exe
 cmp stderr main.stderr
 binsubstr main$exe 'teststring' 'imported var value'
-! binsubstr main$exe 'ImportedVar'
+! binsubstr main$exe 'ImportedVar' ${SEED1}
 
 [short] stop # the extra checks are relatively expensive
 
@@ -23,7 +27,7 @@ cp stderr importedpkg-seed-static-1
 cp main$exe main_seed1$exe
 rm main$exe
 garble -seed=${SEED1}= build -v
-#! stderr .
+! stderr .
 bincmp main$exe main_seed1$exe
 
 exec ./main$exe test/main/imported