Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

argparse support for "python -m module" in help #66436

Closed
tebeka mannequin opened this issue Aug 20, 2014 · 25 comments
Closed

argparse support for "python -m module" in help #66436

tebeka mannequin opened this issue Aug 20, 2014 · 25 comments
Assignees
Labels
3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@tebeka
Copy link
Mannequin

tebeka mannequin commented Aug 20, 2014

BPO 22240
Nosy @tebeka, @ncoghlan, @jwilk, @zertrin, @SamuelMarks, @septatrix
Files
  • prog.diff: Patch
  • prog2.diff: Patch
  • prog3.diff: Patch
  • prog3.diff: Patch
  • prog-module-name-handler.patch: Patch for handling python -m syntax in argument parser ouptut
  • patch.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2014-08-20.19:59:11.381>
    labels = ['type-feature', 'library']
    title = 'argparse support for "python -m module" in help'
    updated_at = <Date 2021-08-08.22:41:00.591>
    user = 'https://github.com/tebeka'

    bugs.python.org fields:

    activity = <Date 2021-08-08.22:41:00.591>
    actor = 'Nils Kattenbeck'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2014-08-20.19:59:11.381>
    creator = 'tebeka'
    dependencies = []
    files = ['36424', '36453', '36461', '36476', '48643', '50174']
    hgrepos = []
    issue_num = 22240
    keywords = ['patch']
    message_count = 23.0
    messages = ['225586', '225608', '225669', '225831', '225834', '225838', '225862', '225863', '225865', '225911', '225913', '226578', '226606', '226622', '226697', '226706', '226707', '353987', '353989', '397764', '398076', '398698', '399244']
    nosy_count = 9.0
    nosy_names = ['tebeka', 'ncoghlan', 'peter.otten', 'bethard', 'jwilk', 'paul.j3', 'zertrin', 'samuelmarks', 'Nils Kattenbeck']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue22240'
    versions = ['Python 3.5']

    Linked PRs

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Aug 20, 2014

    "python -m module -h" starts with
    usage: __main__.py

    It should be
    usage: python -m module

    @tebeka tebeka mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Aug 20, 2014
    @ncoghlan
    Copy link
    Contributor

    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).

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Aug 22, 2014

    For zip file the help should probably be:
    usage: python file.zip

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Aug 24, 2014

    New patch with handling of zip files.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Aug 24, 2014

    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.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Aug 24, 2014

    What if the package is run without -m?

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Aug 25, 2014

    Made the test more robust by using sys.executable in the comparison.

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Aug 25, 2014

    How can you run a package without -m?

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Aug 25, 2014

    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.

    @ncoghlan
    Copy link
    Contributor

    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 :)

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Aug 26, 2014

    Support for directory invocation as well.

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Sep 8, 2014

    Anything else I need to solve to get this patch accepted?

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Sep 8, 2014

    When I apply prog3.diff to my 3.5.0dev, and run

    python3 -m unittest [Lib/test/test_argparse.py](https://github.com/python/cpython/blob/main/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).

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Sep 9, 2014

    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.

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Sep 10, 2014

    Thanks Paul, will work on that.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Sep 10, 2014

    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'.

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Sep 10, 2014

    I don't like changing code just to accommodate testing. Will try to think of a way to solve this.

    @SamuelMarks
    Copy link
    Mannequin

    SamuelMarks mannequin commented Oct 5, 2019

    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])
    )
    

    @SamuelMarks
    Copy link
    Mannequin

    SamuelMarks mannequin commented Oct 5, 2019

    See https://bugs.python.org/msg353987 for manual test

    @Septatrix
    Copy link
    Mannequin

    Septatrix mannequin commented Jul 18, 2021

    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?

    @Septatrix
    Copy link
    Mannequin

    Septatrix mannequin commented Jul 23, 2021

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

     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:

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Aug 1, 2021

    (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 (

    def make_script(script_dir, script_basename, source, omit_suffix=False):
    ) 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.

    @Septatrix
    Copy link
    Mannequin

    Septatrix mannequin commented Aug 8, 2021

    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]?

    septatrix/cpython@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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @serhiy-storchaka serhiy-storchaka self-assigned this Sep 30, 2024
    @serhiy-storchaka serhiy-storchaka added the 3.14 new features, bugs and security fixes label Sep 30, 2024
    serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Sep 30, 2024
    It can now have one of three forms:
    
    * basename(argv0) -- for simple scripts
    * python arv0 -- for directories, ZIP files, etc
    * python -m module -- for imported modules
    @serhiy-storchaka
    Copy link
    Member

    #124799 mostly implements @ncoghlan's algorithm from #66436 (comment) and adds tests and documentation.

    @ncoghlan, I would appreciate your review of the documentation, as it is difficult to me. Feel free to rewrite it completely.

    serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Sep 30, 2024
    It can now have one of three forms:
    
    * basename(argv0) -- for simple scripts
    * python arv0 -- for directories, ZIP files, etc
    * python -m module -- for imported modules
    @serhiy-storchaka
    Copy link
    Member

    It looks to me that testing __main__.__spec__.name for '__main__' is enough. Additional checks are redundant, and checking that sys.argv[0] is equal to __main__.__spec__.origin with the trailing __main__.py removed is hard due to difference in normalization on Unix and Windows.

    serhiy-storchaka added a commit that referenced this issue Oct 1, 2024
    …-124799)
    
    It can now have one of three forms:
    
    * basename(argv0) -- for simple scripts
    * python arv0 -- for directories, ZIP files, etc
    * python -m module -- for imported modules
    
    Co-authored-by: Alyssa Coghlan <[email protected]>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    Status: Doc issues
    Development

    No branches or pull requests

    2 participants