Closed Bug 1030717 Opened 6 years ago Closed 6 years ago

Don't try to create the mach state directory until it's actually needed

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox-esr31 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
mozilla33
Tracking Status
firefox-esr31 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file, 2 obsolete files)

The state directory is barely used, I suspect most people have it empty, and while they have only been bothered once with it, it's annoying, and doesn't help using mach on automation (cf. https://bugzilla.mozilla.org/show_bug.cgi?id=762358#c25 )
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Modified mach environment not to barf about the state directory after having printed environment data already.
Attachment #8446933 - Flags: review?(gps)
Attachment #8446495 - Attachment is obsolete: true
Attachment #8446495 - Flags: review?(gps)
Comment on attachment 8446933 [details] [diff] [review]
Don't try to create the mach state directory until it's actually needed

Review of attachment 8446933 [details] [diff] [review]:
-----------------------------------------------------------------

I approve of deferring the state dir creation until needed. The current setup annoys me.

But, I'm not crazy about changing the API of the context handler callable to only be invoked on missing attribute. There are consumers of mach outside of mozilla-central and I know at least one has a custom context handler.

How about this.

1) We call the context handler with the context as its only argument (like we do today).
2) We install a wrapper that calls the context handler with the key argument on unknown attribute access.

Implementations of context handlers will be defined as such:

def context_handler(context, key=None):
    if not key:
        # Do default/initial population
    elif key == 'foo':
        # Populate missing keys

This could break existing clients. But it still gives people the opportunity to perform one-time context handler population at a well-defined time: mach dispatch time.

::: python/mach/mach/main.py
@@ +178,5 @@
> +                populate_context_handler(context, key)
> +            It acts as a fallback getter for the mach.base.CommandContext
> +            instance.
> +            This allows to augment the context instance with arbitrary data
> +            for use in command handlers.

Fix grammar in the last sentence.
Attachment #8446933 - Flags: review?(gps) → review-
Is this what you were thinking?
Attachment #8449021 - Flags: review?(gps)
Attachment #8446933 - Attachment is obsolete: true
Comment on attachment 8449021 [details] [diff] [review]
Don't try to create the mach state directory until it's actually needed

Review of attachment 8449021 [details] [diff] [review]:
-----------------------------------------------------------------

Yes it is!
Attachment #8449021 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/59a323e670ba
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.