Last Comment Bug 736066 - Build NSS object files more like the rest of the tree
: Build NSS object files more like the rest of the tree
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on: 797508
Blocks: 632954
  Show dependency treegraph
 
Reported: 2012-03-15 07:14 PDT by Mike Hommey [:glandium]
Modified: 2012-12-11 20:32 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Build NSS object files more like the rest of the tree, and simplify security/manager/Makefile.in (7.89 KB, patch)
2012-03-15 07:18 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Review
stack (3.52 KB, text/plain)
2012-03-31 19:50 PDT, Olli Pettay [:smaug]
no flags Details

Description Mike Hommey [:glandium] 2012-03-15 07:14:33 PDT
Bug 632954 gets easier if NSS object files are just in $objdir/$relative_srcdir instead of $objdir/nss/$library_name.
Comment 1 Mike Hommey [:glandium] 2012-03-15 07:18:07 PDT
Created attachment 606197 [details] [diff] [review]
Build NSS object files more like the rest of the tree, and simplify security/manager/Makefile.in

I tested the hack on windows with both pymake and GNU make (which is why there is the $(shell cd $(topsrcdir);pwd)).

At the same time, I seriously simplified security/manager/Makefile.in

Pushed to try for more confidence:
https://tbpl.mozilla.org/?tree=Try&rev=01ef33e1d8a7
Comment 2 Ted Mielczarek [:ted.mielczarek] 2012-03-28 06:14:04 PDT
Comment on attachment 606197 [details] [diff] [review]
Build NSS object files more like the rest of the tree, and simplify security/manager/Makefile.in

Review of attachment 606197 [details] [diff] [review]:
-----------------------------------------------------------------

Despite being kind of crazy, this patch improves security/manager/Makefile.in an awful lot.

::: security/manager/Makefile.in
@@ +184,5 @@
>  DEFAULT_GMAKE_FLAGS += NSS_DISABLE_DBM=1
>  endif
>  ABS_topsrcdir   := $(call core_abspath,$(topsrcdir))
> +# Hack to force NSS build system to use "normal" object directories
> +DEFAULT_GMAKE_FLAGS += BUILD='$(MOZ_BUILD_ROOT)/security/$$(subst $(shell cd $(topsrcdir); pwd)/security/,,$$(CURDIR))'

I'm a little worried about adding a ton of shell invocations for this pwd. Can you do the pwd once in this Makefile and just use that variable in this assignment?
Comment 3 Mike Hommey [:glandium] 2012-03-28 06:30:49 PDT
(In reply to Ted Mielczarek [:ted] from comment #2)
> Comment on attachment 606197 [details] [diff] [review]
> Build NSS object files more like the rest of the tree, and simplify
> security/manager/Makefile.in
> 
> Review of attachment 606197 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Despite being kind of crazy, this patch improves
> security/manager/Makefile.in an awful lot.
> 
> ::: security/manager/Makefile.in
> @@ +184,5 @@
> >  DEFAULT_GMAKE_FLAGS += NSS_DISABLE_DBM=1
> >  endif
> >  ABS_topsrcdir   := $(call core_abspath,$(topsrcdir))
> > +# Hack to force NSS build system to use "normal" object directories
> > +DEFAULT_GMAKE_FLAGS += BUILD='$(MOZ_BUILD_ROOT)/security/$$(subst $(shell cd $(topsrcdir); pwd)/security/,,$$(CURDIR))'
> 
> I'm a little worried about adding a ton of shell invocations for this pwd.
> Can you do the pwd once in this Makefile and just use that variable in this
> assignment?

The $(shell) is invoked once, when appending to DEFAULT_GMAKE_FLAGS. It's subst and CURDIR that are used when building NSS.
Comment 4 Ted Mielczarek [:ted.mielczarek] 2012-03-28 06:43:07 PDT
Aha, thanks.
Comment 6 Olli Pettay [:smaug] 2012-03-31 18:43:50 PDT
Is it possible that this causes crashes if one doesn't do a clobber build?
Some in https://hg.mozilla.org/mozilla-central/pushloghtml?startID=22381&endID=22382 
seems to cause that, and I got crashes in pk11obj.c:633
Comment 7 Ed Morley [:emorley] 2012-03-31 19:16:59 PDT
https://hg.mozilla.org/mozilla-central/rev/369ad04efa1f
Comment 8 Olli Pettay [:smaug] 2012-03-31 19:18:44 PDT
Clobber build didn't help. I'm not at all sure whether this change caused the problem,
but something in the latest merge causes m-c to crash very soon on this 64bit linux.
Comment 9 Olli Pettay [:smaug] 2012-03-31 19:50:42 PDT
Created attachment 611244 [details]
stack

Yes, this causes the crash.
Comment 10 Ed Morley [:emorley] 2012-03-31 19:55:51 PDT
Backed out of m-c:
https://hg.mozilla.org/mozilla-central/rev/4a8a5e8ef78b
Comment 11 Mike Hommey [:glandium] 2012-04-01 00:37:20 PDT
Where is the crash occurring? I don't see anything on tbpl.
Comment 12 Ed Morley [:emorley] 2012-04-01 02:42:39 PDT
Crashes locally for smaug, backing out locally fixed it for him.
Comment 13 Mike Hommey [:glandium] 2012-04-01 02:58:42 PDT
Then I'll need a build log...
Comment 14 Olli Pettay [:smaug] 2012-04-27 05:53:25 PDT
I tried again with gcc, and no crash.
(The previous build was with clang.)
Comment 16 :Ehsan Akhgari (busy, don't ask for review please) 2012-04-30 08:08:00 PDT
https://hg.mozilla.org/mozilla-central/rev/5bc899138eb4

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