hugolib: Fix bundle resource publishing when multiple output formats
authorBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Mon, 15 Apr 2019 10:06:12 +0000 (12:06 +0200)
committerBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Mon, 15 Apr 2019 15:01:39 +0000 (17:01 +0200)
The faulty logic published the bundled resources for the "first output" format.

This worked most of the time, but since the output formats list is sorted,
any output format only used for some of the pages (e.g. CSS) would not work properly.

Fixes #5858

hugolib/page.go
hugolib/page__common.go
hugolib/pagebundler_test.go
hugolib/shortcode_test.go
hugolib/site_render.go

index ec4b23d901214dccc194dc5e00c855c76b3ca2cf..537482fb2695e8f53b9160986af4c94dbc820e14 100644 (file)
@@ -360,40 +360,44 @@ func (p *pageState) setPages(pages page.Pages) {
        p.pages = pages
 }
 
-func (p *pageState) renderResources() error {
-       var toBeDeleted []int
-
-       for i, r := range p.Resources() {
-               if _, ok := r.(page.Page); ok {
-                       // Pages gets rendered with the owning page but we count them here.
-                       p.s.PathSpec.ProcessingStats.Incr(&p.s.PathSpec.ProcessingStats.Pages)
-                       continue
-               }
+func (p *pageState) renderResources() (err error) {
+       p.resourcesPublishInit.Do(func() {
+               var toBeDeleted []int
+
+               for i, r := range p.Resources() {
+                       if _, ok := r.(page.Page); ok {
+                               // Pages gets rendered with the owning page but we count them here.
+                               p.s.PathSpec.ProcessingStats.Incr(&p.s.PathSpec.ProcessingStats.Pages)
+                               continue
+                       }
 
-               src, ok := r.(resource.Source)
-               if !ok {
-                       return errors.Errorf("Resource %T does not support resource.Source", src)
-               }
+                       src, ok := r.(resource.Source)
+                       if !ok {
+                               err = errors.Errorf("Resource %T does not support resource.Source", src)
+                               return
+                       }
 
-               if err := src.Publish(); err != nil {
-                       if os.IsNotExist(err) {
-                               // The resource has been deleted from the file system.
-                               // This should be extremely rare, but can happen on live reload in server
-                               // mode when the same resource is member of different page bundles.
-                               toBeDeleted = append(toBeDeleted, i)
+                       if err := src.Publish(); err != nil {
+                               if os.IsNotExist(err) {
+                                       // The resource has been deleted from the file system.
+                                       // This should be extremely rare, but can happen on live reload in server
+                                       // mode when the same resource is member of different page bundles.
+                                       toBeDeleted = append(toBeDeleted, i)
+                               } else {
+                                       p.s.Log.ERROR.Printf("Failed to publish Resource for page %q: %s", p.pathOrTitle(), err)
+                               }
                        } else {
-                               p.s.Log.ERROR.Printf("Failed to publish Resource for page %q: %s", p.pathOrTitle(), err)
+                               p.s.PathSpec.ProcessingStats.Incr(&p.s.PathSpec.ProcessingStats.Files)
                        }
-               } else {
-                       p.s.PathSpec.ProcessingStats.Incr(&p.s.PathSpec.ProcessingStats.Files)
                }
-       }
 
-       for _, i := range toBeDeleted {
-               p.deleteResource(i)
-       }
+               for _, i := range toBeDeleted {
+                       p.deleteResource(i)
+               }
 
-       return nil
+       })
+
+       return
 }
 
 func (p *pageState) deleteResource(i int) {
index 5bd7223cc2df4c8cd9621b470c4421107e969794..01280bb407ced032027746546214eafbacca2598 100644 (file)
@@ -91,8 +91,9 @@ type pageCommon struct {
        pagesInit sync.Once
 
        // Any bundled resources
-       resources     resource.Resources
-       resourcesInit sync.Once
+       resources            resource.Resources
+       resourcesInit        sync.Once
+       resourcesPublishInit sync.Once
 
        translations    page.Pages
        allTranslations page.Pages
index 870ea3de9a776f226c6a4da9cc9ae22d74de881f..06bd641542f1a805c0e8106b7a54a4dde2e960ec 100644 (file)
@@ -896,3 +896,37 @@ TheContent.
 
        return ps, clean, workDir
 }
+
+// https://github.com/gohugoio/hugo/issues/5858
+func TestBundledResourcesWhenMultipleOutputFormats(t *testing.T) {
+       t.Parallel()
+
+       b := newTestSitesBuilder(t).Running().WithConfigFile("toml", `
+baseURL = "https://example.org"
+[outputs]
+  # This looks odd, but it triggers the behaviour in #5858
+  # The total output formats list gets sorted, so CSS before HTML.
+  home = [ "CSS" ]
+
+`)
+       b.WithContent("mybundle/index.md", `
+---
+title: Page
+date: 2017-01-15
+---
+`,
+               "mybundle/data.json", "MyData",
+       )
+
+       b.CreateSites().Build(BuildCfg{})
+
+       b.AssertFileContent("public/mybundle/data.json", "MyData")
+
+       // Change the bundled JSON file and make sure it gets republished.
+       b.EditFiles("content/mybundle/data.json", "My changed data")
+
+       b.Build(BuildCfg{})
+
+       b.AssertFileContent("public/mybundle/data.json", "My changed data")
+
+}
index 3320e642c44f22cfafba3a0e01a44577691e8c7a..dd01eb5c504a2512da9e9c6e2fa9f8d7257cb495 100644 (file)
@@ -903,6 +903,7 @@ func TestShortcodeParentResourcesOnRebuild(t *testing.T) {
        b.WithTemplatesAdded(
                "index.html", `
 {{ $b := .Site.GetPage "b1" }}
+b1 Content: {{ $b.Content }}
 {{$p := $b.Resources.GetMatch "p1*" }}
 Content: {{ $p.Content }}
 {{ $article := .Site.GetPage "blog/article" }}
@@ -933,20 +934,23 @@ SHORTCODE: {{< c >}}
 
        b.Build(BuildCfg{})
 
-       assert := func() {
-               b.AssertFileContent("public/index.html",
-                       "Parent resource: logo.png: /b1/logo.png",
+       assert := func(matchers ...string) {
+               allMatchers := append(matchers, "Parent resource: logo.png: /b1/logo.png",
                        "Article Content: <p>SHORTCODE: \n\n* Parent resource: logo-article.png: /blog/logo-article.png",
                )
+
+               b.AssertFileContent("public/index.html",
+                       allMatchers...,
+               )
        }
 
        assert()
 
-       b.EditFiles("b1/index.md", pageContent+" Edit.")
+       b.EditFiles("content/b1/index.md", pageContent+" Edit.")
 
        b.Build(BuildCfg{})
 
-       assert()
+       assert("Edit.")
 
 }
 
index 407061b677874cfefd36eb46b656bc1bfb1bbb58..9ade951df1a510e9b07f67cea3030836e22ebfe0 100644 (file)
@@ -35,6 +35,7 @@ type siteRenderContext struct {
        sitesOutIdx int
 
        // Zero based index of the output formats configured within a Site.
+       // Note that these outputs are sorted, so CSS will come before HTML.
        outIdx int
 
        multihost bool
@@ -130,11 +131,9 @@ func pageRenderer(
                        continue
                }
 
-               if ctx.outIdx == 0 {
-                       if err := p.renderResources(); err != nil {
-                               s.SendError(p.errorf(err, "failed to render page resources"))
-                               continue
-                       }
+               if err := p.renderResources(); err != nil {
+                       s.SendError(p.errorf(err, "failed to render page resources"))
+                       continue
                }
 
                layouts, err := p.getLayouts()