From 6bf15241a129ef5293fd9a004b40a7746c7ea21e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Fri, 24 Jul 2026 18:58:46 +0200 Subject: [PATCH] Fix panic on server atomic save edits on MacOS An atomic save (write temp file, rename into place) unlinks the inode the watcher holds, so kqueue reports Remove for a file that's still on disk. That took the delete branch and wiped the entire taxonomy subtree; the following assemble then panicked in createMissingTaxonomies, where the shifting tree.Get hit a not yet assembled *pageMetaSource. Treat Remove of a path that still exists as an update, and use the non-shifting GetRaw when checking for the auto created taxonomy node. Fixes #15130 Co-Authored-By: Claude Opus 4.8 (1M context) --- hugolib/content_map_page_assembler.go | 11 +++-- hugolib/content_map_page_contentnode.go | 13 +++++- .../content_map_page_contentnodeshifter.go | 6 +-- hugolib/integrationtest_builder.go | 39 +++++++++++++++--- hugolib/page.go | 5 +-- hugolib/page__meta.go | 7 ++++ hugolib/page_test.go | 27 ++++++++++++ hugolib/rebuild_test.go | 41 +++++++++++++++++++ hugolib/site.go | 18 +++----- 9 files changed, 137 insertions(+), 30 deletions(-) diff --git a/hugolib/content_map_page_assembler.go b/hugolib/content_map_page_assembler.go index 120438518..54cff0fc3 100644 --- a/hugolib/content_map_page_assembler.go +++ b/hugolib/content_map_page_assembler.go @@ -362,7 +362,7 @@ func (a *allPagesAssembler) doCreatePages(prefix string, depth int) error { switch v := n.(type) { case contentNodeSeq, contentNodes: - return handleContentNodeSeq(contentNodeToSeq(v)) + return handleContentNodeSeq(cnh.contentNodeToSeq(v)) case *pageMetaSource: n2, err = handlePageMetaSource(v, nil, false) if err != nil { @@ -781,7 +781,7 @@ func (a *allPagesAssembler) doCreatePages(prefix string, depth int) error { if _, found := nodes[p.s.siteVector]; !found { var rs *resourceSource - match := cnh.findContentNodeForSiteVector(p.s.siteVector, duplicateResourceFiles, contentNodeToSeq(n)) + match := cnh.findContentNodeForSiteVector(p.s.siteVector, duplicateResourceFiles, cnh.contentNodeToSeq(n)) if match == nil { return true } @@ -1385,8 +1385,11 @@ func (a *allPagesAssembler) createMissingTaxonomies() error { for viewName, languages := range viewLanguages { key := viewName.pluralTreeKey if a.h.isRebuild() { - if v := tree.Get(key); v != nil { - // Already there. + // Note that we cannot use tree.Get here, as the tree at this point + // may hold not yet assembled *pageMetaSource nodes. + // We're only interested in whether the auto created (not file backed) + // taxonomy node is still there. + if n, found := tree.GetRaw(key); found && cnh.hasAutoContentNode(n) { continue } } diff --git a/hugolib/content_map_page_contentnode.go b/hugolib/content_map_page_contentnode.go index 2aeb896f8..d4934af9c 100644 --- a/hugolib/content_map_page_contentnode.go +++ b/hugolib/content_map_page_contentnode.go @@ -28,6 +28,7 @@ var _ contentNode = (*contentNodeSeq)(nil) var ( _ contentNode = (*resourceSource)(nil) + _ contentNodeContentWeightProvider = (*pageMetaSource)(nil) _ contentNodeContentWeightProvider = (*pageState)(nil) _ contentNodeForSites = (*pageState)(nil) _ contentNodePage = (*contentNodes)(nil) @@ -395,7 +396,15 @@ func contentNodeToContentNodesPage(n contentNode) (contentNodesMap, bool) { } } -func contentNodeToSeq(n contentNodeForEach) contentNodeSeq { +// hasAutoContentNode reports whether n holds at least one node that is not backed by a file. +func (h helperContentNode) hasAutoContentNode(n contentNodeForEach) bool { + return !n.forEeachContentNode(func(_ sitesmatrix.Vector, nn contentNode) bool { + wp, ok := nn.(contentNodeContentWeightProvider) + return ok && wp.contentWeight() > 0 + }) +} + +func (h helperContentNode) contentNodeToSeq(n contentNodeForEach) contentNodeSeq { if nn, ok := n.(contentNodeSeq); ok { return nn } @@ -406,7 +415,7 @@ func contentNodeToSeq(n contentNodeForEach) contentNodeSeq { } } -func contentNodeToSeq2(n contentNodeForEach) contentNodeSeq2 { +func (h helperContentNode) contentNodeToSeq2(n contentNodeForEach) contentNodeSeq2 { if nn, ok := n.(contentNodeSeq2); ok { return nn } diff --git a/hugolib/content_map_page_contentnodeshifter.go b/hugolib/content_map_page_contentnodeshifter.go index 1025e1153..04f632f75 100644 --- a/hugolib/content_map_page_contentnodeshifter.go +++ b/hugolib/content_map_page_contentnodeshifter.go @@ -105,7 +105,7 @@ func (s *contentNodeShifter) ForEeachInAllDimensions(n contentNode, f func(conte func (s *contentNodeShifter) ForEeachInDimension(n contentNode, vec sitesmatrix.Vector, d int, f func(contentNode) bool) { LOOP1: - for vec2, v := range contentNodeToSeq2(n) { + for vec2, v := range cnh.contentNodeToSeq2(n) { for i, v := range vec2 { if i != d && v != vec[i] { continue LOOP1 @@ -170,7 +170,7 @@ func (s *contentNodeShifter) Shift(n contentNode, siteVector sitesmatrix.Vector, } // The exact match is an auto page (not backed by a file). // Check if there's a file-backed complement that should take precedence. - if vvv := cnh.findContentNodeForSiteVector(siteVector, fallback, contentNodeToSeq(n)); vvv != nil { + if vvv := cnh.findContentNodeForSiteVector(siteVector, fallback, cnh.contentNodeToSeq(n)); vvv != nil { return vvv, true } return exact, true @@ -180,7 +180,7 @@ func (s *contentNodeShifter) Shift(n contentNode, siteVector sitesmatrix.Vector, return nil, false } - if vvv := cnh.findContentNodeForSiteVector(siteVector, fallback, contentNodeToSeq(n)); vvv != nil { + if vvv := cnh.findContentNodeForSiteVector(siteVector, fallback, cnh.contentNodeToSeq(n)); vvv != nil { return vvv, true } diff --git a/hugolib/integrationtest_builder.go b/hugolib/integrationtest_builder.go index 9db0c0ef9..6898e825b 100644 --- a/hugolib/integrationtest_builder.go +++ b/hugolib/integrationtest_builder.go @@ -231,11 +231,12 @@ type IntegrationTestBuilder struct { Cfg IntegrationTestConfig - changedFiles []string - createdFiles []string - removedFiles []string - renamedFiles []string - renamedDirs []string + changedFiles []string + createdFiles []string + removedFiles []string + renamedFiles []string + atomicSavedFiles []string + renamedDirs []string buildCount int GCCount int @@ -748,6 +749,14 @@ func (s *IntegrationTestBuilder) EditFileReplaceAll(filename, old, new string) * }) } +// EditFileAtomicReplaceAll edits a file and simulates an editor atomic save by +// emitting a fsnotify.Remove event for an existing path (see changeEvents). +func (s *IntegrationTestBuilder) EditFileAtomicReplaceAll(filename, old, new string) *IntegrationTestBuilder { + return s.EditFileAtomicReplaceFunc(filename, func(s string) string { + return strings.ReplaceAll(s, old, new) + }) +} + func (s *IntegrationTestBuilder) EditFileReplaceFunc(filename string, replacementFunc func(s string) string) *IntegrationTestBuilder { absFilename := s.absFilename(filename) b, err := afero.ReadFile(s.fs.Source, absFilename) @@ -758,6 +767,16 @@ func (s *IntegrationTestBuilder) EditFileReplaceFunc(filename string, replacemen return s } +func (s *IntegrationTestBuilder) EditFileAtomicReplaceFunc(filename string, replacementFunc func(s string) string) *IntegrationTestBuilder { + absFilename := s.absFilename(filename) + b, err := afero.ReadFile(s.fs.Source, absFilename) + s.Assert(err, qt.IsNil) + s.atomicSavedFiles = append(s.atomicSavedFiles, absFilename) + oldContent := string(b) + s.writeSource(absFilename, replacementFunc(oldContent)) + return s +} + func (s *IntegrationTestBuilder) EditFileAppend(filename, contnt string) *IntegrationTestBuilder { absFilename := s.absFilename(filename) b, err := afero.ReadFile(s.fs.Source, absFilename) @@ -1043,6 +1062,7 @@ func (s *IntegrationTestBuilder) reset() { s.createdFiles = nil s.removedFiles = nil s.renamedFiles = nil + s.atomicSavedFiles = nil } func (s *IntegrationTestBuilder) build(cfg BuildCfg) error { @@ -1091,6 +1111,15 @@ func (s *IntegrationTestBuilder) changeEvents() []fsnotify.Event { }) } + for _, v := range s.atomicSavedFiles { + events = append(events, fsnotify.Event{ + Name: v, + // The watcher was watching the inode that got replaced by the rename, + // so we get a Remove for a file that's still on disk. + Op: fsnotify.Remove, + }) + } + for _, v := range s.renamedDirs { events = append(events, fsnotify.Event{ Name: v, diff --git a/hugolib/page.go b/hugolib/page.go index 5969670e5..46c7e5c07 100644 --- a/hugolib/page.go +++ b/hugolib/page.go @@ -225,10 +225,7 @@ func (ps *pageState) forEeachContentNode(f func(v sitesmatrix.Vector, n contentN } func (ps *pageState) contentWeight() int { - if ps.m.f == nil { - return 0 - } - return ps.m.f.FileInfo().Meta().Weight + return ps.m.pageMetaSource.contentWeight() } func (ps *pageState) nodeSourceEntryID() any { diff --git a/hugolib/page__meta.go b/hugolib/page__meta.go index 93f85324b..c7141f979 100644 --- a/hugolib/page__meta.go +++ b/hugolib/page__meta.go @@ -98,6 +98,13 @@ func (m *pageMetaSource) String() string { return fmt.Sprintf("pageMetaSource(%s)", m.pathInfo) } +func (m *pageMetaSource) contentWeight() int { + if m.f == nil { + return 0 + } + return m.f.FileInfo().Meta().Weight +} + func (m *pageMetaSource) nodeCategoryPage() { // Marker method. } diff --git a/hugolib/page_test.go b/hugolib/page_test.go index e4b395331..cad453d5b 100644 --- a/hugolib/page_test.go +++ b/hugolib/page_test.go @@ -2341,3 +2341,30 @@ layouts/s1/p3/_views/a.html b.AssertFileContent("public/s1/p2/index.html", "p2: layouts/s1/_views/a.html") // fails b.AssertFileContent("public/s1/p3/index.html", "p3: layouts/s1/p3/_views/a.html") // fails } + +func TestPageConttentWeight(t *testing.T) { + files := ` +-- hugo.toml -- +-- content/mysection/page1.md -- +-- content/myothersection/page2.md -- +-- content/myothersection/_index.md -- +-- layouts/all.html -- +All. +` + b := Test(t, files) + + check := func(p page.Page, ok bool) { + cw := p.(contentNodeContentWeightProvider).contentWeight() + b.Assert(ok, qt.Equals, cw > 0) + } + + s := b.H.Sites[0] + for _, p := range s.RegularPages() { + check(p, true) + } + check(s.home, false) + mysection, _ := s.GetPage("mysection") + check(mysection, false) + myothersection, _ := s.GetPage("myothersection") // backed by a content file. + check(myothersection, true) +} diff --git a/hugolib/rebuild_test.go b/hugolib/rebuild_test.go index f72bfaa31..b6d716c37 100644 --- a/hugolib/rebuild_test.go +++ b/hugolib/rebuild_test.go @@ -897,6 +897,17 @@ func TestRebuildEditSectionRemoveDate(t *testing.T) { b.AssertFileContent("public/mysection/index.html", "all. My Section|Param: |Lastmod: 2021-02-01|") } +// Atomic save (write temp file, then rename into place) of a section's branch bundle. +// See issue 15130. +func TestRebuildEditAtomicSection(t *testing.T) { + t.Parallel() + b := TestRunning(t, rebuildBasicFiles) + b.AssertFileContent("public/mysection/index.html", "My Section") + b.EditFileAtomicReplaceAll("content/mysection/_index.md", "My Section", "My Changed Section").Build() + b.AssertRenderCountPage(5) + b.AssertFileContent("public/mysection/index.html", "My Changed Section") +} + func TestRebuildVariations(t *testing.T) { // t.Parallel() not supported, see https://github.com/fortytw2/leaktest/issues/4 // This leaktest seems to be a little bit shaky on Travis. @@ -2170,3 +2181,33 @@ Foo. b.EditFileReplaceAll("layouts/tags/list.html", "Foo", "Bar").Build() b.AssertFileContent("public/tags/index.html", "Bar.") } + +// Atomic save (write temp file, then rename into place) of a content backed taxonomy root. +// See issue 15130. +func TestRebuildEditAtomicTaxonomyRoot(t *testing.T) { + t.Parallel() + files := ` +-- hugo.toml -- +baseURL = "https://example.com" +disableLiveReload = true +[taxonomies] +object = "objects" +-- content/objects/_index.md -- +--- +title: "Objects" +--- +Objects content. +-- content/p1.md -- +--- +title: "P1" +objects: ["o1"] +--- +-- layouts/all.html -- +{{ .Title }}|{{ .Kind }}|{{ .Content }} +` + b := TestRunning(t, files) + b.AssertFileContent("public/objects/index.html", "Objects|taxonomy|

