1175968 - Can't pass --help to firefox with ./mach run because ./mach itself reads the --help argument

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[:Gijs (he/him)]

gps suggested "./mach run -- --help" but that didn't work either.

Comment 1

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/file/2694ff2ace6a/python/mach/mach/dispatcher.py#l103 is the problematic code. We should ignore arguments after '--' when scanning for the special help flags.

This would be a good first bug.

Comment 2

[Mike Hommey [:glandium]]

I think --help should just be treated like other flags. As in, added to the ArgumentParser instance for the command.

Comment 3

[Wasif Hyder]

Hi. I would like to work on this bug, and it may be my first. How should I go about reproducing this bug?

Comment 4

[Gregory Szorc [:gps]]

$ mach build
$ mach run -- --help

That second command should display Firefox's help message, not mach's.

Also, while I agree with comment #2, integrating --help handling into the ArgumentParser is a bit complicated. Let's defer that to another time and go with a hack to the code linked to in comment #1.

Comment 5

[Supradeep T R]

I was just thinking in the lines of comment #1 by gps, as there is line which calls for command_help if '-h' or '--help' is found. 
We could skip it as you(gps) said by skipping all flags after '-- ' (I think that whitespace would matter), but how do we then forward the rest of the flags to Firefox functions? 
Or for now are you saying just ignore them?

(I am new here, but used to Python scripting and coding, would need a lot of feedback)

And also, how do build only mach?

Comment 6

[Gregory Szorc [:gps]]

(In reply to tr.supradeep from comment #5)
> We could skip it as you(gps) said by skipping all flags after '-- ' (I think
> that whitespace would matter), but how do we then forward the rest of the
> flags to Firefox functions?

This is already implemented and not something you need to be concerned with.

> And also, how do build only mach?

mach doesn't have a build system. It's just a bunch of Python code that is parsed at run-time. Test changes by changing the source code then running `mach`.

Comment 7

[Supradeep T R]

Thank you for the clarifications gps. May I work on this bug? As wasif_hyder hasn't replied yet. This is my first with Mozilla Firefox. 

So I think can't we just remove https://hg.mozilla.org/mozilla-central/file/2694ff2ace6a/python/mach/mach/dispatcher.py#l103 elif conditional statement? 

That is what is finding '-h' or '--help' in the arguments passed and returning the command_help.

Am I right?

Comment 8

[Gregory Szorc [:gps]]

Yes, you can take it.

Removing the elif would break `mach build --help`. You need to modify the code to only honor -h/--help if they come before an '--' argument.

The following should display help:

  $ mach help run
  $ mach run --help
  $ mach run -h

The following should *not* display help:

  $ mach run -- -h
  $ mach run -- --help

Comment 9

[Supradeep T R]

Thank you. I wanted a suggestion for the best editor to use for Python editing. I am facing many problems while using Notepad++ (indentations problems of 'spaces' vs 'tabs'). I tried both 4 spaces and single tab but can't shake off the errors. I used to use PyCharms as preferred IDE.

Also which would be better code?
1. elif '--' == args[0] and restof statements.

or

2. elif args.index('--') < args.index('-h') or args.index('--') < args.index('--help') and rest.

or 

3. Truncate args from '--' onwards such that others command after it are removed and not honored.


Sorry for asking so many questions. I am quite new and will take some time.

Comment 10

[Gregory Szorc [:gps]]

I'd look at https://stackoverflow.com/questions/81584/what-ide-to-use-for-python.

I use vim. But only because I'm old school. I should be using one of the newer ones.

Comment 11

[Supradeep T R]

Attachment: bug-1175968-fix.patch

I have added the requested patch. Your feedback will be gladly accepted.

Comment 12

[Supradeep T R]

Attachment: new_patch.patch

Previous attachment were wrong commits. 

This attachment has it in the right order. Sorry that I had issues with Mercurial.
Using mq seems slightly complicated, and I read your blog post on mq being obsolete. Why is mq still used?

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8632411 [details] [diff] [review]
new_patch.patch

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

Good work. I'll land this for you.

Comment 14

[Gregory Szorc [:gps]]

url:        https://hg.mozilla.org/integration/fx-team/rev/a3a4614025057f20765f16fc6b516b973bec6ef9
changeset:  a3a4614025057f20765f16fc6b516b973bec6ef9
user:       Supradeep T R <[e-mail]>
date:       Mon Jul 13 16:58:54 2015 -0700
description:
Bug 1175968 - Honor '-h' or '--help' only if it appears before '--'; r=gps

Comment 15

[Gregory Szorc [:gps]]

This has landed. Assuming it doesn't break things (it shouldn't), there is nothing more that needs to be done.

Thanks again for contributing this fix!

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a3a461402505

Comment 17

[Supradeep T R]

Thank you very much for guiding and mentoring GPS, it was a delightful learning experience.