Closed Bug 812105 Opened 10 years ago Closed 10 years ago

XULRunner Linux SDK packages need to go on a diet

Categories

(Toolkit Graveyard :: XULRunner, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox21 fixed, firefox22+ fixed)

RESOLVED FIXED
mozilla22
Tracking Status
firefox21 --- fixed
firefox22 + fixed

People

(Reporter: WeirdAl, Assigned: glandium)

References

()

Details

Attachments

(2 files)

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?
libxul.so seems to be eating 99% of the SDK's size.
Are the binaries stripped?  I seem to remember that they aren't.
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.
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
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)
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)
(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
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)
Who should review (since we asked for feedback)?
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 for FF22 since we are committed to shipping XULRunner with each release. Also assigning to Alex given his initial patch proposal.
Assignee: nobody → ajvincent
On IRC Alex suggests that he isn't in the best position to resolve. Glandium/Ted?
Assignee: ajvincent → nobody
Looks like this is causing problems to us now:
https://tbpl.mozilla.org/php/getParsedLog.php?id=20139060&tree=Firefox
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: 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+
https://hg.mozilla.org/mozilla-central/rev/f99a075a5bce
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
The patch didn't work correctly:  see bug 847382 comment 5.
Duplicate of this bug: 857279
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.
(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?
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.