841445 - Add a "mach run" command to launch the application

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Matt Brubeck (:mbrubeck)]

It would be a nice convenience to type "mach run" to run the browser (or whatever application is built) out of the dist/bin directory.

Comment 1

[Gregory Szorc [:gps]]

+1000.

There is lots of logic around for finding the current application's binary and launching it. We should reuse that if possible.

Comment 2

[(not currently active) Ted Mielczarek]

Ideally we could use this same consolidated information to fix bug 648681.

Comment 3

[Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )]

`cfx` in addon-sdk has that magic (using `mozrunner` - https://github.com/mozilla/addon-sdk/blob/master/python-lib/mozrunner/__init__.py#L371) .

Comment 4

[Jeff Hammel]

(In reply to Gregg Lind (User Research - Test Pilot) from comment #3)
> `cfx` in addon-sdk has that magic (using `mozrunner` -
> https://github.com/mozilla/addon-sdk/blob/master/python-lib/mozrunner/
> __init__.py#L371) .

For clarification, cfx uses an older static copy of mozrunner

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Bug 648681 did all the real work; I just want the glory. :)

Comment 6

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

This changes the prefix_chars setting for "mach run" from "-" to "+" so you easily pass arguments starting with "-" like "mach run -no-remote" (instead of "mach run -- -no-remote").

It would be nice to disable prefix_chars completely, but argparse does not seem to provide any way to do this.  (Setting prefix_chars to '' or None just makes it throw an exception; setting it to '\0' mostly works but interferes with its usage output.)

Comment 7

[Matt Brubeck (:mbrubeck)]

Attachment: fix a bug in "mach help run"

"mach help run" currently executes "mach run --help" which in turn runs "/path/to/firefox --help"

This fixes "mach help" to call print_help() directly on the subparser.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 729344 [details] [diff] [review]
patch v2

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

::: python/mozbuild/mozbuild/mach_commands.py
@@ +341,5 @@
>  @CommandProvider
> +class RunProgram(MachCommandBase):
> +    """Launch the compiled binary"""
> +
> +    @Command('run', help='Run the compiled program', prefix_chars='+')

Please add a period after comments and help text. It's what we do.

@@ +345,5 @@
> +    @Command('run', help='Run the compiled program', prefix_chars='+')
> +    @CommandArgument('params', default=None, nargs='*',
> +        help='Command-line arguments to pass to the program')
> +    def run(self, params):
> +        args = [self.get_binary_path('app')]

If the tree isn't built, this will throw. Let's catch that and handle it gracefully.

@@ +347,5 @@
> +        help='Command-line arguments to pass to the program')
> +    def run(self, params):
> +        args = [self.get_binary_path('app')]
> +        if params:
> +            args += params

List concat via += is frowned upon. Use .extend() instead.

@@ +348,5 @@
> +    def run(self, params):
> +        args = [self.get_binary_path('app')]
> +        if params:
> +            args += params
> +        return self.run_process(ensure_exit_code=False, args=args)

We probably want pass_thru=True. This will connect stdout and stderr directly to the terminal.

Comment 9

[Matt Brubeck (:mbrubeck)]

Attachment: patch v3

Addresses review comments.

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 729626 [details] [diff] [review]
patch v3

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

::: python/mozbuild/mozbuild/mach_commands.py
@@ +348,5 @@
> +    def run(self, params):
> +        try:
> +            args = [self.get_binary_path('app')]
> +        except Exception as e:
> +            print(e)

This could probably be more graceful. e.g. "It appears your tree hasn't built yet. Run |mach build| to build it." Even better it would just start building. But, this can be a follow-up. I want this feature to land!

Comment 11

[Matt Brubeck (:mbrubeck)]

(In reply to Gregory Szorc [:gps] from comment #10)
> This could probably be more graceful. e.g. "It appears your tree hasn't
> built yet. Run |mach build| to build it."

I added a friendlier error message (in addition to the exception message, which also contains useful information).

https://hg.mozilla.org/mozilla-central/rev/c9bf19d37fe0
https://hg.mozilla.org/mozilla-central/rev/520cfa198f1d