Closed Bug 503756 Opened 10 years ago Closed 10 years ago

Bug 489579 causes srcdir firefox builds not to start

Categories

(Firefox Build System :: General, defect)

x86
Windows Vista
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: longsonr, Assigned: rain1)

References

Details

(Keywords: regression)

Attachments

(5 files)

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.
Which version of VC++ are you using, and what does your .mozconfig look like?
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
s/the has/the other has/
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?
An objdir build works.
Summary: Bug 489579 causes my firefox builds not to start up → Bug 489579 causes srcdir firefox builds not to start
Ted, is it worth it to try to support srcdir builds?
Yes. We've tried to stop them before and enough people complained that we've decided to continue supporting them for the time being.
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?
(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.
s/an objdir build works/a srcdir build works/
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?
(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.
Keywords: regression
Sorry about the delay.
Assignee: nobody → sid.bugzilla
Status: NEW → ASSIGNED
Attachment #389358 - Flags: review?(benjamin)
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.
Attachment #389362 - Flags: review?(benjamin)
Comment on attachment 389358 [details] [diff] [review]
[checked in] 1. back out 489579 + workaround

Don't need reviews for straight backouts, just do it.
Attachment #389358 - Flags: review?(benjamin) → review+
(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
Attachment #389358 - Attachment description: 1. back out 489579 + workaround → [checked in] 1. back out 489579 + workaround
But if you back something out, you should probably reopen the bug that was backed out.
Attachment #389360 - Flags: review?(kairo) → review+
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.
Attachment #389359 - Flags: review?(jim)
Attachment #389359 - Flags: review?(benjamin)
Attachment #389359 - Flags: review+
Comment on attachment 389359 [details] [diff] [review]
[checked in] 2a. rename js/src/mozilla-config.h

Needs second-review from a JS peer.
(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.
Jim, I don't really care, it seemed good enough. You're welcome to suggest an alternate name.
Attachment #389359 - Flags: review?(jim) → review+
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 on attachment 389359 [details] [diff] [review]
[checked in] 2a. rename js/src/mozilla-config.h

http://hg.mozilla.org/mozilla-central/rev/58d6884a3401
Attachment #389359 - Attachment description: 2a. rename js/src/mozilla-config.h → [checked in] 2a. rename js/src/mozilla-config.h
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
Attachment #389360 - Attachment description: 2b. rename comm-central's mozilla-config.h → [checked in] 2b. rename comm-central's mozilla-config.h
Attachment #389362 - Flags: review?(benjamin) → review+
Comment on attachment 389362 [details] [diff] [review]
[checked in] 3. reland 489579

http://hg.mozilla.org/mozilla-central/rev/ce51f126788d
Attachment #389362 - Attachment description: 3. reland 489579 → [checked in] 3. reland 489579
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3
Target Milestone: mozilla1.9.3 → mozilla1.9.3a1
NB: To modify c-c to help m-c, then not to update c-c ... We need more cooperation!
Attachment #428388 - Flags: review?(bugspam.Callek)
Flags: in-testsuite-
Attachment #428388 - Flags: review?(bugspam.Callek) → review+
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
Attachment #428388 - Attachment description: (Ev1-CC) Port it to comm-central → (Ev1-CC) Port it to comm-central [Checkin: Comment 29]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.