Refine verbose output

Refine the verbose output based on PR feedback from Bill Sacks.

* Verbose output is cumulative as the verbose count increases.

* checkout and status have similar behavior

* dump level status output is indented consistently.

Testing:
  make test - all tests pass
  manual testing of cesm and standalone repos with new verbose checkout and status output.

manic/checkout.py

@@ -23,7 +23,6 @@
 from manic.sourcetree import SourceTree
 from manic.utils import printlog
 from manic.global_constants import VERSION_SEPERATOR, LOG_FILE_NAME
-from manic.global_constants import VERBOSITY_DUMP
 
 if sys.hexversion < 0x02070000:
     print(70 * '*')
@@ -219,8 +218,9 @@ def commandline_arguments(args=None):
 
     parser.add_argument('-v', '--verbose', action='count', default=0,
                         help='Output additional information to '
-                        'the screen and log file. This flag can be used '
-                        'multiple times.')
+                        'the screen and log file. For --status, this flag '
+                        'can be used a second time, increasing the verbosity'
+                        'level each time.')
 
     #
     # developer options
@@ -277,12 +277,8 @@ def main(args):
 
     if args.status:
         # user requested status-only
-        if args.verbose != VERBOSITY_DUMP:
-            for comp in sorted(tree_status.keys()):
-                tree_status[comp].log_status_message(args.verbose)
-        else:  # args.verbose == VERBOSITY_DUMP
-            # user requested verbose status dump of the git/svn status commands
-            source_tree.verbose_status_dump()
+        for comp in sorted(tree_status.keys()):
+            tree_status[comp].log_status_message(args.verbose)
     else:
         # checkout / update the external repositories.
         safe_to_update = check_safe_to_update_repos(tree_status)

manic/externals_status.py

@@ -9,8 +9,8 @@
 from __future__ import print_function
 
 from .global_constants import EMPTY_STR
-from .utils import printlog
-from .global_constants import VERBOSITY_DEFAULT
+from .utils import printlog, indent_string
+from .global_constants import VERBOSITY_VERBOSE, VERBOSITY_DUMP
 
 
 class ExternalStatus(object):
@@ -50,25 +50,26 @@ def __init__(self):
         self.path = EMPTY_STR
         self.current_version = EMPTY_STR
         self.expected_version = EMPTY_STR
+        self.status_output = EMPTY_STR
 
     def log_status_message(self, verbosity):
         """Write status message to the screen and log file
         """
-        if verbosity == VERBOSITY_DEFAULT:
-            msg = self.default_status_message()
-        else:
-            msg = self.verbose_status_message()
-        printlog(msg)
+        self._default_status_message()
+        if verbosity >= VERBOSITY_VERBOSE:
+            self._verbose_status_message()
+        if verbosity >= VERBOSITY_DUMP:
+            self._dump_status_message()
 
-    def default_status_message(self):
+    def _default_status_message(self):
         """Return the default terse status message string
         """
         msg = '{sync}{clean}{src_type} {path}'.format(
             sync=self.sync_state, clean=self.clean_state,
             src_type=self.source_type, path=self.path)
-        return msg
+        printlog(msg)
 
-    def verbose_status_message(self):
+    def _verbose_status_message(self):
         """Return the verbose status message string
         """
         clean_str = self.DEFAULT
@@ -81,9 +82,14 @@ def verbose_status_message(self):
         if self.sync_state != self.STATUS_OK:
             sync_str = '{current} --> {expected}'.format(
                 current=self.current_version, expected=self.expected_version)
