Closed Bug 1461383 Opened 2 years ago Closed 2 years ago
_build .py for local builds
test_build.py fails on local builds with messages like: make: Entering directory '/tmp/tmp65BjCH' make: *** No rule to make target 'buildid.h'. Stop. make: Leaving directory '/tmp/tmp65BjCH' /home/froydnj/src/gecko-dev.git/config/faster/rules.mk:76: recipe for target '/tmp/tmp65BjCH/buildid.h' failed make: *** [/tmp/tmp65BjCH/buildid.h] Error 2 make: *** Waiting for unfinished jobs.... make: Entering directory '/tmp/tmp65BjCH' make: *** No rule to make target 'source-repo.h'. Stop. make: Leaving directory '/tmp/tmp65BjCH' /home/froydnj/src/gecko-dev.git/config/faster/rules.mk:76: recipe for target '/tmp/tmp65BjCH/source-repo.h' failed make: *** [/tmp/tmp65BjCH/source-repo.h] Error 2 make: Leaving directory '/tmp/tmp65BjCH/faster' Makefile:155: recipe for target 'faster' failed The tests pass in automation, however, because automation always defines the rules for buildid.h and source-repo.h in the toplevel Makefile.in. For local builds, however, those rules are not defined to avoid build churn. Let's ensure that the necessary rules are defined during testing as well.
We talked about this over IRC, so I figured you were a reasonable candidate to review this. I'm not *totally* sure about the commit message; I'm not entirely sure how the GENERATED_FILES bits in the root moz.build for buildid.h and source-repo.h play into this...
Comment on attachment 8975542 [details] [diff] [review] fix test_build.py for local builds Redirecting to chmanchester. Although this approach works, I'm not really sure if it's the right way to do it. It looks like faster/rules.mk has the install manifests depend on buildid.h + source-repo.h in , which triggers the rule at , and that fails because we don't have a buildid.h rule generated in backend.mk for the faster-make backend. It seems to me like the FORCE dependency in the top-level Makefile that makes this work is a happy accident, and we probably need to revisit the assumptions in faster/rules.mk so that they reflect how buildid.h is now built as a result of bug 1298328. (ie: do we still need buildid.h/source-repo.h as explicit dependencies here now that they are generated differently? Or do we need to update the rule in  to accommodate them? Or should buildid.h+source-repo.h rules be generated in the faster-make backend?). Unfortunately I don't know the answer to this, so hopefully chmanchester does :)  https://dxr.mozilla.org/mozilla-central/rev/45ec8fd380dd2c308e79dbb396ca87f2ce9b3f9c/config/faster/rules.mk#82  https://dxr.mozilla.org/mozilla-central/rev/45ec8fd380dd2c308e79dbb396ca87f2ce9b3f9c/config/faster/rules.mk#73 >diff --git a/Makefile.in b/Makefile.in >index f08bb62..b573c8d 100644 >--- a/Makefile.in >+++ b/Makefile.in >@@ -30,16 +30,23 @@ DIST_GARBAGE = config.cache config.log config.status* config-defs.h \ > netwerk/necko-config.h xpcom/xpcom-config.h xpcom/xpcom-private.h \ > .mozconfig.mk > > ifndef MOZ_PROFILE_USE > # Automation builds should always have a new buildid, but for the sake of not > # re-linking libxul on every incremental build we do not enforce this for > # developer builds. > ifneq (,$(MOZ_AUTOMATION)$(MOZ_BUILD_DATE)) >+needs_buildid_rule := 1 >+endif >+# Tests need a new buildid always. >+ifdef TEST_MOZBUILD >+needs_buildid_rule := 1 >+endif >+ifdef needs_buildid_rule If we do go with this approach, I think this could be simplified by doing: ifneq (,$(MOZ_AUTOMATION)$(MOZ_BUILD_DATE)$(TEST_MOZBUILD)) Since the ifneq construction is effectively an 'if A or B or C'
Comment on attachment 8975542 [details] [diff] [review] fix test_build.py for local builds Review of attachment 8975542 [details] [diff] [review]: ----------------------------------------------------------------- I don't have great answers for mshal's questions about faster-make and buildid.h, but removing those buildid.h/source-repo.h dependencies in faster/rules.mk isn't entirely straightforward. This patch is a fine workaround (I agree with Mike's comment about avoiding the new variable), and in the context of the test I don't think it's doing any harm.
Attachment #8975542 - Flags: review?(cmanchester) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/73dd77918ce5 fix test_build.py for local builds; r=chmanchester,f=mshal
You need to log in before you can comment on or make changes to this bug.