Closed Bug 1424417 Opened 3 years ago Closed 3 years ago

config.status --dry-run writes out backend.mk files

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: glandium, Assigned: mshal)

References

Details

Attachments

(1 file)

config.status --dry-run is useful when modifying frontend or backend code, and along --diff allows to see whether the changes made alters the output or not. The problem is that when it does alter the output of backend.mk files, they are overwritten (but not other files)

The problem comes from the fact that BackendMakeFile is using FileAvoidWrite directly instead of BuildBackend._write_file. BackendTupfile has the same problem for that matter.

Mike, would you be willing to take a look at this?
Flags: needinfo?(mshal)
See Also: → 1425540
I filed bug 1425540 as a followup for the tup backend since that seems to be less straightforward. For the RecursiveMake backend we can just pass the dry_run flag into BackendMakefile.

Separately, why do we have separate flags for --diff and --dry-run? Personally I would only ever want a --diff to also be a --dry-run, and I don't think a standalone --dry-run is particularly useful without showing a diff. Are there other use-cases I'm not considering? Can we just make --diff imply a dry-run and remove the separate flag?
Flags: needinfo?(mshal)
Assignee: nobody → mshal
(In reply to Michael Shal [:mshal] from comment #1)
> I filed bug 1425540 as a followup for the tup backend since that seems to be
> less straightforward. For the RecursiveMake backend we can just pass the
> dry_run flag into BackendMakefile.
> 
> Separately, why do we have separate flags for --diff and --dry-run?
> Personally I would only ever want a --diff to also be a --dry-run, and I
> don't think a standalone --dry-run is particularly useful without showing a
> diff. Are there other use-cases I'm not considering? Can we just make --diff
> imply a dry-run and remove the separate flag?

No reason besides --dry-run having been implemented well after --diff.
Comment on attachment 8937136 [details]
Bug 1424417 - Use dry-run flag for backend.mk files;

https://reviewboard.mozilla.org/r/207832/#review213998
Attachment #8937136 - Flags: review?(mh+mozilla) → review+
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/009736d49487
Use dry-run flag for backend.mk files; r=glandium
https://hg.mozilla.org/mozilla-central/rev/009736d49487
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.