Closed Bug 1461795 Opened 7 years ago Closed 7 years ago

Use FileAvoidWrite for mozinfo.json

Categories

(Firefox Build System :: General, enhancement)

3 Branch
enhancement
Not set
normal

Tracking

(firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(1 file)

mozinfo.json is loaded by mozbuild/base.py in from_environment(), which means anything that imports buildconfig (such as the file_generate action) depends on this file. Configure always writes mozinfo.json, so running configure causes tup to rebuild all generated files even if mozinfo.json is unchanged. We should use FileAvoidWrite on mozinfo.json to avoid rebuilding things unnecessarily. https://bugzilla.mozilla.org/show_bug.cgi?id=968245#c0 mentions using FileAvoidWrite for this, but didn't end up landing. In a meeting, gps mentioned that mozinfo.json may no longer be necessary now that config.status is in python, but I'm not sure I want to go down that rabbit hole at this time.
Attachment #8975937 - Flags: review?(core-build-config-reviews) → review?(cmanchester)
Comment on attachment 8975937 [details] Bug 1461795 - Use FileAvoidWrite when writing mozinfo.json; https://reviewboard.mozilla.org/r/244138/#review250128 ::: python/mozbuild/mozbuild/config_status.py:120 (Diff revision 1) > if 'WRITE_MOZINFO' in os.environ: > - write_mozinfo(os.path.join(topobjdir, 'mozinfo.json'), env, os.environ) > + with FileAvoidWrite(os.path.join(topobjdir, 'mozinfo.json')) as f: > + write_mozinfo(f, env, os.environ) I believe with this we can also remove the `WRITE_MOZINFO` environment variable and end up writing out mozinfo.json whenever config_status is run and its contents change, effectively fixing the issue in bug 968245 as well. If that's more than you want to tackle for now or causes other issues this is fine to land.
Attachment #8975937 - Flags: review?(cmanchester) → review+
(In reply to Michael Shal [:mshal] from comment #0) > https://bugzilla.mozilla.org/show_bug.cgi?id=968245#c0 mentions using > FileAvoidWrite for this, but didn't end up landing. In a meeting, gps > mentioned that mozinfo.json may no longer be necessary now that > config.status is in python, but I'm not sure I want to go down that rabbit > hole at this time. mozinfo.json is primarily generated for use by our test harnesses (it's what we feed to the test manifest parser for use in evaluating `skip-if` expressions) so I don't think this is true.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3) > (In reply to Michael Shal [:mshal] from comment #0) > > https://bugzilla.mozilla.org/show_bug.cgi?id=968245#c0 mentions using > > FileAvoidWrite for this, but didn't end up landing. In a meeting, gps > > mentioned that mozinfo.json may no longer be necessary now that > > config.status is in python, but I'm not sure I want to go down that rabbit > > hole at this time. > > mozinfo.json is primarily generated for use by our test harnesses (it's what > we feed to the test manifest parser for use in evaluating `skip-if` > expressions) so I don't think this is true. Ahh, I misunderstood then. Is it true we could remove it as part of 'mach build' then? It looks like base.py just uses it to get topsrcdir, topobjdir, and the mozconfig. Not that we need to for this - just curious.
Comment on attachment 8975937 [details] Bug 1461795 - Use FileAvoidWrite when writing mozinfo.json; https://reviewboard.mozilla.org/r/244138/#review250294 ::: python/mozbuild/mozbuild/config_status.py:120 (Diff revision 1) > if 'WRITE_MOZINFO' in os.environ: > - write_mozinfo(os.path.join(topobjdir, 'mozinfo.json'), env, os.environ) > + with FileAvoidWrite(os.path.join(topobjdir, 'mozinfo.json')) as f: > + write_mozinfo(f, env, os.environ) Good call - fixed.
Pushed by mshal@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d90a5a4aab56 Use FileAvoidWrite when writing mozinfo.json; r=chmanchester
(In reply to Michael Shal [:mshal] from comment #4) > Ahh, I misunderstood then. Is it true we could remove it as part of 'mach > build' then? It looks like base.py just uses it to get topsrcdir, topobjdir, > and the mozconfig. Not that we need to for this - just curious. I don't know for sure. I originally added it as a way to feed extra info from the build into mozinfo so we could have things like `skip-if = debug` in test manifests. Along the way things like topsrcdir/topobjdir got stuffed into it, and it also got tied up in the `MozbuildObject.from_environment` code to locate an objdir, so I'm not sure what relies on it now.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: