From cb6e27b32a1d09956027b8e45bae0c18c1593d5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Wed, 23 Oct 2024 19:26:13 +0200 Subject: [PATCH] hugolib/commands: Fix stuck server error issues Fixes #11378 --- commands/commandeer.go | 2 +- commands/hugobuilder.go | 58 +++++++++++-------- commands/server.go | 20 ++++--- hugolib/hugo_sites.go | 3 - hugolib/hugo_sites_build.go | 10 ---- hugolib/integrationtest_builder.go | 5 -- hugolib/page__new.go | 13 +++-- hugolib/site.go | 10 ---- .../server__error_recovery_edit_config.txt | 42 ++++++++++++++ .../server__error_recovery_edit_content.txt | 42 ++++++++++++++ tpl/template.go | 1 - tpl/tplimpl/template.go | 21 ++----- 12 files changed, 143 insertions(+), 84 deletions(-) create mode 100644 testscripts/commands/server__error_recovery_edit_config.txt create mode 100644 testscripts/commands/server__error_recovery_edit_content.txt diff --git a/commands/commandeer.go b/commands/commandeer.go index ad2adf3a2..69077ad73 100644 --- a/commands/commandeer.go +++ b/commands/commandeer.go @@ -507,7 +507,7 @@ func (r *rootCommand) createLogger(running bool) (loggers.Logger, error) { return loggers.New(optsLogger), nil } -func (r *rootCommand) Reset() { +func (r *rootCommand) resetLogs() { r.logger.Reset() loggers.Log().Reset() } diff --git a/commands/hugobuilder.go b/commands/hugobuilder.go index 42bf68a37..95129018f 100644 --- a/commands/hugobuilder.go +++ b/commands/hugobuilder.go @@ -27,7 +27,6 @@ import ( "sync/atomic" "time" - "github.com/bep/logg" "github.com/bep/simplecobra" "github.com/fsnotify/fsnotify" "github.com/gohugoio/hugo/common/herrors" @@ -136,10 +135,6 @@ func (e *hugoBuilderErrState) wasErr() bool { return e.waserr } -func (c *hugoBuilder) errCount() int { - return c.r.logger.LoggCount(logg.LevelError) + loggers.Log().LoggCount(logg.LevelError) -} - // getDirList provides NewWatcher() with a list of directories to watch for changes. func (c *hugoBuilder) getDirList() ([]string, error) { h, err := c.hugo() @@ -345,7 +340,6 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa for { select { case changes := <-c.r.changesFromBuild: - c.errState.setBuildErr(nil) unlock, err := h.LockBuild() if err != nil { c.r.logger.Errorln("Failed to acquire a build lock: %s", err) @@ -358,7 +352,7 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa } if c.s != nil && c.s.doLiveReload { doReload := c.changeDetector == nil || len(c.changeDetector.changed()) > 0 - doReload = doReload || c.showErrorInBrowser && c.errCount() > 0 + doReload = doReload || c.showErrorInBrowser && c.errState.buildErr() != nil if doReload { livereload.ForceRefresh() } @@ -372,7 +366,7 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa return } c.handleEvents(watcher, staticSyncer, evs, configSet) - if c.showErrorInBrowser && c.errCount() > 0 { + if c.showErrorInBrowser && c.errState.buildErr() != nil { // Need to reload browser to show the error livereload.ForceRefresh() } @@ -419,11 +413,17 @@ func (c *hugoBuilder) build() error { } func (c *hugoBuilder) buildSites(noBuildLock bool) (err error) { - h, err := c.hugo() + defer func() { + c.errState.setBuildErr(err) + }() + + var h *hugolib.HugoSites + h, err = c.hugo() if err != nil { - return err + return } - return h.Build(hugolib.BuildCfg{NoBuildLock: noBuildLock}) + err = h.Build(hugolib.BuildCfg{NoBuildLock: noBuildLock}) + return } func (c *hugoBuilder) copyStatic() (map[string]uint64, error) { @@ -619,6 +619,9 @@ func (c *hugoBuilder) fullRebuild(changeType string) { // Set the processing on pause until the state is recovered. c.errState.setPaused(true) c.handleBuildErr(err, "Failed to reload config") + if c.s.doLiveReload { + livereload.ForceRefresh() + } } else { c.errState.setPaused(false) } @@ -1081,37 +1084,44 @@ func (c *hugoBuilder) printChangeDetected(typ string) { c.r.logger.Println(htime.Now().Format(layout)) } -func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) error { +func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) (err error) { + defer func() { + c.errState.setBuildErr(err) + }() if err := c.errState.buildErr(); err != nil { ferrs := herrors.UnwrapFileErrorsWithErrorContext(err) for _, err := range ferrs { events = append(events, fsnotify.Event{Name: err.Position().Filename, Op: fsnotify.Write}) } } - c.errState.setBuildErr(nil) - h, err := c.hugo() + var h *hugolib.HugoSites + h, err = c.hugo() if err != nil { - return err + return } - - return h.Build(hugolib.BuildCfg{NoBuildLock: true, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()}, events...) + err = h.Build(hugolib.BuildCfg{NoBuildLock: true, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()}, events...) + return } -func (c *hugoBuilder) rebuildSitesForChanges(ids []identity.Identity) error { - c.errState.setBuildErr(nil) - h, err := c.hugo() +func (c *hugoBuilder) rebuildSitesForChanges(ids []identity.Identity) (err error) { + defer func() { + c.errState.setBuildErr(err) + }() + + var h *hugolib.HugoSites + h, err = c.hugo() if err != nil { - return err + return } whatChanged := &hugolib.WhatChanged{} whatChanged.Add(ids...) err = h.Build(hugolib.BuildCfg{NoBuildLock: true, WhatChanged: whatChanged, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()}) - c.errState.setBuildErr(err) - return err + + return } func (c *hugoBuilder) reloadConfig() error { - c.r.Reset() + c.r.resetLogs() c.r.configVersionID.Add(1) if err := c.withConfE(func(conf *commonConfig) error { diff --git a/commands/server.go b/commands/server.go index 84d4165f0..6b801b158 100644 --- a/commands/server.go +++ b/commands/server.go @@ -648,9 +648,8 @@ func (c *serverCommand) setServerInfoInConfig() error { } func (c *serverCommand) getErrorWithContext() any { - errCount := c.errCount() - - if errCount == 0 { + buildErr := c.errState.buildErr() + if buildErr == nil { return nil } @@ -659,7 +658,7 @@ func (c *serverCommand) getErrorWithContext() any { m["Error"] = cleanErrorLog(c.r.logger.Errors()) m["Version"] = hugo.BuildVersionString() - ferrors := herrors.UnwrapFileErrorsWithErrorContext(c.errState.buildErr()) + ferrors := herrors.UnwrapFileErrorsWithErrorContext(buildErr) m["Files"] = ferrors return m @@ -830,22 +829,25 @@ func (c *serverCommand) fixURL(baseURLFromConfig, baseURLFromFlag string, port i return u.String(), nil } -func (c *serverCommand) partialReRender(urls ...string) error { +func (c *serverCommand) partialReRender(urls ...string) (err error) { defer func() { c.errState.setWasErr(false) }() - c.errState.setBuildErr(nil) visited := types.NewEvictingStringQueue(len(urls)) for _, url := range urls { visited.Add(url) } - h, err := c.hugo() + var h *hugolib.HugoSites + h, err = c.hugo() if err != nil { - return err + return } + // Note: We do not set NoBuildLock as the file lock is not acquired at this stage. - return h.Build(hugolib.BuildCfg{NoBuildLock: false, RecentlyVisited: visited, PartialReRender: true, ErrRecovery: c.errState.wasErr()}) + err = h.Build(hugolib.BuildCfg{NoBuildLock: false, RecentlyVisited: visited, PartialReRender: true, ErrRecovery: c.errState.wasErr()}) + + return } func (c *serverCommand) serve() error { diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go index 659a772f2..a5186fd44 100644 --- a/hugolib/hugo_sites.go +++ b/hugolib/hugo_sites.go @@ -179,9 +179,6 @@ type hugoSitesInit struct { // Loads the data from all of the /data folders. data *lazy.Init - // Performs late initialization (before render) of the templates. - layouts *lazy.Init - // Loads the Git info and CODEOWNERS for all the pages if enabled. gitInfo *lazy.Init } diff --git a/hugolib/hugo_sites_build.go b/hugolib/hugo_sites_build.go index 65ce946e9..dd548be51 100644 --- a/hugolib/hugo_sites_build.go +++ b/hugolib/hugo_sites_build.go @@ -250,10 +250,6 @@ func (h *HugoSites) process(ctx context.Context, l logg.LevelLogger, config *Bui l = l.WithField("step", "process") defer loggers.TimeTrackf(l, time.Now(), nil, "") - if _, err := h.init.layouts.Do(ctx); err != nil { - return err - } - if len(events) > 0 { // This is a rebuild triggered from file events. return h.processPartialFileEvents(ctx, l, config, init, events) @@ -1067,8 +1063,6 @@ func (h *HugoSites) processPartialFileEvents(ctx context.Context, l logg.LevelLo } if tmplChanged || i18nChanged { - // TODO(bep) we should split this, but currently the loading of i18n and layout files are tied together. See #12048. - h.init.layouts.Reset() if err := loggers.TimeTrackfn(func() (logg.LevelLogger, error) { // TODO(bep) this could probably be optimized to somehow // only load the changed templates and its dependencies, but that is non-trivial. @@ -1141,10 +1135,6 @@ func (s *Site) handleContentAdapterChanges(bi pagesfromdata.BuildInfo, buildConf } func (h *HugoSites) processContentAdaptersOnRebuild(ctx context.Context, buildConfig *BuildCfg) error { - // Make sure the layouts are initialized. - if _, err := h.init.layouts.Do(context.Background()); err != nil { - return err - } g := rungroup.Run[*pagesfromdata.PagesFromTemplate](ctx, rungroup.Config[*pagesfromdata.PagesFromTemplate]{ NumWorkers: h.numWorkers, Handle: func(ctx context.Context, p *pagesfromdata.PagesFromTemplate) error { diff --git a/hugolib/integrationtest_builder.go b/hugolib/integrationtest_builder.go index b45defb42..b806ad7c1 100644 --- a/hugolib/integrationtest_builder.go +++ b/hugolib/integrationtest_builder.go @@ -246,11 +246,6 @@ func (s *IntegrationTestBuilder) AssertBuildCountGitInfo(count int) { s.Assert(s.H.init.gitInfo.InitCount(), qt.Equals, count) } -func (s *IntegrationTestBuilder) AssertBuildCountLayouts(count int) { - s.Helper() - s.Assert(s.H.init.layouts.InitCount(), qt.Equals, count) -} - func (s *IntegrationTestBuilder) AssertFileCount(dirname string, expected int) { s.Helper() fs := s.fs.WorkingDirReadOnly diff --git a/hugolib/page__new.go b/hugolib/page__new.go index b7d9b10f2..9a4972d07 100644 --- a/hugolib/page__new.go +++ b/hugolib/page__new.go @@ -34,6 +34,15 @@ import ( var pageIDCounter atomic.Uint64 func (h *HugoSites) newPage(m *pageMeta) (*pageState, *paths.Path, error) { + p, pth, err := h.doNewPage(m) + if err != nil { + // Make sure that any partially created page part is marked as stale. + m.MarkStale() + } + return p, pth, err +} + +func (h *HugoSites) doNewPage(m *pageMeta) (*pageState, *paths.Path, error) { m.Staler = &resources.AtomicStaler{} if m.pageMetaParams == nil { m.pageMetaParams = &pageMetaParams{ @@ -231,10 +240,6 @@ func (h *HugoSites) newPage(m *pageMeta) (*pageState, *paths.Path, error) { } return ps, nil }() - // Make sure to evict any cached and now stale data. - if err != nil { - m.MarkStale() - } if ps == nil { return nil, nil, err diff --git a/hugolib/site.go b/hugolib/site.go index 08031390b..24ee5dcc5 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -344,7 +344,6 @@ func newHugoSites(cfg deps.DepsCfg, d *deps.Deps, pageTrees *pageTrees, sites [] skipRebuildForFilenames: make(map[string]bool), init: &hugoSitesInit{ data: lazy.New(), - layouts: lazy.New(), gitInfo: lazy.New(), }, } @@ -400,15 +399,6 @@ func newHugoSites(cfg deps.DepsCfg, d *deps.Deps, pageTrees *pageTrees, sites [] return nil, nil }) - h.init.layouts.Add(func(context.Context) (any, error) { - for _, s := range h.Sites { - if err := s.Tmpl().(tpl.TemplateManager).MarkReady(); err != nil { - return nil, err - } - } - return nil, nil - }) - h.init.gitInfo.Add(func(context.Context) (any, error) { err := h.loadGitInfo() if err != nil { diff --git a/testscripts/commands/server__error_recovery_edit_config.txt b/testscripts/commands/server__error_recovery_edit_config.txt new file mode 100644 index 000000000..664d99272 --- /dev/null +++ b/testscripts/commands/server__error_recovery_edit_config.txt @@ -0,0 +1,42 @@ +# Test the hugo server command when adding an error to a config file +# and then fixing it. + +hugo server & + +waitServer + +httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1' + +replace $WORK/hugo.toml 'title =' 'titlefoo' +httpget ${HUGOTEST_BASEURL_0}p1/ 'failed' + +replace $WORK/hugo.toml 'titlefoo' 'title =' +httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1' + +stopServer + +-- hugo.toml -- +title = "Hugo Server Test" +baseURL = "https://example.org/" +disableKinds = ["taxonomy", "term", "sitemap"] +-- layouts/index.html -- +Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}| +-- layouts/_default/single.html -- +Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}| +-- content/_index.md -- +--- +title: Hugo Home +--- +-- content/p1/index.md -- +--- +title: P1 +--- +-- content/p2/index.md -- +--- +title: P2 +--- +-- static/staticfiles/static.txt -- +static + + + diff --git a/testscripts/commands/server__error_recovery_edit_content.txt b/testscripts/commands/server__error_recovery_edit_content.txt new file mode 100644 index 000000000..f5ea7e94b --- /dev/null +++ b/testscripts/commands/server__error_recovery_edit_content.txt @@ -0,0 +1,42 @@ +# Test the hugo server command when adding a front matter error to a content file +# and then fixing it. + +hugo server & + +waitServer + +httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1' + +replace $WORK/content/p1/index.md 'title:' 'titlecolon' +httpget ${HUGOTEST_BASEURL_0}p1/ 'failed' + +replace $WORK/content/p1/index.md 'titlecolon' 'title:' +httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1' + +stopServer + +-- hugo.toml -- +title = "Hugo Server Test" +baseURL = "https://example.org/" +disableKinds = ["taxonomy", "term", "sitemap"] +-- layouts/index.html -- +Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}| +-- layouts/_default/single.html -- +Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}| +-- content/_index.md -- +--- +title: Hugo Home +--- +-- content/p1/index.md -- +--- +title: P1 +--- +-- content/p2/index.md -- +--- +title: P2 +--- +-- static/staticfiles/static.txt -- +static + + + diff --git a/tpl/template.go b/tpl/template.go index cb8d2b321..18a31e231 100644 --- a/tpl/template.go +++ b/tpl/template.go @@ -40,7 +40,6 @@ type TemplateManager interface { TemplateHandler TemplateFuncGetter AddTemplate(name, tpl string) error - MarkReady() error } // TemplateVariants describes the possible variants of a template. diff --git a/tpl/tplimpl/template.go b/tpl/tplimpl/template.go index 04ccdaad2..fd07db68e 100644 --- a/tpl/tplimpl/template.go +++ b/tpl/tplimpl/template.go @@ -168,6 +168,10 @@ func newTemplateHandlers(d *deps.Deps) (*tpl.TemplateHandlers, error) { return nil, err } + if err := h.main.createPrototypes(); err != nil { + return nil, err + } + e := &templateExec{ d: d, executor: exec, @@ -312,28 +316,11 @@ func (t *templateExec) GetFunc(name string) (reflect.Value, bool) { return v, found } -func (t *templateExec) MarkReady() error { - var err error - t.readyInit.Do(func() { - // We only need the clones if base templates are in use. - if len(t.needsBaseof) > 0 { - err = t.main.createPrototypes() - if err != nil { - return - } - } - }) - - return err -} - type templateHandler struct { main *templateNamespace needsBaseof map[string]templateInfo baseof map[string]templateInfo - readyInit sync.Once - // This is the filesystem to load the templates from. All the templates are // stored in the root of this filesystem. layoutsFs afero.Fs