Closed Bug 1040649 Opened 5 years ago Closed 5 years ago

Detect changes to frozen variables between rules.mk include and the end of Makefile.in

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla33

People

(Reporter: glandium, Assigned: glandium)

Details

Attachments

(1 file)

We have a bunch of Makefiles that include rules.mk and do things after that. It would be better to check they don't change frozen variables. Fortunately, we have another file included after everything from the Makefile.in: recurse.mk (that include is added by the recursemake backend).
Comment on attachment 8458533 [details] [diff] [review]
Detect changes to frozen variables between rules.mk include and the end of Makefile.in

>+# Make sure that anything that needs to be defined in moz.build wasn't
>+# overwritten.
>+_eval_for_side_effects := $(CHECK_MOZBUILD_VARIABLES)

>+# We may have modified "frozen" variables in rules.mk (we do that), but we don't
>+# want Makefile.in doing that, so collect the possibly modified variables here,
>+# and check them again in recurse.mk, which is always included after Makefile.in
>+# contents.
>+$(foreach var,$(_MOZBUILD_EXTERNAL_VARIABLES),$(eval $(var)_FROZEN := '$($(var))'))
>+$(foreach var,$(_DEPRECATED_VARIABLES),$(eval $(var)_FROZEN := '$($(var))'))

It might be helpful to add a token of some kind in the comments to make it obvious which _eval_for_side_effects lines up with which $(foreach var...) block. Even something simple like adding "1st check" to the comments for the original {config/rules.mk, config/config.mk} version, and "2nd check" to these comments for the new {config/recurse.mk, config/rules.mk} version would help understanding. Aside from your comment here that mentions we "check them again in recurse.mk", it is a little hard to follow the overall strategy.
Attachment #8458533 - Flags: review?(mshal) → review+
https://hg.mozilla.org/mozilla-central/rev/8614bc69e290
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.