Merge ~weii-wang/charm-k8s-discourse:fix-terser-precompile into charm-k8s-discourse:master

Proposed by Weii Wang
Status: Merged
Approved by: Tom Haddon
Approved revision: ada4ba97b28ab53dd0a619b3a9325b7a981afac1
Merge reported by: Tom Haddon
Merged at revision: ada4ba97b28ab53dd0a619b3a9325b7a981afac1
Proposed branch: ~weii-wang/charm-k8s-discourse:fix-terser-precompile
Merge into: charm-k8s-discourse:master
Diff against target: 80 lines (+10/-26)
4 files modified
dev/null (+0/-13)
image/build_scripts/build_app (+0/-3)
image/build_scripts/get_app_dependencies (+9/-1)
image/scripts/pod_setup (+1/-9)
Reviewer Review Type Date Requested Status
Tom Haddon Approve
Barry Price Approve
Franco Luciano Forneron Buschiazzo Pending
Review via email: mp+427723@code.launchpad.net

Commit message

Enable terser to uglify JS assets

Set up the terser package and the SKIP_NODE_UGLIFY environment variable
correctly so discourse will use terser instead of uglifyjs to compress
all the Javascript assets.

LP: #1952681

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Barry Price (barryprice) wrote :

Thanks for your work on this, looks good to me - couple of minor nitpicks inline below.

review: Approve
Revision history for this message
Weii Wang (weii-wang) wrote :

> Thanks for your work on this, looks good to me - couple of minor nitpicks
> inline below.

I made some modifications based on that, new commit has been squashed and uploaded.
Thanks!

Revision history for this message
Tom Haddon (mthaddon) wrote :

LGTM, thx

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision d86ae45b79d2685989b869200a3a2566f832ed3b

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/image/build_scripts/build_app b/image/build_scripts/build_app
index 9d6d04a..6a0333c 100755
--- a/image/build_scripts/build_app
+++ b/image/build_scripts/build_app
@@ -16,9 +16,6 @@ git clone --depth 1 --branch ${CONTAINER_APP_VERSION} https://github.com/discour
16# Apply patch for LP#190369516# Apply patch for LP#1903695
17git -C "${CONTAINER_APP_VERSION}" apply /srv/build_scripts/lp1903695.patch17git -C "${CONTAINER_APP_VERSION}" apply /srv/build_scripts/lp1903695.patch
1818
19# Apply patch to use terser rather than uglifier
20git -C "${CONTAINER_APP_VERSION}" apply /srv/build_scripts/terser.patch
21
22chown -R ${CONTAINER_APP_USERNAME}:${CONTAINER_APP_GROUP} ${CONTAINER_APP_VERSION}19chown -R ${CONTAINER_APP_USERNAME}:${CONTAINER_APP_GROUP} ${CONTAINER_APP_VERSION}
23ln -s ${CONTAINER_APP_ROOT}/${CONTAINER_APP_VERSION} app20ln -s ${CONTAINER_APP_ROOT}/${CONTAINER_APP_VERSION} app
24cd ${CONTAINER_APP_ROOT}/app21cd ${CONTAINER_APP_ROOT}/app
diff --git a/image/build_scripts/get_app_dependencies b/image/build_scripts/get_app_dependencies
index 8f9a0f6..2295656 100755
--- a/image/build_scripts/get_app_dependencies
+++ b/image/build_scripts/get_app_dependencies
@@ -16,7 +16,7 @@ apt-get install -y brotli \
16 libxml2-dev \16 libxml2-dev \
17 libxslt1-dev \17 libxslt1-dev \
18 libz-dev \18 libz-dev \
19 node-terser \19 uglifyjs.terser \
20 node-uglify \20 node-uglify \
21 optipng \21 optipng \
22 pngquant \22 pngquant \
@@ -27,3 +27,11 @@ apt-get install -y brotli \
27 ruby2.7-dev \27 ruby2.7-dev \
28 zlib1g-dev28 zlib1g-dev
2929
30# Older versions of the uglifyjs.terser package install a uglifyjs.terser
31# command but not the terser command, terser command exists in $PATH is
32# vital to trigger js assets compression using node. Manually create the
33# terser command if it does not exist. Please remove this line if the
34# base image is >= 22.04. Also, please consider removing the node-uglify
35# when upgrading to discourse >= 2.8.0, since the node-uglify is not
36# required to trigger node js assets compression, only terser will do fine.
37which terser || ln -s $(which uglifyjs.terser) /usr/local/bin/terser
30\ No newline at end of file38\ No newline at end of file
diff --git a/image/build_scripts/terser.patch b/image/build_scripts/terser.patch
31deleted file mode 10064439deleted file mode 100644
index 55e20ff..0000000
--- a/image/build_scripts/terser.patch
+++ /dev/null
@@ -1,13 +0,0 @@
1diff --git a/config/environments/production.rb b/config/environments/production.rb
2index a523888a8d..7f41985b17 100644
3--- a/config/environments/production.rb
4+++ b/config/environments/production.rb
5@@ -14,7 +14,7 @@ Discourse::Application.configure do
6 # Disable Rails's static asset server (Apache or nginx will already do this)
7 config.public_file_server.enabled = GlobalSetting.serve_static_assets || false
8
9- config.assets.js_compressor = :uglifier
10+ config.assets.js_compressor = :terser
11
12 # stuff should be pre-compiled
13 config.assets.compile = false
diff --git a/image/scripts/pod_setup b/image/scripts/pod_setup
index 4893723..ca5f8d4 100755
--- a/image/scripts/pod_setup
+++ b/image/scripts/pod_setup
@@ -36,15 +36,7 @@ else
36fi36fi
37### MIGRATION LOCK WORKAROUND ENDS HERE37### MIGRATION LOCK WORKAROUND ENDS HERE
3838
39# It is safe to build assets in the background to improve startup time.39su -s /bin/bash -c "bin/bundle exec rake assets:precompile RAILS_ENV=$RAILS_ENV" ${CONTAINER_APP_USERNAME} 2>&1 | sed 's/^/asset-build: /'
40export SKIP_NODE_UGLIFY="true"
41
42# LP#1952681: First run consistently fails to process discourse/tests/theme_qunit_helper JS
43su -s /bin/bash -c "bin/bundle exec rake assets:precompile RAILS_ENV=$RAILS_ENV" ${CONTAINER_APP_USERNAME} 2>&1 |sed 's/^/asset-build-1: /'
44# LP#1952681: Second run consistently fails to process application JS
45su -s /bin/bash -c "bin/bundle exec rake assets:precompile RAILS_ENV=$RAILS_ENV" ${CONTAINER_APP_USERNAME} 2>&1 |sed 's/^/asset-build-2: /'
46# LP#1952681: Third run consistently succeeds
47su -s /bin/bash -c "bin/bundle exec rake assets:precompile RAILS_ENV=$RAILS_ENV" ${CONTAINER_APP_USERNAME} 2>&1 |sed 's/^/asset-build-3: /'
4840
49if [ ! -z "${DISCOURSE_USE_S3}" ] && [ "${DISCOURSE_USE_S3}" == "true" ]; then41if [ ! -z "${DISCOURSE_USE_S3}" ] && [ "${DISCOURSE_USE_S3}" == "true" ]; then
50 echo "Discourse is configured to use S3:"42 echo "Discourse is configured to use S3:"

Subscribers

People subscribed via source and target branches