812105 - XULRunner Linux SDK packages need to go on a diet

Status: RESOLVED FIXED

(Toolkit Graveyard :: XULRunner, defect)

Description

[Alex Vincent [:WeirdAl]]

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?

Comment 1

[Alex Vincent [:WeirdAl]]

libxul.so seems to be eating 99% of the SDK's size.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Are the binaries stripped?  I seem to remember that they aren't.

Comment 3

[Alex Vincent [:WeirdAl]]

Kyle, I have no idea what that means.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Stripping a binary removes the embedded debugging symbols.  I think you can use | file libxul.so | to see if the binary is stripped.

Comment 5

[Alex Vincent [:WeirdAl]]

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

Comment 6

[Alex Vincent [:WeirdAl]]

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?

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I have no idea.  bsmedberg?

Comment 8

[Benjamin Smedberg]

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.

Comment 9

[Alex Vincent [:WeirdAl]]

(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

Comment 10

[Alex Vincent [:WeirdAl]]

Attachment: brute-force patch

I don't like the way this patch fixes the problem, but it works... my Makefile fu is not that strong.

Comment 11

[Phil Ringnalda (:philor)]

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 12

[(not currently active) Ted Mielczarek]

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.

Comment 13

[Alex Vincent [:WeirdAl]]

Who should review (since we asked for feedback)?

Comment 14

[Mike Hommey [:glandium]]

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.

Comment 15

[Phil Ringnalda (:philor)]

Apparently we need to track this for 22, since releases will hit the same problem.

Comment 16

[Alex Keybl [:akeybl]]

Tracking for FF22 since we are committed to shipping XULRunner with each release. Also assigning to Alex given his initial patch proposal.

Comment 17

[Alex Keybl [:akeybl]]

On IRC Alex suggests that he isn't in the best position to resolve. Glandium/Ted?

Comment 18

[Mike Hommey [:glandium]]

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

Comment 19

[Mike Hommey [:glandium]]

Attachment: 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.

Comment 20

[(not currently active) Ted Mielczarek]

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.

Comment 21

[Mike Hommey [:glandium]]

https://hg.mozilla.org/mozilla-central/rev/f99a075a5bce

Comment 22

[Alex Vincent [:WeirdAl]]

The patch didn't work correctly:  see bug 847382 comment 5.

Comment 24

[[e-mail] (:bhearsum)]

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.

Comment 25

[Mike Hommey [:glandium]]

(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 26

[[e-mail] (:bhearsum)]

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.

Comment 27

[[e-mail] (:bhearsum)]

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