shithub: hugo

Info  •  Files  •  Log  •  Branches

hugolib: Fix possible .Content cut

There have been one report of a site with truncated `.Content` after the Hugo `0.40.1` release.

This commit fixes this so that race should not be possible anymore. It also adds a stress test with focus on content rendering and multiple output formats.

Fixes #4706

--- a/hugolib/hugo_sites.go

+++ b/hugolib/hugo_sites.go

@@ -559,14 +559,22 @@

 	}

 }

-func (s *Site) preparePagesForRender(start bool) {

+func (s *Site) preparePagesForRender(start bool) error {

 	for _, p := range s.Pages {

 		p.setContentInit(start)

+		if err := p.initMainOutputFormat(); err != nil {

+			return err

+		}

 	}

 	for _, p := range s.headlessPages {

 		p.setContentInit(start)

+		if err := p.initMainOutputFormat(); err != nil {

+			return err

+		}

 	}

+

+	return nil

 }

 // Pages returns all pages for all sites.

--- a/hugolib/hugo_sites_build.go

+++ b/hugolib/hugo_sites_build.go

@@ -234,7 +234,9 @@

 				isRenderingSite := s == s2

-				s2.preparePagesForRender(isRenderingSite && i == 0)

+				if err := s2.preparePagesForRender(isRenderingSite && i == 0); err != nil {

+					return err

+				}

 			}

--- a/hugolib/hugo_sites_build_test.go

+++ b/hugolib/hugo_sites_build_test.go

@@ -734,6 +734,119 @@

 	b.AssertFileContent("public/sect/doc2/index.html", "Single", "Hello")

 }

+// https://github.com/gohugoio/hugo/issues/4706

+func TestContentStressTest(t *testing.T) {

+	b := newTestSitesBuilder(t)

+

+	numPages := 500

+

+	contentTempl := `

+---

+%s

+title: %q

+weight: %d

+multioutput: %t

+---

+

+# Header

+

+CONTENT

+

+The End.

+`

+

+	contentTempl = strings.Replace(contentTempl, "CONTENT", strings.Repeat(`

+	

+## Another header

+

+Some text. Some more text.

+

+`, 100), -1)

+

+	var content []string

+	defaultOutputs := `outputs: ["html", "json", "rss" ]`

+

+	for i := 1; i <= numPages; i++ {

+		outputs := defaultOutputs

+		multioutput := true

+		if i%3 == 0 {

+			outputs = `outputs: ["json"]`

+			multioutput = false

+		}

+		section := "s1"

+		if i%10 == 0 {

+			section = "s2"

+		}

+		content = append(content, []string{fmt.Sprintf("%s/page%d.md", section, i), fmt.Sprintf(contentTempl, outputs, fmt.Sprintf("Title %d", i), i, multioutput)}...)

+	}

+

+	content = append(content, []string{"_index.md", fmt.Sprintf(contentTempl, defaultOutputs, fmt.Sprintf("Home %d", 0), 0, true)}...)

+	content = append(content, []string{"s1/_index.md", fmt.Sprintf(contentTempl, defaultOutputs, fmt.Sprintf("S %d", 1), 1, true)}...)

+	content = append(content, []string{"s2/_index.md", fmt.Sprintf(contentTempl, defaultOutputs, fmt.Sprintf("S %d", 2), 2, true)}...)

+

+	b.WithSimpleConfigFile()

+	b.WithTemplates("layouts/_default/single.html", `Single: {{ .Content }}`)

+	b.WithTemplates("layouts/_default/myview.html", `View: {{ len .Content }}`)

+	b.WithTemplates("layouts/_default/single.json", `Single JSON: {{ .Content }}`)

+	b.WithTemplates("layouts/_default/list.html", `

+Page: {{ .Paginator.PageNumber }}

+P: {{ path.Join .Path }}

+List: {{ len .Paginator.Pages }}|List Content: {{ len .Content }}

+{{ $shuffled :=  where .Site.RegularPages "Params.multioutput" true | shuffle }}

+{{ $first5 := $shuffled | first 5 }}

+L1: {{ len .Site.RegularPages }} L2: {{ len $first5 }}

+{{ range $i, $e := $first5 }}

+Render {{ $i }}: {{ .Render "myview" }}

+{{ end }}

+END

+`)

+

+	b.WithContent(content...)

+

+	b.CreateSites().Build(BuildCfg{})

+

+	contentMatchers := []string{"<h2 id=\"another-header\">Another header</h2>", "<h2 id=\"another-header-99\">Another header</h2>", "<p>The End.</p>"}

+

+	for i := 1; i <= numPages; i++ {

+		if i%3 != 0 {

+			section := "s1"

+			if i%10 == 0 {

+				section = "s2"

+			}

+			checkContent(b, fmt.Sprintf("public/%s/page%d/index.html", section, i), 8343, contentMatchers...)

+		}

+	}

+

+	for i := 1; i <= numPages; i++ {

+		section := "s1"

+		if i%10 == 0 {

+			section = "s2"

+		}

+		checkContent(b, fmt.Sprintf("public/%s/page%d/index.json", section, i), 8348, contentMatchers...)

+	}

+

+	checkContent(b, "public/s1/index.html", 184, "P: s1/_index.md\nList: 10|List Content: 8335\n\n\nL1: 500 L2: 5\n\nRender 0: View: 8335\n\nRender 1: View: 8335\n\nRender 2: View: 8335\n\nRender 3: View: 8335\n\nRender 4: View: 8335\n\nEND\n")

+	checkContent(b, "public/s2/index.html", 184, "P: s2/_index.md\nList: 10|List Content: 8335", "Render 4: View: 8335\n\nEND")

+	checkContent(b, "public/index.html", 181, "P: _index.md\nList: 10|List Content: 8335", "4: View: 8335\n\nEND")

+

+	// Chek paginated pages

+	for i := 2; i <= 9; i++ {

+		checkContent(b, fmt.Sprintf("public/page/%d/index.html", i), 181, fmt.Sprintf("Page: %d", i), "Content: 8335\n\n\nL1: 500 L2: 5\n\nRender 0: View: 8335", "Render 4: View: 8335\n\nEND")

+	}

+}

