-
Notifications
You must be signed in to change notification settings - Fork 14.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump Docsy to 0.4.x #48722
base: main
Are you sure you want to change the base?
Bump Docsy to 0.4.x #48722
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
LGTM label has been added. Git tree hash: 425988602e055e2c0ebf8c13b2c9ae6e88258acc
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the changes suggested below, we can then successfully run both local and container build and serve.
@@ -34,7 +34,7 @@ module-init: ## Initialize required submodules. | |||
all: build ## Build site with production settings and put deliverables in ./public | |||
|
|||
build: module-check ## Build site with non-production settings and put deliverables in ./public | |||
hugo --cleanDestinationDir --minify --environment development | |||
hugo --cleanDestinationDir --minify --environment development --themesdir node_modules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hugo --cleanDestinationDir --minify --environment development --themesdir node_modules | |
hugo --cleanDestinationDir --minify --environment development --themesDir node_modules |
"dependencies": { | ||
"docsy": "github:google/docsy#v0.4.0" | ||
}, | ||
"devDependencies": { | ||
"autoprefixer": "^9.8.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All are devDependencies
:
"dependencies": { | |
"docsy": "github:google/docsy#v0.4.0" | |
}, | |
"devDependencies": { | |
"autoprefixer": "^9.8.4", | |
"devDependencies": { | |
"autoprefixer": "^9.8.4", | |
"docsy": "google/docsy#semver:0.4.0", |
|
||
FROM docker.io/library/golang:1.23.0-alpine3.20 | ||
# was previously based on his Dockerfile at | ||
# https://github.com/jguyomard/docker-hugo/blob/master/Dockerfile. | ||
|
||
LABEL maintainer="Luc Perkins <[email protected]>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line causes an error for me -- I have to comment it out.
Btw, Luc isn't maintainer anymore, and no longer works for the LF/CNCF.
npm && \ | ||
npm install -D autoprefixer postcss-cli | ||
WORKDIR /opt/npm | ||
RUN npm install -D -g autoprefixer postcss-cli google/docsy#semver:0.4.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RUN npm install -D -g autoprefixer postcss-cli google/docsy#semver:0.4.0 | |
RUN npm install -g autoprefixer postcss-cli google/docsy#semver:0.4.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually found a better way (IMHO), which copies the project's packages files rather that duplicating dependencies here. I'll share that tomorrow, I'm calling it a day.
@@ -97,11 +97,11 @@ docker-push: ## Build a multi-architecture image and push that into the registry | |||
rm Dockerfile.cross | |||
|
|||
container-build: module-check | |||
$(CONTAINER_RUN) --read-only --mount type=tmpfs,destination=/tmp,tmpfs-mode=01777 $(CONTAINER_IMAGE) sh -c "npm ci && hugo --minify --environment development" | |||
$(CONTAINER_RUN) --read-only --mount type=tmpfs,destination=/tmp,tmpfs-mode=01777 $(CONTAINER_IMAGE) sh -c "npm ci && hugo --minify --environment development --themesDir /usr/local/lib/node_modules" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/src
is read-only so we can't run npm ci
here (and don't need to):
$(CONTAINER_RUN) --read-only --mount type=tmpfs,destination=/tmp,tmpfs-mode=01777 $(CONTAINER_IMAGE) sh -c "npm ci && hugo --minify --environment development --themesDir /usr/local/lib/node_modules" | |
$(CONTAINER_RUN) --read-only --mount type=tmpfs,destination=/tmp,tmpfs-mode=01777 $(CONTAINER_IMAGE) sh -c "hugo --minify --environment development --destination /tmp/hugo --themesDir /usr/local/lib/node_modules --noBuildLock" |
Actually, here's the proposal I made earlier that copies the website's NPM package files for use in the container image; this avoids a global install (which isn't usually a good thing for non CLI packages), and keeps things more DRY: diff --git a/.dockerignore b/.dockerignore
index 1d085cacc9..be2f2c26c4 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -1 +1,3 @@
**
+!package.json
+!package-lock.json diff --git a/Dockerfile b/Dockerfile
index 9e07877af0..9cde91c750 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -25,8 +25,11 @@ RUN apt-get update && apt-get install -y \
RUN rm -rf /var/cache/* # partial cleanup
-WORKDIR /opt/npm
-RUN npm install -g autoprefixer postcss-cli google/docsy#semver:0.4.0
+RUN mkdir -p /npm-dep
+COPY package.json package-lock.json /npm-dep
+
+WORKDIR /npm-dep
+RUN npm install
RUN useradd -m --user-group -u 60000 -d /var/hugo hugo && \
chown -R hugo: /var/hugo && \ diff --git a/Makefile b/Makefile
index e5716e1193..c9e52333da 100644
--- a/Makefile
+++ b/Makefile
@@ -101,7 +101,7 @@ container-build: module-check
# no build lock to allow for read-only mounts
container-serve: module-check ## Boot the development server using container.
- $(CONTAINER_RUN) --cap-drop=ALL --cap-add=AUDIT_WRITE --read-only --mount type=tmpfs,destination=/tmp,tmpfs-mode=01777 -p 1313:1313 $(CONTAINER_IMAGE) hugo server --buildFuture --environment development --themesDir /usr/local/lib/node_modules --bind 0.0.0.0 --destination /tmp/hugo --cleanDestinationDir --noBuildLock
+ $(CONTAINER_RUN) --cap-drop=ALL --cap-add=AUDIT_WRITE --read-only --mount type=tmpfs,destination=/tmp,tmpfs-mode=01777 -p 1313:1313 $(CONTAINER_IMAGE) hugo server --buildFuture --environment development --themesDir /npm-dep/node_modules --bind 0.0.0.0 --destination /tmp/hugo --cleanDestinationDir --noBuildLock
test-examples:
scripts/test_examples.sh install |
This PR upgrades to a newer version of Docsy: 0.4.0 (preview)
Fixes #32905
Importantly, the container image now installs dependencies using NPM, not a Git submodule.
In the testing I've done, I haven't found any different look or behavior that still needs fixing.
/area web-development
This PR incorporates the commits from PR #48721