301158 - cannot make xpinstall/packager when building deerpark --with-system-nspr

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

In the makeinstall phase of my RPM, I traditionally perform the following:

cd xpinstall/packager/
make MOZILLA_BIN="\$(DIST)/bin/firefox-bin" STRIP=/bin/true
cd -

Using deerpark alpha2 source, I get a bunch of 'cannot stat: no such file'
errors posted on screen.

Relevant portion of the log attached.  This is not an issue when
--without-system-nspr is used, only --with-system-nspr

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: tail of error log

Comment 2

[Christopher Aillon (sabbatical, not receiving bugmail)]

toolkit/mozapps/installer/packager.mk

213                ifdef MOZ_NATIVE_NSPR
214                ifndef EXCLUDE_NSPR_LIBS
215                	@echo "Copying NSPR libs..."
216                	@cp -p $(NSPR_LDIR)/*$(DLL_SUFFIX) $(DIST)/$(MOZ_PKG_APPNAME)
217                	@chmod 755 $(DIST)/$(MOZ_PKG_APPNAME)/*$(DLL_SUFFIX)
218                endif
219                endif


We appear to attempt to copy over everything in the NSPR_LDIR when using
MOZ_NATIVE_NSPR, which in this case is /usr/lib.  That seems rather wrong.  :-(

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Proposed patch

I think that the logic is reversed -- we want to copy NSPR over only when we
are not building using the native libraries.

Comment 4

[Wolfgang Rosenauer [:wolfiR]]

https://bugzilla.mozilla.org/show_bug.cgi?id=288647#c12

had a short discussion about that issue. As there was an easy workaround I
didn't drive this forward but IMHO your patch is correct.

Comment 5

[Benjamin Smedberg]

Packaging Firefox should use browser/installer not xpinstall/packager. Likewise
tbird should be using mail/installer.

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Taking bug.

Comment 7

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked into trunk 2005-07-18 12:21.

Comment 8

[Wan-Teh Chang]

Comment on attachment 189654 [details] [diff] [review]
Proposed patch

>-ifdef MOZ_NATIVE_NSPR
>+ifndef MOZ_NATIVE_NSPR
> ifndef EXCLUDE_NSPR_LIBS
> 	@echo "Copying NSPR libs..."
> 	@cp -p $(NSPR_LDIR)/*$(DLL_SUFFIX) $(DIST)/$(MOZ_PKG_APPNAME)
> 	@chmod 755 $(DIST)/$(MOZ_PKG_APPNAME)/*$(DLL_SUFFIX)
> endif
> endif

While this change should be correct, the "cp" command
copies more than just the NSPR libs because by default
$(NSPR_LDIR) is $(DIST)/lib.  Similarly, the "chmod"
command operates on more than just the NSPR libs in
$(DIST)/$(MOZ_PKG_APPNAME).

Mozilla's build system owner should review these two
issues.

Comment 9

[Wan-Teh Chang]

Comment on attachment 189654 [details] [diff] [review]
Proposed patch

To elaborate on my previous comment:

1. The "cp" and "chmod" commands are only intended to operate
on the three NSPR libs (libnspr4.so, libplc4.so, libplds4.so).
The use of "*" in these two commands makes them operate on more
files than intended.

2. Previously, because of the reversed logic with MOZ_NATIVE_NSPR,
we may have depended on the overly general side effects of these
two commands.  Now that the logic is fixed, we may need to add
"cp" or "chmod" command to copy other files or change the file
permissions to 755 on other files.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

I'm removing that copying in Bug 301327.