Skip to content

Commit

Permalink
Fix some server rebuild issues for non-HTML custom output formats
Browse files Browse the repository at this point in the history
The failing test case here is

* A custom search output format defined on the home page, marked as `noAlternative` and not `permalinkable`
* In fast render mode, when making a change to a data source for that search output format, the JSON file was not refreshed.

There are variants of the above, but the gist of it is:

* The change set was correctly determined, but since the search JSON file was not in the recently visited browser stack, we skipped rendering it.

Running with `hugo server --disableFastRender` would be a workaround for the above.

This commit fixes this by:

* Adding a check for the HTTP request header `Sec-Fetch-Mode = navigation` to the condition for if we should track server request as a user navigation (and not e.g. a HTTP request for a linked CSS stylesheet).
* Making sure that we compare against the real relative URL for non-permalinkable output formats.

Fixes #13014
  • Loading branch information
bep committed Jan 24, 2025
1 parent a563783 commit 44fbcd5
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 42 deletions.
6 changes: 3 additions & 3 deletions commands/hugobuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ type hugoBuilder struct {

// Currently only set when in "fast render mode".
changeDetector *fileChangeDetector
visitedURLs *types.EvictingStringQueue
visitedURLs *types.EvictingQueue[string]

fullRebuildSem *semaphore.Weighted
debounce func(f func())
Expand Down Expand Up @@ -1103,7 +1103,7 @@ func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) (err error) {
if err != nil {
return
}
err = h.Build(hugolib.BuildCfg{NoBuildLock: true, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()}, events...)
err = h.Build(hugolib.BuildCfg{NoBuildLock: true, RecentlyTouched: c.visitedURLs, ErrRecovery: c.errState.wasErr()}, events...)
return
}

Expand All @@ -1119,7 +1119,7 @@ func (c *hugoBuilder) rebuildSitesForChanges(ids []identity.Identity) (err error
}
whatChanged := &hugolib.WhatChanged{}
whatChanged.Add(ids...)
err = h.Build(hugolib.BuildCfg{NoBuildLock: true, WhatChanged: whatChanged, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()})
err = h.Build(hugolib.BuildCfg{NoBuildLock: true, WhatChanged: whatChanged, RecentlyTouched: c.visitedURLs, ErrRecovery: c.errState.wasErr()})

return
}
Expand Down
12 changes: 8 additions & 4 deletions commands/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const (
)

