diff --git a/main.go b/main.go index c39b189..197ef45 100644 --- a/main.go +++ b/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++ } diff --git a/shared.go b/shared.go index d95ac65..c770f27 100644 --- a/shared.go +++ b/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) } diff --git a/testdata/mod/gopkg.in_garbletest.v2_v2.999.0.txt b/testdata/mod/gopkg.in_garbletest.v2_v2.999.0.txt index 308e50a..41f4a1e 100644 --- a/testdata/mod/gopkg.in_garbletest.v2_v2.999.0.txt +++ b/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 diff --git a/testdata/mod/rsc.io_sampler_v1.3.0.txt b/testdata/mod/rsc.io_sampler_v1.3.0.txt index febe51f..5a316bb 100644 --- a/testdata/mod/rsc.io_sampler_v1.3.0.txt +++ b/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 diff --git a/testdata/mod/rsc.io_sampler_v1.99.99.txt b/testdata/mod/rsc.io_sampler_v1.99.99.txt index e89cacc..e4dbba0 100644 --- a/testdata/mod/rsc.io_sampler_v1.99.99.txt +++ b/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 diff --git a/testdata/scripts/imports.txt b/testdata/scripts/imports.txt index 4d83b75..246ef84 100644 --- a/testdata/scripts/imports.txt +++ b/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= diff --git a/testdata/scripts/seed-cache.txt b/testdata/scripts/seed-cache.txt new file mode 100644 index 0000000..0cb945a --- /dev/null +++ b/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() } diff --git a/testdata/scripts/seed.txt b/testdata/scripts/seed.txt index 21b1480..dd53e3e 100644 --- a/testdata/scripts/seed.txt +++ b/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