736066 - Build NSS object files more like the rest of the tree

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

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]]

Attachment: 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

[(not currently active) Ted Mielczarek]

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]]

(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

[(not currently active) Ted Mielczarek]

Aha, thanks.

Comment 5

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/369ad04efa1f

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

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]]

https://hg.mozilla.org/mozilla-central/rev/369ad04efa1f

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

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][[e-mail]]]

Attachment: stack

Yes, this causes the crash.

Comment 10

[Ed Morley [:emorley]]

Backed out of m-c:
https://hg.mozilla.org/mozilla-central/rev/4a8a5e8ef78b

Comment 11

[Mike Hommey [:glandium]]

Where is the crash occurring? I don't see anything on tbpl.

Comment 12

[Ed Morley [:emorley]]

Crashes locally for smaug, backing out locally fixed it for him.

Comment 13

[Mike Hommey [:glandium]]

Then I'll need a build log...

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

I tried again with gcc, and no crash.
(The previous build was with clang.)

Comment 15

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5bc899138eb4

Comment 16

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/5bc899138eb4