commit bb930461cee056de210816e58c36a8d6dfd122be
author Mike Frysinger <vapier@google.com> Tue Feb 25 15:18:31 2020 -0500
committer David Pursehouse <dpursehouse@collab.net> Tue Mar 17 00:08:52 2020 +0000
tree 3f98c279580f02ecf7c1608a20caa294f1075f45
parent d3639c53d56feaea81474ffd28395a124744dab7

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>

main.py

diff --git a/main.py b/main.py
index b309fad..1393b40 100755
--- a/main.py
+++ b/main.py
@@ -204,7 +204,7 @@
     SetDefaultColoring(gopts.color)
 
     try:
-      cmd = self.commands[name]
+      cmd = self.commands[name]()
     except KeyError:
       print("repo: '%s' is not a repo command.  See 'repo help'." % name,
             file=sys.stderr)

subcmds/__init__.py

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

subcmds/help.py

diff --git a/subcmds/help.py b/subcmds/help.py
index 5e24ed0..1e16019 100644
--- a/subcmds/help.py
+++ b/subcmds/help.py
@@ -43,7 +43,7 @@
     fmt = '  %%-%ds  %%s' % maxlen
 
     for name in commandNames:
-      command = all_commands[name]
+      command = all_commands[name]()
       try:
         summary = command.helpSummary.strip()
       except AttributeError:
@@ -134,7 +134,7 @@
 
   def _PrintAllCommandHelp(self):
     for name in sorted(all_commands):
-      cmd = all_commands[name]
+      cmd = all_commands[name]()
       cmd.manifest = self.manifest
       self._PrintCommandHelp(cmd, header_prefix='[%s] ' % (name,))
 
@@ -159,7 +159,7 @@
       name = args[0]
 
       try:
-        cmd = all_commands[name]
+        cmd = all_commands[name]()
       except KeyError:
         print("repo: '%s' is not a repo command." % name, file=sys.stderr)
         sys.exit(1)

subcmds/version.py

diff --git a/subcmds/version.py b/subcmds/version.py
index 91dbe68..8721bf4 100644
--- a/subcmds/version.py
+++ b/subcmds/version.py
@@ -44,9 +44,9 @@
     print('repo version %s' % rp_ver)
     print('       (from %s)' % rem.url)
 
-    if Version.wrapper_path is not None:
-      print('repo launcher version %s' % Version.wrapper_version)
-      print('       (from %s)' % Version.wrapper_path)
+    if self.wrapper_path is not None:
+      print('repo launcher version %s' % self.wrapper_version)
+      print('       (from %s)' % self.wrapper_path)
 
       if src_ver != rp_ver:
         print('       (currently at %s)' % src_ver)