From ec1c97e7e9d62ce5245135e0906fdedf14af0cae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Sat, 4 Mar 2023 11:09:03 +0100 Subject: [PATCH] Work around --gc failure on Windows <= 10 This applies two related fixes/improvements: * The --gc now keeps empty `_resources/_gen/images` etc folders, even if empty. This should have been the behaviour from the start. * Also, if removal of an empty dir on Windows fails with the "used by another process" error, just ignore it for now. Fixes #10781 --- cache/filecache/filecache_pruner.go | 22 +++++- cache/filecache/integration_test.go | 102 ++++++++++++++++++++++++++++ hugolib/integrationtest_builder.go | 8 +++ 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 cache/filecache/integration_test.go diff --git a/cache/filecache/filecache_pruner.go b/cache/filecache/filecache_pruner.go index 5734af199..41aec15d3 100644 --- a/cache/filecache/filecache_pruner.go +++ b/cache/filecache/filecache_pruner.go @@ -17,6 +17,8 @@ import ( "fmt" "io" "os" + "runtime" + "strings" "github.com/gohugoio/hugo/common/herrors" "github.com/gohugoio/hugo/hugofs" @@ -66,6 +68,7 @@ func (c *Cache) Prune(force bool) (int, error) { if info.IsDir() { f, err := c.Fs.Open(name) + if err != nil { // This cache dir may not exist. return nil @@ -74,7 +77,24 @@ func (c *Cache) Prune(force bool) (int, error) { _, err = f.Readdirnames(1) if err == io.EOF { // Empty dir. - err = c.Fs.Remove(name) + if name == "." { + // e.g. /_gen/images -- keep it even if empty. + err = nil + } else { + err = c.Fs.Remove(name) + if err != nil { + if runtime.GOOS == "windows" { + if strings.Contains(err.Error(), "used by another process") { + // See https://github.com/gohugoio/hugo/issues/10781 + // This is a known issue on Windows with Go 1.20. + // There's not much we can do about it. + // So just return nil. + err = nil + + } + } + } + } } if err != nil && !herrors.IsNotExist(err) { diff --git a/cache/filecache/integration_test.go b/cache/filecache/integration_test.go new file mode 100644 index 000000000..eaddf9da6 --- /dev/null +++ b/cache/filecache/integration_test.go @@ -0,0 +1,102 @@ +// Copyright 2023 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package filecache_test + +import ( + "path/filepath" + "runtime" + "testing" + "time" + + qt "github.com/frankban/quicktest" + "github.com/gohugoio/hugo/hugolib" +) + +// See issue #10781. That issue wouldn't have been triggered if we kept +// the empty root directories (e.g. _resources/gen/images). +// It's still an upstream Go issue that we also need to handle, but +// this is a test for the first part. +func TestPruneShouldPreserveEmptyCacheRoots(t *testing.T) { + files := ` +-- hugo.toml -- +baseURL = "https://example.com" +-- content/_index.md -- +--- +title: "Home" +--- + +` + + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{T: t, TxtarString: files, RunGC: true, NeedsOsFS: true}, + ).Build() + + _, err := b.H.BaseFs.ResourcesCache.Stat(filepath.Join("_gen", "images")) + + b.Assert(err, qt.IsNil) + +} + +func TestPruneImages(t *testing.T) { + files := ` +-- hugo.toml -- +baseURL = "https://example.com" +[caches] +[caches.images] +maxAge = "200ms" +dir = ":resourceDir/_gen" +-- content/_index.md -- +--- +title: "Home" +--- +-- assets/a/pixel.png -- +iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg== +-- layouts/index.html -- +{{ $img := resources.GetMatch "**.png" }} +{{ $img = $img.Resize "3x3" }} +{{ $img.RelPermalink }} + + + +` + + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{T: t, TxtarString: files, RunGC: true, NeedsOsFS: true}, + ).Build() + + b.Assert(b.GCCount, qt.Equals, 0) + + imagesCacheDir := filepath.Join("_gen", "images") + _, err := b.H.BaseFs.ResourcesCache.Stat(imagesCacheDir) + + b.Assert(err, qt.IsNil) + + // TODO(bep) we need a way to test full rebuilds. + // For now, just sleep a little so the cache elements expires. + time.Sleep(300 * time.Millisecond) + + b.RenameFile("assets/a/pixel.png", "assets/b/pixel2.png").Build() + b.Assert(b.GCCount, qt.Equals, 1) + // Build it again to GC the empty a dir. + b.Build() + if runtime.GOOS != "windows" { + // See issue #58860 -- this sometimes fails on Windows, + // but the empty directory will be removed on the next run. + _, err = b.H.BaseFs.ResourcesCache.Stat(filepath.Join(imagesCacheDir, "a")) + b.Assert(err, qt.Not(qt.IsNil)) + } + _, err = b.H.BaseFs.ResourcesCache.Stat(imagesCacheDir) + b.Assert(err, qt.IsNil) + +} diff --git a/hugolib/integrationtest_builder.go b/hugolib/integrationtest_builder.go index f0e3c504d..8bd458bc1 100644 --- a/hugolib/integrationtest_builder.go +++ b/hugolib/integrationtest_builder.go @@ -84,6 +84,7 @@ type IntegrationTestBuilder struct { renamedFiles []string buildCount int + GCCount int counters *testCounters logBuff lockingBuffer @@ -193,6 +194,9 @@ func (s *IntegrationTestBuilder) Build() *IntegrationTestBuilder { if s.Cfg.Verbose || err != nil { fmt.Println(s.logBuff.String()) } + if s.Cfg.RunGC { + s.GCCount, err = s.H.GC() + } s.Assert(err, qt.IsNil) return s } @@ -263,6 +267,7 @@ func (s *IntegrationTestBuilder) RenameFile(old, new string) *IntegrationTestBui absNewFilename := s.absFilename(new) s.renamedFiles = append(s.renamedFiles, absOldFilename) s.createdFiles = append(s.createdFiles, absNewFilename) + s.Assert(s.fs.Source.MkdirAll(filepath.Dir(absNewFilename), 0777), qt.IsNil) s.Assert(s.fs.Source.Rename(absOldFilename, absNewFilename), qt.IsNil) return s } @@ -488,6 +493,9 @@ type IntegrationTestConfig struct { // Whether it needs the real file system (e.g. for js.Build tests). NeedsOsFS bool + // Whether to run GC after each build. + RunGC bool + // Do not remove the temp dir after the test. PrintAndKeepTempDir bool