So now when I touch content/media/webaudio/test/Makefile.in, and build in content/media, I get a "reticulating splines" message followed by the whole directory being rebuilt. When I then build in dom/bindings, the whole directory get rebuilt again. This is super annoying. I _think_ it's a regression from the CPPSRCS move, or at least that's when I noticed it.

I think this is actually fallout from a change gps made, but I can't find the bug offhand.

This is by design. The Makefile is listed as a global dependency for many targets in rules.mk, which means that touching Makefile.in or Makefile will invalidate previously-built targets. This is needed to ensure Makefile changes result in rebuilds. The downside is that not all Makefile updates need to invalidate targets. We can be more intelligent about this. That will be done as part of other work and it isn't worth tracking in an explicit bug.

(In reply to comment #2) > This is by design. The Makefile is listed as a global dependency for many > targets in rules.mk, which means that touching Makefile.in or Makefile will > invalidate previously-built targets. This is needed to ensure Makefile changes > result in rebuilds. Is this going to go away in the moz.build world? > The downside is that not all Makefile updates need to invalidate targets. We > can be more intelligent about this. That will be done as part of other work and > it isn't worth tracking in an explicit bug. The problem here is that touching a Makefile.in in the "test" directory causes the contents of the parent to be rebuilt, which is a regression, and it's actually not needed for proper dependency tracking as far as I can tell. Can you please tell me which other bug is tracking this fix?

On deeper investigation, I can reproduce this. Likely regressed from bug 874543.

STR: $ ./config.status $ make -s -C content/media ... AudioAvailableEventManager.cpp AudioChannelFormat.cpp AudioNodeEngine.cpp AudioNodeStream.cpp AudioSegment.cpp AudioStream.cpp AudioStreamTrack.cpp DOMMediaStream.cpp DecoderTraits.cpp FileBlockCache.cpp MediaCache.cpp MediaDecoder.cpp MediaDecoderReader.cpp MediaDecoderStateMachine.cpp MediaResource.cpp MediaStreamGraph.cpp MediaStreamTrack.cpp StreamBuffer.cpp TextTrack.cpp TextTrackCue.cpp TextTrackCueList.cpp TextTrackList.cpp VideoFrameContainer.cpp VideoSegment.cpp VideoStreamTrack.cpp VideoUtils.cpp WebVTTLoadListener.cpp Interestingly, if we run |make -C content| instead of |make -C content/media|, no rebuilding occurs! Huh?!

Woah: $ ./config.status $ make -s -C content <no .cpp files are rebuilt> $ make -s -C content/media <the .cpp files from the previous comment are rebuilt> ?!

So, Makefile is definitely causing invalidation here. Prerequisite `Makefile' is newer than target `AudioAvailableEventManager.o'. Now to figure out why.

So, content/media/Makefile is being touched as part of |make -C content/media|. Digging into |make -d|: Prerequisite `../../backend.RecursiveMakeBackend.built' is newer than target `Makefile'. Must remake target `Makefile'. make: Entering directory `/home/gps/src/firefox/obj-firefox/content/media' Putting child 0x00c02320 (Makefile) PID 5188 on the chain. Live child 0x00c02320 (Makefile) PID 5188 Reaping winning child 0x00c02320 PID 5188 Removing child 0x00c02320 PID 5188 from chain. Successfully remade target file `Makefile'. make: Leaving directory `/home/gps/src/firefox/obj-firefox/content/media' Re-executing[1]: make -d -C content/media ... No need to remake target `../../backend.RecursiveMakeBackend.built'. Finished prerequisites of target file `Makefile'. Prerequisite `../../backend.RecursiveMakeBackend.built' is older than target `Makefile'. No need to remake target `Makefile'. Updating goal targets.... Considering target file `default'. It's not apparent from the output, but I guess GNU make touches Makefile as part of self-reevaluation.

Oh, this is from rules.mk, not a GNU make internal mechanism: # Since Makefile is listed as a global dependency, this has the # unfortunate side-effect of invalidating all targets if it is executed. # So e.g. if you are in /dom/bindings and /foo/moz.build changes, # /dom/bindings will get invalidated. The upside is if the current # Makefile/backend.mk is updated as a result of backend regeneration, we # actually pick up the changes. This should reduce the amount of # required clobbers and is thus the lesser evil. Makefile: $(DEPTH)/backend.RecursiveMakeBackend.built @$(TOUCH) $@

I consider the crux of this bug WONTFIX because I believe the current mechanism for direct make invocation is the best we can get it. However, I've provided this patch to extensively document behavior so it's not spread out over half a dozen bugs (as it is now). In order to make things better, we'll need to a) sacrifice correctness, b) have people execute make with a wrapper that knows how to regenerate the build backend, or c) go down a deep rabbit hole in the backend code (that may not pan out). I'm concerned "c" won't work and thus am ruling it out. I'll put "a" on the table because it will make some developers happy. However, it's a giant paper cut that I fear many will hit and will inevitably result in developers filing bugs "I ran make but my changes to Makefile.in or moz.build weren't picked up." I think "b" is the long term solution and I'd like to move us in that direction. (FWIW "b" is required for a Tup backend.) Regardless of what we do here, we should file a bug so |mach build| knows about backend regeneration. Perhaps there is room for a compromise. Perhaps we could remove the Makefile touch and add a warning message instead? Perhaps we could offer an escape hatch by way of environment variable to disable the touch. I'll entertain proposals. tl;dr There is no perfect solution here. I wish there was, but there isn't. This is an unfortunate regression due to the introduction of moz.build files. We have to look forward and deal with it, I fear.

Comment on attachment 766350 [details] [diff] [review] Thoroughly document backend generation semantics Review of attachment 766350 [details] [diff] [review]: ----------------------------------------------------------------- Wait a minute. If backend.RecursiveMakeBackend.built is decided to be outdated because of a change in a moz.build, and config.status is re-run, and the Makefile has not changed, it means the moz.build change that triggered all that didn't change the Makefile. Now, it may change a backend.mk, and make doesn't handle that, but if you make the Makefile depend on backend.mk and backend.mk on backend.RecursiveMakeBackend.built instead, then things should work much better. At least, it works with the small following testcase: all: echo $@ Makefile: backend.mk touch $@ backend.mk: rebuilt : rebuilt: moz.build touch $@ # config.status touch backend.mk $(warning foo) This would also work if config.status was made to touch Makefile when it updates the corresponding backend.mk. Then the following works: all: echo $@ Makefile: rebuilt : rebuilt: moz.build touch $@ # config.status touch backend.mk touch Makefile $(warning foo)

So here's what I think should happen The rule at[0] exists because moz.build files can have "spooky action at a distance" effects. We rerun config.status if any moz.build (or Makefile.in) has changed anywhere in the tree. That updates the backend file and next time we run in a leaf directory the Makefile is touched which causes everything to rebuild. This all exists to rerun things if you change foo/moz.build and then build in bar/moz.build and foo/moz.build affects what happens in bar/. I think this is rare enough that we should just drop support for it. If you do this you need to do a top-level rebuild, otherwise we just check the Makefile.in/moz.build in the current directory. This avoids what effectively rebuilds the entire tree each time anything changes. [0] http://hg.mozilla.org/mozilla-central/annotate/7ff96bd19c1c/config/rules.mk#l663

The solution of having the build driver check backed update state is the simplest. I believe this is what khuey proposed in the last comment. This patch implements that. Please note that this will likely regress build system behavior for people not building through mach. The two big complaints about mach I've heard from people have been the timestamp prefixing (fixed in bug 847175) and some lingering objdir detection issues (vlad gave me enough info to track down the problem and I should have a patch in bug 920679 shortly). Assuming bug 920679 fixes the last environment detection issue, there should be no excuses left for people to not build through mach. Even the power users (like bz) should be able to adapt their build scripts to invoke mach instead of make. Annoying, yes. But it should be a one-time transition. There are so many wins from having builds go through a wrapper script. It's about time we enforced it. Anyway on to this patch. Before, we checked if config.status was stale in any entrant Makefile (top level or child directory). This had undesirable side-effects for partial tree builds, notably that if the build backend was out of date, the current Makefile was invalidated. With this patch, we removed backend update checking from child Makefiles. We now perform the backend update checked in the toplevel Makefile only. Since we don't want to break partial tree builds, |mach build| will call out to this make target when performing partial tree builds. Since no file touching is involved, no entrant Makefile targets are invalided.

Comment on attachment 812033 [details] [diff] [review] Change when build backend update check it performed Review of attachment 812033 [details] [diff] [review]: ----------------------------------------------------------------- How about adding a backend target at top-level, that would do this too, and a failure when one does make -C foo and backend files under foo/ are outdated? (saying to use make backend) ::: Makefile.in @@ +62,5 @@ > +# > +# The mach build driver will ensure the backend is up to date for partial tree > +# builds. This cleanly avoids most of the pain. > + > +ifndef MOZBUILD_BACKEND_CHECKED sounds like you can remove the ifndef and the export.

Incorporated review feedback. Instead of adding yet another invisible make target, I created a mach command. I figure we'll need to implement this command when we support multiple build backends anyway. So we might as well implement it now.

Comment on attachment 812750 [details] [diff] [review] Change when build backend update check it performed Review of attachment 812750 [details] [diff] [review]: ----------------------------------------------------------------- r+ if you fix rules.mk to error out. ::: config/rules.mk @@ +442,3 @@ > define SUBMAKE # $(call SUBMAKE,target,directory,static) > +@$(UPDATE_TITLE) > ++$(MAKE) $(if $(2),-C $(2)) $(1) As a followup, you could remove the now useless third argument in SUBMAKE calls. @@ +642,5 @@ > else > HOST_OUTOPTION = -o # eol > endif > ################################################################################ > You should still add something like: include $(DEPTH)/backend.RecursiveMakeBackend.built.pp $(DEPTH)/backend.RecursiveMakeBackend.built.pp: $(error Build configuration changed. Run mach build-backend) ::: python/mozbuild/mozbuild/mach_commands.py @@ +505,5 @@ > + description='Generate a backend used to build the tree.') > + def build_backend(self): > + # When we support multiple build backends (Tup, Visual Studio, etc), > + # this command will be expanded to support choosing what to generate. > + # Until then, hardcode config.status. fwiw, i think even in the future with multiple build backends it should still be config.status creating it.

(In reply to Mike Hommey [:glandium] from comment #17) > You should still add something like: > > include $(DEPTH)/backend.RecursiveMakeBackend.built.pp > > $(DEPTH)/backend.RecursiveMakeBackend.built.pp: > $(error Build configuration changed. Run mach build-backend) This won't actually do anything since all backend.RecursiveMakeBackend.built* references have been removed from rules.mk and therefore they don't show up in the default target list. We can't simply add a "Makefile: $(DEPTH)/backend.RecursiveMakeBackend.built" reference back in because if config.status is executed, there's no guarantee the local Makefile or backend.mk will be updated. Checking if the backend is up to date is expensive - as make will need to stat() a few thousand moz.build, Makefile.in, and test manifest files. If we add checking of backend.RecursiveMakeBackend.built back into rules.mk, we'll need the MOZBUILD_BACKEND_CHECKED exported variable again. Is this what you're advocating? If so, the patch would effectively be preserving the existing rules for non-toplevel directories except we'd replace $(touch) Makefile and config.status execution with $(error)s. I think the easiest way to handle things is to have rules.mk enforce that non-toplevel builds are using mach. e.g. ifndef MACH $(error). But that's been controversial and will surely draw ire. It will arguably happen eventually. Do we want to do that now or should we continue to wait? Do you have a better idea?

Incorporated review feedback. Partial tree builds on an outdated build config not building through mach will get a build error. Said error message strongly encourages building through mach.

Comment on attachment 821947 [details] [diff] [review] Change when build backend update check it performed Review of attachment 821947 [details] [diff] [review]: ----------------------------------------------------------------- ::: Makefile.in @@ +66,5 @@ > +# > +# The mach build driver will ensure the backend is up to date for partial tree > +# builds. This cleanly avoids most of the pain. > + > +backend.RecursiveMakeBackend.built: You've been bitrotted ;) ::: python/mozbuild/mozbuild/mach_commands.py @@ +357,5 @@ > > + # Ensure build backend is up to date. While there are rules in > + # rules.mk to do this for non-toplevel builds, the side-effects > + # are weird (it touches the invoked Makefile, invalidating most > + # targets). rules.mk doesn't have rules for non-toplevel builds anymore, right?

https://hg.mozilla.org/integration/mozilla-inbound/rev/9d0bf12c1d1d With unbitrotting and fixed comment.