classification
Title: argparse support for "python -m module" in help
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Nils Kattenbeck, bethard, jwilk, ncoghlan, paul.j3, peter.otten, samuelmarks, tebeka, zertrin
Priority: normal Keywords: patch

Created on 2014-08-20 19:59 by tebeka, last changed 2021-08-08 22:41 by Nils Kattenbeck.

Files
File name Uploaded Description Edit
prog.diff tebeka, 2014-08-20 19:59 Patch review
prog2.diff tebeka, 2014-08-24 17:42 Patch review
prog3.diff tebeka, 2014-08-25 03:45 Patch review
prog3.diff tebeka, 2014-08-26 11:18 Patch review
prog-module-name-handler.patch samuelmarks, 2019-10-05 04:05 Patch for handling `python -m` syntax in argument parser ouptut
patch.diff Nils Kattenbeck, 2021-07-23 16:08
Messages (23)
msg225586 - (view) Author: Miki Tebeka (tebeka) * Date: 2014-08-20 19:59
"python -m module -h" starts with
    usage: __main__.py

It should be
    usage: python -m module
msg225608 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-08-21 14:04
I suspect resolving this will actually need special casing on the argparse side - __main__.__spec__ will have the original details for __main__ when executed via -m of zipfile/directory execution.

Things to check for:

* "__main__.__spec__ is not None" indicates a "-m" invocation
* __main__.__spec__.name will give you the module name that was executed
* if it is exactly "__main__" then this is likely zipfile or directory execution, and sys.argv[0] will still give the correct name
* if it ends with a ".__main__", it is likely package execution, and the last segment should be dropped
* otherwise, it is normal module execution

