fix for local builds

RESOLVED FIXED in Firefox 62


Last year
Last year


(Reporter: froydnj, Assigned: froydnj)



Firefox Tracking Flags

(firefox62 fixed)



(1 attachment)



Last year 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/ 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/ 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
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.

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 for buildid.h and
source-repo.h play into this...
Attachment #8975542 - Flags: review?(mshal)
Comment on attachment 8975542 [details] [diff] [review]
fix 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/ 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 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/ 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 :)


>diff --git a/ b/
>index f08bb62..b573c8d 100644
>--- a/
>+++ b/
>@@ -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 \
> # 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.
>+needs_buildid_rule := 1
>+# Tests need a new buildid always.
>+needs_buildid_rule := 1
>+ifdef needs_buildid_rule

If we do go with this approach, I think this could be simplified by doing:


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 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/ 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
fix for local builds; r=chmanchester,f=mshal

Comment 5

Last year
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.