hugolib: Fix some shortcode vs .Content corner cases

This is a follow-up to #4632. There were some assumptions in that implementation that did not hold water in all situations.

This commit simplifies the content lazy initalization making it more robust.

Fixes #4664

hugolib/hugo_sites.go

@@ -559,41 +559,14 @@ func (h *HugoSites) setupTranslations() {
 	}
 }
 
-func (s *Site) preparePagesForRender(cfg *BuildCfg) {
-
-	pageChan := make(chan *Page)
-	wg := &sync.WaitGroup{}
-
-	numWorkers := getGoMaxProcs() * 4
-
-	for i := 0; i < numWorkers; i++ {
-		wg.Add(1)
-		go func(pages <-chan *Page, wg *sync.WaitGroup) {
-			defer wg.Done()
-			for p := range pages {
-				p.setContentInit(cfg)
-
-				// In most cases we could delay the content init until rendering time,
-				// but there could be use cases where the templates would depend
-				// on state set in the shortcodes (.Page.Scratch.Set), so we
-				// need to do this early. This will do the needed recursion.
-				p.initContent()
-			}
-		}(pageChan, wg)
-	}
-
+func (s *Site) preparePagesForRender(start bool) {
 	for _, p := range s.Pages {
-		pageChan <- p
+		p.setContentInit(start)
 	}
 
 	for _, p := range s.headlessPages {
-		pageChan <- p
+		p.setContentInit(start)
 	}
-
-	close(pageChan)
-
-	wg.Wait()
-
 }
 
 // Pages returns all pages for all sites.

hugolib/hugo_sites_build.go

@@ -222,10 +222,21 @@ func (h *HugoSites) assemble(config *BuildCfg) error {
 func (h *HugoSites) render(config *BuildCfg) error {
 	for _, s := range h.Sites {
 		s.initRenderFormats()
+	}
+
+	for _, s := range h.Sites {
 		for i, rf := range s.renderFormats {
-			s.rc = &siteRenderingContext{Format: rf}
+			for _, s2 := range h.Sites {
+				// We render site by site, but since the content is lazily rendered
+				// and a site can "borrow" content from other sites, every site
+				// needs this set.
+				s2.rc = &siteRenderingContext{Format: rf}
 
-			s.preparePagesForRender(config)
+				isRenderingSite := s == s2
+
+				s2.preparePagesForRender(isRenderingSite && i == 0)
+
+			}
 
 			if !config.SkipRender {
 				if err := s.render(config, i); err != nil {

hugolib/page.go

@@ -38,6 +38,7 @@ import (
 	"path"
 	"path/filepath"
 	"regexp"
+	"runtime"
 	"strings"
 	"sync"
 	"time"
@@ -129,9 +130,6 @@ type Page struct {
 	// Params contains configuration defined in the params section of page frontmatter.
 	params map[string]interface{}
 
-	// Called when needed to init the content (render shortcodes etc.).
-	contentInitFn func(p *Page) func()
-
 	// Content sections
 	contentv        template.HTML
 	summary         template.HTML
@@ -270,7 +268,14 @@ type Page struct {
 	targetPathDescriptorPrototype *targetPathDescriptor
 }
 
+func stackTrace() string {
+	trace := make([]byte, 2000)
+	runtime.Stack(trace, true)
+	return string(trace)
+}
+
 func (p *Page) initContent() {
+
 	p.contentInit.Do(func() {
 		// This careful dance is here to protect against circular loops in shortcode/content
 		// constructs.
@@ -284,9 +289,12 @@ func (p *Page) initContent() {
 			p.contentInitMu.Lock()
 			defer p.contentInitMu.Unlock()
 
-			if p.contentInitFn != nil {
-				p.contentInitFn(p)()
+			err = p.prepareForRender()
+			if err != nil {
+				p.s.Log.ERROR.Printf("Failed to prepare page %q for render: %s", p.Path(), err)
+				return
 			}
+
 			if len(p.summary) == 0 {
 				if err = p.setAutoSummary(); err != nil {
 					err = fmt.Errorf("Failed to set user auto summary for page %q: %s", p.pathOrTitle(), err)
@@ -297,7 +305,7 @@ func (p *Page) initContent() {
 
 		select {
 		case <-ctx.Done():
-			p.s.Log.WARN.Printf(`WARNING: Timed out creating content for page %q (.Content will be empty). This is most likely a circular shortcode content loop that should be fixed. If this is just a shortcode calling a slow remote service, try to set "timeout=20000" (or higher, value is in milliseconds) in config.toml.`, p.pathOrTitle())
+			p.s.Log.WARN.Printf(`WARNING: Timed out creating content for page %q (.Content will be empty). This is most likely a circular shortcode content loop that should be fixed. If this is just a shortcode calling a slow remote service, try to set "timeout=20000" (or higher, value is in milliseconds) in config.toml.\n`, p.pathOrTitle())
 		case err := <-c:
 			if err != nil {
 				p.s.Log.ERROR.Println(err)
@@ -420,14 +428,8 @@ type pageContentInit struct {
 	plainWordsInit sync.Once
 }
 
-func (p *Page) resetContent(init func(page *Page) func()) {
+func (p *Page) resetContent() {
 	p.pageContentInit = &pageContentInit{}
-	if init == nil {
-		init = func(page *Page) func() {
-			return func() {}
-		}
-	}
-	p.contentInitFn = init
 }
 
 // IsNode returns whether this is an item of one of the list types in Hugo,
@@ -1165,49 +1167,40 @@ func (p *Page) subResourceTargetPathFactory(base string) string {
 	return path.Join(p.relTargetPathBase, base)
 }
 
-func (p *Page) setContentInit(cfg *BuildCfg) error {
-	if !p.shouldRenderTo(p.s.rc.Format) {
-		// No need to prepare
-		return nil
-	}
+func (p *Page) setContentInit(start bool) error {
 
-	var shortcodeUpdate bool
-	if p.shortcodeState != nil {
-		shortcodeUpdate = p.shortcodeState.updateDelta()
+	if start {
+		// This is a new language.
+		p.shortcodeState.clearDelta()
 	}
-
-	resetFunc := func(page *Page) func() {
-		return func() {
-			err := page.prepareForRender(cfg)
-			if err != nil {
-				p.s.Log.ERROR.Printf("Failed to prepare page %q for render: %s", page.Path(), err)
-			}
-		}
+	updated := true
+	if p.shortcodeState != nil {
+		updated = p.shortcodeState.updateDelta()
 	}
 
-	if shortcodeUpdate || cfg.whatChanged.other {
-		p.resetContent(resetFunc)
+	if updated {
+		p.resetContent()
 	}
 
-	// Handle bundled pages.
 	for _, r := range p.Resources.ByType(pageResourceType) {
-		shortcodeUpdate = false
+		p.s.PathSpec.ProcessingStats.Incr(&p.s.PathSpec.ProcessingStats.Pages)
 		bp := r.(*Page)
-
+		if start {
+			bp.shortcodeState.clearDelta()
+		}
 		if bp.shortcodeState != nil {
-			shortcodeUpdate = bp.shortcodeState.updateDelta()
+			updated = bp.shortcodeState.updateDelta()
 		}
-
-		if shortcodeUpdate || cfg.whatChanged.other {
-			p.s.PathSpec.ProcessingStats.Incr(&p.s.PathSpec.ProcessingStats.Pages)
-			bp.resetContent(resetFunc)
+		if updated {
+			bp.resetContent()
 		}
 	}
 
 	return nil
+
 }
 
-func (p *Page) prepareForRender(cfg *BuildCfg) error {
+func (p *Page) prepareForRender() error {
 	s := p.s
 
 	// If we got this far it means that this is either a new Page pointer
@@ -1217,7 +1210,7 @@ func (p *Page) prepareForRender(cfg *BuildCfg) error {
 	// If in watch mode or if we have multiple output formats,
 	// we need to keep the original so we can
 	// potentially repeat this process on rebuild.
-	needsACopy := p.s.running() || len(p.outputFormats) > 1
+	needsACopy := s.running() || len(p.outputFormats) > 1
 	var workContentCopy []byte
 	if needsACopy {
 		workContentCopy = make([]byte, len(p.workContent))

hugolib/pageSort_test.go

@@ -15,7 +15,6 @@ package hugolib
 
 import (
 	"fmt"
-	"html/template"
 	"path/filepath"
 	"testing"
 	"time"
@@ -168,11 +167,16 @@ func setSortVals(dates [4]time.Time, titles [4]string, weights [4]int, pages Pag
 		pages[len(dates)-1-i].linkTitle = pages[i].title + "l"
 		pages[len(dates)-1-i].PublishDate = dates[i]
 		pages[len(dates)-1-i].ExpiryDate = dates[i]
-		pages[len(dates)-1-i].contentv = template.HTML(titles[i] + "_content")
+		pages[len(dates)-1-i].workContent = []byte(titles[i] + "_content")
 	}
 	lastLastMod := pages[2].Lastmod
 	pages[2].Lastmod = pages[1].Lastmod
 	pages[1].Lastmod = lastLastMod
+
+	for _, p := range pages {
+		p.resetContent()
+	}
+
 }
 
 func createSortTestPages(s *Site, num int) Pages {

hugolib/page_test.go

@@ -525,7 +525,7 @@ func checkPageDate(t *testing.T, page *Page, time time.Time) {
 }
 
 func checkTruncation(t *testing.T, page *Page, shouldBe bool, msg string) {
-	if page.summary == "" {
+	if page.Summary() == "" {
 		t.Fatal("page has no summary, can not check truncation")
 	}
 	if page.truncated != shouldBe {
@@ -722,8 +722,8 @@ func TestPageWithDelimiterForMarkdownThatCrossesBorder(t *testing.T) {
 
 	p := s.RegularPages[0]
 
-	if p.summary != template.HTML("<p>The <a href=\"http://gohugo.io/\">best static site generator</a>.<sup class=\"footnote-ref\" id=\"fnref:1\"><a href=\"#fn:1\">1</a></sup>\n</p>") {
-		t.Fatalf("Got summary:\n%q", p.summary)
+	if p.Summary() != template.HTML("<p>The <a href=\"http://gohugo.io/\">best static site generator</a>.<sup class=\"footnote-ref\" id=\"fnref:1\"><a href=\"#fn:1\">1</a></sup>\n</p>") {
+		t.Fatalf("Got summary:\n%q", p.Summary())
 	}
 
 	if p.content() != template.HTML("<p>The <a href=\"http://gohugo.io/\">best static site generator</a>.<sup class=\"footnote-ref\" id=\"fnref:1\"><a href=\"#fn:1\">1</a></sup>\n</p>\n<div class=\"footnotes\">\n\n<hr />\n\n<ol>\n<li id=\"fn:1\">Many people say so.\n <a class=\"footnote-return\" href=\"#fnref:1\"><sup>[return]</sup></a></li>\n</ol>\n</div>") {
@@ -876,8 +876,8 @@ func TestSummaryWithHTMLTagsOnNextLine(t *testing.T) {
 
 	assertFunc := func(t *testing.T, ext string, pages Pages) {
 		p := pages[0]
-		require.Contains(t, p.summary, "Happy new year everyone!")
-		require.NotContains(t, p.summary, "User interface")
+		require.Contains(t, p.Summary(), "Happy new year everyone!")
+		require.NotContains(t, p.Summary(), "User interface")
 	}
 
 	testAllMarkdownEnginesForPages(t, assertFunc, nil, `---
@@ -1511,8 +1511,8 @@ func TestPageSimpleMethods(t *testing.T) {
 	} {
 
 		p, _ := s.NewPage("Test")
-		p.contentv = "<h1>Do Be Do Be Do</h1>"
-		p.initContent()
+		p.workContent = []byte("<h1>Do Be Do Be Do</h1>")
+		p.resetContent()
 		if !this.assertFunc(p) {
 			t.Errorf("[%d] Page method error", i)
 		}

hugolib/shortcode.go

@@ -366,7 +366,12 @@ func (s *shortcodeHandler) updateDelta() bool {
 		s.contentShortcodes = createShortcodeRenderers(s.shortcodes, s.p.withoutContent())
 	})
 
-	contentShortcodes := s.contentShortcodesForOutputFormat(s.p.s.rc.Format)
+	if !s.p.shouldRenderTo(s.p.s.rc.Format) {
+		// TODO(bep) add test for this re translations
+		return false
+	}
+	of := s.p.s.rc.Format
+	contentShortcodes := s.contentShortcodesForOutputFormat(of)
 
 	if s.contentShortcodesDelta == nil || s.contentShortcodesDelta.Len() == 0 {
 		s.contentShortcodesDelta = contentShortcodes
@@ -387,13 +392,19 @@ func (s *shortcodeHandler) updateDelta() bool {
 	return delta.Len() > 0
 }
 
+func (s *shortcodeHandler) clearDelta() {
+	if s == nil {
+		return
+	}
+	s.contentShortcodesDelta = newOrderedMap()
+}
+
 func (s *shortcodeHandler) contentShortcodesForOutputFormat(f output.Format) *orderedMap {
 	contentShortcodesForOuputFormat := newOrderedMap()
 	lang := s.p.Lang()
 
 	for _, key := range s.shortcodes.Keys() {
 		shortcodePlaceholder := key.(string)
-		//	shortcodePlaceholder := s.shortcodes.getShortcode(key)
 
 		key := newScKeyFromLangAndOutputFormat(lang, f, shortcodePlaceholder)
 		renderFn, found := s.contentShortcodes.Get(key)