Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32177 closed Bug (fixed)

ManagementUtility instantiates CommandParser without passing already-computed prog argument

Reported by: William Schwartz Owned by: William Schwartz
Component: Core (Management commands) Version: dev
Severity: Normal Keywords: freezers
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

ManagementUtility goes to the trouble to parse the program name from the argv it's passed rather than from sys.argv:

    def __init__(self, argv=None):
        self.argv = argv or sys.argv[:]
        self.prog_name = os.path.basename(self.argv[0])
        if self.prog_name == '__main__.py':
            self.prog_name = 'python -m django'

But then when it needs to parse --pythonpath and --settings, it uses the program name from sys.argv:

        parser = CommandParser(usage='%(prog)s subcommand [options] [args]', add_help=False, allow_abbrev=False)

Above "%(prog)s" refers to sys.argv[0]. Instead, it should refer to self.prog_name. This can fixed as follows:

        parser = CommandParser(
            prog=self.prog_name,
            usage='%(prog)s subcommand [options] [args]',
            add_help=False,
            allow_abbrev=False)

I'm aware that execute_from_command_line is a private API, but it'd be really convenient for me if it worked properly in my weird embedded environment where sys.argv[0] is incorrectly None. If passing my own argv to execute_from_command_line avoided all the ensuing exceptions, I wouldn't have to modify sys.argv[0] globally as I'm doing in the meantime.

Change History (7)

comment:1 by Douglas Cueva, 3 years ago

Owner: changed from nobody to Douglas Cueva
Status: newassigned

comment:2 by Mariusz Felisiak, 3 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Tentatively accepted, looks valid but I was not able to reproduce and invalid message (even with mocking sys.argv), so a regression test is crucial.

in reply to:  2 comment:3 by William Schwartz, 3 years ago

Needs tests: unset

Replying to Mariusz Felisiak:

...I was not able to reproduce and invalid message (even with mocking sys.argv), so a regression test is crucial.

I've created a patch with a test case in PR. (I didn't see Douglas Cueva's patch GH-13652 before I pushed mine. Sorry.) Is this a backport candidate?

The test I added passes now, and failed like below before I applied the fix.

(venv) ➜  django git:(mgmt-argv0) ✗ tox -e py38 -- admin_scripts.tests.ExecuteFromCommandLine
<snip>
py38 run-test: commands[0] | django/.tox/py38/bin/python runtests.py admin_scripts.tests.ExecuteFromCommandLine
Testing against Django installed in 'django/django'
System check identified no issues (0 silenced).
E
======================================================================
ERROR: test_prog_name_from_argv0 (admin_scripts.tests.ExecuteFromCommandLine)
Program name is computed once from argv argument, not sys.argv (#32177).
----------------------------------------------------------------------
Traceback (most recent call last):
  File "django/tests/admin_scripts/tests.py", line 1882, in test_prog_name_from_argv0
    execute_from_command_line([''] + args)
  File "django/django/core/management/__init__.py", line 414, in execute_from_command_line
    utility.execute()
  File "django/django/core/management/__init__.py", line 347, in execute
    parser = CommandParser(usage='%(prog)s subcommand [options] [args]', add_help=False, allow_abbrev=False)
  File "django/django/core/management/base.py", line 54, in __init__
    super().__init__(**kwargs)
  File "/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/argparse.py", line 1660, in __init__
    prog = _os.path.basename(_sys.argv[0])
  File "/usr/local/opt/python@3.8/bin/../Frameworks/Python.framework/Versions/3.8/lib/python3.8/posixpath.py", line 142, in basename
    p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not NoneType

----------------------------------------------------------------------
Ran 1 test in 0.005s

FAILED (errors=1)

comment:4 by Mariusz Felisiak, 3 years ago

Owner: changed from Douglas Cueva to William Schwartz

Thanks for the patch.

Is this a backport candidate?

No, this doesn't qualify for a backport.

comment:5 by Mariusz Felisiak, 3 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In cc226935:

Fixed #32177 -- Made execute_from_command_line() use program name from the argv argument.

This caused crash in environments where sys.argv[0] is incorrectly set
to None.

comment:7 by Mariusz Felisiak, 3 years ago

Keywords: freezers added
Note: See TracTickets for help on using tickets.
Back to Top