func newHugoBuilder(r *rootCommand, s *serverCommand, onConfigLoaded ...func(reloaded bool) error) *hugoBuilder {
var visitedURLs *types.EvictingStringQueue
var visitedURLs *types.EvictingQueue[string]
if s != nil && !s.disableFastRender {
visitedURLs = types.NewEvictingStringQueue(20)

Check failure on line 90 in commands/server.go

View workflow job for this annotation

GitHub Actions / test (1.22.x, ubuntu-latest)

cannot infer T (/home/runner/work/hugo/hugo/common/types/evictingqueue.go:33:29) (compile)

Check failure on line 90 in commands/server.go

View workflow job for this annotation

GitHub Actions / test (1.22.x, windows-latest)

cannot infer T (D:/a/hugo/hugo/common/types/evictingqueue.go:33:29)

Check failure on line 90 in commands/server.go

View workflow job for this annotation

GitHub Actions / test (1.23.x, windows-latest)

in call to types.NewEvictingStringQueue, cannot infer T (D:/a/hugo/hugo/common/types/evictingqueue.go:33:29)
}
Expand Down Expand Up @@ -364,7 +364,11 @@ func (f *fileServer) createEndpoint(i int) (*http.ServeMux, net.Listener, string
}

if f.c.fastRenderMode && f.c.errState.buildErr() == nil {
if strings.HasSuffix(requestURI, "/") || strings.HasSuffix(requestURI, "html") || strings.HasSuffix(requestURI, "htm") {
// Sec-Fetch-Dest = document
// Sec-Fetch-Mode should be sent by all recent browser versions, see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Mode#navigate
// Fall back to the file extension if not set.
// The main take here is that we don't want to have CSS/JS files etc. partake in this logic.
if r.Header.Get("Sec-Fetch-Mode") == "navigate" || strings.HasSuffix(requestURI, "/") || strings.HasSuffix(requestURI, "html") || strings.HasSuffix(requestURI, "htm") {
if !f.c.visitedURLs.Contains(requestURI) {
// If not already on stack, re-render that single page.
if err := f.c.partialReRender(requestURI); err != nil {
Expand Down Expand Up @@ -838,7 +842,7 @@ func (c *serverCommand) partialReRender(urls ...string) (err error) {
defer func() {
c.errState.setWasErr(false)
}()
visited := types.NewEvictingStringQueue(len(urls))
visited := types.NewEvictingStringQueue[string](len(urls))
for _, url := range urls {
visited.Add(url)
}
Expand All @@ -850,7 +854,7 @@ func (c *serverCommand) partialReRender(urls ...string) (err error) {
}

// Note: We do not set NoBuildLock as the file lock is not acquired at this stage.
err = h.Build(hugolib.BuildCfg{NoBuildLock: false, RecentlyVisited: visited, PartialReRender: true, ErrRecovery: c.errState.wasErr()})
err = h.Build(hugolib.BuildCfg{NoBuildLock: false, RecentlyTouched: visited, PartialReRender: true, ErrRecovery: c.errState.wasErr()})

return
}
Expand Down
35 changes: 19 additions & 16 deletions common/types/evictingqueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,24 @@ import (
"sync"
)

// EvictingStringQueue is a queue which automatically evicts elements from the head of
// EvictingQueue is a queue which automatically evicts elements from the head of
// the queue when attempting to add new elements onto the queue and it is full.
// This queue orders elements LIFO (last-in-first-out). It throws away duplicates.
// Note: This queue currently does not contain any remove (poll etc.) methods.
type EvictingStringQueue struct {
type EvictingQueue[T comparable] struct {
size int
vals []string
set map[string]bool
vals []T
set map[T]bool
mu sync.Mutex
zero T
}

// NewEvictingStringQueue creates a new queue with the given size.
func NewEvictingStringQueue(size int) *EvictingStringQueue {
return &EvictingStringQueue{size: size, set: make(map[string]bool)}
func NewEvictingStringQueue[T comparable](size int) *EvictingQueue[T] {
return &EvictingQueue[T]{size: size, set: make(map[T]bool)}
}

// Add adds a new string to the tail of the queue if it's not already there.
func (q *EvictingStringQueue) Add(v string) *EvictingStringQueue {
func (q *EvictingQueue[T]) Add(v T) *EvictingQueue[T] {
q.mu.Lock()
if q.set[v] {
q.mu.Unlock()
Expand All @@ -54,7 +54,7 @@ func (q *EvictingStringQueue) Add(v string) *EvictingStringQueue {
return q
}

func (q *EvictingStringQueue) Len() int {
func (q *EvictingQueue[T]) Len() int {
if q == nil {
return 0
}
Expand All @@ -64,7 +64,7 @@ func (q *EvictingStringQueue) Len() int {
}

// Contains returns whether the queue contains v.
func (q *EvictingStringQueue) Contains(v string) bool {
func (q *EvictingQueue[T]) Contains(v T) bool {
if q == nil {
return false
}
Expand All @@ -74,22 +74,25 @@ func (q *EvictingStringQueue) Contains(v string) bool {
}

// Peek looks at the last element added to the queue.
func (q *EvictingStringQueue) Peek() string {
func (q *EvictingQueue[T]) Peek() T {
q.mu.Lock()
l := len(q.vals)
if l == 0 {
q.mu.Unlock()
return ""
return q.zero
}
elem := q.vals[l-1]
q.mu.Unlock()
return elem
}

// PeekAll looks at all the elements in the queue, with the newest first.
func (q *EvictingStringQueue) PeekAll() []string {
func (q *EvictingQueue[T]) PeekAll() []T {
if q == nil {
return nil
}
q.mu.Lock()
vals := make([]string, len(q.vals))
vals := make([]T, len(q.vals))
copy(vals, q.vals)
q.mu.Unlock()
for i, j := 0, len(vals)-1; i < j; i, j = i+1, j-1 {
Expand All @@ -99,9 +102,9 @@ func (q *EvictingStringQueue) PeekAll() []string {
}

// PeekAllSet returns PeekAll as a set.
func (q *EvictingStringQueue) PeekAllSet() map[string]bool {
func (q *EvictingQueue[T]) PeekAllSet() map[T]bool {
all := q.PeekAll()
set := make(map[string]bool)
set := make(map[T]bool)
for _, v := range all {
set[v] = true
}
Expand Down
26 changes: 17 additions & 9 deletions hugolib/hugo_sites.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,8 @@ type BuildCfg struct {
// Set in server mode when the last build failed for some reason.
ErrRecovery bool

// Recently visited URLs. This is used for partial re-rendering.
RecentlyVisited *types.EvictingStringQueue
// Recently visited or touched URLs. This is used for partial re-rendering.
RecentlyTouched *types.EvictingQueue[string]

// Can be set to build only with a sub set of the content source.
ContentInclusionFilter *glob.FilenameFilter
Expand All @@ -428,8 +428,14 @@ type BuildCfg struct {
testCounters *buildCounters
}

// TouchedURL represents a URL that has been recently visited or touched.
type TouchedURL struct {
URL string
Reason string // Used for logging.
}

// shouldRender returns whether this output format should be rendered or not.
func (cfg *BuildCfg) shouldRender(p *pageState) bool {
func (cfg *BuildCfg) shouldRender(infol logg.LevelLogger, p *pageState) bool {
if p.skipRender() {
return false
}
Expand Down Expand Up @@ -457,18 +463,20 @@ func (cfg *BuildCfg) shouldRender(p *pageState) bool {
return false
}

if p.outputFormat().IsHTML {
// This is fast render mode and the output format is HTML,
// rerender if this page is one of the recently visited.
return cfg.RecentlyVisited.Contains(p.RelPermalink())
if relURL := p.getRelURL(); relURL != "" {
if cfg.RecentlyTouched.Contains(relURL) {
infol.Logf("render recently touched %s (%s)", relURL, p.outputFormat().Name)
return true
}
}

// In fast render mode, we want to avoid re-rendering the sitemaps etc. and
// other big listings whenever we e.g. change a content file,
// but we want partial renders of the recently visited pages to also include
// but we want partial renders of the recently touched pages to also include
// alternative formats of the same HTML page (e.g. RSS, JSON).
for _, po := range p.pageOutputs {
if po.render && po.f.IsHTML && cfg.RecentlyVisited.Contains(po.RelPermalink()) {
if po.render && po.f.IsHTML && cfg.RecentlyTouched.Contains(po.getRelURL()) {
infol.Logf("render recently touched %s, %s version of %s", po.getRelURL(), po.f.Name, p.outputFormat().Name)
return true
}
}
Expand Down
6 changes: 3 additions & 3 deletions hugolib/hugo_sites_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ func (h *HugoSites) render(l logg.LevelLogger, config *BuildCfg) error {
loggers.TimeTrackf(l, start, h.buildCounters.loggFields(), "")
}()

siteRenderContext := &siteRenderContext{cfg: config, multihost: h.Configs.IsMultihost}
siteRenderContext := &siteRenderContext{cfg: config, infol: l, multihost: h.Configs.IsMultihost}

renderErr := func(err error) error {
if err == nil {
Expand Down Expand Up @@ -902,12 +902,12 @@ func (h *HugoSites) processPartialFileEvents(ctx context.Context, l logg.LevelLo

needsPagesAssemble = true

if config.RecentlyVisited != nil {
if config.RecentlyTouched != nil {
// Fast render mode. Adding them to the visited queue
// avoids rerendering them on navigation.
for _, id := range changes {
if p, ok := id.(page.Page); ok {
config.RecentlyVisited.Add(p.RelPermalink())
config.RecentlyTouched.Add(p.RelPermalink())
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions hugolib/integrationtest_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,11 +487,11 @@ func (s *IntegrationTestBuilder) BuildPartialE(urls ...string) (*IntegrationTest
if !s.Cfg.Running {
panic("BuildPartial can only be used in server mode")
}
visited := types.NewEvictingStringQueue(len(urls))
visited := types.NewEvictingStringQueue[string](len(urls))
for _, url := range urls {
visited.Add(url)
}
buildCfg := BuildCfg{RecentlyVisited: visited, PartialReRender: true}
buildCfg := BuildCfg{RecentlyTouched: visited, PartialReRender: true}
return s, s.build(buildCfg)
}

Expand Down
8 changes: 7 additions & 1 deletion hugolib/page__paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,17 @@ func newPagePaths(ps *pageState) (pagePaths, error) {
// Use the main format for permalinks, usually HTML.
permalinksIndex := 0
if f.Permalinkable {
// Unless it's permalinkable
// Unless it's permalinkable.
permalinksIndex = i
}

relURL := relPermalink
if relURL == "" {
relURL = paths.RelPermalink(s.PathSpec)
}

targets[f.Name] = targetPathsHolder{
relURL: relURL,
paths: paths,
OutputFormat: pageOutputFormats[permalinksIndex],
}
Expand Down
10 changes: 9 additions & 1 deletion hugolib/page__per_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,13 +469,21 @@ type pagePerOutputProviders interface {

type targetPather interface {
targetPaths() page.TargetPaths
getRelURL() string
}

type targetPathsHolder struct {
paths page.TargetPaths
// relURL is usually the same as OutputFormat.RelPermalink, but can be different
// for non-permalinkable output formats. These shares RelPermalink with the main (first) output format.
relURL string
paths page.TargetPaths
page.OutputFormat
}

func (t targetPathsHolder) getRelURL() string {
return t.relURL
}

func (t targetPathsHolder) targetPaths() page.TargetPaths {
return t.paths
}
Expand Down
47 changes: 45 additions & 2 deletions hugolib/rebuild_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,8 @@ RegularPages: {{ range .Site.RegularPages }}{{ .RelPermalink }}|{{ end }}$
}

func TestRebuildRenameDirectoryWithBranchBundleFastRender(t *testing.T) {
recentlyVisited := types.NewEvictingStringQueue(10).Add("/a/b/c/")
b := TestRunning(t, rebuildFilesSimple, func(cfg *IntegrationTestConfig) { cfg.BuildCfg = BuildCfg{RecentlyVisited: recentlyVisited} })
recentlyVisited := types.NewEvictingStringQueue[string](10).Add("/a/b/c/")
b := TestRunning(t, rebuildFilesSimple, func(cfg *IntegrationTestConfig) { cfg.BuildCfg = BuildCfg{RecentlyTouched: recentlyVisited} })
b.RenameDir("content/mysection", "content/mysectionrenamed").Build()
b.AssertFileContent("public/mysectionrenamed/index.html", "My Section")
b.AssertFileContent("public/mysectionrenamed/mysectionbundle/index.html", "My Section Bundle")
Expand Down Expand Up @@ -1181,6 +1181,49 @@ Content: {{ .Content }}
b.AssertFileContent("public/index.html", "Content: <p>Home</p>")
}

// Issue #13014.
func TestRebuildEditNotPermalinkableCustomOutputFormatTemplateInFastRenderMode(t *testing.T) {
t.Parallel()

files := `
-- hugo.toml --
baseURL = "https://example.com/docs/"
disableLiveReload = true
[internal]
fastRenderMode = true
disableKinds = ["taxonomy", "term", "sitemap", "robotsTXT", "404"]
[outputFormats]
[outputFormats.SearchIndex]
baseName = 'Search'
isPlainText = true
mediaType = 'text/plain'
noAlternative = true
permalinkable = false
[outputs]
home = ['HTML', 'SearchIndex']
-- content/_index.md --
---
title: "Home"
---
Home.
-- layouts/index.html --
Home.
-- layouts/_default/index.searchindex.txt --
Text. {{ .Title }}|{{ .RelPermalink }}|
`
b := TestRunning(t, files, TestOptInfo())

b.AssertFileContent("public/search.txt", "Text.")

b.EditFileReplaceAll("layouts/_default/index.searchindex.txt", "Text.", "Text Edited.").Build()

b.BuildPartial("/docs/search.txt")

b.AssertFileContent("public/search.txt", "Text Edited.")
}

func TestRebuildVariationsAssetsJSImport(t *testing.T) {
t.Parallel()
files := `
Expand Down
5 changes: 4 additions & 1 deletion hugolib/site_render.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strings"
"sync"

"github.com/bep/logg"
"github.com/gohugoio/hugo/common/herrors"
"github.com/gohugoio/hugo/hugolib/doctree"

Expand All @@ -33,6 +34,8 @@ import (
type siteRenderContext struct {
cfg *BuildCfg

infol logg.LevelLogger

// languageIdx is the zero based index of the site.
languageIdx int

Expand Down Expand Up @@ -86,7 +89,7 @@ func (s *Site) renderPages(ctx *siteRenderContext) error {
Tree: s.pageMap.treePages,
Handle: func(key string, n contentNodeI, match doctree.DimensionFlag) (bool, error) {
if p, ok := n.(*pageState); ok {
if cfg.shouldRender(p) {
if cfg.shouldRender(ctx.infol, p) {
select {
case <-s.h.Done():
return true, nil
Expand Down

0 comments on commit 44fbcd5

Please sign in to comment.