XULRunner Linux SDK packages need to go on a diet

RESOLVED FIXED in Firefox 21

Status

RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: WeirdAl, Assigned: glandium)

Tracking

unspecified
mozilla22
x86_64
Linux

Firefox Tracking Flags

(firefox21 fixed, firefox22+ fixed)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
I hope there's a legitimate reason for the Linux SDK's packages to be over six times the size of the Mac & Windows SDK's.  Especially since the corresponding runtime packages are much more reasonably sized.

khuey, can you think of a build engineer willing to investigate?
(Reporter)

Comment 1

6 years ago
libxul.so seems to be eating 99% of the SDK's size.
Are the binaries stripped?  I seem to remember that they aren't.
(Reporter)

Comment 3

6 years ago
Kyle, I have no idea what that means.
Stripping a binary removes the embedded debugging symbols.  I think you can use | file libxul.so | to see if the binary is stripped.
(Reporter)

Comment 5

6 years ago
FF17 beta 6 SDK:
[ajvincent@Fedora-64 bin]$ file libxul.so 
libxul.so: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, stripped
(Reporter)

Comment 6

6 years ago
There's something odd going on here.

[ajvincent@Fedora-64 test]$ du -h xulrunner-sdk/lib/libxul.so
498M	xulrunner-sdk/lib/libxul.so
[ajvincent@Fedora-64 test]$ du -h xulrunner-sdk/bin/libxul.so 
33M	xulrunner-sdk/bin/libxul.so

Why do we have two different libxul.so files?
I have no idea.  bsmedberg?
Flags: needinfo?(benjamin)

Comment 8

6 years ago
Is the other one unstripped? We may just not be stripping the lib version properly.

I believe that we needed to have libxul.so in both locations because people typically linked using -Lsdk/lib -lxul. I don't see any reason that the lib/libxul.so shouldn't just be a symlink to the bin/libxul.so if that can happen.
Flags: needinfo?(benjamin)
(Reporter)

Comment 9

6 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #8)
> Is the other one unstripped? We may just not be stripping the lib version
> properly.

I confirmed that lib/libxul.so is not stripped.

> I believe that we needed to have libxul.so in both locations because people
> typically linked using -Lsdk/lib -lxul. I don't see any reason that the
> lib/libxul.so shouldn't just be a symlink to the bin/libxul.so if that can
> happen.

Is this where I need to make the change?
http://hg.mozilla.org/mozilla-central/annotate/b47078825e68/toolkit/mozapps/installer/packager.mk#l932
(Reporter)

Comment 10

6 years ago
Created attachment 689558 [details] [diff] [review]
brute-force patch

