Global target for moz.build traversal

RESOLVED FIXED in mozilla24

Status

defect
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

unspecified
mozilla24
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

13:05 < khuey> what is backend.mk?
13:11 < khuey> Error remaking makefiles (ignored)
13:11 < khuey> No rule to remake missing include file backend.mk
13:11 < khuey> c:\dev\mozilla-inbound\config\makefiles\target_export.mk:27:0: command 'c:/mozil
13:11 < khuey> la-build/python/python.exe c:/dev/mozilla-inbound/build/pymake/pymake/../make.py -C bindings export' failed, return code 2
13:12 < khuey> gps: ^
13:13 < gps> khuey: how did you manage that?
13:13 < gps> backend.mk is the moz.build derived make file
13:14 < khuey> gps: I built?
13:14 < gps> :/
13:14 < khuey> though
13:14 < khuey> I did do a rm -rf objdir/dom/bindings
13:14 < khuey> and then a top-level rebuild
13:14 < gps> run ./config.status
13:14 < khuey> to try to clear out a different build issue :-/
13:15 < gps> if that fixes it, we might have a missing dependency to force config.status on directory deletion
13:15 < khuey> yeah that fixed it
13:15 < gps> I'll file a bug

I reckon we regressed something with moz.build switchover. I haven't verified this locally. But, it should be easy enough to reproduce.
I ran into this today, and running $OBJDIR/config.status fixed it.  Not sure how I managed this; possibly rm -rf $OBJDIR/mobile/android/base && make -C $OJBIDR/mobile/android/base.  Not sure what the long term fix is.
I am going to add a global target for config.status that triggers if *any* moz.build change occurs.

I'll explain in the patch why this is necessary.
Assignee: nobody → gps
Blocks: 850380
Status: NEW → ASSIGNED
Summary: Possible missing dependency on missing make files in subdirectory → Global target for moz.build traversal
Depends on: 860957
One of the first actions an invoked Makefile now does is check to see if
*any* moz.build file or Makefile.in is out of date. If so, config.status
is executed to rebuild the build backend.

Since we always perform this check as part of a build, we no longer need
special handling for out of date moz.build files during traversals. This
results in the removal of a significant amount of code!

Another upside of the change is that if a moz.build file is modified
during building, we don't (potentially) modify the build backend from
under the in-progress build. Thus the only race condition that remains
is if a moz.build is mutated during moz.build reading. This window (a
few seconds) is significantly shorter than the time of a full build
(minutes).
Attachment #750820 - Flags: review?(mh+mozilla)
Oh, I just remembered something. I tried to put the logic in rules.mk (instead of root Makefile.in). However, I kept running into path normalization issues. I could probably work this out with enough effort. But, I was feeling a wee bit lazy when I authored this.

Try at https://tbpl.mozilla.org/?tree=Try&rev=782cec96aa8d
No longer depends on: 860957
Comment on attachment 750820 [details] [diff] [review]
Check for moz.build traversal at top of build

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

::: Makefile.in
@@ +35,5 @@
>     netwerk/necko-config.h xpcom/xpcom-config.h xpcom/xpcom-private.h \
>     $(topsrcdir)/.mozconfig.mk $(topsrcdir)/.mozconfig.out
>  
>  ifndef MOZ_PROFILE_USE
> +default alldep all:: CLOBBER $(topsrcdir)/configure config.status backend.RecursiveMakeBackend.built.pp

That's the kind of file that should be in a .deps subdirectory.

@@ +80,5 @@
> +else
> +include backend.RecursiveMakeBackend.built.pp
> +endif
> +
> +export MOZBUILD_BACKEND_CHECKED

I don't expect this to work without giving it a value.

::: config/rules.mk
@@ +615,5 @@
> +# per traversal, hence the ifdef and the export. This rule needs to come before
> +# other rules for the default target or else it may not run in time.
> +ifndef MOZBUILD_BACKEND_CHECKED
> +default::
> +	$(MAKE) -C $(DEPTH) backend.RecursiveMakeBackend.built

Note that because of bug 780824, default is not the default rule in a few directories (this is just to bring awareness, not something you need to handle here).
Attachment #750820 - Flags: review?(mh+mozilla) → review+
I added a value to the export statement.

I couldn't move the .pp file to a .deps directory easily because of path normalization issues. If we normalized topobjdir to an absolute path (bug 873325), this would be much easier.

I'm not too worried about the very few directories where default is not the default target.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d14e9efe0b00
Target Milestone: --- → mozilla24
Blocks: 873629
https://hg.mozilla.org/mozilla-central/rev/d14e9efe0b00
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Blocks: 873809
Blocks: 874078
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.