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) <noreply@anthropic.com>
This commit is contained in:
Bjørn Erik Pedersen
2026-07-24 18:58:46 +02:00
parent 861ede6d10
commit 6bf15241a1
9 changed files with 137 additions and 30 deletions
+7 -4
View File
@@ -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
}
}
+11 -2
View File
@@ -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
}
@@ -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
}
+34 -5
View File
@@ -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,
+1 -4
View File
@@ -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 {
+7
View File
@@ -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.
}
+27
View File
@@ -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)
}
+41
View File
@@ -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|<p>Objects content.</p>")
b.EditFileAtomicReplaceAll("content/objects/_index.md", "Objects content.", "Objects content edited.").Build()
b.AssertFileContent("public/objects/index.html", "Objects|taxonomy|<p>Objects content edited.</p>")
b.AssertFileContent("public/objects/o1/index.html", "O1|term|")
}
+6 -12
View File
@@ -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