-        msg = '{path} :\n    {clean}, {sync}'.format(
-            path=self.path, clean=clean_str, sync=sync_str)
-        return msg
+        msg = '        {clean}, {sync}'.format(clean=clean_str, sync=sync_str)
+        printlog(msg)
+
+    def _dump_status_message(self):
+        """Return the dump status message string
+        """
+        msg = indent_string(self.status_output, 12)
+        printlog(msg)
 
     def safe_to_update(self):
         """Report if it is safe to update a repository. Safe is defined as:

manic/repository.py

@@ -49,14 +49,6 @@ def status(self, stat, repo_dir_path):  # pylint: disable=unused-argument
                'repository classes! {0}'.format(self.__class__.__name__))
         fatal_error(msg)
 
-    def verbose_status_dump(self, repo_dir_path):  # pylint: disable=unused-argument
-        """Display the raw repo status to the user.
-
-        """
-        msg = ('DEV_ERROR: status method must be implemented in all '
-               'repository classes! {0}'.format(self.__class__.__name__))
-        fatal_error(msg)
-
     def url(self):
         """Public access of repo url.
         """

manic/repository_git.py

@@ -14,7 +14,7 @@
 from .repository import Repository
 from .externals_status import ExternalStatus
 from .utils import expand_local_url, split_remote_url, is_remote_url
-from .utils import log_process_output, fatal_error, printlog
+from .utils import fatal_error, printlog
 from .utils import execute_subprocess
 
 
@@ -80,13 +80,6 @@ def status(self, stat, repo_dir_path):
         if os.path.exists(repo_dir_path):
             self._status_summary(stat, repo_dir_path)
 
-    def verbose_status_dump(self, repo_dir_path):
-        """Display the raw repo status to the user.
-
-        """
-        if os.path.exists(repo_dir_path):
-            self._status_verbose_dump(repo_dir_path)
-
     # ----------------------------------------------------------------
     #
     # Internal work functions
@@ -508,23 +501,16 @@ def _status_summary(self, stat, repo_dir_path):
         cwd = os.getcwd()
         os.chdir(repo_dir_path)
         git_output = self._git_status_porcelain_v1z()
-        os.chdir(cwd)
         is_dirty = self._status_v1z_is_dirty(git_output)
         if is_dirty:
             stat.clean_state = ExternalStatus.DIRTY
         else:
             stat.clean_state = ExternalStatus.STATUS_OK
 
-    def _status_verbose_dump(self, repo_dir_path):
-        """Display raw git status output to the user
-
-        """
-        cwd = os.getcwd()
-        os.chdir(repo_dir_path)
-        git_output = self._git_status_verbose()
+        # Now save the verbose status output incase the user wants to
+        # see it.
+        stat.status_output = self._git_status_verbose()
         os.chdir(cwd)
-        log_process_output(git_output)
-        print(git_output)
 
     @staticmethod
     def _status_v1z_is_dirty(git_output):
@@ -648,7 +634,7 @@ def _git_clone(url, repo_dir_name, verbosity):
         """Run git clone for the side effect of creating a repository.
         """
         cmd = ['git', 'clone', url, repo_dir_name]
-        if verbosity == VERBOSITY_VERBOSE:
+        if verbosity >= VERBOSITY_VERBOSE:
             printlog('    {0}'.format(' '.join(cmd)))
         execute_subprocess(cmd)
 
@@ -675,6 +661,6 @@ def _git_checkout_ref(ref, verbosity):
 
         """
         cmd = ['git', 'checkout', ref]
-        if verbosity == VERBOSITY_VERBOSE:
+        if verbosity >= VERBOSITY_VERBOSE:
             printlog('    {0}'.format(' '.join(cmd)))
         execute_subprocess(cmd)

manic/repository_svn.py

@@ -12,7 +12,7 @@
 from .global_constants import EMPTY_STR, VERBOSITY_VERBOSE
 from .repository import Repository
 from .externals_status import ExternalStatus
-from .utils import fatal_error, log_process_output, indent_string, printlog
+from .utils import fatal_error, indent_string, printlog
 from .utils import execute_subprocess
 
 
@@ -87,13 +87,6 @@ def status(self, stat, repo_dir_path):
         if os.path.exists(repo_dir_path):
             self._status_summary(stat, repo_dir_path)
 
