fix test_build.py for local builds

RESOLVED FIXED in Firefox 62

Status

defect
RESOLVED FIXED
Last year
Last year

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

unspecified
mozilla62

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

Last year
test_build.py fails on local builds with messages like:

make[4]: Entering directory '/tmp/tmp65BjCH'
make[4]: *** No rule to make target 'buildid.h'.  Stop.
make[4]: Leaving directory '/tmp/tmp65BjCH'
/home/froydnj/src/gecko-dev.git/config/faster/rules.mk:76: recipe for target '/tmp/tmp65BjCH/buildid.h' failed
make[3]: *** [/tmp/tmp65BjCH/buildid.h] Error 2
make[3]: *** Waiting for unfinished jobs....
make[4]: Entering directory '/tmp/tmp65BjCH'
make[4]: *** No rule to make target 'source-repo.h'.  Stop.
make[4]: 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[3]: *** [/tmp/tmp65BjCH/source-repo.h] Error 2
make[3]: 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.
Assignee

Comment 1

Last year
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...
Attachment #8975542 - Flags: review?(mshal)
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 [1], which triggers the rule at [2], 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 [2] 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 :)

[1] https://dxr.mozilla.org/mozilla-central/rev/45ec8fd380dd2c308e79dbb396ca87f2ce9b3f9c/config/faster/rules.mk#82
[2] 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'
Attachment #8975542 - Flags: review?(mshal)
Attachment #8975542 - Flags: review?(cmanchester)
Attachment #8975542 - Flags: feedback+
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+

Comment 4

Last year
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73dd77918ce5
fix test_build.py for local builds; r=chmanchester,f=mshal

Comment 5

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/73dd77918ce5
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.