503756 - Bug 489579 causes srcdir firefox builds not to start

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Robert Longson [:longsonr]]

This check in is the problem:
http://hg.mozilla.org/mozilla-central/rev/0cb931e6aaa7

With that change, I get this on start up.

*** Registering components in: ZipWriterModule
*** Registering components in: Apprunner
++DOCSHELL 0269FD40 == 1
++DOMWINDOW == 1 (027059D0) [serial = 1] [outer = 00000000]
WARNING: NS_ENSURE_TRUE(em) failed: file c:/moz2/mozilla/toolkit/xre/nsAppRunner.cpp, line 3275

This line is returning null in nsAppRunner.cpp

            nsCOMPtr<nsIExtensionManager> em(do_GetService("@mozilla.org/extensions/manager;1"));

And then start up terminates.

This affects both machines I use to build firefox. I've tried reinstalling the build tools and this has had no effect.

Downloaded nightlies still start. Safe mode and/or a new profile have no effect.

Comment 1

[Rain [:rain1] (inactive)]

Which version of VC++ are you using, and what does your .mozconfig look like?

Comment 2

[Robert Longson [:longsonr]]

One machine has Visual C++ 2005 Express Edition on Vista, the has uses Visual C++ 2005 Professional on Windows XP.

I don't think my .mozconfig is very interesting ;-)

. $topsrcdir/browser/config/mozconfig

ac_add_options --disable-optimize
ac_add_options --enable-debug

Comment 3

[Robert Longson [:longsonr]]

s/the has/the other has/

Comment 4

[Rain [:rain1] (inactive)]

Hm, the mozconfig _is_ interesting ;) you're doing a srcdir build.

Could you configure an objdir as described in <https://developer.mozilla.org/en/Configuring_Build_Options#Building_with_an_Objdir>, clobber, and try again?

Comment 5

[Robert Longson [:longsonr]]

An objdir build works.

Comment 6

[Rain [:rain1] (inactive)]

Ted, is it worth it to try to support srcdir builds?

Comment 7

[Benjamin Smedberg]

Yes. We've tried to stop them before and enough people complained that we've decided to continue supporting them for the time being.

Comment 8

[Benjamin Smedberg]

From bug 489579, it appears we're picking up js/src/mozilla-config.h instead of the real mozilla-config.h

I'm confused why we stopped using $(DEPTH)/mozilla-config.h and are instead using the bare version... can somebody explain that?

Comment 9

[Rain [:rain1] (inactive)]