+

+func checkContent(s *sitesBuilder, filename string, length int, matches ...string) {

+	content := readDestination(s.T, s.Fs, filename)

+	for _, match := range matches {

+		if !strings.Contains(content, match) {

+			s.Fatalf("No match for %q in content for %s\n%q", match, filename, content)

+		}

+	}

+	if len(content) != length {

+		s.Fatalf("got %d expected %d", len(content), length)

+	}

+}

+

 func TestTableOfContentsInShortcodes(t *testing.T) {

 	t.Parallel()

--- a/hugolib/page.go

+++ b/hugolib/page.go

@@ -416,7 +416,6 @@

 	languageInit        sync.Once

 	pageMenusInit       sync.Once

 	pageMetaInit        sync.Once

-	pageOutputInit      sync.Once

 	renderingConfigInit sync.Once

 	withoutContentInit  sync.Once

 }

@@ -1178,6 +1177,24 @@

 	return path.Join(p.relTargetPathBase, base)

 }

+func (p *Page) initMainOutputFormat() error {

+	if p.mainPageOutput != nil {

+		return nil

+	}

+

+	outFormat := p.outputFormats[0]

+	pageOutput, err := newPageOutput(p, false, false, outFormat)

+

+	if err != nil {

+		return fmt.Errorf("Failed to create output page for type %q for page %q: %s", outFormat.Name, p.pathOrTitle(), err)

+	}

+

+	p.mainPageOutput = pageOutput

+

+	return nil

+

+}

