Skip to content

Commit 99dddcb

Browse files
regenvanwalbeek-wfregenvanwalbeek-wf
committed
Fix: Don't bust cache so aggressively when compiling test sources
1 parent 0b2280d commit 99dddcb

File tree

3 files changed

+63
-22
lines changed

3 files changed

+63
-22
lines changed

‎build/build.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,6 @@ func NewSession(options *Options) (*Session, error) {
464464
GOPATH: options.GOPATH,
465465
BuildTags: options.BuildTags,
466466
Minify: options.Minify,
467-
TestedPackage: options.TestedPackage,
468467
}
469468
s.Types = make(map[string]*types.Package)
470469
if options.Watch {
@@ -608,7 +607,7 @@ func (s *Session) BuildPackage(pkg *PackageData) (*compiler.Archive, error) {
608607
}
609608

610609
if !s.options.NoCache {
611-
archive := s.buildCache.LoadArchive(pkg.ImportPath)
610+
archive := s.buildCache.LoadArchive(pkg.ImportPath, s.options.TestedPackage)
612611
if archive != nil && !pkg.SrcModTime.After(archive.BuildTime) {
613612
if err := archive.RegisterTypes(s.Types); err != nil {
614613
panic(fmt.Errorf("Failed to load type information from %v: %w", archive, err))
@@ -649,7 +648,7 @@ func (s *Session) BuildPackage(pkg *PackageData) (*compiler.Archive, error) {
649648
fmt.Println(pkg.ImportPath)
650649
}
651650

652-
s.buildCache.StoreArchive(archive)
651+
s.buildCache.StoreArchive(archive, s.options.TestedPackage)
653652
s.UpToDateArchives[pkg.ImportPath] = archive
654653

655654
return archive, nil

‎build/cache/cache.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func Clear() error {
5858
// the cache. For example, any artifacts that were cached for a minified build
5959
// must not be reused for a non-minified build. GopherJS version change also
6060
// invalidates the cache. It is callers responsibility to ensure that artifacts
61-
// passed the the StoreArchive function were generated with the same build
61+
// passed to the StoreArchive function were generated with the same build
6262
// parameters as the cache is configured.
6363
//
6464
// There is no upper limit for the total cache size. It can be cleared
@@ -77,11 +77,6 @@ type BuildCache struct {
7777
GOPATH string
7878
BuildTags []string
7979
Minify bool
80-
// When building for tests, import path of the package being tested. The
81-
// package under test is built with *_test.go sources included, and since it
82-
// may be imported by other packages in the binary we can't reuse the "normal"
83-
// cache.
84-
TestedPackage string
8580
}
8681

8782
func (bc BuildCache) String() string {
@@ -90,11 +85,11 @@ func (bc BuildCache) String() string {
9085

9186
// StoreArchive compiled archive in the cache. Any error inside this method
9287
// will cause the cache not to be persisted.
93-
func (bc *BuildCache) StoreArchive(a *compiler.Archive) {
88+
func (bc *BuildCache) StoreArchive(a *compiler.Archive, testedPackage string) {
9489
if bc == nil {
9590
return // Caching is disabled.
9691
}
97-
path := cachedPath(bc.archiveKey(a.ImportPath))
92+
path := cachedPath(bc.archiveKey(a.ImportPath, testedPackage))
9893
if err := os.MkdirAll(filepath.Dir(path), 0750); err != nil {
9994
log.Warningf("Failed to create build cache directory: %v", err)
10095
return
@@ -125,11 +120,11 @@ func (bc *BuildCache) StoreArchive(a *compiler.Archive) {
125120
//
126121
// The returned archive would have been built with the same configuration as
127122
// the build cache was.
128-
func (bc *BuildCache) LoadArchive(importPath string) *compiler.Archive {
123+
func (bc *BuildCache) LoadArchive(importPath string, testedPackage string) *compiler.Archive {
129124
if bc == nil {
130125
return nil // Caching is disabled.
131126
}
132-
path := cachedPath(bc.archiveKey(importPath))
127+
path := cachedPath(bc.archiveKey(importPath, testedPackage))
133128
f, err := os.Open(path)
134129
if err != nil {
135130
if os.IsNotExist(err) {
@@ -156,6 +151,11 @@ func (bc *BuildCache) commonKey() string {
156151
}
157152

158153
// archiveKey returns a full cache key for a package's compiled archive.
159-
func (bc *BuildCache) archiveKey(importPath string) string {
160-
return path.Join("archive", bc.commonKey(), importPath)
154+
func (bc *BuildCache) archiveKey(importPath string, testedPackage string) string {
155+
// When building for tests, testedPackage is import path of the package being tested.
156+
// testedPackage is built with *_test.go sources included, and since it
157+
// may be imported by other packages in the binary we can't reuse the "normal"
158+
// cache.
159+
compiledWithTests := testedPackage == importPath
160+
return path.Join("archive", bc.commonKey(), importPath, fmt.Sprintf("compiledWithTests: %v", compiledWithTests))
161161
}

‎build/cache/cache_test.go

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,20 @@ func TestStore(t *testing.T) {
1616
}
1717

1818
bc := BuildCache{}
19-
if got := bc.LoadArchive(want.ImportPath); got != nil {
19+
if got := bc.LoadArchive(want.ImportPath, ""); got != nil {
2020
t.Errorf("Got: %s was found in the cache. Want: empty cache.", got.ImportPath)
2121
}
22-
bc.StoreArchive(want)
23-
got := bc.LoadArchive(want.ImportPath)
22+
bc.StoreArchive(want, "")
23+
got := bc.LoadArchive(want.ImportPath, "")
2424
if got == nil {
25-
t.Errorf("Got: %s wan not found in the cache. Want: archive is can be loaded after store.", want.ImportPath)
25+
t.Errorf("Got: %s was not found in the cache. Want: archive can be loaded after store.", want.ImportPath)
2626
}
2727
if diff := cmp.Diff(want, got); diff != "" {
2828
t.Errorf("Loaded archive is different from stored (-want,+got):\n%s", diff)
2929
}
3030

3131
// Make sure the package names are a part of the cache key.
32-
if got := bc.LoadArchive("fake/other"); got != nil {
32+
if got := bc.LoadArchive("fake/other", ""); got != nil {
3333
t.Errorf("Got: fake/other was found in cache: %#v. Want: nil for packages that weren't cached.", got)
3434
}
3535
}
@@ -61,15 +61,57 @@ func TestInvalidation(t *testing.T) {
6161

6262
for _, test := range tests {
6363
a := &compiler.Archive{ImportPath: "package/fake"}
64-
test.cache1.StoreArchive(a)
64+
test.cache1.StoreArchive(a, "")
6565

66-
if got := test.cache2.LoadArchive(a.ImportPath); got != nil {
66+
if got := test.cache2.LoadArchive(a.ImportPath, ""); got != nil {
6767
t.Logf("-cache1,+cache2:\n%s", cmp.Diff(test.cache1, test.cache2))
6868
t.Errorf("Got: %v loaded from cache. Want: build parameter change invalidates cache.", got)
6969
}
7070
}
7171
}
7272

73+
func TestDoNotReuseCacheWhenLoadingPackageForTest(t *testing.T) {
74+
cacheForTest(t)
75+
76+
sharedPkgArchive := &compiler.Archive{
77+
ImportPath: "fake/sharedTestPkg",
78+
}
79+
80+
bc := BuildCache{}
81+
if got := bc.LoadArchive(sharedPkgArchive.ImportPath, "fake/pkg1"); got != nil {
82+
t.Errorf("Got: %s was found in the cache. Want: empty cache.", got.ImportPath)
83+
}
84+
bc.StoreArchive(sharedPkgArchive, "fake/package_1")
85+
86+
// sharedPkgArchive has not been stored with *_test.go sources, so it cannot be reused
87+
if got := bc.LoadArchive(sharedPkgArchive.ImportPath, sharedPkgArchive.ImportPath); got != nil {
88+
t.Errorf("Got: %s was found in the cache. Want: empty cache.", got.ImportPath)
89+
}
90+
}
91+
92+
func TestDifferentSourcesCanShareSameArchive(t *testing.T) {
93+
cacheForTest(t)
94+
95+
sharedPkgArchive := &compiler.Archive{
96+
ImportPath: "fake/sharedTestPkg",
97+
}
98+
99+
bc := BuildCache{}
100+
if got := bc.LoadArchive(sharedPkgArchive.ImportPath, "fake/pkg1"); got != nil {
101+
t.Errorf("Got: %s was found in the cache. Want: empty cache.", got.ImportPath)
102+
}
103+
bc.StoreArchive(sharedPkgArchive, "fake/package_1")
104+
105+
// sharedPkgArchive has been stored without *_test.go sources, so it can be reused
106+
if got := bc.LoadArchive(sharedPkgArchive.ImportPath, "fake/pkg2"); got == nil {
107+
t.Errorf("Got: %s was not found in the cache. Want: archive can be loaded after store.", sharedPkgArchive.ImportPath)
108+
}
109+
110+
if got := bc.LoadArchive(sharedPkgArchive.ImportPath, ""); got == nil {
111+
t.Errorf("Got: %s was not found in the cache. Want: archive can be loaded after store.", sharedPkgArchive.ImportPath)
112+
}
113+
}
114+
73115
func cacheForTest(t *testing.T) {
74116
t.Helper()
75117
originalRoot := cacheRoot

0 commit comments

Comments
 (0)