NSPR Linux opt target does not strip shared libraries

VERIFIED FIXED

Status

SeaMonkey
Build Config
P1
normal
VERIFIED FIXED
18 years ago
13 years ago

People

(Reporter: dougt, Assigned: larryh (gone))

Tracking

({embed})

Trunk
x86
Linux
embed

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3-])

Attachments

(7 attachments)

(Reporter)

Description

18 years ago
The standalone nspr target with the right options do not seems to build it
properly (eg stripped).  Take a look: 

From the nightly tar: 
[dougt@z package]$ file libnspr4.so 
libnspr4.so: ELF 32-bit LSB shared object, Intel 80386, version 1, stripped 

From my local optimized tree: 
[dougt@z package]$ file libnspr4.so 
libnspr4.so: ELF 32-bit LSB shared object, Intel 80386, version 1, not stripped

Comment 1

18 years ago
So the NSPR library in the nightly build is
built in a different way?
Status: NEW → ASSIGNED
(Reporter)

Comment 2

18 years ago
no idea.  Leaf, cls, can you shed some light on this please?

Comment 3

18 years ago
It isn't built differently, but part of the packaging process in the build
automation strips everything under dist/bin, which is why there's a difference.

Comment 4

18 years ago
well, we do build nspr before building mozilla and put it in a separate OBJDIR
(something I mean to change), but leaf is correct.  We strip everything under
dist/bin other than a few types of non-binary files as part of the packaging.
See mozilla/xpinstall/packager/Makefile.in.

We use the standard configure flags.  See /u/cltbld/logs/make_moz.linux22 for
the configure lines and environment variables.

Comment 5

18 years ago
nominating, this adds bloat to embedding clients.
Keywords: embed, nsbeta3

Comment 6

17 years ago
Reassigned to larryh.
Assignee: wtc → larryh
Status: ASSIGNED → NEW
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 7

17 years ago
A work-around for this bug already exists: The installer strips symbols. This 
seems like an OK solution.
(Assignee)

Comment 8

17 years ago
Absent objections by 29-Sep-2000, NSPR will mark this closed WONTFIX.
(Reporter)

Comment 9

17 years ago
Larry, most customers of nspr do not use this installer.  We need to fix the
build.
(Assignee)

Comment 10

17 years ago
No other products using NSPR need to have the optimized build further processed. 
I would think that the mozilla build process would do what it needs to do after 
we've built standard parts suitable for the rest of the universe.

I also have to ask: why is the provided installer not used?

Comment 11

17 years ago
embedding isn't far enough along to have an installer (it's not clear what an
embedding installer would even do). it would seem to me that NSPR should clean
up the bits as best as it can before people use them.

Comment 12

17 years ago
Do you only want the optimized libraries to be stripped?
I assume that you want the symbols in the debug libraries,
right?

Do we lose anything if we strip a library?  Are all the
system libraries (e.g., libc.so, libpthread.so) stripped?
(Assignee)

Comment 13

17 years ago
Created attachment 15274 [details] [diff] [review]
proposed patch
(Assignee)

Comment 14

17 years ago
See the proposed patch in the attachment.
The strip operation reduces NSPR's .so from 214K to 169K, about 20%

Wan-Teh, Please review.
(Assignee)

Comment 15

17 years ago
Created attachment 15321 [details] [diff] [review]
proposed patch, as "cvs diff -u" output
(Assignee)

Comment 16

17 years ago
added attachment. patch is "cvs diff -u" output.

Comment 17

17 years ago
wan-teh, can we get a plus on this and get it in?
(Assignee)

Comment 18

17 years ago
cls,
Please review my patch.
Is there anything that needs doing for autoconf?
/Larry.

Comment 19

17 years ago
Judson, this bug is P3, nsbeta3.  I can't check it in
unless it gets the appropriate priority and status
according to the current policy.

Comment 20

17 years ago
Created attachment 15380 [details] [diff] [review]
changes to autoconf bits needed for strip

