848530 - Global target for moz.build traversal

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

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.

Comment 1

[Nick Alexander :nalexander [he/him]]

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.

Comment 2

[Gregory Szorc [:gps]]

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.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Check for moz.build traversal at top of build

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).

Comment 4

[Gregory Szorc [:gps]]

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

Comment 5

[Mike Hommey [:glandium]]

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).

Comment 6

[Gregory Szorc [:gps]]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d14e9efe0b00