Objects content.

") + b.EditFileAtomicReplaceAll("content/objects/_index.md", "Objects content.", "Objects content edited.").Build() + b.AssertFileContent("public/objects/index.html", "Objects|taxonomy|

Objects content edited.

") + b.AssertFileContent("public/objects/o1/index.html", "O1|term|") +} diff --git a/hugolib/site.go b/hugolib/site.go index 409a3e30d..c23b756d8 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -1257,20 +1257,14 @@ func (h *HugoSites) fileEventsApplyInfo(events []fsnotify.Event) []fileEventInfo removed := false added := false - if ev.Op&fsnotify.Remove == fsnotify.Remove { - removed = true - } - fi, statErr := h.Fs.Source.Stat(ev.Name) - // Some editors (Vim) sometimes issue only a Rename operation when writing an existing file - // Sometimes a rename operation means that file has been renamed other times it means - // it's been updated. - if ev.Op.Has(fsnotify.Rename) { - // If the file is still on disk, it's only been updated, if it's not, it's been moved - if statErr != nil { - removed = true - } + // Some editors (Vim) sometimes issue only a Rename operation when writing an existing file, + // and an atomic save (write temp file, then rename it into place) makes the watcher + // report the replaced file as removed (kqueue/macOS). + // So, if the file is still on disk, it's only been updated, if it's not, it's gone. + if ev.Op.Has(fsnotify.Remove) || ev.Op.Has(fsnotify.Rename) { + removed = statErr != nil } if ev.Op.Has(fsnotify.Create) { added = true