In the latter two cases, sys.executable can be used to figure out what to put before the "-m" (we don't expose the raw C level argv to Python, although there is an open RFE to do so somewhere on the tracker).
msg225669 - (view) Author: Miki Tebeka (tebeka) * Date: 2014-08-22 14:41
For zip file the help should probably be:
    usage: python file.zip
msg225831 - (view) Author: Miki Tebeka (tebeka) * Date: 2014-08-24 17:42
New patch with handling of zip files.
msg225834 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-08-24 20:03
The patch tests assume the test is being run in a particular way:

    python -m unittest ...

But I often use

    python3 -m unittest ...

or

    python3 test_argparse.py

both of which fail these tests.

Testing this change might be more difficult than implementing it.
msg225838 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-08-24 21:19
What if the package is run without `-m`?
msg225862 - (view) Author: Miki Tebeka (tebeka) * Date: 2014-08-25 03:45
Made the test more robust by using sys.executable in the comparison.
msg225863 - (view) Author: Miki Tebeka (tebeka) * Date: 2014-08-25 03:45
How can you run a package without -m?
msg225865 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-08-25 05:13
Package might be the wrong term.  How about a directory with a `__main__.py` file, and no local imports (relative or not)?  In my tests it runs with and without the -m.
msg225911 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-08-26 07:47
If you have:

  <curdir>
    /subdir
        __main__.py

Then in 3.3+, both of the following will work:

    python3 subdir
    python3 -m subdir

They do slightly different things, though.

In the first case, "subdir" will be added to sys.path, and then python will execute the equivalent of "python3 -m __main__"

In the second case, the current directory will be added to sys.path, and python will execute the equivalent of "python3 -m subdir.__main__"

The first case is the directory execution support that was added way back in Python 2.6.

The second case is a combination of the package execution support added in Python 2.7/3.1 and the implicit namespace packages support that was added in Python 3.3.

Interesting find - the possibility of the latter situation hadn't occurred to me before :)
msg225913 - (view) Author: Miki Tebeka (tebeka) * Date: 2014-08-26 11:18
Support for directory invocation as well.
msg226578 - (view) Author: Miki Tebeka (tebeka) * Date: 2014-09-08 13:00
Anything else I need to solve to get this patch accepted?
msg226606 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-09-08 19:54
When I apply `prog3.diff` to my 3.5.0dev, and run

    python3 -m unittest Lib/test/test_argparse.py

I get 2 failures, both over 'usage: python3 -m [unit]test'.

It also fails with

    python3 test_argparse.py

I suspect it would also fail if I ran the tests with `nosetests` (though I'm not setup to use that in the development version).
msg226622 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-09-09 05:43
In argparse.py, I would put a utility function like `_prog_name` near the start in the 'Utility functions and classes' block.  

In test_argparse.py, I don't see the purpose of the `progname` changes in the TestParentParsers class.  That class wasn't producing an error, even though 'unittest' has a '__main__'.  Is there some other way of running the test that produces an error with the existing code?  

Most tests for 'usage' and 'help', use 'prog="PROG"' to get around the various ways that the tests can be run.  I don't see what TestParentParsers gains by not doing the same.  The 'prog' attribute has nothing to do with 'parents', at least not that I can tell.

I think a patch like this needs a new test(s), one that fails with existing  code, and run fine with the patch.  It also needs to work with almost any method of running the tests.  But writing such a test might be lot harder than writing the fix, since it involves system issues, and possibly mocking a package and a zip file.

Nick's suggestion regarding '__spec__' also needs to be considered.  It is an area of active development.  I'm not sure that 'argparse' should be getting into these details regarding how the script is invoked and/or packaged.

Finally, does this patch accomplish anything that the user can't do by directly setting the 'prog' attribute?  The user could even use a function like '_prog_name' to create that name on the fly.

'unittest.__main__' is an example of a package that gets around this '__main__' problem by redefining 'sys.argv[0]' before invoking the parser (optparse).  'Nose' also does this.
msg226697 - (view) Author: Miki Tebeka (tebeka) * Date: 2014-09-10 14:36
Thanks Paul, will work on that.
msg226706 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-09-10 18:52
One way to reduce the testing burden, and to be extra safe regarding backward compatibility is to make this action optional, rather than the default.

For example, make `_prog_name` importable (i.e. change the name), and then expect the user to use it explicitly with:

    parser = argparse.ArgumentParser(prog=argparse.prog_name(), ...)

and leave the `parse_args` stack unchanged.

This would require an addition to the documentation.  The user can then check for themselves whether `prog_name` gets the right name, given their packaging and calling method.  It's a little more work for a package creator, but arguably it's a good thing to aware of.  

The added tests, if any, can focus on the output of this function, rather than the output of the 'print_help'.
msg226707 - (view) Author: Miki Tebeka (tebeka) * Date: 2014-09-10 19:04
I don't like changing code just to accommodate testing. Will try to think of a way to solve this.
msg353987 - (view) Author: Samuel Marks (samuelmarks) * Date: 2019-10-05 03:43
Until this is accepted, I've modified my codebase:
```
from argparse import ArgumentParser

ArgumentParser(
  prog=None if globals().get('__spec__') is None else 'python -m {}'.format(__spec__.name.partition('.')[0])
)
```
msg353989 - (view) Author: Samuel Marks (samuelmarks) * Date: 2019-10-05 04:05
See https://bugs.python.org/msg353987 for manual test
msg397764 - (view) Author: Nils Kattenbeck (Nils Kattenbeck) * Date: 2021-07-18 23:07
I am not sure if the patch correctly handles calling a nested module (e.g. `python3 -m serial.tools.miniterm`). Would it also be possible to detect if python or python3 was used for the invocation?
msg398076 - (view) Author: Nils Kattenbeck (Nils Kattenbeck) * Date: 2021-07-23 16:08
I expanded the patch from tebeka to also work with invocations like `python3 -m serial.tools.miniterm` where `miniterm.py` is a file and not a directory with a `__main__.py`. This was able to handle everything I threw at it.

However due to the import of zipfile which itself imports binascii the build of CPython itself fails at the `sharedmods` stage...


```text
 CC='gcc -pthread' LDSHARED='gcc -pthread -shared    ' OPT='-DNDEBUG -g -fwrapv -O3 -Wall' 	_TCLTK_INCLUDES='' _TCLTK_LIBS='' 	./python -E ./setup.py  build
Traceback (most recent call last):
  File "/home/septatrix/Documents/programming/cpython/./setup.py", line 3, in <module>
    import argparse
    ^^^^^^^^^^^^^^^
  File "/home/septatrix/Documents/programming/cpython/Lib/argparse.py", line 93, in <module>
    from zipfile import is_zipfile as _is_zipfile
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/septatrix/Documents/programming/cpython/Lib/zipfile.py", line 6, in <module>
    import binascii
    ^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named 'binascii'
make: *** [Makefile:639: sharedmods] Error 1
```

I guess this is because binascii is a c module and not yet build at that point in time. Does anyone who knows more about the build system have an idea how to resolve this?

---

Resolving this bug would also allow the removal of several workarounds for this in the stdlib:

* https://github.com/python/cpython/blob/83d1430ee5b8008631e7f2a75447e740eed065c1/Lib/unittest/__main__.py#L4
* https://github.com/python/cpython/blob/83d1430ee5b8008631e7f2a75447e740eed065c1/Lib/json/tool.py#L19
* https://github.com/python/cpython/blob/83d1430ee5b8008631e7f2a75447e740eed065c1/Lib/venv/__init__.py#L426
msg398698 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2021-08-01 14:13
(Note: this is an old enough ticket that it started out with patch files on the tracker rather than PRs on GitHub. That's just a historical artifact, so feel free to open a PR as described in the devguide if you would prefer to work that way)

There are two common ways of solving import bootstrapping problems that affect the build process:

1. Make the problematic import lazy, so it only affects the specific operation that needs the binary dependency; or
2. Wrap a try/except around the import, and do something reasonable in the failing case

However, I think in this case it should be possible to avoid the zipfile dependency entirely, and instead make the determination based on the contents of __main__ and __main__.__spec__ using the suggestions I noted in an earlier comment (see https://docs.python.org/3/library/importlib.html#importlib.machinery.ModuleSpec for the info the module spec makes available).

This should also solve a problem I believe the current patch has, where I think directory execution will give the wrong result (returning "python -m __main__" as the answer instead of the path to the directory containing the __main__.py file).

The three cases to cover come from https://docs.python.org/3/using/cmdline.html#interface-options:

* if `__main__.__spec__` is None, the result should continue to be `_os.path.basename(_sys.argv[0])` (as it is in the patch)
* if `__main__.__spec__.name` is exactly the string "__main__", `__main__.__spec__.parent` is the empty string, and `__main__.__spec__.has_location` is true, and sys.argv[0] is equal to `__main__.__spec__.origin` with the trailing `__main__.py` removed, then we have a directory or zipfile path execution case, and the result should be a Python path execution command using f'{py} {arg0}
* otherwise, we have a `-m` invocation, using f'{py} -m {mod.__spec__.name.removesuffix(".__main__")}'


The patch gets the three potential results right, but it gets the check for the second case wrong by looking specifically for zipfiles instead of looking at the contents of __main__.__spec__ and seeing if it refers to a __main__.py file located inside the sys.argv[0] path (which may be a directory, zipfile, or any other valid sys.path entry).

For writing robust automated tests, I'd suggest either looking at test.support.script_helper for programmatic creation of executable zipfiles and directories ( https://github.com/python/cpython/blob/208a7e957b812ad3b3733791845447677a704f3e/Lib/test/support/script_helper.py#L209 ) or else factoring the logic out such that there is a helper function that receives "__main__.__spec__" and "sys.argv[0]" as parameters, allowing the test suite to easily control them.

The latter approach would require some up front manual testing to ensure the behaviour of the different scenarios was being emulated correctly, but would execute a lot faster than actually running subprocesses would.
msg399244 - (view) Author: Nils Kattenbeck (Nils Kattenbeck) * Date: 2021-08-08 22:41
I implemented the logic and adjusted the existing tests to have a fixed program name. I also fixed the build error by changing how zip files are detected. Based on you comment Nick you however even had a different idea. Currently I check if __spec__.__loader__ is a zip loader but if I understood correct you suggest the check if __spec__.__location__ is a proper subdir of sys.argv[0]?

https://github.com/septatrix/cpython/compare/main...septatrix:enhance/argparse-prog-name.

When I have some more time I will check whether mocking works and otherwise checkout test.support.script_helper or making the function accept spec/argv0
History
Date User Action Args
2021-08-08 22:41:00Nils Kattenbecksetmessages: + msg399244
2021-08-05 19:11:38jwilksetnosy: + jwilk
2021-08-01 14:13:07ncoghlansetmessages: + msg398698
2021-07-23 16:08:12Nils Kattenbecksetfiles: + patch.diff

messages: + msg398076
2021-07-18 23:07:52Nils Kattenbecksetnosy: + Nils Kattenbeck
messages: + msg397764
2019-10-05 04:05:03samuelmarkssetfiles: + prog-module-name-handler.patch

messages: + msg353989
2019-10-05 03:43:23samuelmarkssetnosy: + samuelmarks
messages: + msg353987
2017-03-29 15:01:39BreamoreBoysetnosy: - BreamoreBoy
2017-03-29 11:10:02zertrinsetnosy: + zertrin
2014-09-10 19:04:36tebekasetmessages: + msg226707
2014-09-10 18:52:57paul.j3setmessages: + msg226706
2014-09-10 14:36:23tebekasetmessages: + msg226697
2014-09-09 05:43:58paul.j3setmessages: + msg226622
2014-09-08 19:54:31paul.j3setmessages: + msg226606
2014-09-08 13:25:36berker.peksagsetstage: patch review
2014-09-08 13:00:23tebekasetmessages: + msg226578
2014-08-26 11:18:32tebekasetfiles: + prog3.diff

messages: + msg225913
2014-08-26 07:47:06ncoghlansetmessages: + msg225911
2014-08-25 05:13:15paul.j3setmessages: + msg225865
2014-08-25 03:45:42tebekasetmessages: + msg225863
2014-08-25 03:45:19tebekasetfiles: + prog3.diff

messages: + msg225862
2014-08-24 21:19:41paul.j3setmessages: + msg225838
2014-08-24 20:03:02paul.j3setmessages: + msg225834
2014-08-24 17:42:58tebekasetfiles: + prog2.diff

messages: + msg225831
2014-08-22 14:41:08tebekasetmessages: + msg225669
2014-08-21 18:20:16BreamoreBoysetnosy: + BreamoreBoy, paul.j3
2014-08-21 17:16:03peter.ottensetnosy: + peter.otten
2014-08-21 14:04:53ncoghlansetnosy: + bethard
messages: + msg225608
2014-08-21 13:51:32brett.cannonsetnosy: + ncoghlan
2014-08-20 19:59:11tebekacreate