Code

Opened 5 years ago

Closed 4 weeks ago

#11407 closed Bug (fixed)

--version weirdness

Reported by: kmtracey Owned by:
Component: Core (Management commands) Version: master
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 marclurr 3 years ago.
11407-2.diff (4.7 KB) - added by claudep 16 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 5 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:3 Changed 3 years ago by marclurr

  • Easy pickings unset
  • Owner changed from nobody to marclurr
  • Status changed from new to assigned
  • UI/UX unset

Changed 3 years ago by marclurr

comment:4 Changed 3 years ago by marclurr

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 Changed 3 years ago by marclurr

  • Has patch set

comment:6 Changed 2 years ago by jezdez

  • Patch needs improvement set

This breaks the admin_scripts tests currently.

Changed 16 months ago by claudep

comment:7 Changed 16 months ago by claudep

  • Owner marclurr deleted
  • Patch needs improvement unset
  • Status changed from assigned to new

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 Changed 8 months ago by Siecje

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

comment:9 Changed 4 weeks ago by claudep

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 Changed 4 weeks ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.