Closed Bug 1040649 Opened 5 years ago Closed 5 years ago

Detect changes to frozen variables between include and the end of


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: glandium, Assigned: glandium)



(1 file)

We have a bunch of Makefiles that include 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 (that include is added by the recursemake backend).
Comment on attachment 8458533 [details] [diff] [review]
Detect changes to frozen variables between include and the end of

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

>+# We may have modified "frozen" variables in (we do that), but we don't
>+# want doing that, so collect the possibly modified variables here,
>+# and check them again in, which is always included after
>+# 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/, config/} version, and "2nd check" to these comments for the new {config/, config/} version would help understanding. Aside from your comment here that mentions we "check them again in", it is a little hard to follow the overall strategy.
Attachment #8458533 - Flags: review?(mshal) → review+
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.