Updated

17 years ago
Priority: P3 → P2
Whiteboard: [nsbeta3+]

Comment 21

17 years ago
now it's P1. please proceed w/ checkin process.
Priority: P2 → P1
(Assignee)

Comment 22

17 years ago
Created attachment 15449 [details] [diff] [review]
patch. Merged autoconf & other patch. file: configure not included
(Assignee)

Comment 23

17 years ago
attached merged patch for Chris' autoconf patch and non-autoconf patches, fitted 
to NSPRPUB_CLIENT_BRANCH.

Review, please.

Comment 24

17 years ago
It looks like a hunk fell out during the merge.  This was part of larry's
original patch and is needed for the strip to actually occur when using
--enable-nspr-autoconf.  Add this in and r=cls.

Index: rules.mk
===================================================================
RCS file: /cvsroot/mozilla/nsprpub/config/rules.mk,v
retrieving revision 3.31
diff -u -r3.31 rules.mk
--- rules.mk    2000/08/20 23:52:43     3.31
+++ rules.mk    2000/09/22 17:53:01
@@ -315,6 +315,7 @@
        rm -f $@
 ifdef USE_AUTOCONF
        $(MKSHLIB) $(OBJS) $(EXTRA_LIBS) $(OS_LIBS)
+       $(STRIP) $@
 else
 ifeq ($(OS_ARCH)$(OS_RELEASE), AIX4.1)
        echo "#!" > $(OBJDIR)/lib$(LIBRARY_NAME)_syms

Comment 25

17 years ago
cls: should we only strip the libs if BUILD_OPT is defined
(i.e., when building optimized)?

Comment 26

17 years ago
Oops, that's right.  Hmmm...we're not setting BUILD_OPT if --enable-optimize is
given to configure.  I'll whip up a patch to fix that.

Comment 27

17 years ago
per email with Jud, changing nsbeta3+ to nsbeta3- on all "embed" keyword bugs
since embedding changes will not be made in the mn6 branch. If you feel this bug
fix needs to go into mn6 branch, please list the reasons/user impact/risk and
nominate for rtm. Thanks.
Whiteboard: [nsbeta3+] → [nsbeta3-]

Comment 28

17 years ago
Created attachment 15658 [details] [diff] [review]
Only call STRIP if BUILD_OPT is set. Includes all previous patches minus configure changes

Comment 29

17 years ago
so who's checking in the patches? they need to go into the trunk.
(Assignee)

Comment 30

17 years ago
Checked in parts to the NSPR tip of tree.

Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.27; previous revision: 1.26
done
config/Linux.mk
Checking in config/Linux.mk;
/cvsroot/mozilla/nsprpub/config/Linux.mk,v  <--  Linux.mk
new revision: 3.22; previous revision: 3.21
done
config/UNIX.mk
Checking in config/UNIX.mk;
/cvsroot/mozilla/nsprpub/config/UNIX.mk,v  <--  UNIX.mk
new revision: 3.16; previous revision: 3.15
done
config/autoconf.mk.in
Checking in config/autoconf.mk.in;
/cvsroot/mozilla/nsprpub/config/autoconf.mk.in,v  <--  autoconf.mk.in
new revision: 1.9; previous revision: 1.8
done
config/rules.mk
Checking in config/rules.mk;
/cvsroot/mozilla/nsprpub/config/rules.mk,v  <--  rules.mk
new revision: 3.32; previous revision: 3.31
done

Comment 31

17 years ago
This patch breaks the NSPR build on Windows
because STRIP is not defined.

Comment 32

17 years ago
then lets do it for unixes. agreed?

Comment 33

17 years ago
Judson: yes.  This is easy to fix.  Just need to
move the $(STRIP) command inside the ifdef for Unixes.

Comment 34

17 years ago
I'd prefer to just set STRIP=echo in WIN32.mk rather than have more ifdefs.