+

 func (p *Page) setContentInit(start bool) error {

 	if start {

@@ -1962,12 +1979,17 @@

 // copy creates a copy of this page with the lazy sync.Once vars reset

 // so they will be evaluated again, for word count calculations etc.

-func (p *Page) copy() *Page {

+func (p *Page) copy(initContent bool) *Page {

 	p.contentInitMu.Lock()

 	c := *p

 	p.contentInitMu.Unlock()

 	c.pageInit = &pageInit{}

-	c.pageContentInit = &pageContentInit{}

+	if initContent {

+		if len(p.outputFormats) < 2 {

+			panic(fmt.Sprintf("programming error: page %q should not need to rebuild content as it has only %d outputs", p.Path(), len(p.outputFormats)))

+		}

+		c.pageContentInit = &pageContentInit{}

+	}

 	return &c

 }

--- a/hugolib/page_output.go

+++ b/hugolib/page_output.go

@@ -55,7 +55,7 @@

 	return tp, nil

 }

-func newPageOutput(p *Page, createCopy bool, f output.Format) (*PageOutput, error) {

+func newPageOutput(p *Page, createCopy, initContent bool, f output.Format) (*PageOutput, error) {

 	// TODO(bep) This is only needed for tests and we should get rid of it.

 	if p.targetPathDescriptorPrototype == nil {

 		if err := p.initPaths(); err != nil {

@@ -64,7 +64,7 @@

 	}

 	if createCopy {

-		p = p.copy()

+		p = p.copy(initContent)

 	}

 	td, err := p.createTargetPathDescriptor(f)

@@ -82,8 +82,8 @@

 // copy creates a copy of this PageOutput with the lazy sync.Once vars reset

 // so they will be evaluated again, for word count calculations etc.

-func (p *PageOutput) copyWithFormat(f output.Format) (*PageOutput, error) {

-	c, err := newPageOutput(p.Page, true, f)

+func (p *PageOutput) copyWithFormat(f output.Format, initContent bool) (*PageOutput, error) {

+	c, err := newPageOutput(p.Page, true, initContent, f)

 	if err != nil {

 		return nil, err

 	}

@@ -92,7 +92,7 @@

 }

 func (p *PageOutput) copy() (*PageOutput, error) {

-	return p.copyWithFormat(p.outputFormat)

+	return p.copyWithFormat(p.outputFormat, false)

 }

 func (p *PageOutput) layouts(layouts ...string) ([]string, error) {

@@ -142,24 +142,6 @@

 }

 func (p *Page) Render(layout ...string) template.HTML {

-	p.pageOutputInit.Do(func() {

-		if p.mainPageOutput != nil {

-			return

-		}

-		// If Render is called in a range loop, the page output isn't available.

-		// So, create one.

-		outFormat := p.outputFormats[0]

-		pageOutput, err := newPageOutput(p, true, outFormat)

-

-		if err != nil {

-			p.s.Log.ERROR.Printf("Failed to create output page for type %q for page %q: %s", outFormat.Name, p.pathOrTitle(), err)

-			return

-		}

-

-		p.mainPageOutput = pageOutput

-

-	})

-

 	return p.mainPageOutput.Render(layout...)

 }

--- a/hugolib/pagination_test.go

+++ b/hugolib/pagination_test.go

@@ -279,8 +279,8 @@

 	require.NoError(t, err)

 	pages := createTestPages(s, 12)

-	n1, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

-	n2, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

+	n1, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

+	n2, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

 	n1.Data["Pages"] = pages

 	var paginator1 *Pager

@@ -306,7 +306,7 @@

 	require.Equal(t, paginator1, samePaginator)

 	pp, _ := s.NewPage("test")

-	p, _ := newPageOutput(pp, false, output.HTMLFormat)

+	p, _ := newPageOutput(pp, false, false, output.HTMLFormat)

 	_, err = p.Paginator()

 	require.NotNil(t, err)

@@ -315,7 +315,7 @@

 func TestPaginatorWithNegativePaginate(t *testing.T) {

 	t.Parallel()

 	s := newTestSite(t, "paginate", -1)

-	n1, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

+	n1, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

 	_, err := n1.Paginator()

 	require.Error(t, err)

 }

@@ -378,8 +378,8 @@

 	}

 	pages := createTestPages(s, 6)

-	n1, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

-	n2, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

+	n1, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

+	n2, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

 	var paginator1, paginator2 *Pager

@@ -404,7 +404,7 @@

 	require.Equal(t, paginator2, paginator1.Next())

 	pp, err := s.NewPage("test")

-	p, _ := newPageOutput(pp, false, output.HTMLFormat)

+	p, _ := newPageOutput(pp, false, false, output.HTMLFormat)

 	_, err = p.Paginate(pages)

 	require.NotNil(t, err)

@@ -413,7 +413,7 @@

 func TestInvalidOptions(t *testing.T) {

 	t.Parallel()

 	s := newTestSite(t)

-	n1, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

+	n1, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

 	_, err := n1.Paginate(createTestPages(s, 1), 1, 2)

 	require.NotNil(t, err)

@@ -431,7 +431,7 @@

 	s, err := NewSiteForCfg(deps.DepsCfg{Cfg: cfg, Fs: fs})

 	require.NoError(t, err)

-	n, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

+	n, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

 	_, err = n.Paginate(createTestPages(s, 2))

 	require.NotNil(t, err)

@@ -458,8 +458,8 @@

 func TestPaginatorFollowedByPaginateShouldFail(t *testing.T) {

 	t.Parallel()

 	s := newTestSite(t, "paginate", 10)

-	n1, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

-	n2, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

+	n1, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

+	n2, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

 	_, err := n1.Paginator()

 	require.Nil(t, err)

@@ -475,8 +475,8 @@

 	t.Parallel()

 	s := newTestSite(t, "paginate", 10)

-	n1, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

-	n2, _ := newPageOutput(s.newHomePage(), false, output.HTMLFormat)

+	n1, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

+	n2, _ := newPageOutput(s.newHomePage(), false, false, output.HTMLFormat)

 	p1 := createTestPages(s, 2)

 	p2 := createTestPages(s, 10)

--- a/hugolib/site_render.go

+++ b/hugolib/site_render.go

@@ -73,7 +73,7 @@

 			// Avoid double work.

 			continue

 		}

-		pageOutput, err := newPageOutput(page, false, outFormat)

+		pageOutput, err := newPageOutput(page, false, false, outFormat)

 		if err == nil {

 			page.mainPageOutput = pageOutput

 			err = pageOutput.renderResources()

@@ -92,6 +92,11 @@

 		for i, outFormat := range page.outputFormats {

+			if outFormat != page.s.rc.Format {

+				// Will be rendered  ... later.

+				continue

+			}

+

 			var (

 				pageOutput *PageOutput

 				err        error

@@ -98,24 +103,20 @@

 			)

 			if i == 0 {

-				pageOutput, err = newPageOutput(page, false, outFormat)

-				page.mainPageOutput = pageOutput

+				pageOutput = page.mainPageOutput

+			} else {

+				pageOutput, err = page.mainPageOutput.copyWithFormat(outFormat, true)

 			}

-			if outFormat != page.s.rc.Format {

-				// Will be rendered  ... later.

+			if err != nil {

+				s.Log.ERROR.Printf("Failed to create output page for type %q for page %q: %s", outFormat.Name, page, err)

 				continue

 			}

 			if pageOutput == nil {

-				pageOutput, err = page.mainPageOutput.copyWithFormat(outFormat)

+				panic("no pageOutput")

 			}

-			if err != nil {

-				s.Log.ERROR.Printf("Failed to create output page for type %q for page %q: %s", outFormat.Name, page, err)

-				continue

-			}

-

 			// We only need to re-publish the resources if the output format is different

 			// from all of the previous (e.g. the "amp" use case).

 			shouldRender := i == 0

@@ -291,7 +292,7 @@

 	htmlOut := output.HTMLFormat

 	htmlOut.BaseName = "404"

-	pageOutput, err := newPageOutput(p, false, htmlOut)

+	pageOutput, err := newPageOutput(p, false, false, htmlOut)

 	if err != nil {

 		return err

 	}

@@ -373,7 +374,7 @@

 	rLayouts := []string{"robots.txt", "_default/robots.txt", "_internal/_default/robots.txt"}

-	pageOutput, err := newPageOutput(p, false, output.RobotsTxtFormat)

+	pageOutput, err := newPageOutput(p, false, false, output.RobotsTxtFormat)

 	if err != nil {

 		return err

 	}

--- a/hugolib/testhelpers_test.go

+++ b/hugolib/testhelpers_test.go

@@ -39,6 +39,8 @@

 	Fs  *hugofs.Fs

 	T   testing.TB

+	logger *jww.Notepad

+

 	dumper litter.Options

 	// Aka the Hugo server mode.

@@ -88,6 +90,11 @@

 	return s

 }

+func (s *sitesBuilder) WithLogger(logger *jww.Notepad) *sitesBuilder {

+	s.logger = logger

+	return s

+}

+

 func (s *sitesBuilder) WithWorkingDir(dir string) *sitesBuilder {

 	s.workingDir = dir

 	return s

@@ -282,7 +289,7 @@

 		s.Cfg = cfg

 	}

-	sites, err := NewHugoSites(deps.DepsCfg{Fs: s.Fs, Cfg: s.Cfg, Running: s.running})

+	sites, err := NewHugoSites(deps.DepsCfg{Fs: s.Fs, Cfg: s.Cfg, Logger: s.logger, Running: s.running})

 	if err != nil {

 		s.Fatalf("Failed to create sites: %s", err)

 	}

-- 

⑨