Closed Bug 1467205 Opened 2 years ago Closed 2 years ago

minify non-JS files on desktop builds

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Shipping smaller installers is a win, and we already do this on
Fennec.  (The only files that get modified currently are .properties
files, which have their comment lines stripped.)  We don't minify JS
files at the moment due to needing to have a larger conversation around
debuggability of JS, and because the setup for minifying needs more love
for non-Android platforms.

Just doing this, however, wins about 400k of on-disk space and ~100k
installer size, so it seems like a reasonable first step.
r? to build peers; r? to Pike to make sure that we're OK stripping all the
comments from .properties files.  These comments typically contain localization
notes, and while I suspect that localizers work from the .properties files in
the source tree rather than the files in omni.ja, it'd be good to have someone
verify that suspicion.

Pike, please do redirect if I asked the wrong person, thanks!
Attachment #8983876 - Flags: review?(l10n)
Attachment #8983876 - Flags: review?(core-build-config-reviews)
Comment on attachment 8983876 [details] [diff] [review]
minify non-JS files on desktop builds

Review of attachment 8983876 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/installer/Makefile.in
@@ +113,5 @@
> +# Don't minify JS yet, due to concerns about debuggability.
> +# Also, the JS minification setup really only works correctly on Android:
> +# we need extra setup to use the newly-built shell for Linux and Windows,
> +# and Mac requires some extra care due to cross-compilation.
> +

This comment here makes it a little unclear how it relates to the variables MOZ_PACKAGER_MINIFY and MOZ_PACKAGER_MINIFY_JS, can we make it mention those or put it closer to something relevant happening in this file?
Attachment #8983876 - Flags: review?(core-build-config-reviews) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb0f9b9ac3c2
minify non-JS files on desktop builds; r=chmanchester
https://hg.mozilla.org/mozilla-central/rev/bb0f9b9ac3c2
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Comment on attachment 8983876 [details] [diff] [review]
minify non-JS files on desktop builds

We seems to have landed this and gone through uplift without significant problems.
Attachment #8983876 - Flags: review?(l10n)
Blocks: 1505119
You need to log in before you can comment on or make changes to this bug.