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

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Andrew Halberstadt [:ahal]]

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.

Comment 1

[Andrew Halberstadt [:ahal]]

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.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: 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 b66d80b9f5566eef25db2cf35e47e87d5194599a https://reviewboard-hg.mozilla.org/gecko/

Comment 3

[Gregory Szorc [: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.

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8603592 [details]
MozReview Request: bz://1163112/ahal

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

Great cleanup. Thanks.

Comment 5

[Andrew Halberstadt [:ahal]]

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/

Comment 6

[Andrew Halberstadt [:ahal]]

I belatedly realized I broke a couple mach tests, latest push fixes that.

Comment 7

[Andrew Halberstadt [:ahal]]

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 8

[Gregory Szorc [:gps]]

Comment on attachment 8603592 [details]
MozReview Request: bz://1163112/ahal

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

Ship It!

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8603592 [details]
MozReview Request: bz://1163112/ahal

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

Ship It!

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8df1ea116b91

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8df1ea116b91

Comment 13

[Andrew Halberstadt [:ahal]]

Attachment: MozReview Request: Bug 1163112 - [mach core] Consolidate functionality between Main._run and Registrar.dispatch, r=gps