I don't like the way this patch fixes the problem, but it works... my Makefile fu is not that strong.
Attachment #689558 - Flags: feedback?(khuey)
Linux32 went over 500mb last week, which makes it too big for signing and burns the build. At 494mb (and, rather suspiciously, growing , Linux64 will probably make it a couple weeks before it will also burn, unless something big lands.
Comment on attachment 689558 [details] [diff] [review]
brute-force patch

Not sure if bug 780561 has bitrotted this patch, but we should definitely fix this.
Attachment #689558 - Flags: feedback?(khuey) → feedback?(mh+mozilla)
(Reporter)

Comment 13

6 years ago
Who should review (since we asked for feedback)?
(Assignee)

Comment 14

6 years ago
Comment on attachment 689558 [details] [diff] [review]
brute-force patch

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

::: toolkit/mozapps/installer/packager.mk
@@ +952,5 @@
>  	$(NSINSTALL) -D $(DIST)/$(SDK_PATH)
> +ifeq (Linux,$(OS_TARGET))
> +ifndef PKG_SKIP_STRIP
> +	$(STRIP) $(STRIP_FLAGS) $(DIST)/$(MOZ_APP_NAME)-sdk/lib/*.so
> +	$(STRIP) $(STRIP_FLAGS) $(DIST)/$(MOZ_APP_NAME)-sdk/sdk/lib/*.so

If we're going to strip builds, we should do so on all platforms. Sure, the debugging info is only there on linux, taking so much space, but iirc on mac, for instance, the static symbols are there too, and we strip them on linux, we should strip them on mac.
Attachment #689558 - Flags: feedback?(mh+mozilla) → feedback-
Apparently we need to track this for 22, since releases will hit the same problem.
tracking-firefox22: --- → ?
Tracking for FF22 since we are committed to shipping XULRunner with each release. Also assigning to Alex given his initial patch proposal.
Assignee: nobody → ajvincent
tracking-firefox22: ? → +
On IRC Alex suggests that he isn't in the best position to resolve. Glandium/Ted?
Assignee: ajvincent → nobody
(Assignee)

Comment 18

6 years ago
Looks like this is causing problems to us now:
https://tbpl.mozilla.org/php/getParsedLog.php?id=20139060&tree=Firefox
(Assignee)

Comment 19

6 years ago
Created attachment 719410 [details] [diff] [review]
Strip all files that can be stripped in the SDK

Eventually, I want the SDK to be packaged proper, but that requires more work on the packager side. This is good enough for the short term.
Attachment #719410 - Flags: review?(ted)
(Assignee)

Updated

6 years ago
Assignee: nobody → mh+mozilla
Comment on attachment 719410 [details] [diff] [review]
Strip all files that can be stripped in the SDK

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

::: toolkit/mozapps/installer/strip.py
@@ +16,5 @@
> +    # that can be stripped, and copying ExecutableFiles defaults to
> +    # stripping them unless buildconfig.substs['PKG_SKIP_STRIP'] is set.
> +    for p, f in FileFinder(dir):
> +        copier.add(p, f)
> +    copier.copy(dir)

It's a little confusing to copy a dir to itself, but this seems fine.
Attachment #719410 - Flags: review?(ted) → review+
(Assignee)

Comment 21

6 years ago
https://hg.mozilla.org/mozilla-central/rev/f99a075a5bce
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
(Reporter)

Comment 22

6 years ago
The patch didn't work correctly:  see bug 847382 comment 5.
Duplicate of this bug: 857279
(Assignee)

Updated

6 years ago
status-firefox21: --- → affected
Is there any reason why this can't be backported to mozilla-beta? We shouldn't be shipping a huge SDK for Linux users when we have a fix IMO.
(Assignee)

Comment 25

6 years ago
(In reply to Ben Hearsum [:bhearsum] from comment #24)
> Is there any reason why this can't be backported to mozilla-beta? We
> shouldn't be shipping a huge SDK for Linux users when we have a fix IMO.

Besides https://bugzilla.mozilla.org/show_bug.cgi?id=857279#c3 nothing. Note bug 847382 fixes an issue with the patch in this bug.
Comment on attachment 719410 [details] [diff] [review]
Strip all files that can be stripped in the SDK

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Linux XULRunner SDKs are 10x the size they need to be, causing longer downloads for users of them.
Testing completed (on m-c, etc.): Applied patch locally.
Attachment #719410 - Flags: approval-mozilla-beta?
Comment on attachment 719410 [details] [diff] [review]
Strip all files that can be stripped in the SDK

10:37 < bhearsum|buildduty> lsblakk: i  just asked for approval for https://bugzilla.mozilla.org/show_bug.cgi?id=812105 and https://bugzilla.mozilla.org/show_bug.cgi?id=847382, but then glandium pointed out to me that a=NPOTB 
                            has been used for xulrunner things before. can i go ahead and land them on beta?
10:43 < lsblakk> ya

https://hg.mozilla.org/releases/mozilla-beta/rev/7c65fa3a6187
https://hg.mozilla.org/releases/mozilla-beta/rev/76d7f0959eb0
Attachment #719410 - Flags: approval-mozilla-beta?

Updated

6 years ago
status-firefox21: affected → fixed
(Assignee)

Updated

6 years ago
status-firefox22: --- → fixed
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.