-    def verbose_status_dump(self, repo_dir_path):
-        """Display the raw repo status to the user.
-
-        """
-        if os.path.exists(repo_dir_path):
-            self._status_verbose_dump(repo_dir_path)
-
     # ----------------------------------------------------------------
     #
     # Internal work functions
@@ -187,13 +180,9 @@ def _status_summary(self, stat, repo_dir_path):
         else:
             stat.clean_state = ExternalStatus.STATUS_OK
 
-    def _status_verbose_dump(self, repo_dir_path):
-        """Display the raw svn status output to the user.
-
-        """
-        svn_output = self._svn_status_verbose(repo_dir_path)
-        log_process_output(svn_output)
-        print(svn_output)
+        # Now save the verbose status output incase the user wants to
+        # see it.
+        stat.status_output = self._svn_status_verbose(repo_dir_path)
 
     @staticmethod
     def xml_status_is_dirty(svn_output):
@@ -265,7 +254,7 @@ def _svn_checkout(url, repo_dir_path, verbosity):
         Checkout a subversion repository (repo_url) to checkout_dir.
         """
         cmd = ['svn', 'checkout', url, repo_dir_path]
-        if verbosity == VERBOSITY_VERBOSE:
+        if verbosity >= VERBOSITY_VERBOSE:
             printlog('    {0}'.format(' '.join(cmd)))
         execute_subprocess(cmd)
 
@@ -275,6 +264,6 @@ def _svn_switch(url, verbosity):
         Switch branches for in an svn sandbox
         """
         cmd = ['svn', 'switch', url]
-        if verbosity == VERBOSITY_VERBOSE:
+        if verbosity >= VERBOSITY_VERBOSE:
             printlog('    {0}'.format(' '.join(cmd)))
         execute_subprocess(cmd)

manic/sourcetree.py

@@ -42,6 +42,7 @@ def __init__(self, root_dir, name, ext_description):
         self._repo = None
         self._externals = EMPTY_STR
         self._externals_sourcetree = None
+        self._stat = ExternalStatus()
         # Parse the sub-elements
 
         # _path : local path relative to the containing source tree
@@ -86,38 +87,37 @@ def status(self):
         If load_all is True, also load all of the the externals sub-externals.
         """
 
-        stat = ExternalStatus()
-        stat.path = self.get_local_path()
+        self._stat.path = self.get_local_path()
         if not self._required:
-            stat.source_type = ExternalStatus.OPTIONAL
+            self._stat.source_type = ExternalStatus.OPTIONAL
         elif self._local_path == LOCAL_PATH_INDICATOR:
             # LOCAL_PATH_INDICATOR, '.' paths, are standalone
             # component directories that are not managed by
             # checkout_externals.
-            stat.source_type = ExternalStatus.STANDALONE
+            self._stat.source_type = ExternalStatus.STANDALONE
         else:
             # managed by checkout_externals
-            stat.source_type = ExternalStatus.MANAGED
+            self._stat.source_type = ExternalStatus.MANAGED
 
         ext_stats = {}
 
         if not os.path.exists(self._repo_dir_path):
-            stat.sync_state = ExternalStatus.EMPTY
+            self._stat.sync_state = ExternalStatus.EMPTY
             msg = ('status check: repository directory for "{0}" does not '
                    'exist.'.format(self._name))
             logging.info(msg)
-            stat.current_version = 'not checked out'
+            self._stat.current_version = 'not checked out'
             # NOTE(bja, 2018-01) directory doesn't exist, so we cannot
             # use repo to determine the expected version. We just take
             # a best-guess based on the assumption that only tag or
             # branch should be set, but not both.
             if not self._repo:
-                stat.expected_version = 'unknown'
+                self._stat.expected_version = 'unknown'
             else:
-                stat.expected_version = self._repo.tag() + self._repo.branch()
+                self._stat.expected_version = self._repo.tag() + self._repo.branch()
         else:
             if self._repo:
-                self._repo.status(stat, self._repo_dir_path)
+                self._repo.status(self._stat, self._repo_dir_path)
 
             if self._externals and self._externals_sourcetree:
                 # we expect externals and they exist
@@ -134,29 +134,13 @@ def status(self):
         if self._local_path != LOCAL_PATH_INDICATOR:
             # store the stats under tha local_path, not comp name so
             # it will be sorted correctly
-            all_stats[stat.path] = stat
+            all_stats[self._stat.path] = self._stat
 
         if ext_stats:
             all_stats.update(ext_stats)
 
         return all_stats
 
-    def verbose_status_dump(self):
-        """Display the verbose status to the user. This is just the raw output
-        from the repository 'status' command.
-
-        """
-        if not os.path.exists(self._repo_dir_path):
-            msg = ('status check: repository directory for "{0}" does not '
-                   'exist!'.format(self._name))
-            logging.info(msg)
-        else:
-            cwd = os.getcwd()
-            os.chdir(self._repo_dir_path)
-            if self._repo:
-                self._repo.verbose_status_dump(self._repo_dir_path)
-            os.chdir(cwd)
-
     def checkout(self, verbosity, load_all):
         """
         If the repo destination directory exists, ensure it is correct (from
@@ -180,11 +164,17 @@ def checkout(self, verbosity, load_all):
                         self._base_dir_path)
                     fatal_error(msg)
 
