Closed Bug 1163112 Opened 6 years ago Closed 6 years ago

Registrar.dispatch() bypasses features like handler.parser and handler.conditions

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(1 file, 1 obsolete file)

There exists a function that sort of "fakes" a mach invocation:
https://dxr.mozilla.org/mozilla-central/source/python/mach/mach/registrar.py#41

This is useful for one command to invoke another without shelling out to the system. However using this function bypasses certain features, such as argument parsers, subcommand_parsers and condition functions. In the case of mochitest, the argument parser contains a lot of validation logic that is essential to running the command, so bypassing it means the command will fail.

There is a TODO in the link above about consolidating Registrar.dispatch() with Main._run(). This would make sure all invocations go through the same code path, and therefore all of the bypassed features would no longer be bypassed. So that seems like the best approach to solving this, I'll dig into it some more.
To clarify, running any mochitests using |mach test| is broken until this is fixed (see bug 1163037). It was originally regressed by my patch in bug 1155338, but I believe this bug is the proper way to fix it.
Attached file MozReview Request: bz://1163112/ahal (obsolete) —
/r/8441 - Bug 1163112 - [mach core] Consolidate functionality between Main._run and Registrar.dispatch, r=gps

Pull down this commit:

hg pull -r b66d80b9f5566eef25db2cf35e47e87d5194599a https://reviewboard-hg.mozilla.org/gecko/
Attachment #8603592 - Flags: review?(gps)
https://reviewboard.mozilla.org/r/8441/#review7183

::: python/mach/mach/main.py:153
(Diff revision 1)
> -            except AttributeError, TypeError:
> +            except (AttributeError, TypeError):

Yikes. Good catch.
Attachment #8603592 - Flags: review?(gps) → review+
Comment on attachment 8603592 [details]
MozReview Request: bz://1163112/ahal

https://reviewboard.mozilla.org/r/8439/#review7185

Great cleanup. Thanks.
Comment on attachment 8603592 [details]
MozReview Request: bz://1163112/ahal

/r/8441 - Bug 1163112 - [mach core] Consolidate functionality between Main._run and Registrar.dispatch, r=gps

Pull down this commit:

hg pull -r 51a17e235abc6bc5138b4b34a325242ee1f23367 https://reviewboard-hg.mozilla.org/gecko/
I belatedly realized I broke a couple mach tests, latest push fixes that.
Comment on attachment 8603592 [details]
MozReview Request: bz://1163112/ahal

/r/8441 - Bug 1163112 - [mach core] Consolidate functionality between Main._run and Registrar.dispatch, r=gps

Pull down this commit:

hg pull -r 37fb8698c388b63b212c4b02b387dd8814eeecde https://reviewboard-hg.mozilla.org/gecko/
Comment on attachment 8603592 [details]
MozReview Request: bz://1163112/ahal

https://reviewboard.mozilla.org/r/8439/#review7239

Ship It!
Attachment #8603592 - Flags: review+
Comment on attachment 8603592 [details]
MozReview Request: bz://1163112/ahal

https://reviewboard.mozilla.org/r/8439/#review7241

Ship It!
Attachment #8603592 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/8df1ea116b91
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Attachment #8603592 - Attachment is obsolete: true
Attachment #8620261 - Flags: review+
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.