Closed Bug 841445 Opened 8 years ago Closed 8 years ago

Add a "mach run" command to launch the application

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla22

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

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.
+1000.

There is lots of logic around for finding the current application's binary and launching it. We should reuse that if possible.
Ideally we could use this same consolidated information to fix bug 648681.
(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
Attached patch patch (obsolete) — Splinter Review
Bug 648681 did all the real work; I just want the glory. :)
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Attachment #729236 - Flags: review?(gps)
Depends on: 648681
Attached patch patch v2 (obsolete) — Splinter Review
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.)
Attachment #729236 - Attachment is obsolete: true
Attachment #729236 - Flags: review?(gps)
Attachment #729344 - Flags: review?(gps)
"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.
Attachment #729345 - Flags: review?(gps)
Attachment #729345 - Flags: review?(gps) → review+
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.
Attachment #729344 - Flags: review?(gps)
Attached patch patch v3Splinter Review
Addresses review comments.
Attachment #729344 - Attachment is obsolete: true
Attachment #729626 - Flags: review?(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!
Attachment #729626 - Flags: review?(gps) → review+
(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
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Duplicate of this bug: 773913
Blocks: 860071
Blocks: 863092
Depends on: 881582
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.