Changing the code that generates config.status may require a clobber

RESOLVED FIXED in Firefox 53

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: cmanchester, Unassigned)

Tracking

(Blocks 1 bug)

unspecified
mozilla53
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment)

I found working on bug 1316140 that un-applying that patch can require a clobber, because |./mach build| is trusting that the config.status it finds is up to date.
Comment on attachment 8811008 [details]
Bug 1317778 - Emit a depfile with python configure dependencies so Make will know when to re-run configure.

https://reviewboard.mozilla.org/r/93266/#review94846

There's actually more than configure.py that can affect config.status (and in fact, configure.py doesn't change enough that adding it manually makes a difference).

We should probably make configure emit a proper dependency file and include it from client.mk.
Attachment #8811008 - Flags: review?(mh+mozilla) → review-
No longer blocks: artifact
Comment on attachment 8811008 [details]
Bug 1317778 - Emit a depfile with python configure dependencies so Make will know when to re-run configure.

https://reviewboard.mozilla.org/r/93266/#review97418

::: configure.py:36
(Diff revision 2)
>      if sandbox._help:
>          return 0
>  
> -    return config_status(config)
> +    mk = Makefile()
> +    rule = mk.create_rule()
> +    rule.add_targets([os.path.join(config['TOPSRCDIR'], 'configure'),

It will be better for make to:
- use a literal '$(TOPSRCDIR)', because in some corner cases, config['TOPSRCDIR'] might be different, and that would make the dependencies unused.
- use mozpath instead of os.path, because forward slashes are better.
- and in fact, the right target for the dependencies is not $(TOPSRCDIR)/**/configure, but $(OBJDIR)/config.status.

This should also go into the config_status function.
Attachment #8811008 - Flags: review?(mh+mozilla)
Comment on attachment 8811008 [details]
Bug 1317778 - Emit a depfile with python configure dependencies so Make will know when to re-run configure.

https://reviewboard.mozilla.org/r/93266/#review98002

::: client.mk:333
(Diff revision 3)
>    $(TOPSRCDIR)/testing/mozbase/packages.txt \
>    $(OBJDIR)/.mozconfig.json \
>    $(NULL)
>  
> +# Include a dep file emitted by configure to track Python files that
> +# may incluence the result of configure.

typo

::: configure.py:36
(Diff revision 3)
>      if sandbox._help:
>          return 0
>  
> -    return config_status(config)
> +    return config_status(config, sandbox)
>  
> -
> +def config_status(config, sandbox):

Mmmmm it would be better if we could avoid the signature change, so that we don't break comm-central. How about a set_config() at the end of moz.configure to be able to grab _all_paths from config instead of sandbox?

But I won't stop you if you don't feel like it's necessary ; after all, that only requires a one-liner in comm-central to be fixed.
Attachment #8811008 - Flags: review?(mh+mozilla) → review+
Attachment #8811008 - Flags: review+ → review?(mh+mozilla)
Comment on attachment 8811008 [details]
Bug 1317778 - Emit a depfile with python configure dependencies so Make will know when to re-run configure.

https://reviewboard.mozilla.org/r/93266/#review100554

Sorry for the delay.
Attachment #8811008 - Flags: review?(mh+mozilla) → review+
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/202f46bb664d
Emit a depfile with python configure dependencies so Make will know when to re-run configure. r=glandium
https://hg.mozilla.org/mozilla-central/rev/202f46bb664d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.