+        if self._stat.source_type != ExternalStatus.STANDALONE:
+            if verbosity >= VERBOSITY_VERBOSE:
+                # NOTE(bja, 2018-01) probably do not want to pass
+                # verbosity in this case, because if (verbosity ==
+                # VERBOSITY_DUMP), then the previous status output would
+                # also be dumped, adding noise to the output.
+                self._stat.log_status_message(VERBOSITY_VERBOSE)
+
         if self._repo:
-            self._repo.checkout(
-                self._base_dir_path,
-                self._repo_dir_name,
-                verbosity)
+            self._repo.checkout(self._base_dir_path,
+                                self._repo_dir_name, verbosity)
 
     def checkout_externals(self, verbosity, load_all):
         """Checkout the sub-externals for this object
@@ -255,6 +245,7 @@ def status(self, relative_path_base=LOCAL_PATH_INDICATOR):
 
         summary = {}
         for comp in load_comps:
+            printlog('{0}, '.format(comp), end='')
             stat = self._all_components[comp].status()
             for name in stat.keys():
                 # check if we need to append the relative_path_base to
@@ -271,15 +262,6 @@ def status(self, relative_path_base=LOCAL_PATH_INDICATOR):
 
         return summary
 
-    def verbose_status_dump(self):
-        """Display verbose status to the user. This is just the raw output of
-        the git and svn status commands.
-
-        """
-        load_comps = self._all_components.keys()
-        for comp in load_comps:
-            self._all_components[comp].verbose_status_dump()
-
     def checkout(self, verbosity, load_all, load_comp=None):
         """
         Checkout or update indicated components into the the configured
@@ -289,7 +271,7 @@ def checkout(self, verbosity, load_all, load_comp=None):
         If load_all is False, load_comp is an optional set of components to load.
         If load_all is True and load_comp is None, only load the required externals.
         """
-        if verbosity == VERBOSITY_VERBOSE:
+        if verbosity >= VERBOSITY_VERBOSE:
             printlog('Checking out externals: ')
         else:
             printlog('Checking out externals: ', end='')
@@ -303,10 +285,12 @@ def checkout(self, verbosity, load_all, load_comp=None):
 
         # checkout the primary externals
         for comp in load_comps:
-            if verbosity == VERBOSITY_VERBOSE:
-                printlog('\n  {0} : '.format(comp))
-            else:
+            if verbosity < VERBOSITY_VERBOSE:
                 printlog('{0}, '.format(comp), end='')
+            else:
+                # verbose output handled by the _External object, just
+                # output a newline
+                printlog(EMPTY_STR)
             self._all_components[comp].checkout(verbosity, load_all)
         printlog('')