Updating Hugo detection logic and bumping up version #2009

harryli0108 · 2023-05-31T20:22:53Z

Due to new Hugo versions changed the config name convention from config.toml etc. to hugo.toml etc., this PR updates the detection logic and make sure the new naming convention is also supported while not affecting the old ones. This PR also added a new modified sample app and a test case in order to make sure new naming convention is correctly supported.

And because this change starts from Hugo version 0.110.0, this PR also upgrades the current hugo version to the latest 0.112.5.

cormacpayne

A few comments to take a look at

build/constants.yaml

src/Detector/Hugo/HugoDetector.cs

cormacpayne · 2023-05-31T20:41:52Z

doc/runtimes/hugo.md

@@ -16,6 +16,8 @@ The Hugo toolset is run when one of following conditions met:
 2. Recursively look up config directory: `config/**/*.yaml`, `config/**/*.toml`,
   `config/**/*.yml` or `config/**/*.json` configuration files in sub-directories.

+**Note**: Starting from [Hugo version `0.110.0`](https://gohugo.io/getting-started/configuration/#hugotoml-vs-configtoml), configuration file name has changed from `config.toml` etc. to `hugo.toml` etc. Please note the old naming convention is still supported.


We could probably scrap this line in favor of updating the (1.) bullet above to mention that we look for the hugo.* equivalent files.

cormacpayne · 2023-05-31T20:42:31Z

doc/runtimes/hugo.md

@@ -16,6 +16,8 @@ The Hugo toolset is run when one of following conditions met:
 2. Recursively look up config directory: `config/**/*.yaml`, `config/**/*.toml`,
   `config/**/*.yml` or `config/**/*.json` configuration files in sub-directories.

+**Note**: Starting from [Hugo version `0.110.0`](https://gohugo.io/getting-started/configuration/#hugotoml-vs-configtoml), configuration file name has changed from `config.toml` etc. to `hugo.toml` etc. Please note the old naming convention is still supported.


nit: let's keep the maximum length of a line in Markdown to 120 characters

pauld-msft

Few minor things. Overall, looks good to me, so happy to approve once a few minor things are resolved.

Also, thanks for adding the test!

Edit: also noticed builds are failing, and cormac had some good comments to look at :)

src/Detector/Hugo/HugoDetector.cs

build/constants.yaml

src/Detector/Hugo/HugoDetector.cs

daniv-msft · 2023-05-31T23:52:57Z

doc/runtimes/hugo.md

@@ -12,7 +12,7 @@ details on components and configuration of build images.

 The Hugo toolset is run when one of following conditions met:

-1. `config.yaml`, `config.toml`, `config.yml` or `config.json` configuration files in root of repo.
+1. `config.yaml`, `config.toml`, `config.yml`, `config.json` or `hugo.*` equivalent configuration files in root of repo.


Please consider writing specifically hugo.toml, hugo.yaml, or hugo.json. I feel like in the doc being very precise about what we support is important, especially as customers might look for specific strings/find these from search engines.

That makes sense! Updating it right now

daniv-msft · 2023-06-01T00:04:26Z

tests/Detector.Tests/Hugo/HugoDetectorTest.cs

@@ -78,8 +79,8 @@ public void IsHugoApp_ReturnsTrue_ForAllSupportedConfigurationKeys_InTomlFile(st
        }

        [Theory]
-        [InlineData(HugoConstants.JsonFileName)]
-        [InlineData(HugoConstants.ConfigFolderName, HugoConstants.JsonFileName)]
+        [InlineData("config.json")]


Just curious: we can't directly use HugoConstants.JsonFileNames.First() in these InlineData fields to remain consistent?

So the InlineData requires const variables, and since HugoConstants.JsonFileNames and other file names are now a static readonly list, it doesn't allow to use it there.

daniv-msft

Looks good to me! Thanks for fixing this

waliMSFT

Looks good to me as well.

cormacpayne

A few comments we should take care of before merging

cormacpayne · 2023-06-01T17:39:59Z

doc/runtimes/hugo.md

@@ -12,7 +12,7 @@ details on components and configuration of build images.

 The Hugo toolset is run when one of following conditions met:

-1. `config.yaml`, `config.toml`, `config.yml` or `config.json` configuration files in root of repo.
+1. `config/hugo.yaml`, `config/hugo.toml`, `config/hugo.yml` or `config/hugo.json` configuration files in root of repo.


I actually think we should word this line differently as users may interpret the config part of each file type as the directory name, especially because our second bullet below mentions we search the config directory recursively.

I'd be in favor of writing this as something like the following:

1. Any supported config.* files at the root of the repository: config.toml, config.yaml, config.yml, config.json
2. Any supported hugo.* files at the root of the repository: hugo.toml, hugo.yaml, hugo.yml, hugo.json
3. Recursively look up supported files in the config directory: config/**/*.toml, config/**/*.toml
config/**/*.yml, config/**/*.json

cormacpayne · 2023-06-01T17:41:48Z

tests/Detector.Tests/Hugo/HugoDetectorTest.cs

-        [InlineData(HugoConstants.TomlFileName)]
-        [InlineData(HugoConstants.ConfigFolderName, HugoConstants.TomlFileName)]
+        [InlineData("config.toml")]
+        [InlineData(HugoConstants.ConfigFolderName, "config.toml")]
        public void IsHugoApp_ReturnsTrue_ForAppWithConfigTomlFile(params string[] subPaths)


Let's introduce the same set of tests that we have in this file but for the hugo.* files that we now support to ensure that we're detecting them properly.

cormacpayne · 2023-06-01T17:45:29Z

tests/Detector.Tests/Hugo/HugoDetectorTest.cs

@@ -63,7 +64,7 @@ public void IsHugoApp_ReturnsTrue_ForAllSupportedConfigurationKeys_InTomlFile(st
        {
            // Arrange
            var appDir = CreateAppDir();
-            WriteFile($"{configurationKeyName}=\"test\"", appDir, HugoConstants.TomlFileName);
+            WriteFile($"{configurationKeyName}=\"test\"", appDir, HugoConstants.TomlFileNames.First());


We should extract out the name of the file to be an additional argument of this test to ensure that the support configuration keys work for all versions of .toml file.

This comment also applies for the other tests below where we've made the same change.

cormacpayne

jpconstantineau · 2023-06-09T15:48:05Z

Has this been released yet?
I am having an issue that this PR solves. (wrong/old hugo version)
How do I get my github action to use this fix? Will it once it get released?

harryli0108 · 2023-06-09T18:26:06Z

@jpconstantineau Hi! Sorry for the delay here. We have a release inbound today that includes this fix.
In order to use this in GH Action, depending on if you're using Azure Web Apps task or the new Container App task. With the Web Apps one, I think you might need to wait for the App service team to include this fix in their new release. But with Container Apps task, you can either specify the oryx build image used in your Dockerfile, or you might need to wait for our team to release the next builder.

Meanwhile, I would encourage you to post this as an issue if you continue to experience it since comments in closed PRs could be accidentally ignored.

jpconstantineau · 2023-06-09T18:33:16Z

Thanks. It's web app indeed. I'll wait for the release here first. Hopefully, the GitHub action doesn't need updating itself but simply picks up the latest release (or the one the web app team points to through config)

Updating Hugo detection logic and bumping up version

ed4a437

harryli0108 requested a review from a team as a code owner May 31, 2023 20:22

Update hugo.md

1b3965e

harryli0108 mentioned this pull request May 31, 2023

Hugo (Golang SSG) does not get detected due to new hugo.toml that replaces config.toml #2007

Closed

cormacpayne suggested changes May 31, 2023

View reviewed changes

pauld-msft reviewed May 31, 2023

View reviewed changes

src/Detector/Hugo/HugoDetector.cs Outdated Show resolved Hide resolved

build/constants.yaml Outdated Show resolved Hide resolved

src/Detector/Hugo/HugoDetector.cs Outdated Show resolved Hide resolved

harryli0108 added 2 commits May 31, 2023 15:44

Addressing comments

1521a9f

Updated md

1992e7d

daniv-msft reviewed May 31, 2023

View reviewed changes

daniv-msft reviewed Jun 1, 2023

View reviewed changes

daniv-msft previously approved these changes Jun 1, 2023

View reviewed changes

waliMSFT reviewed Jun 1, 2023

View reviewed changes

Addressing comments

379c531

harryli0108 dismissed daniv-msft’s stale review via 379c531 June 1, 2023 17:24

cormacpayne suggested changes Jun 1, 2023

View reviewed changes

Updated test cases and md file

46fca3b

cormacpayne approved these changes Jun 2, 2023

View reviewed changes

harryli0108 merged commit a4feb35 into main Jun 5, 2023
1 check passed

malte610 mentioned this pull request Jul 7, 2023

The Oryx version used does not detect hugo apps correctly. Azure/static-web-apps#1219

Closed

snehapar9 mentioned this pull request Jul 24, 2023

Errors when building hugo on Azure Static Sites #2078

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating Hugo detection logic and bumping up version #2009

Updating Hugo detection logic and bumping up version #2009

harryli0108 commented May 31, 2023 •

edited

Loading

cormacpayne left a comment

cormacpayne May 31, 2023

cormacpayne May 31, 2023

pauld-msft left a comment •

edited

Loading

daniv-msft May 31, 2023 •

edited

Loading

harryli0108 Jun 1, 2023

daniv-msft Jun 1, 2023

harryli0108 Jun 1, 2023

daniv-msft left a comment

waliMSFT left a comment

cormacpayne left a comment

cormacpayne Jun 1, 2023

cormacpayne Jun 1, 2023

cormacpayne Jun 1, 2023

cormacpayne left a comment

jpconstantineau commented Jun 9, 2023

harryli0108 commented Jun 9, 2023

jpconstantineau commented Jun 9, 2023

Updating Hugo detection logic and bumping up version #2009

Updating Hugo detection logic and bumping up version #2009

Conversation

harryli0108 commented May 31, 2023 • edited Loading

cormacpayne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pauld-msft left a comment • edited Loading

Choose a reason for hiding this comment

daniv-msft May 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniv-msft left a comment

Choose a reason for hiding this comment

waliMSFT left a comment

Choose a reason for hiding this comment

cormacpayne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cormacpayne left a comment

Choose a reason for hiding this comment

jpconstantineau commented Jun 9, 2023

harryli0108 commented Jun 9, 2023

jpconstantineau commented Jun 9, 2023

harryli0108 commented May 31, 2023 •

edited

Loading

pauld-msft left a comment •

edited

Loading

daniv-msft May 31, 2023 •

edited

Loading