Skip to content

Commit

Permalink
hugolib/commands: Fix stuck server error issues
Browse files Browse the repository at this point in the history
Fixes #11378
  • Loading branch information
bep committed Oct 24, 2024
1 parent 5bbe95f commit 6e78cbd
Show file tree
Hide file tree
Showing 12 changed files with 118 additions and 66 deletions.
2 changes: 1 addition & 1 deletion commands/commandeer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
28 changes: 17 additions & 11 deletions commands/hugobuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -78,6 +77,11 @@ type hugoBuilder struct {

var errConfigNotSet = errors.New("config not set")

func (c *hugoBuilder) resetBuildErr() {
c.errState.setBuildErr(nil)
c.r.resetLogs()
}

func (c *hugoBuilder) withConfE(fn func(conf *commonConfig) error) error {
c.confmu.Lock()
defer c.confmu.Unlock()
Expand Down Expand Up @@ -136,10 +140,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()
Expand Down Expand Up @@ -345,7 +345,7 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa
for {
select {
case changes := <-c.r.changesFromBuild:
c.errState.setBuildErr(nil)
c.resetBuildErr()
unlock, err := h.LockBuild()
if err != nil {
c.r.logger.Errorln("Failed to acquire a build lock: %s", err)
Expand All @@ -358,7 +358,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()
}
Expand All @@ -372,7 +372,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()
}
Expand Down Expand Up @@ -423,7 +423,13 @@ func (c *hugoBuilder) buildSites(noBuildLock bool) (err error) {
if err != nil {
return err
}
return h.Build(hugolib.BuildCfg{NoBuildLock: noBuildLock})
err = h.Build(hugolib.BuildCfg{NoBuildLock: noBuildLock})
if err != nil {
c.errState.setBuildErr(err)
} else {
c.resetBuildErr()
}
return err
}

func (c *hugoBuilder) copyStatic() (map[string]uint64, error) {
Expand Down Expand Up @@ -1098,7 +1104,7 @@ func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) error {
}

func (c *hugoBuilder) rebuildSitesForChanges(ids []identity.Identity) error {
c.errState.setBuildErr(nil)
c.resetBuildErr()
h, err := c.hugo()
if err != nil {
return err
Expand All @@ -1111,7 +1117,7 @@ func (c *hugoBuilder) rebuildSitesForChanges(ids []identity.Identity) error {
}

func (c *hugoBuilder) reloadConfig() error {
c.r.Reset()
c.r.resetLogs()
c.r.configVersionID.Add(1)

if err := c.withConfE(func(conf *commonConfig) error {
Expand Down
6 changes: 2 additions & 4 deletions commands/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,9 +648,7 @@ func (c *serverCommand) setServerInfoInConfig() error {
}

func (c *serverCommand) getErrorWithContext() any {
errCount := c.errCount()

if errCount == 0 {
if c.errState.builderr == nil {
return nil
}

Expand Down Expand Up @@ -834,7 +832,7 @@ func (c *serverCommand) partialReRender(urls ...string) error {
defer func() {
c.errState.setWasErr(false)
}()
c.errState.setBuildErr(nil)
c.resetBuildErr()
visited := types.NewEvictingStringQueue(len(urls))
for _, url := range urls {
visited.Add(url)
Expand Down
3 changes: 0 additions & 3 deletions hugolib/hugo_sites.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 0 additions & 10 deletions hugolib/hugo_sites_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 0 additions & 5 deletions hugolib/integrationtest_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions hugolib/page__new.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions hugolib/site.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
}
Expand Down Expand Up @@ -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 {
Expand Down
43 changes: 43 additions & 0 deletions testscripts/unfinished/server__error_recovery_edit_config.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# 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 ='
sleep 9
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



42 changes: 42 additions & 0 deletions testscripts/unfinished/server__error_recovery_edit_content.txt
Original file line number Diff line number Diff line change
@@ -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



1 change: 0 additions & 1 deletion tpl/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ type TemplateManager interface {
TemplateHandler
TemplateFuncGetter
AddTemplate(name, tpl string) error
MarkReady() error
}

// TemplateVariants describes the possible variants of a template.
Expand Down
21 changes: 4 additions & 17 deletions tpl/tplimpl/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 6e78cbd

Please sign in to comment.