Last Comment Bug 715048 - .mozconfig.mk is generated into $topsrcdir instead of the $objdir
: .mozconfig.mk is generated into $topsrcdir instead of the $objdir
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla12
Assigned To: Nicholas Nethercote [:njn]
:
: Gregory Szorc [:gps]
Mentors:
Depends on: 731985 762358
Blocks: 715793
  Show dependency treegraph
 
Reported: 2012-01-03 18:24 PST by Nicholas Nethercote [:njn]
Modified: 2012-06-07 02:06 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (759 bytes, patch)
2012-01-04 14:28 PST, Nicholas Nethercote [:njn]
khuey: review+
Details | Diff | Splinter Review

Description Nicholas Nethercote [:njn] 2012-01-03 18:24:12 PST
I discovered that if you have two objdirs for a single tree, and you build them alternately, configure is re-run every time even if you haven't changed anything.

E.g. if your tree is at $TREE, and you have objdirs $TREE/debug64 and $TREE/opt64, you'll see this if you build debug64, then opt64, then debug64, etc.

I captured the output of -d, I'm pretty sure these are the key lines:

   ...
     Finished prerequisites of target file `/home/njn/moz/mi4/d64/config.status'.
     ...
     Prerequisite `/home/njn/moz/mi4/.mozconfig.mk' is newer than target `/home/njn/moz/mi4/d64/config.status'.
     ...
    Must remake target `/home/njn/moz/mi4/d64/config.status'.
   ...
  Must remake target `configure-files'.
 ...
Must remake target `configure'.


The problem seems to be that .mozconfig.mk is not generated into the objdir.  Presumably it should be.  $(topsrcdir)/.mozconfig.mk appears in a couple of makefiles, as does $(topsrcdir)/.mozconfig.out.
Comment 1 Ted Mielczarek [:ted.mielczarek] 2012-01-03 19:08:00 PST
This is the way it's supposed to work. .mozconfig.mk contains options that are used by client.mk, including things like *the name of the objdir*, so it has to be somewhere that can be included without an objdir present.

I'm not sure why we force rebuilds when the .mk changes, though. There aren't all that many options you can reasonably set for client.mk. It's primarily used to set MOZ_OBJDIR and MOZ_MAKE_FLAGS nowadays.
Comment 2 Nicholas Nethercote [:njn] 2012-01-04 14:28:43 PST
Created attachment 585893 [details] [diff] [review]
patch

This patch avoids re-running configure when .mozconfig.mk changes.  Does anything else need to change?  It'd probably be worth adding a comment to explain why .mozconfig.mk isn't in CONFIG_STATUS_DEPS.
Comment 3 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-04 14:30:41 PST
Comment on attachment 585893 [details] [diff] [review]
patch

I don't think we need comments here.  It's kind of nonsensical for the make options to be in the configure deps to begin with.
Comment 4 Nicholas Nethercote [:njn] 2012-01-04 21:11:16 PST
I guess there's a race here -- if you trigger two builds at the same time there's a small chance that a .mozconfig.mk might be overwritten by the second build before it's read by the first build.  But that has always been there, and this patch doesn't change it.
Comment 5 Nicholas Nethercote [:njn] 2012-01-04 21:32:59 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/c87c1aedefd4
Comment 6 Justin Wood (:Callek) 2012-01-04 22:23:06 PST
(In reply to Nicholas Nethercote [:njn] from comment #4)
> I guess there's a race here -- if you trigger two builds at the same time
> there's a small chance that a .mozconfig.mk might be overwritten by the
> second build before it's read by the first build.  But that has always been
> there, and this patch doesn't change it.

we could bake in a rando-hash type thing for the race-condition case, if we feel its worth it

auto add to the generated .mk "MOZ_MAKE_SANITY=foobarRandomHash" and pass MOZ_MAKE_SANITY_CHECK=foobarRandomHash on the next launch of client.mk when recursed, or simply check it later if not :-)

But I agree "not this bug"
Comment 7 Marco Bonardo [::mak] 2012-01-05 08:28:37 PST
https://hg.mozilla.org/mozilla-central/rev/c87c1aedefd4
Comment 8 :aceman 2012-01-05 12:13:53 PST
Is this relevant for comm-central too?
Comment 9 Nicholas Nethercote [:njn] 2012-01-05 13:56:03 PST
(In reply to :aceman from comment #8)
> Is this relevant for comm-central too?

I think so.
http://hg.mozilla.org/comm-central/file/a038320a5ee6/client.mk#l295 has .mozconfig.mk in CONFIG_STATUS_DEPS.
Comment 10 Martin Stránský 2012-03-20 11:16:11 PDT
Guys, this patch causes a regression in build system. I reported it for thunderbird as Bug 731985 but it applies to Firefox too now (with this patch).

Reproduction steps:

1) download mozilla tree, set up .mozconfig
2) build firefox (make -f client.mk build)
3) build fails or does not fail, it does not matter here
4) change .mozconfig (tweak build config etc.)
5) run "make -f client.mk build" - the changes in .mozconfig are ignored.

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