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

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Mike Hommey [:glandium]]

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?

Comment 1

[Michael Shal [:mshal]]

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?

Comment 2

[Michael Shal [:mshal]]

Attachment: Bug 1424417 - Use dry-run flag for backend.mk files;

For cases like BackendMakefile where we don't have access to
BuildBackend._write_file(), we need to make sure that FileAvoidWrite
uses the dry-run flag in order to avoid writing out files during a
config.status --dry-run.

Review commit: https://reviewboard.mozilla.org/r/207832/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207832/

Comment 3

[Mike Hommey [:glandium]]

(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 4

[Mike Hommey [:glandium]]

Comment on attachment 8937136 [details]
Bug 1424417 - Use dry-run flag for backend.mk files;

https://reviewboard.mozilla.org/r/207832/#review213998

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/009736d49487
Use dry-run flag for backend.mk files; r=glandium

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/009736d49487