Opened 15 years ago

Closed 10 years ago

#11407 closed Bug (fixed)

--version weirdness

Reported by: Karen Tracey Owned by:
Component: Core (Management commands) Version: dev
Severity: Normal Keywords: management django-admin.py manage.py
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There's some weirdness that happens if you pass both --version and a subcommand, or another option, to manage.py (or django-admin.py):

kmt@lbox:~/software/web/playground$ ./manage.py --version --help
1.1 beta 1 SVN-11115
Unknown command: '--version'
Type 'manage.py help' for usage.
kmt@lbox:~/software/web/playground$ ./manage.py runserver --version
1.1 beta 1 SVN-11115
1.1 beta 1 SVN-11115
kmt@lbox:~/software/web/playground$ ./manage.py test --version
1.1 beta 1 SVN-11115
1.1 beta 1 SVN-11115
kmt@lbox:~/software/web/playground$ ./manage.py lskdjfljtest --version
1.1 beta 1 SVN-11115
Unknown command: 'lskdjfljtest'
Type 'manage.py help' for usage.
kmt@lbox:~/software/web/playground$

--version as provided by optparse.OptionParser is documented to print the version string and exit. This is done when parse_args is called. However the LaxOptionParser defined and used within django.core.management.__init__ overrides _process_args so that the SystemExit raised when --version is processed is swallowed/ignored. Thus the version gets printed but the program does not exit but rather proceeds. Things then may get a little confused depending on what else has been specified on the command line. In the case of no subcommand the --version argument is attempted to be interpreted as a subcommand. In the case of a valid subcommand being specified, a regular OptionParser is called to process the arguments for it, and that one will again print the version string but then actually exit. In the case of an invalid subcommand, you'll get a message to that effect. This last one actually looks like reasonable behavior but the others are a bit confusing, so it would probably be better if the program did consistently exit on the first processing of --version, ignoring whatever else may have been specified.

Attachments (2)

command_parser.diff (3.4 KB ) - added by Mark Barrett 12 years ago.
11407-2.diff (4.7 KB ) - added by Claude Paroz 11 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 by Alex Gaynor, 15 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Julien Phalip, 13 years ago

Severity: Normal
Type: Bug

comment:3 by Mark Barrett, 12 years ago

Easy pickings: unset
Owner: changed from nobody to Mark Barrett
Status: newassigned
UI/UX: unset

by Mark Barrett, 12 years ago

Attachment: command_parser.diff added

comment:4 by Mark Barrett, 12 years ago

Explanation of changes in command_parser.diff:

  1. Modified _process_args on LaxOptionParser so that it doesn't catch the SystemExit thrown by the exit() function
  2. Modified print_help on LaxOptionParser to print the help message, rather than doing nothing
  3. Renamed LaxOptionParser to ManagementUtilityOptionParser, as it is pretty specific to the management utility and isn't used anywhere else
  4. Removed --version, --help and -h logic from ManagementUtility.execute
  5. Removed exception handling for the call to LaxOptionParser.parse_args() as it's only purpose seemed to be to catch SystemExit throw when --version or --help are specified on the commandline, so allow the specific logic below to run. Since this logic has now been removed the exception handling is not required

comment:5 by Mark Barrett, 12 years ago

Has patch: set

comment:6 by Jannis Leidel, 12 years ago

Patch needs improvement: set

This breaks the admin_scripts tests currently.

by Claude Paroz, 11 years ago

Attachment: 11407-2.diff added

comment:7 by Claude Paroz, 11 years ago

Owner: Mark Barrett removed
Patch needs improvement: unset
Status: assignednew

Just uploaded a new patch. It's somewhat similar to the one marclurr posted, except I didn't rename the subclassed parser. All tests pass with sqlite3.

comment:8 by Cody Scott, 10 years ago

Keywords: management django-admin.py manage.py added

comment:9 by Claude Paroz, 10 years ago

After the partial rewrite needed by the conversion to argparse (#19973), only the first "weirdness" partially subsists:

$ ./manage.py --version --help
Unknown command: '--version'
Type 'manage.py help' for usage.

or

$ ./manage.py --help --version
Unknown command: '--help'
Type 'manage.py help' for usage.

While individually using --help or --version produces the expected output. Do we want to specifically support that sort of combination?

comment:10 by Aymeric Augustin, 10 years ago

Resolution: fixed
Status: newclosed

That wouldn't be a good use of your time. I don't see the use case for asking for both help and version at the same time.

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