Closed Bug 1279564 Opened 3 years ago Closed 3 years ago

Move state directory creation code from mach_bootstrap.py into python/mozboot

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox50 fixed)

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(3 files)

Bug 1277406 introduced duplication of code for the state directory. We should make the canonical home under python/mozboot and have mach_bootstrap.py import and use that code.
We'll be consolidating code from mach_bootstrap.py and mozboot.
We don't want mach_bootstrap.py to import bootstrap.py because it
imports nearly every module under mozboot. So establish a standalone
module with minimal dependencies to hold utility code. Move
get_state_dir() there.

Review commit: https://reviewboard.mozilla.org/r/58994/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58994/
Attachment #8762115 - Flags: review?(mh+mozilla)
Attachment #8762116 - Flags: review?(mh+mozilla)
Attachment #8762117 - Flags: review?(mh+mozilla)
This matches the implementation from mach_bootstrap.py.

Review commit: https://reviewboard.mozilla.org/r/58996/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58996/
Import mozboot.util and use the function from there.

Review commit: https://reviewboard.mozilla.org/r/58998/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58998/
Attachment #8762115 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8762116 [details]
Bug 1279564 - Return a 2-tuple from get_state_dir();

https://reviewboard.mozilla.org/r/58996/#review56280

::: python/mozboot/mozboot/bootstrap.py:214
(Diff revision 1)
>          # The state directory code is largely duplicated from mach_bootstrap.py.
>          # We can't easily import mach_bootstrap.py because the bootstrapper may
>          # run in self-contained mode and only the files in this directory will
>          # be available. We /could/ refactor parts of mach_bootstrap.py to be
>          # part of this directory to avoid the code duplication.
> -        state_dir = get_state_dir()
> +        state_dir = get_state_dir()[0]

state_dir, _ = get_state_dir() ?
Attachment #8762116 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8762117 [details]
Bug 1279564 - Use get_state_dir() from mozboot;

https://reviewboard.mozilla.org/r/58998/#review56282
Attachment #8762117 - Flags: review?(mh+mozilla) → review+
It feels to me something should be done about STATE_DIR_FIRST_RUN and STATE_DIR_INFO.
Comment on attachment 8762115 [details]
Bug 1279564 - Move get_state_dir() to own module;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58994/diff/1-2/
Comment on attachment 8762116 [details]
Bug 1279564 - Return a 2-tuple from get_state_dir();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58996/diff/1-2/
Comment on attachment 8762117 [details]
Bug 1279564 - Use get_state_dir() from mozboot;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58998/diff/1-2/
https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=2812ef2db597
Assignee: nobody → gps
Status: NEW → ASSIGNED
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/910bb13e5929
Move get_state_dir() to own module; r=glandium
https://hg.mozilla.org/integration/autoland/rev/cc353a894d96
Return a 2-tuple from get_state_dir(); r=glandium
https://hg.mozilla.org/mozilla-central/rev/910bb13e5929
https://hg.mozilla.org/mozilla-central/rev/cc353a894d96
https://hg.mozilla.org/mozilla-central/rev/2812ef2db597
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.