Closed Bug 486698 Opened 11 years ago Closed 11 years ago

Facilitate the building of major components independently and in a chain manner by downstream distributions

Categories

(NSS :: Build, defect)

3.12.3
x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
3.12.4

People

(Reporter: elio.maldonado.batiz, Assigned: elio.maldonado.batiz)

Details

Attachments

(1 file, 2 obsolete files)

The build system should change to enable downstream distributions such as Fedora to build and package the full NSS, NSS softoken and NSS-util by themselves. NSS requires softoken (and util) to build and softoken requires util. The downlstream distribution should be able to build then in a chained fashion where the dependent piece can tell the NSS build system to pick up headers and shared libraries from the system locations instead of the build area dist directory if need be.  Please refer to Bug 482742 where this proposal was originally made in an incomplete fashion.
Attachment #370880 - Flags: review?(nelson)
How is this bug distinct from bug 482742?  Is this a duplicate of bug 482742?
What goal does this bug have that that other bug does not, and vice versa?
Elio, Bob, before I review this patch, I need to know
where you plan to install the headers and libraries of
the new nss-util and nss-softoken packages in Fedora.

Also, the coreconf variables MODULE and REQUIRES can
be used to separate nss-util and nss-softoken builds
from the main NSS build.  These two variables can
be used to ensure the correct dependency between
nss-util, nss-softoken, and nss when you build all
of them together.  You should consider modifying
the manifest.mn files throughout the NSS source tree
to set MODULE and REQUIRES properly to reflect the
fact that nss-util and nss-softoken are built as
separate modules.
(In reply to comment #1)
> How is this bug distinct from bug 482742?  Is this a duplicate of bug 482742?
> What goal does this bug have that that other bug does not, and vice versa?

I made it a separate bug for the sake of clarity as this bug is a request
motivated by the needs of the Fedora/RHEL build system to separate packaging of
NSS, softoken and utils whereas the other bug was driven by own NSS building
needs not separate packaging.
> You should consider modifying the manifest.mn files throughout the NSS 
> source tree to set MODULE and REQUIRES properly to reflect the fact that 
> nss-util and nss-softoken are built as separate modules.

I agree.  Please do more than consider it.  :)
I was wasn't aware of MODULE and REQUIRES. Let me study this then.
Elio, based on your definition of the purposes of this bug
and bug 482742, my suggestion of using the coreconf MODULE
and REQUIRES variables is intended for bug 482742.
Okay, I will pose my questions on MODULE and REQUIRES on the other bug then :-)
(In reply to comment #6)
> Elio, based on your definition of the purposes of this bug
> and bug 482742, my suggestion of using the coreconf MODULE
> and REQUIRES variables is intended for bug 482742.
That approach is not yielding fruits at the momment, see latest comments on Bug 482742. Even with the split I think I would need this patch in order for the Fedora build system to query the configuration information that the installed packages expose and based on this set and export environment variables so the NSS build system can find the include and library directories as system directories (on a split building the dist directory would not have all the headers and libraries). I modelled this patch after what I observed the Fedora build does with NSPR where the NSPR header and library are found in system locations.
Assignee: nobody → emaldona
Attachment #370880 - Flags: review?(nelson)
Comment on attachment 370880 [details] [diff] [review]
Changes to build files and coreconf to allow chained building

Temporarily removing the request for review until I do some checks downstream.
Comment on attachment 370880 [details] [diff] [review]
Changes to build files and coreconf to allow chained building

Reactivating review request.
Attachment #370880 - Flags: review?(nelson)
Comment on attachment 370880 [details] [diff] [review]
Changes to build files and coreconf to allow chained building

r=nelson
Thanks for going over the requirements with me in detail this morning.
Attachment #370880 - Flags: review?(nelson) → review+
Comment on attachment 370880 [details] [diff] [review]
Changes to build files and coreconf to allow chained building

lib/nss/config.mk and lib/smime/config.mk should not need
-L$(SOFTOKEN_LIB_DIR).  Only lib/ssl/config.mk needs
-L$(SOFTOKEN_LIB_DIR) because it needs libfreebl.a.

You should also need to add -L$(NSSUTIL_LIB_DIR) to
lib/softoken/legacydb/config.mk and cmd/platlibs.mk.  This
LXR query shows the makefiles that contain -lnssutil3 now:
http://mxr.mozilla.org/security/search?string=-lnssutil3&find=&findi=&filter=^[^\0]*$&hitlimit=&tree=security

Some of the programs in cmd also need -L$(SOFTOKEN_LIB_DIR)
because the need libfreebl.a.
(In reply to comment #12) Thank you Wann-Teh. Searching for programs in cmd that include blapi.h I found: bltest, ecperf, fipstest, rsaperf, signtool. Also shlibsign which includes the freebl header "shsign.h". I may find more. In cmd I don't see config.mk files like in lib but their files do include platlibs.mk which is global so I don't want change that one. I do see in their makefiles these two sections:
# (6) Execute "component" rules. (OPTIONAL)                           #
and
# (7) Execute "local" rules. (OPTIONAL).                              #
What's the meaning of component versus local rules?
Its seems that I should add
EXTRA_SHARED_LIBS += \
	-L$(SOFTOKEN_LIB_DIR) \
	$(NULL)
to one of these sections. Would that be the correct way and if so, to which one?
I found that we shouldn't use -L$(SOFTOKEN_LIB_DIR) in any
makefile, because we link with libfreebl.a using its full
pathname, for example, in lib/ssl/config.mk, we now have:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/config.mk&rev=1.26&mark=76,90#74

  CRYPTOLIB=$(DIST)/lib/$(LIB_PREFIX)freebl.$(LIB_SUFFIX)

We need to change it to

  CRYPTOLIB=$(SOFTOKEN_LIB_DIR)/$(LIB_PREFIX)freebl.$(LIB_SUFFIX)

This applies to both lib/ssl/config.mk and cmd/platlibs.mk.
This change to cmd/platlibs.mk will take care of the programs
(bltest, ecperf, fipstest, rsaperf, signtool, shlibsign) that
use blapi.h/libfreebl.a.  You should not need to change other
makefiles under cmd.
Attachment #370880 - Attachment is obsolete: true
Attachment #372099 - Flags: review?
Attachment #372099 - Flags: review? → review?(wtc)
Comment on attachment 372099 [details] [diff] [review]
Changes to build files and coreconf to support chained building

Temporarily withdrawing from review until I investigate some test failures.
Attachment #372099 - Flags: review?(wtc)
Comment on attachment 372099 [details] [diff] [review]
Changes to build files and coreconf to support chained building

Thanks for the patch.  Please fix the following bugs in this
patch.

Note: these bugs show that either you didn't test this patch
or you didn't test the patch correctly.

1. cmd/platlibs.mk

You only fixed one instance of
$(DIST)/lib/$(LIB_PREFIX)freebl.$(LIB_SUFFIX).  There is
another instance that also needs to be fixed.  Do not fix
the instance inside ifdef MOZILLA_BSAFE_BUILD because that
is dead code.  (We should remove the dead code, but you
don't need to do that here.)

>@@ -202,6 +202,7 @@ endif
> EXTRA_SHARED_LIBS += \
> 	-L$(DIST)/lib \
> 	$(SQLITE) \
>+	-L$(NSS_LIB_DIR) \
> 	-lnssutil3 \

This should be -L$(NSSUTIL_LIB_DIR).

2. lib/ssl/config.mk

>@@ -90,7 +91,7 @@ else
> CRYPTOLIB=$(DIST)/lib/$(LIB_PREFIX)freebl.$(LIB_SUFFIX)
> CRYPTODIR=../freebl
> ifdef MOZILLA_SECURITY_BUILD
>-	CRYPTOLIB=$(DIST)/lib/$(LIB_PREFIX)crypto.$(LIB_SUFFIX)
>+	CRYPTOLIB=$(SOFTOKEN_LIB_DIR)/lib/$(LIB_PREFIX)crypto.$(LIB_SUFFIX)
> 	CRYPTODIR=../crypto
> endif

This instance is dead code, so you don't need to fix it.
You didn't fix the two instances that need to be fixed.
Attachment #372099 - Flags: review-
Fixing the second instance (en the else) in lib/ssl/config.mk caused the build to fail with make: *** No rule to make target ../../../../dist/Linux2.6_x86_64_glibc_PTH_64_DBG.OBJ/lib/lib/libfreebl.a.

(By the same token, I should fix the second instance in cmd/platlib.mk
and when I tried to, it caused the error mentioned above for lib/ssl.)

The static library, freebl.a, is where it is supposed to be. Why does make think it needs to build it? Some other change maybe needed. I haven't figured out what yet.
Elio, make sure you define CRYTOLIB as I did in comment 14,
rather than what you did in comment 17.  I should have
pointed out that what you did in comment 17 was wrong.
I didn't mention that because you modified dead code.
The mistake was that you have an extra 'lib' after
$(SOFTOKEN_LIB_DIR), which causes the pathname in the
make error message in comment 18 to have "lib/lib".
Thanks, that fixes it. Will send patch for review later in the day. Will leave dead code as is and it's very likely to be removed.
Yes, don't update the dead code.  I will remove it in
bug 487858.
Attachment #372099 - Attachment is obsolete: true
Attachment #372221 - Flags: review?
Attachment #372221 - Flags: review? → review?(wtc)
Comment on attachment 372221 [details] [diff] [review]
Changes to build files and coreconf to allow chained building

r=wtc.  The following two instances of CRYPTOLIB still have an extra
'lib' after $(SOFTOKEN_LIB_DIR).  They must be fixed before you check
this patch in.

In security/nss/cmd/platlibs.mk

>@@ -87,7 +87,7 @@ ifeq ($(OS_ARCH), WINNT)
> 
> DEFINES += -DNSS_USE_STATIC_LIBS
> # $(PROGRAM) has explicit dependencies on $(EXTRA_LIBS)
>-CRYPTOLIB=$(DIST)/lib/$(LIB_PREFIX)freebl.$(LIB_SUFFIX)
>+CRYPTOLIB=$(SOFTOKEN_LIB_DIR)/lib/$(LIB_PREFIX)freebl.$(LIB_SUFFIX)
> ifdef MOZILLA_SECURITY_BUILD
> 	CRYPTOLIB=$(DIST)/lib/crypto.lib
> endif

This one is wrong.

In security/nss/lib/ssl/config.mk

>@@ -73,7 +74,7 @@ EXTRA_SHARED_LIBS += \
> endif # NS_USE_GCC
> 
> # $(PROGRAM) has explicit dependencies on $(EXTRA_LIBS)
>-CRYPTOLIB=$(DIST)/lib/$(LIB_PREFIX)freebl.$(LIB_SUFFIX)
>+CRYPTOLIB=$(SOFTOKEN_LIB_DIR)/lib/$(LIB_PREFIX)freebl.$(LIB_SUFFIX)
> CRYPTODIR=../freebl
> ifdef MOZILLA_SECURITY_BUILD
> 	CRYPTOLIB=$(DIST)/lib/$(LIB_PREFIX)crypto.$(LIB_SUFFIX)

This one is wrong.
Attachment #372221 - Flags: review?(wtc) → review+
Changed committed:
/cvsroot/mozilla/security/coreconf/location.mk,v  <--  location.mk
new revision: 1.12; previous revision: 1.11
/cvsroot/mozilla/security/nss/cmd/platlibs.mk,v  <--  platlibs.mk
new revision: 1.60; previous revision: 1.59
/cvsroot/mozilla/security/nss/lib/freebl/config.mk,v  <--  config.mk
new revision: 1.23; previous revision: 1.22
/cvsroot/mozilla/security/nss/lib/nss/config.mk,v  <--  config.mk
new revision: 1.33; previous revision: 1.32
/cvsroot/mozilla/security/nss/lib/smime/config.mk,v  <--  config.mk
new revision: 1.27; previous revision: 1.26
/cvsroot/mozilla/security/nss/lib/softoken/config.mk,v  <--  config.mk
new revision: 1.26; previous revision: 1.25
/cvsroot/mozilla/security/nss/lib/softoken/legacydb/config.mk,v  <--  config.mk
new revision: 1.9; previous revision: 1.8
/cvsroot/mozilla/security/nss/lib/ssl/config.mk,v  <--  config.mk
new revision: 1.27; previous revision: 1.26
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Seems like the last patch to the this bug brakes builds on macs osx. I've just downloaded the fresh source tree and I was not able to build with a default make command without additional parameters to gmake or additional variable settings.

...
../../dist/Darwin9.6.0_DBG.OBJ/include  -I../../../../dist/public/nss -I../../../../dist/private/nss  ssl3ecc.c
cc -o Darwin9.6.0_DBG.OBJ/unix_err.o -c -g -fPIC -Di386 -Wmost -fpascal-strings -fno-common -pipe -DDARWIN -DHAVE_STRERROR -DHAVE_BSD_FLOCK  -DXP_UNIX -DDEBUG -UNDEBUG -DDEBUG_volkov -DNSS_ENABLE_ECC -DPKIX_ERROR_DESCRIPTION -DUSE_UTIL_DIRECTLY -I../../../../dist/Darwin9.6.0_DBG.OBJ/include  -I../../../../dist/public/nss -I../../../../dist/private/nss  unix_err.c
rm -f Darwin9.6.0_DBG.OBJ/libssl.a
ar cr Darwin9.6.0_DBG.OBJ/libssl.a Darwin9.6.0_DBG.OBJ/derive.o Darwin9.6.0_DBG.OBJ/prelib.o Darwin9.6.0_DBG.OBJ/ssl3con.o Darwin9.6.0_DBG.OBJ/ssl3gthr.o Darwin9.6.0_DBG.OBJ/sslauth.o Darwin9.6.0_DBG.OBJ/sslcon.o Darwin9.6.0_DBG.OBJ/ssldef.o Darwin9.6.0_DBG.OBJ/sslenum.o Darwin9.6.0_DBG.OBJ/sslerr.o Darwin9.6.0_DBG.OBJ/ssl3ext.o Darwin9.6.0_DBG.OBJ/sslgathr.o Darwin9.6.0_DBG.OBJ/sslmutex.o Darwin9.6.0_DBG.OBJ/sslnonce.o Darwin9.6.0_DBG.OBJ/sslreveal.o Darwin9.6.0_DBG.OBJ/sslsecur.o Darwin9.6.0_DBG.OBJ/sslsnce.o Darwin9.6.0_DBG.OBJ/sslsock.o Darwin9.6.0_DBG.OBJ/ssltrace.o Darwin9.6.0_DBG.OBJ/sslver.o Darwin9.6.0_DBG.OBJ/authcert.o Darwin9.6.0_DBG.OBJ/cmpcert.o Darwin9.6.0_DBG.OBJ/nsskea.o Darwin9.6.0_DBG.OBJ/sslinfo.o Darwin9.6.0_DBG.OBJ/ssl3ecc.o Darwin9.6.0_DBG.OBJ/unix_err.o
ranlib Darwin9.6.0_DBG.OBJ/libssl.a
grep -v ';+' ssl.def | grep -v ';-' | sed -e 's; DATA ;;' -e 's,;;,,' -e 's,;.*,,' -e 's,^,_,' > Darwin9.6.0_DBG.OBJ/ssl.def
gmake: *** No rule to make target /libfreebl.a.  Stop.
gmake[1]: *** [/libfreebl.a] Error 1
gmake: *** [libs] Error 2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #25)
> Seems like the last patch to the this bug brakes builds on macs osx. 

The Darwin tinderboxes are green.
Sorry, was problem on my side. Didn't do get the whole tree.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.