subcmds: stop instantiating at import time

The current subcmds design has singletons in all_commands.  This isn't
exactly unusual, but the fact that our main & help subcommand will then
attach members to the classes before invoking them is.  This makes it
hard to keep track of what members a command has access to, and the two
code paths (main & help) attach different members depending on what APIs
they then invoke.

Lets pull this back a step by storing classes in all_commands and leave
the instantiation step to when they're used.  This doesn't fully clean
up the confusion, but gets us closer.

Change-Id: I6a768ff97fe541e6f3228358dba04ed66c4b070a
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/259154
Tested-by: Mike Frysinger <vapier@google.com>
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
This commit is contained in:
Mike Frysinger 2020-02-25 15:18:31 -05:00 committed by David Pursehouse
parent d3639c53d5
commit bb930461ce
4 changed files with 9 additions and 8 deletions

View File

@ -204,7 +204,7 @@ class _Repo(object):
SetDefaultColoring(gopts.color) SetDefaultColoring(gopts.color)
try: try:
cmd = self.commands[name] cmd = self.commands[name]()
except KeyError: except KeyError:
print("repo: '%s' is not a repo command. See 'repo help'." % name, print("repo: '%s' is not a repo command. See 'repo help'." % name,
file=sys.stderr) file=sys.stderr)

View File

@ -16,6 +16,7 @@
import os import os
# A mapping of the subcommand name to the class that implements it.
all_commands = {} all_commands = {}
my_dir = os.path.dirname(__file__) my_dir = os.path.dirname(__file__)
@ -37,7 +38,7 @@ for py in os.listdir(my_dir):
['%s' % name]) ['%s' % name])
mod = getattr(mod, name) mod = getattr(mod, name)
try: try:
cmd = getattr(mod, clsn)() cmd = getattr(mod, clsn)
except AttributeError: except AttributeError:
raise SyntaxError('%s/%s does not define class %s' % ( raise SyntaxError('%s/%s does not define class %s' % (
__name__, py, clsn)) __name__, py, clsn))

View File

@ -43,7 +43,7 @@ Displays detailed usage information about a command.
fmt = ' %%-%ds %%s' % maxlen fmt = ' %%-%ds %%s' % maxlen
for name in commandNames: for name in commandNames:
command = all_commands[name] command = all_commands[name]()
try: try:
summary = command.helpSummary.strip() summary = command.helpSummary.strip()
except AttributeError: except AttributeError:
@ -134,7 +134,7 @@ Displays detailed usage information about a command.
def _PrintAllCommandHelp(self): def _PrintAllCommandHelp(self):
for name in sorted(all_commands): for name in sorted(all_commands):
cmd = all_commands[name] cmd = all_commands[name]()
cmd.manifest = self.manifest cmd.manifest = self.manifest
self._PrintCommandHelp(cmd, header_prefix='[%s] ' % (name,)) self._PrintCommandHelp(cmd, header_prefix='[%s] ' % (name,))
@ -159,7 +159,7 @@ Displays detailed usage information about a command.
name = args[0] name = args[0]
try: try:
cmd = all_commands[name] cmd = all_commands[name]()
except KeyError: except KeyError:
print("repo: '%s' is not a repo command." % name, file=sys.stderr) print("repo: '%s' is not a repo command." % name, file=sys.stderr)
sys.exit(1) sys.exit(1)

View File

@ -44,9 +44,9 @@ class Version(Command, MirrorSafeCommand):
print('repo version %s' % rp_ver) print('repo version %s' % rp_ver)
print(' (from %s)' % rem.url) print(' (from %s)' % rem.url)
if Version.wrapper_path is not None: if self.wrapper_path is not None:
print('repo launcher version %s' % Version.wrapper_version) print('repo launcher version %s' % self.wrapper_version)
print(' (from %s)' % Version.wrapper_path) print(' (from %s)' % self.wrapper_path)
if src_ver != rp_ver: if src_ver != rp_ver:
print(' (currently at %s)' % src_ver) print(' (currently at %s)' % src_ver)