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 i, rf := range s.renderFormats {
-			s.rc = &siteRenderingContext{Format: rf}
+	}
 
-			s.preparePagesForRender(config)
+	for _, s := range h.Sites {
+		for i, rf := range s.renderFormats {
+			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}
+
+				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 start {
+		// This is a new language.
+		p.shortcodeState.clearDelta()
+	}
+	updated := true
 	if p.shortcodeState != nil {
-		shortcodeUpdate = p.shortcodeState.updateDelta()
+		updated = p.shortcodeState.updateDelta()
 	}
 
-	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)
-			}
-		}
+	if updated {
+		p.resetContent()
 	}
 
-	if shortcodeUpdate || cfg.whatChanged.other {
-		p.resetContent(resetFunc)
-	}
-
-	// Handle bundled pages.
 	for _, r := range p.Resources.ByType(pageResourceType) {
-		shortcodeUpdate = false
-		bp := r.(*Page)
-
-		if bp.shortcodeState != nil {
-			shortcodeUpdate = bp.shortcodeState.updateDelta()
-		}
-
-		if shortcodeUpdate || cfg.whatChanged.other {
 		p.s.PathSpec.ProcessingStats.Incr(&p.s.PathSpec.ProcessingStats.Pages)
-			bp.resetContent(resetFunc)
+		bp := r.(*Page)
+		if start {
+			bp.shortcodeState.clearDelta()
+		}
+		if bp.shortcodeState != nil {
+			updated = bp.shortcodeState.updateDelta()
+		}
+		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)

hugolib/shortcode_test.go

@@ -22,6 +22,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/spf13/viper"
+
 	jww "github.com/spf13/jwalterweatherman"
 
 	"github.com/spf13/afero"
@@ -884,7 +886,83 @@ func TestScKey(t *testing.T) {
 
 }
 
-func TestPreserveShortcodeOrder(t *testing.T) {
+func TestShortcodeGetContent(t *testing.T) {
+	t.Parallel()
+	assert := require.New(t)
+
+	contentShortcode := `
+{{- $t := .Get 0 -}}
+{{- $p := .Get 1 -}}
+{{- $k := .Get 2 -}}
+{{- $page := $.Page.Site.GetPage "page" $p -}}
+{{ if $page }}
+{{- if eq $t "bundle" -}}
+{{- .Scratch.Set "p" ($page.Resources.GetMatch (printf "%s*" $k)) -}}
+{{- else -}}
+{{- $.Scratch.Set "p" $page -}}
+{{- end -}}P1:{{ .Page.Content }}|P2:{{ $p := ($.Scratch.Get "p") }}{{ $p.Title }}/{{ $p.Content }}|
+{{- else -}}
+{{- errorf "Page %s is nil" $p -}}
+{{- end -}}
+`
+
+	var templates []string
+	var content []string
+
+	contentWithShortcodeTemplate := `---
+title: doc%s
+weight: %d
+---
+Logo:{{< c "bundle" "b1" "logo.png" >}}:P1: {{< c "page" "section1/p1" "" >}}:BP1:{{< c "bundle" "b1" "bp1" >}}`
+
+	simpleContentTemplate := `---
+title: doc%s
+weight: %d
+---
+C-%s`
+
+	v := viper.New()
+
+	v.Set("timeout", 500)
+
+	templates = append(templates, []string{"shortcodes/c.html", contentShortcode}...)
+	templates = append(templates, []string{"_default/single.html", "Single Content: {{ .Content }}"}...)
+	templates = append(templates, []string{"_default/list.html", "List Content: {{ .Content }}"}...)
+
+	content = append(content, []string{"b1/index.md", fmt.Sprintf(contentWithShortcodeTemplate, "b1", 1)}...)
+	content = append(content, []string{"b1/logo.png", "PNG logo"}...)
+	content = append(content, []string{"b1/bp1.md", fmt.Sprintf(simpleContentTemplate, "bp1", 1, "bp1")}...)
+
+	content = append(content, []string{"section1/_index.md", fmt.Sprintf(contentWithShortcodeTemplate, "s1", 2)}...)
+	content = append(content, []string{"section1/p1.md", fmt.Sprintf(simpleContentTemplate, "s1p1", 2, "s1p1")}...)
+
+	content = append(content, []string{"section2/_index.md", fmt.Sprintf(simpleContentTemplate, "b1", 1, "b1")}...)
+	content = append(content, []string{"section2/s2p1.md", fmt.Sprintf(contentWithShortcodeTemplate, "bp1", 1)}...)
+
+	builder := newTestSitesBuilder(t).WithDefaultMultiSiteConfig()
+
+	builder.WithViper(v).WithContent(content...).WithTemplates(templates...).CreateSites().Build(BuildCfg{})
+	s := builder.H.Sites[0]
+	assert.Equal(3, len(s.RegularPages))
+
+	builder.AssertFileContent("public/section1/index.html",
+		"List Content: <p>Logo:P1:|P2:logo.png/PNG logo|:P1: P1:|P2:docs1p1/<p>C-s1p1</p>\n|",
+		"BP1:P1:|P2:docbp1/<p>C-bp1</p>",
+	)
+
+	builder.AssertFileContent("public/b1/index.html",
+		"Single Content: <p>Logo:P1:|P2:logo.png/PNG logo|:P1: P1:|P2:docs1p1/<p>C-s1p1</p>\n|",
+		"P2:docbp1/<p>C-bp1</p>",
+	)
+
+	builder.AssertFileContent("public/section2/s2p1/index.html",
+		"Single Content: <p>Logo:P1:|P2:logo.png/PNG logo|:P1: P1:|P2:docs1p1/<p>C-s1p1</p>\n|",
+		"P2:docbp1/<p>C-bp1</p>",
+	)
+
+}
+
+func TestShortcodePreserveOrder(t *testing.T) {
 	t.Parallel()
 	assert := require.New(t)
 
@@ -897,28 +975,30 @@ weight: %d
 {{< s1 >}}{{< s2 >}}{{< s3 >}}{{< s4 >}}{{< s5 >}}
 
 {{< nested >}}
-{{< ordinal >}}
-{{< ordinal >}}
-{{< ordinal >}}
+{{< ordinal >}} {{< scratch >}}
+{{< ordinal >}} {{< scratch >}}
+{{< ordinal >}} {{< scratch >}}
 {{< /nested >}}
 
-
 `
 
-	ordinalShortcodeTemplate := `ordinal: {{ .Ordinal }}`
+	ordinalShortcodeTemplate := `ordinal: {{ .Ordinal }}{{ .Page.Scratch.Set "ordinal" .Ordinal }}`
 
 	nestedShortcode := `outer ordinal: {{ .Ordinal }} inner: {{ .Inner }}`
-
-	shortCodeTemplate := `v%d: {{ .Ordinal }}|`
+	scratchGetShortcode := `scratch ordinal: {{ .Ordinal }} scratch get ordinal: {{ .Page.Scratch.Get "ordinal" }}`
+	shortcodeTemplate := `v%d: {{ .Ordinal }} sgo: {{ .Page.Scratch.Get "o2" }}{{ .Page.Scratch.Set "o2" .Ordinal }}|`
 
 	var shortcodes []string
 	var content []string
 
 	shortcodes = append(shortcodes, []string{"shortcodes/nested.html", nestedShortcode}...)
 	shortcodes = append(shortcodes, []string{"shortcodes/ordinal.html", ordinalShortcodeTemplate}...)
+	shortcodes = append(shortcodes, []string{"shortcodes/scratch.html", scratchGetShortcode}...)
 
 	for i := 1; i <= 5; i++ {
-		shortcodes = append(shortcodes, []string{fmt.Sprintf("shortcodes/s%d.html", i), fmt.Sprintf(shortCodeTemplate, i)}...)
+		sc := fmt.Sprintf(shortcodeTemplate, i)
+		sc = strings.Replace(sc, "%%", "%", -1)
+		shortcodes = append(shortcodes, []string{fmt.Sprintf("shortcodes/s%d.html", i), sc}...)
 	}
 
 	for i := 1; i <= 3; i++ {
@@ -932,18 +1012,10 @@ weight: %d
 	s := builder.H.Sites[0]
 	assert.Equal(3, len(s.RegularPages))
 
-	p1 := s.RegularPages[0]
-
-	if !strings.Contains(string(p1.content()), `v1: 0|v2: 1|v3: 2|v4: 3|v5: 4|`) {
-		t.Fatal(p1.content())
-	}
-
-	// Check nested behaviour
-	if !strings.Contains(string(p1.content()), `outer ordinal: 5 inner: 
-ordinal: 0
-ordinal: 1
-ordinal: 2`) {
-		t.Fatal(p1.content())
-	}
+	builder.AssertFileContent("public/en/p1/index.html", `v1: 0 sgo: |v2: 1 sgo: 0|v3: 2 sgo: 1|v4: 3 sgo: 2|v5: 4 sgo: 3`)
+	builder.AssertFileContent("public/en/p1/index.html", `outer ordinal: 5 inner: 
+ordinal: 0 scratch ordinal: 1 scratch get ordinal: 0
+ordinal: 2 scratch ordinal: 3 scratch get ordinal: 2
+ordinal: 4 scratch ordinal: 5 scratch get ordinal: 4`)
 
 }

hugolib/site.go

@@ -180,6 +180,7 @@ func (s *Site) reset() *Site {
 		titleFunc:           s.titleFunc,
 		relatedDocsHandler:  newSearchIndexHandler(s.relatedDocsHandler.cfg),
 		outputFormats:       s.outputFormats,
+		rc:                  s.rc,
 		outputFormatsConfig: s.outputFormatsConfig,
 		frontmatterHandler:  s.frontmatterHandler,
 		mediaTypesConfig:    s.mediaTypesConfig,
@@ -266,6 +267,7 @@ func newSite(cfg deps.DepsCfg) (*Site, error) {
 		titleFunc:           titleFunc,
 		relatedDocsHandler:  newSearchIndexHandler(relatedContentConfig),
 		outputFormats:       outputFormats,
+		rc:                  &siteRenderingContext{output.HTMLFormat},
 		outputFormatsConfig: siteOutputFormatsConfig,
 		mediaTypesConfig:    siteMediaTypesConfig,
 		frontmatterHandler:  frontMatterHandler,
@@ -546,7 +548,7 @@ func (s *SiteInfo) RelRef(ref string, page *Page, options ...string) (string, er
 }
 
 func (s *Site) running() bool {
-	return s.owner.running
+	return s.owner != nil && s.owner.running
 }
 
 func init() {