82324 - objdir builds do not get coreconf & nss updates

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P1)

Description

[cls]

I had to clobber security/ on the Irix tinderbox for it to grab the updates from
bug 81181.  Because the entire nss & coreconf directories are copied into the
objdir upon the *first* export, the updates never made it into the working tree.

Comment 1

[Javier Delgadillo]

Attachment: quick and dirty patch to do what you want..

Comment 2

[Javier Delgadillo]

The above patch will give you what you want, but it also slows the build down
quite a bit which is sub-optimal do to the cvs update.

Perhaps we should *always* copy over the files?

Comment 3

[cls]

Always copying over the files means that we will always be rebuilding them. I
seem to recall a posting to m.crypto where relyea claimed to have objdir builds
working for NSS with a recent checkin.

news://news.mozilla.org/3AF9B1E3.9090609%40netscape.com

Comment 4

[cls]

Attachment: Use BUILD_TREE per relyea's posting

Comment 5

[Javier Delgadillo]

Did the BUILD_TREE functionality make it to the NSS_CLIENT_TAG?  I thought it
was only on the trunk. 

Comment 6

[cls]

Ah. That would explain why it didn't work in my build. ;-(

Comment 7

[cls]

When are the BUILD_TREE changes going to land on the CLIENT_TAG?  Since Mozilla
has always supported objdir builds, we're going to need those changes. 
Preferably for 0.9.1.

Comment 8

[Wan-Teh Chang]

Chris,

1. The severity of this bug is definitely not critical
   (crashes, loss of data, severe memory leak).  It should
   be major at best, or minor as there is a workaround.

2. I've verified that your patch works with the trunk of
   NSS on Linux.  It still needs work in order to handle
   srcdir builds.  When doing srcdir builds, BUILD_TREE
   should not be set.

3. I created the static tag NSS_BUILD_TREE_TAG off the
   trunk of NSS that you can test your patch against.  It
   would be nice to test on one of Solaris and HP-UX, and
   on OS/2.

4. I need two weeks to land the BUILD_TREE changes on
   NSS_CLIENT_TAG.  It probably will just take a couple
   of days if I work hard.

5. I would target mozilla0.9.2.  I am sure there are many
   other bugs that we should fix before this one for 0.9.1.
   At this point in the 0.9.1 schedule, we need to focus on
   the product itself rather than the build system.

Comment 9

[cls]

Ok, this is going to be one of *those* cases....

1.  I don't consider not getting cvs updates into my working tree as they occur
a minor issue.  Especially when we're going to put this on the tinderboxes for
developers to use.  I'll concede major as there's no dataloss involved.

2. "should not be set" or "doesn't need to be set"?  Regardless of srcdir or
objdir builds, MOZ_BUILD_ROOT is always set.  What makes the srcdir case any
different wrt BUILD_TREE?

3. cc'ing mcafee, jdunn & mkaply

4. *sigh* and this is what I should have asked about in that meeting.  I pointed
out these build issues (no objdir support, etc) almost 5 months ago.   Bob
landed a fix to the tip only (for whatever reason*) almost 2 weeks ago.   And
now you're saying that you need *another* 2 weeks to fix the problem?  I hope
that I'm not the only one who sees that something's wrong here.

So, for future reference, when does the 2 week turnaround period start:
when the problem is reported?
when you acknowledge that it is a problem?
when you have a fix in hand?

5.  As long as you do not have a dedicated build person, *someone* from your
team is going to have to worry about the build system instead of the features. 
If no one does, then you're creating (or leaving) a problem for the users of
your build system.  <insert standard argument for alternate build systems that
do not have this problem>

* I'm sure that "whatever reason" includes not wanting to move the static tag
which would grab numerous other changes; all of which could be avoided by using
a branch instead of a static tag.

Comment 10

[Chris McAfee]

Tinderbox doesn't need to do objdir builds.  If we don't
expect tag changes to happen for the next few weeks it
would probably be Ok to wait for wtc/0.9.2 target fix.

If coreconf and nss start changing more soon, we'll need
to go ping wtc and ask for a higher priority and/or
get him some help.

Comment 11

[Stephane Saux]

Why is this PSM?

Comment 12

[Chris McAfee]

Stephane is pointing at the product setting, should this be browser, cls?

Comment 13

[Wan-Teh Chang]

The product setting for this bug is right.  To fix
this bug requires changes to both NSS and PSM.  I
just opened an NSS bug (bug #83225: support for building
outside the source tree) and have this bug depend on it.

Comment 14

[Wan-Teh Chang]

Attachment: Proposed patch: cls's patch plus srcdir build support.

Comment 15

[Wan-Teh Chang]

I also attached a patch to the related NSS bug (bug #83225).
I'll ask Bob Relyea to do a final review of the NSS patch
tomorrow.  Meantime, I am verifying my patches on Linux,
Solaris, and Windows, both objdir and srcdir builds.  I do
not expect any problems.

I should be ready to check in my patches on Monday, June 4
afternoon.

Comment 16

[Brian Ryner (not reading)]

wtc: could you please put the definition of ABS_topsrcdir and the addition of
BUILD_TREE to DEFAULT_GMAKE_FLAGS inside an "ifndef MOZ_NSS_AUTOCONF", as those
are not needed by the autoconf branch build?

Thanks.

Comment 17

[Wan-Teh Chang]

bryner: the definition of ABS_topsrcdir and the addition of
BUILD_TREE to DEFAULT_GMAKE_FLAGS are already inside an
"ifndef MOZ_NSS_AUTOCONF".  Could you verify that?

By the way, I forgot to note in this bug report, although I
did note in the NSS bug #83225, that I've already verified this
patch works on Windows, Linux, and Solaris, both srcdir and objdir
builds.  This patch will very likely break OS/2, but it should
only require minor tweaking.

Let me know what you Mozilla guys want to do.  I'm ready to
check in.

Comment 18

[Brian Ryner (not reading)]

wtc: ah, you're right, nevermind.

I do have two more bugs I'd like to see addressed (let me know if I should file
these separately):

1) the build tree should end up in $(OBJDIR)/security/nss, not $(OBJDIR)/nss. 
We want the object tree to exactly mirror the structure of the source tree.

2) For mozilla, you can specify the path to the C compiler by setting CC in the
environment.  It appears that for this to take effect for NSS,
security/manager/Makefile.in needs to add CC=$(CC) to DEFAULT_GMAKE_FLAGS.

Comment 19

[jhp (no longer active)]

This and 83225 build and work fine for os/2 with no changes.

Comment 20

[Wan-Teh Chang]

bryner:

1. This problem can be fixed.

2. The overriding of CC is not supported with the current build
   system.  I'll need to do a code review to know if it works.
   It may just require simple modifications.

   I would like to know what symbols you guys usually override.
   CC, CXX, CFLAGS.  Anything else?

In any case, if we fix #1, the BUILD_TREE variable will be a
strict improvement over what we have now.  Agree?

Comment 21

[Wan-Teh Chang]

I checked in the patch to use the new BUILD_TREE feature
of NSS to build NSS outside the source tree.

bryner: please file two separate bugs for the two issues
you raised.  For the second one, please describe what
variables you want to override (CC, CXX, etc.).  Thank
you.

Comment 22

[John Unruh]

Verified per wtc's comment.