Last Comment Bug 360818 - No RPATH set for signtool and signver
: No RPATH set for signtool and signver
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Build (show other bugs)
: 3.11.3
: All All
: P1 normal (vote)
: 3.11.4
Assigned To: Neil Williams
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-11-15 12:10 PST by Christophe Ravel
Modified: 2006-11-16 19:03 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
move the block of RPATH settings to start of platlibs.mk (3.65 KB, patch)
2006-11-15 17:43 PST, Neil Williams
christophe.ravel.bugs: review+
nelson: review+
wtc: superreview+
Details | Diff | Splinter Review
incorporates Wan-Teh's suggestion (2.56 KB, patch)
2006-11-16 12:26 PST, Neil Williams
wtc: review+
nelson: review+
Details | Diff | Splinter Review

Description Christophe Ravel 2006-11-15 12:10:50 PST
Unlike certutil and many other NSS tools, there is no RPATH set for signtool and signver.

# ldd signtool
        libplc4.so =>    (file not found)
        libplds4.so =>   (file not found)
        libnspr4.so =>   (file not found)
Comment 1 Neil Williams 2006-11-15 14:43:54 PST
There has been quite a bit of work done on signtool to make it use dynamic libraries (see bug 263948). This isn't a duplicate since it is preferable to have signtool use the shared libraries, but when that happens the patch to fix this bug should be undone.
Comment 2 Neil Williams 2006-11-15 14:45:28 PST
There has been quite a bit of work done on signtool to make it use dynamic libraries (see bug 263948). This isn't a duplicate since it is preferable to have signtool use the shared libraries, but when that happens the patch to fix this bug should be undone.
Comment 3 Neil Williams 2006-11-15 17:40:02 PST
Mistakenly changed status with last comment.
Comment 4 Neil Williams 2006-11-15 17:43:38 PST
Created attachment 245712 [details] [diff] [review]
move the block of RPATH settings to start of platlibs.mk
Comment 5 Neil Williams 2006-11-15 17:58:18 PST
Comment on attachment 245712 [details] [diff] [review]
move the block of RPATH settings to start of platlibs.mk

This patch need not be backed out when bug 263948 is fixed--unless someone dislikes the readability quotient.
Comment 6 Christophe Ravel 2006-11-15 18:06:38 PST
Comment on attachment 245712 [details] [diff] [review]
move the block of RPATH settings to start of platlibs.mk

Build and fix tested on Solaris SPARC, Solaris x86, Linux and HP-UX.
Comment 7 Christophe Ravel 2006-11-15 18:56:45 PST
Tested all.sh on Solaris SPARC, Solaris x86, Linux, HP-UX: no error.
Comment 8 Nelson Bolyard (seldom reads bugmail) 2006-11-16 00:26:44 PST
Comment on attachment 245712 [details] [diff] [review]
move the block of RPATH settings to start of platlibs.mk

Wan-Teh, do you agree with these changes for Linux and other non-Solaris platforms?
Comment 9 Wan-Teh Chang 2006-11-16 06:49:03 PST
Comment on attachment 245712 [details] [diff] [review]
move the block of RPATH settings to start of platlibs.mk

This patch has a harmless bug -- it incorrectly moves the
code for the -rpath-link linker flag.

I encourage you to submit a new patch that doesn't move
the code for the -rpath-linker flag.

If you are pressed for time, you may check in this patch
as is on the NSS_3_11_BRANCH.

But on the NSS trunk, please do not move the code for the
-rpath-linker flag.  It should stay where it is, next to
the Darwin code for the -dylib_file linker flag.
Comment 10 Wan-Teh Chang 2006-11-16 06:53:48 PST
My previous comment has two instances of a typo.

By "the -rpath-linker flag", I meant "the -rpath-link linker flag".
Specifically, I'm referring to this block of code.

-# If GNU ld is used, we must use the -rpath-link option to tell
-# the linker where to find libsoftokn3.so, an implicit dependency
-# of libnss3.so.
-ifeq (,$(filter-out BSD_OS FreeBSD Linux NetBSD, $(OS_ARCH)))
-EXTRA_SHARED_LIBS += -Wl,-rpath-link,$(DIST)/lib
-endif
-
-ifeq ($(OS_ARCH), SunOS)
-ifdef NS_USE_GCC
-ifdef GCC_USE_GNU_LD
-EXTRA_SHARED_LIBS += -Wl,-rpath-link,$(DIST)/lib
-endif
-endif
-endif

It should not be moved.
Comment 11 Neil Williams 2006-11-16 12:26:35 PST
Created attachment 245777 [details] [diff] [review]
incorporates Wan-Teh's suggestion
Comment 12 Neil Williams 2006-11-16 18:14:42 PST
Checking in cmd/platlibs.mk;
/cvsroot/mozilla/security/nss/cmd/platlibs.mk,v  <--  platlibs.mk
new revision: 1.43.2.4; previous revision: 1.43.2.3
done

Checking in cmd/platlibs.mk;
/cvsroot/mozilla/security/nss/cmd/platlibs.mk,v  <--  platlibs.mk
new revision: 1.47; previous revision: 1.46
done

Note You need to log in before you can comment on or make changes to this bug.