[Django] #32177: ManagementUtility instantiates CommandParser without passing already-computed prog argument
Django
computed prog argument
-------------------------------------+-------------------------------------
Reporter: William | Owner: nobody
Schwartz |
Type: Bug | Status: new
Component: Core | Version: master
(Management commands) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
`ManagementUtility`
[https://github.com/django/django/blob/354c1524b38c9b9f052c1d78dcbfa6ed5559aeb3/django/core/management/__init__.py#L188-L192
goes to the trouble] to parse the program name from the `argv` it's passed
rather than from `sys.argv`:
{{{
#!python
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
[https://github.com/django/django/blob/354c1524b38c9b9f052c1d78dcbfa6ed5559aeb3/django/core/management/__init__.py#L347
uses] the program name from `sys.argv`:
{{{
#!python
parser = CommandParser(usage='%(prog)s subcommand [options]
[args]', add_help=False, allow_abbrev=False)
}}}
Above `"%(prog)s"` [https://docs.python.org/3/library/argparse.html#prog
refers] to `sys.argv[0]`. Instead, it should refer to `self.prog_name`.
This can fixed as follows:
{{{
#!python
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
[https://github.com/indygreg/PyOxidizer/issues/307 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32177>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
computed prog argument
-------------------------------------+-------------------------------------
| Cueva
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Douglas Cueva
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32177#comment:1>
Django
computed prog argument
-------------------------------------+-------------------------------------
| Cueva
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32177#comment:2>
Django
computed prog argument
-------------------------------------+-------------------------------------
| Cueva
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 1 => 0
Comment:
Replying to [comment:2 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
[https://github.com/django/django/pull/13658 PR]. (I didn't see Douglas
Cueva's patch GH-13652 before I pushed mine. Sorry.) Is this a backport
candidate?
The
[https://github.com/wkschwartz/django/blob/c8a6dab82841abb43dfc7dc8859482d329c8334a/tests/admin_scripts/tests.py#L1868-L1882
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)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32177#comment:3>
Django
computed prog argument
-------------------------------------+-------------------------------------
| Schwartz
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Douglas Cueva => William Schwartz
Comment:
Thanks for the patch.
> Is this a backport candidate?
No, this doesn't qualify for a backport.
--
Ticket URL: <https://code.djangoproject.com/ticket/32177#comment:4>
Django
computed prog argument
-------------------------------------+-------------------------------------
| Schwartz
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32177#comment:5>
Django
computed prog argument
-------------------------------------+-------------------------------------
| Schwartz
Component: Core (Management | Version: master
commands) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"cc2269350548307e3fe31723ff4e40a879a7a173" cc226935]:
{{{
#!CommitTicketReference repository=""
revision="cc2269350548307e3fe31723ff4e40a879a7a173"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32177#comment:6>
Django
computed prog argument
-------------------------------------+-------------------------------------
| Schwartz
Type: Bug | Status: closed
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution: fixed
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* keywords: => freezers
--
Ticket URL: <https://code.djangoproject.com/ticket/32177#comment:7>
Django
computed prog argument
-------------------------------------+-------------------------------------
| Schwartz
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: freezers | Triage Stage: Ready for
| checkin
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
I believe the same problem exists with the `CommandParser` used by
subcommands: At the end of `ManagementUtility.execute` (see 1), the
subcommand is run with
`self.fetch_command(subcommand).run_from_argv(self.argv)`. In
`run_from_argv`, a parser is created with `self.argv[0]` via `parser =
self.create_parser(argv[0], argv[1])` (see 2). Thus, when running `python
-m django check --help` (or any other subcommand), the output starts with
`usage: __main__.py check`.
Maybe it would be better to just update `self.argv[0]` in
`ManagementUtility` instead of having `prog_name`?
Either way, should I open a separate ticket and/or implement the proposed
change?
- (1):
https://github.com/django/django/blob/4a3ad9eebbc16ce80b348644b557c84ecc741be7/django/core/management/__init__.py#L436
- (2):
https://github.com/django/django/blob/4a3ad9eebbc16ce80b348644b557c84ecc741be7/django/core/management/base.py#L408
--
Ticket URL: <https://code.djangoproject.com/ticket/32177#comment:8>