Brian Ryner has pointed out other problems with the patches...
1) configure.in is missing a AC_SUBST(STRIP)
2) configure was never checked in.  Afaik, it's not autogen'd like mozilla's is.
3) I added the strip option to the $(LIBRARY) target which doesn't cause
problems on my box but does on others.

And the changes never made it onto the NSPRPUB_CLIENT_BRANCH.

Comment 35

17 years ago
Created attachment 15899 [details] [diff] [review]
Added STRIP define to other <platform>.mks, removes strip from $(LIBRARY) target, includes configure changes

Comment 36

17 years ago
Created attachment 15901 [details] [diff] [review]
Contains sum of previous changes needed to be checked into the NSPRPUB_CLIENT_BRANCH branch
r=bryner

Comment 38

17 years ago
Changes from attach 15889 have been checked in.

Checking in nsprpub/configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.24; previous revision: 1.23
done
Checking in nsprpub/configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.28; previous revision: 1.27
done
Checking in nsprpub/config/BeOS.mk;
/cvsroot/mozilla/nsprpub/config/BeOS.mk,v  <--  BeOS.mk
new revision: 3.5; previous revision: 3.4
done
Checking in nsprpub/config/OS2.mk;
/cvsroot/mozilla/nsprpub/config/OS2.mk,v  <--  OS2.mk
new revision: 3.12; previous revision: 3.11
done
Checking in nsprpub/config/WIN32.mk;
/cvsroot/mozilla/nsprpub/config/WIN32.mk,v  <--  WIN32.mk
new revision: 3.13; previous revision: 3.12
done
Checking in nsprpub/config/rules.mk;
/cvsroot/mozilla/nsprpub/config/rules.mk,v  <--  rules.mk
new revision: 3.33; previous revision: 3.32
done
Checking in nsprpub/config/win16.mk;
/cvsroot/mozilla/nsprpub/config/win16.mk,v  <--  win16.mk
new revision: 3.7; previous revision: 3.6
done

Comment 39

17 years ago
Changes from attach 15901 have been checked into the NSPRPUB_CLIENT_BRANCH :

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.17.2.5; previous revision: 1.17.2.4
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.18.2.5; previous revision: 1.18.2.4
done
Checking in config/BeOS.mk;
/cvsroot/mozilla/nsprpub/config/BeOS.mk,v  <--  BeOS.mk
new revision: 3.2.72.1; previous revision: 3.2
done
Checking in config/Linux.mk;
/cvsroot/mozilla/nsprpub/config/Linux.mk,v  <--  Linux.mk
new revision: 3.16.2.3; previous revision: 3.16.2.2
done
Checking in config/OS2.mk;
/cvsroot/mozilla/nsprpub/config/OS2.mk,v  <--  OS2.mk
new revision: 3.7.18.3; previous revision: 3.7.18.2
done
Checking in config/UNIX.mk;
/cvsroot/mozilla/nsprpub/config/UNIX.mk,v  <--  UNIX.mk
new revision: 3.13.72.1; previous revision: 3.13
done
Checking in config/WIN32.mk;
/cvsroot/mozilla/nsprpub/config/WIN32.mk,v  <--  WIN32.mk
new revision: 3.9.32.2; previous revision: 3.9.32.1
done
Checking in config/autoconf.mk.in;
/cvsroot/mozilla/nsprpub/config/autoconf.mk.in,v  <--  autoconf.mk.in
new revision: 1.8.2.1; previous revision: 1.8
done
Checking in config/rules.mk;
/cvsroot/mozilla/nsprpub/config/rules.mk,v  <--  rules.mk
new revision: 3.26.2.5; previous revision: 3.26.2.4
done
Checking in config/win16.mk;
/cvsroot/mozilla/nsprpub/config/win16.mk,v  <--  win16.mk
new revision: 3.4.72.1; previous revision: 3.4
done

Comment 40

17 years ago
Marking fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 41

17 years ago
marking verified.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.