Closed Bug 1591538 Opened 1 year ago Closed 11 months ago

Improve the SpiderMonkey tarball

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox72 fixed)

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: nox, Assigned: nox)

Details

Attachments

(9 files)

I'll push a few patches to that issue to slim down the tarball generated by make-source-package.sh and improve it a bit.

This is what brew installs through the autoconf213 package.

/testing/taskcluster/tasks/branches/base_jobs.yml doesn't exist
anymore.

${tgtpath} always contain "js" by default, so this command is doing
absolutely nothing for most people out there, and if it was working
it would be deleting every Cargo.toml file from /third_party/rust
anyway.

This lets us exclude things more surgically.

This was done in bug 956597 where including its files won over
massaging the imports to not be reached from the tarball's
build system.

There are now only three references to mozwebidlcodegen in the
tarball:

  • /python/mozbuild/mozbuild/frontend/data.py in which it is
    imported from a method on WebIDLCollection (there are no
    WebIDL collections in the tarball);

  • /python/mozbuild/mozbuild/frontend/emitter.py to emit
    WebIDL collections;

  • /python/mozbuild/mozbuild/backend/common.py in which it was
    made local to the single function using it in bug 1272976.

I don't think it is necessary to include it anymore, but given
it's only 56KB I'm not sure it matters either way.

We can just not add the directory to DIRS if JS_STANDALONE is true.

Its only reference is an include that has been gated not to apply
to SM tarballs in bug 1273006. Unfortunately we can't omit
package-name.mk because that seems to be used when the crash
reporter is enabled, which can be when using autospider.py as
far as I understand from bug 1410528, but I could be wrong about
that.

This Python package takes 8MB and is not used by SM at all, as stated
in bug 956597.

Remove tests (they are not run AFAIK), the tools (they are not used),
and the Unicode data (we don't look at them, we use the compiled version
from /config/external/icu/data).

Attachment #9104504 - Flags: review?(sphink)
Attachment #9104505 - Flags: review?(sphink)
Attachment #9104506 - Flags: review?(sphink)
Attachment #9104507 - Flags: review?(sphink)
Attachment #9104508 - Flags: review?(sphink)
Attachment #9104509 - Flags: review?(sphink)
Attachment #9104510 - Flags: review?(sphink)
Attachment #9104511 - Flags: review?(sphink)
Attachment #9104512 - Flags: review?(sphink)
Attachment #9104504 - Attachment description: Bug 1591538 - Also try autoconf213 when making the SM tarball → Bug 1591538 - Also try autoconf213 when making the SM tarball;
Attachment #9104505 - Attachment description: Bug 1591538 - Remove obsolete comment in make-source-package.sh → Bug 1591538 - Fix obsolete comment in make-source-package.sh;
Attachment #9104506 - Attachment description: Bug 1591538 - Remove removal of Cargo.toml files in the SM tarball → Bug 1591538 - Remove removal of Cargo.toml files in the SM tarball;
Attachment #9104507 - Attachment description: Bug 1591538 - Use rsync to generate the SM tarball → Bug 1591538 - Use rsync to generate the SM tarball;
Attachment #9104508 - Attachment description: Bug 1591538 - Don't include mozwebidlcodegen in SM tarball → Bug 1591538 - Don't include mozwebidlcodegen in SM tarball;
Attachment #9104509 - Attachment description: Bug 1591538 - Don't include taskcluster stuff in SM tarball → Bug 1591538 - Don't include taskcluster stuff in SM tarball;
Attachment #9104510 - Attachment description: Bug 1591538 - Don't include upload-files.mk in SM tarball → Bug 1591538 - Don't include upload-files.mk in SM tarball;
Attachment #9104511 - Attachment description: Bug 1591538 - Don't include gyp anymore in SM tarball → Bug 1591538 - Don't include gyp anymore in SM tarball;
Attachment #9104512 - Attachment description: Bug 1591538 - Trim down ICU → Bug 1591538 - Trim down ICU;

I wanted to do a sm-package-linux64 try run before landing but apparently I don't have try privileges anymore, so I'll look at that on Monday.

Successfully ran SM(pkg) in a try run, I am confident I can land this now.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c0f9eb35bf8ef632f0ae8d281ff61619cb9b4dcd

Pushed by aramine@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0356b9960dfd
Also try autoconf213 when making the SM tarball; r=sfink
https://hg.mozilla.org/integration/autoland/rev/48984ec2df7a
Fix obsolete comment in make-source-package.sh; r=sfink
https://hg.mozilla.org/integration/autoland/rev/08a184798910
Remove removal of Cargo.toml files in the SM tarball; r=sfink
https://hg.mozilla.org/integration/autoland/rev/752245d75539
Use rsync to generate the SM tarball; r=sfink
https://hg.mozilla.org/integration/autoland/rev/a475978e4f2f
Don't include mozwebidlcodegen in SM tarball; r=sfink
https://hg.mozilla.org/integration/autoland/rev/cbf72b9e705a
Don't include taskcluster stuff in SM tarball; r=sfink
https://hg.mozilla.org/integration/autoland/rev/3f447c49dc77
Don't include upload-files.mk in SM tarball; r=sfink
https://hg.mozilla.org/integration/autoland/rev/f919b76a1ec6
Don't include gyp anymore in SM tarball; r=sfink
https://hg.mozilla.org/integration/autoland/rev/48919539e15c
Trim down ICU; r=sfink

Thank you for the improvements, the tarball has always been a bit of a bloated mess. It was very understandable if someone downloaded and unpacked the tarball, then looked at the directory listing and thought there must be some kind of mistake. ;-)

You're welcome, learnt a few things about rsync in the process (truth is that I was too lazy to read the damn man page in depth). I have a few more trimming ideas but I need to clean up the patches first, I'll properly request reviews next batch of changes. :)

Anthony, do you know if we have a treeherder job doing that automatically? (for users and to make sure we don't regress)
merci!

That's what SM(pkg) is supposed to be, AFAIK.

Comment on attachment 9104504 [details]
Bug 1591538 - Also try autoconf213 when making the SM tarball;

(clearing out redundant review request flags to make my bugzilla number of shame go down)

Attachment #9104504 - Flags: review?(sphink)
Attachment #9104505 - Flags: review?(sphink)
Attachment #9104506 - Flags: review?(sphink)
Attachment #9104507 - Flags: review?(sphink)
Attachment #9104508 - Flags: review?(sphink)
Attachment #9104509 - Flags: review?(sphink)
Attachment #9104510 - Flags: review?(sphink)
Attachment #9104511 - Flags: review?(sphink)
Attachment #9104512 - Flags: review?(sphink)
You need to log in before you can comment on or make changes to this bug.