(In reply to comment #8)
> From bug 489579, it appears we're picking up js/src/mozilla-config.h instead of
> the real mozilla-config.h
> 
> I'm confused why we stopped using $(DEPTH)/mozilla-config.h and are instead
> using the bare version... can somebody explain that?

We did that because of bug 489579 comment 7. We were picking up the comm-central mozilla-config.h instead of the m-c one, since we look for LOCAL_INCLUDES before the current directory, and any LOCAL_INCLUDE which has a depth one less than the current dir would cause the comm-central version to be matched. An example of such a LOCAL_INCLUDE is <http://mxr.mozilla.org/mozilla-central/source/content/xbl/src/Makefile.in#112>.

A few options at this point:
1. Back this workaround out, and use the other one in bug 489579. (Robert or Wei, could you back attachment 383168 [details] [diff] [review] out, apply attachment 383167 [details] [diff] [review], and see if an objdir build works?)
2. Rename the comm-central and js versions to comm-config.h and js-config.h. I don't really think it's a good idea to keep different headers with the same name around.
3. Back out bug 489579 entirely. Greatly reduced console/tinderbox spew, while nice, isn't worth breaking builds I guess.

Comment 10

[Rain [:rain1] (inactive)]

s/an objdir build works/a srcdir build works/

Comment 11

[Benjamin Smedberg]

I agree with #2, but I'm not sure we can do it quickly. Can we back out the original bug, fix the comm-config.js and js-config.h thing (be aware there is already a js-config.h, so you'll need a different name) and reland it when that's done?

Comment 12

[Robert Longson [:longsonr]]

(In reply to comment #9)
> A few options at this point:
> 1. Back this workaround out, and use the other one in bug 489579. (Robert or
> Wei, could you back attachment 383168 [details] [diff] [review] out, apply attachment 383167 [details] [diff] [review], and see if
> a srcdir build works?)

A srcdir build does work in the above circumstances.

Comment 13

[Rain [:rain1] (inactive)]

Attachment: [checked in] 1. back out 489579 + workaround

Sorry about the delay.

Comment 14

[Rain [:rain1] (inactive)]

Attachment: [checked in] 2a. rename js/src/mozilla-config.h

Comment 15

[Rain [:rain1] (inactive)]

Attachment: [checked in] 2b. rename comm-central's mozilla-config.h

Comment 16

[Rain [:rain1] (inactive)]

Attachment: [checked in] 3. reland 489579

I added a bit of extra protection with the $(DEPTH)/dist/include, just in case...

The try server seemed happy enough with all the patches.

Comment 17

[Benjamin Smedberg]

Comment on attachment 389358 [details] [diff] [review]
[checked in] 1. back out 489579 + workaround

Don't need reviews for straight backouts, just do it.

Comment 18

[Rain [:rain1] (inactive)]

(In reply to comment #17)
> (From update of attachment 389358 [details] [diff] [review])
> Don't need reviews for straight backouts, just do it.

http://hg.mozilla.org/mozilla-central/rev/079d432f4ebb

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

But if you back something out, you should probably reopen the bug that was backed out.

Comment 20

[Robert Kaiser]

Comment on attachment 389360 [details] [diff] [review]
[checked in] 2b. rename comm-central's mozilla-config.h

While I generally don't like increasing the diff between comm-central and mozilla-central build system files, I'm OK with this if the general approach is signed off by Ted and/or Benjamin.

Comment 21

[Benjamin Smedberg]

Comment on attachment 389359 [details] [diff] [review]
[checked in] 2a. rename js/src/mozilla-config.h

Needs second-review from a JS peer.

Comment 22

[Jim Blandy :jimb]

(In reply to comment #21)
> (From update of attachment 389359 [details] [diff] [review])
> Needs second-review from a JS peer.

Benjamin, do you think js-defines.h is a good name?  It'd be nice to suggest that it's configure-derived, perhaps js-confdefs.h.

Comment 23

[Benjamin Smedberg]

Jim, I don't really care, it seemed good enough. You're welcome to suggest an alternate name.

Comment 24

[Jim Blandy :jimb]

Comment on attachment 389359 [details] [diff] [review]
[checked in] 2a. rename js/src/mozilla-config.h

Approved, but please name the generated file js-confdefs.h instead of js-defines.h.

Comment 25

[Rain [:rain1] (inactive)]

Comment on attachment 389359 [details] [diff] [review]
[checked in] 2a. rename js/src/mozilla-config.h

http://hg.mozilla.org/mozilla-central/rev/58d6884a3401

Comment 26

[Rain [:rain1] (inactive)]

Comment on attachment 389360 [details] [diff] [review]
[checked in] 2b. rename comm-central's mozilla-config.h

http://hg.mozilla.org/comm-central/rev/65d02f78e088

Comment 27

[Rain [:rain1] (inactive)]

Comment on attachment 389362 [details] [diff] [review]
[checked in] 3. reland 489579

http://hg.mozilla.org/mozilla-central/rev/ce51f126788d

Comment 28

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1-CC) Port it to comm-central [Checkin: Comment 29]

NB: To modify c-c to help m-c, then not to update c-c ... We need more cooperation!

Comment 29

[Serge Gautherie (:sgautherie)]

Comment on attachment 428388 [details] [diff] [review]
(Ev1-CC) Port it to comm-central
[Checkin: Comment 29]


http://hg.mozilla.org/comm-central/rev/ccf1f72d7221