Squashed 'manage_externals/' changes from 0427305..fc5acda

fc5acda Merge pull request #114 from billsacks/fix_large_output_hang
aa2eb71 Try getting travis-ci working on MacOS
96842b4 Fix pylint errors
813fe3c pylint: disable useless-object-inheritance
c49d878 Rework execute_subprocess timeout handling
8fc0e5f Cleanup from 'make style'
b0b23a6 Merge pull request #110 from gold2718/help_fix
3cbcd16 Fixed and clarified help documentation
025e6cb Merge pull request #107 from jedwards4b/ignore_empty_git_dir
489842b if you encounter an empty directory clone into it
0c5a2f6 Merge pull request #106 from billsacks/remove_logfile_message
7799e99 Remove message about checking the log file for more details

git-subtree-dir: manage_externals
git-subtree-split: fc5acda

.travis.yml

@@ -17,6 +17,7 @@ matrix:
       # NOTE(bja, 2017-11) update is slow, 2.7.12 installed by default, good enough!
       # - brew update
       # - brew outdated python2 || brew upgrade python2
+      - pip install virtualenv
       - virtualenv env -p python2
       - source env/bin/activate
 install:
@@ -29,4 +30,3 @@ script:
 after_success:
   - cd test; make coverage
   - cd test; coveralls
-

manic/__init__.py

@@ -1,7 +1,7 @@
 """Public API for the manage_externals library
 """
 
-import manic.checkout as checkout
+from manic import checkout
 from manic.utils import printlog
 
 __all__ = [

manic/checkout.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 
 """
-Tool to assemble respositories represented in a model-description file.
+Tool to assemble repositories represented in a model-description file.
 
 If loaded as a module (e.g., in a component's buildcpp), it can be used
 to check the validity of existing subdirectories and load missing sources.
@@ -48,17 +48,9 @@ def commandline_arguments(args=None):
     description = '''
 
 %(prog)s manages checking out groups of externals from revision
-control based on a externals description file. By default only the
+control based on an externals description file. By default only the
 required externals are checkout out.
 
-Operations performed by manage_externals utilities are explicit and
-data driven. %(prog)s will always make the working copy *exactly*
-match what is in the externals file when modifying the working copy of
-a repository.
-
-If %(prog)s isn't doing what you expected, double check the contents
-of the externals description file.
-
 Running %(prog)s without the '--status' option will always attempt to
 synchronize the working copy to exactly match the externals description.
 '''
@@ -76,7 +68,7 @@ def commandline_arguments(args=None):
     $ git clone [email protected]/{SOME_ORG}/some-project some-project-dev
 
 and you need to checkout the sub-project externals, then the root of the
-source tree is /path/to/some-project-dev. Do *NOT* run %(prog)s
+source tree remains /path/to/some-project-dev. Do *NOT* run %(prog)s
 from within /path/to/some-project-dev/sub-project
 
 The root of the source tree will be referred to as `${SRC_ROOT}` below.
@@ -109,7 +101,7 @@ def commandline_arguments(args=None):
     description file:
 
         $ cd ${SRC_ROOT}
-        $ ./manage_externals/%(prog)s --excernals my-externals.cfg
+        $ ./manage_externals/%(prog)s --externals my-externals.cfg
 
   * Status summary of the repositories managed by %(prog)s:
 
@@ -178,8 +170,9 @@ def commandline_arguments(args=None):
 
     Note: 'externals_only' will only process the external's own
     external description file without trying to manage a repository
-    for the component. This is used for retreiving externals for
-    standalone components like cam and clm. If the source root of the
+    for the component. This is used for retrieving externals for
+    standalone components like cam and ctsm which also serve as
+    sub-components within a larger project. If the source root of the
     externals_only component is the same as the main source root, then
     the local path must be set to '.', the unix current working
     directory, e. g. 'local_path = .'
@@ -219,21 +212,40 @@ def commandline_arguments(args=None):
 
   * externals (string) : used to make manage_externals aware of
     sub-externals required by an external. This is a relative path to
-    the external's root directory. For example, the main externals
-    description has an external checkout out at 'src/useful_library'.
-    useful_library requires additional externals to be complete.
-    Those additional externals are managed from the source root by the
-    externals description file pointed 'useful_library/sub-xternals.cfg',
-    Then the main 'externals' field in the top level repo should point to
-    'sub-externals.cfg'.
+    the external's root directory. For example, if LIBX is often used
+    as a sub-external, it might have an externals file (for its
+    externals) called Externals_LIBX.cfg. To use libx as a standalone
+    checkout, it would have another file, Externals.cfg with the
+    following entry:
+
+    [ libx ]
+    local_path = .
+    protocol = externals_only
+    externals = Externals_LIBX.cfg
+    required = True
+
+    Now, %(prog)s will process Externals.cfg and also process
+    Externals_LIBX.cfg as if it was a sub-external.
 
-  * Lines begining with '#' or ';' are comments and will be ignored.
+  * Lines beginning with '#' or ';' are comments and will be ignored.
 
 # Obtaining this tool, reporting issues, etc.
 
   The master repository for manage_externals is
   https://github.com/ESMCI/manage_externals. Any issues with this tool
   should be reported there.
+
+# Troubleshooting
+
+Operations performed by manage_externals utilities are explicit and
+data driven. %(prog)s will always attempt to make the working copy
+*exactly* match what is in the externals file when modifying the
+working copy of a repository.
+
+If %(prog)s is not doing what you expected, double check the contents
+of the externals description file or examine the output of
+./manage_externals/%(prog)s --status
+
 '''
 
     parser = argparse.ArgumentParser(
@@ -244,7 +256,7 @@ def commandline_arguments(args=None):
     # user options
     #
     parser.add_argument("components", nargs="*",
-                        help="Specific component(s) to checkout. By default"
+                        help="Specific component(s) to checkout. By default, "
                         "all required externals are checked out.")
 
     parser.add_argument('-e', '--externals', nargs='?',
@@ -258,9 +270,9 @@ def commandline_arguments(args=None):
                         'optional externals.')
 
     parser.add_argument('-S', '--status', action='store_true', default=False,
-                        help='Output status of the repositories managed by '
+                        help='Output the status of the repositories managed by '
                         '%(prog)s. By default only summary information '
-                        'is provided. Use verbose output to see details.')
+                        'is provided. Use the verbose option to see details.')
 
     parser.add_argument('-v', '--verbose', action='count', default=0,
                         help='Output additional information to '

manic/repository_git.py

@@ -55,7 +55,9 @@ def checkout(self, base_dir_path, repo_dir_name, verbosity):
         branch or tag.
         """
         repo_dir_path = os.path.join(base_dir_path, repo_dir_name)
-        if not os.path.exists(repo_dir_path):
+        repo_dir_exists = os.path.exists(repo_dir_path)
+        if (repo_dir_exists and not os.listdir(
+                repo_dir_path)) or not repo_dir_exists:
             self._clone_repo(base_dir_path, repo_dir_name, verbosity)
         self._checkout_ref(repo_dir_path, verbosity)
 

manic/utils.py

@@ -12,9 +12,9 @@
 import os
 import subprocess
 import sys
-import time
+from threading import Timer
 
-from .global_constants import LOCAL_PATH_INDICATOR, LOG_FILE_NAME
+from .global_constants import LOCAL_PATH_INDICATOR
 
 # ---------------------------------------------------------------------
 #
@@ -65,13 +65,15 @@ def last_n_lines(the_string, n_lines, truncation_message=None):
 
     lines = the_string.splitlines(True)
     if len(lines) <= n_lines:
-        return the_string
+        return_val = the_string
     else:
         lines_subset = lines[-n_lines:]
         str_truncated = ''.join(lines_subset)
         if truncation_message:
             str_truncated = truncation_message + '\n' + str_truncated
-        return str_truncated
+        return_val = str_truncated
+
+    return return_val
 
 
 def indent_string(the_string, indent_level):
@@ -119,9 +121,9 @@ def str_to_bool(bool_str):
     """
     value = None
     str_lower = bool_str.lower()
-    if (str_lower == 'true') or (str_lower == 't'):
+    if str_lower in ('true', 't'):
         value = True
-    elif (str_lower == 'false') or (str_lower == 'f'):
+    elif str_lower in ('false', 'f'):
         value = False
     if value is None:
         msg = ('ERROR: invalid boolean string value "{0}". '
@@ -199,80 +201,30 @@ def expand_local_url(url, field):
 # subprocess
 #
 # ---------------------------------------------------------------------
-_TIMEOUT_MSG = """ Timout errors typically occur when svn or git requires
-authentication to access a private repository. On some systems, svn
-and git requests for authentication information will not be displayed
-to the user.  In this case, the program will appear to hang and
-generate a timeout error. Ensure you can run svn and git manually and
-access all repositories without entering your authentication
-information."""
-
-_TIMEOUT_SEC = 300
-_POLL_DELTA_SEC = 0.02
-
-
-def _poll_subprocess(commands, status_to_caller, output_to_caller,
-                     timeout_sec=_TIMEOUT_SEC):
-    """Create a subprocess and poll the process until complete.
 
-    Impose a timeout limit and checkout process output for known
-    conditions that require user interaction.
+# Give the user a helpful message if we detect that a command seems to
+# be hanging.
+_HANGING_SEC = 300
 
-    NOTE: the timeout_delta has significant impact on run time. If it
-    is too long, and the many quick local subprocess calls will
-    drastically increase the run time, especially in tests.
-
-    NOTE: This function is broken out into for ease of
-    understanding. It does no error checking. It should only be called
-    from execute_subprocess, never directly.
-
-    """
-    logging.info(' '.join(commands))
-    output = []
-    start = time.time()
-
-    proc = subprocess.Popen(commands,
-                            shell=False,
-                            stdout=subprocess.PIPE,
-                            stderr=subprocess.STDOUT,
-                            universal_newlines=True)
-    while proc.poll() is None:
-        time.sleep(_POLL_DELTA_SEC)
-        if time.time() - start > timeout_sec:
-            proc.kill()
-            time.sleep(_POLL_DELTA_SEC * 5)
-            msg = ("subprocess call to  '{0}' has exceeded timeout limit of "
-                   "{1} seconds.\n{2}".format(commands[0], timeout_sec,
-                                              _TIMEOUT_MSG))
-            fatal_error(msg)
-    finish = time.time()
 
-    run_time_msg = "run time : {0:.2f} seconds".format(finish - start)
-    logging.info(run_time_msg)
-    output = proc.stdout.read()
-    log_process_output(output)
-    status = proc.returncode
+def _hanging_msg(working_directory, command):
+    print("""
 
-    # NOTE(bja, 2018-03) need to cleanup open files. In python3 use
-    # "with subprocess.Popen(...) as proc:", but that is not available
-    # with python2 unless we create a context manager.
-    proc.stdout.close()
+Command '{command}'
+from directory {working_directory}
+has taken {hanging_sec} seconds. It may be hanging.
 
-    if status != 0:
-        raise subprocess.CalledProcessError(returncode=status,
-                                            cmd=commands,
-                                            output=output)
-
-    if status_to_caller and output_to_caller:
-        ret_value = (status, output)
-    elif status_to_caller:
-        ret_value = status
-    elif output_to_caller:
-        ret_value = output
-    else:
-        ret_value = None
+The command will continue to run, but you may want to abort
+manage_externals with ^C and investigate. A possible cause of hangs is
+when svn or git require authentication to access a private
+repository. On some systems, svn and git requests for authentication
+information will not be displayed to the user. In this case, the program
+will appear to hang. Ensure you can run svn and git manually and access
+all repositories without entering your authentication information.
 
-    return ret_value
+""".format(command=command,
+           working_directory=working_directory,
+           hanging_sec=_HANGING_SEC))
 
 
 def execute_subprocess(commands, status_to_caller=False,
@@ -288,19 +240,24 @@ def execute_subprocess(commands, status_to_caller=False,
     return code, otherwise execute_subprocess treats non-zero return
     status as an error and raises an exception.
 
-    NOTE(bja, 2018-03) we are polling the running process to avoid
-    having it hang indefinitely if there is input that we don't
-    detect. Some large checkouts are multiple minutes long. For now we
-    are setting the timeout interval to five minutes.
-
     """
-    msg = 'In directory: {0}\nexecute_subprocess running command:'.format(
-        os.getcwd())
+    cwd = os.getcwd()
+    msg = 'In directory: {0}\nexecute_subprocess running command:'.format(cwd)
     logging.info(msg)
-    logging.info(commands)
+    commands_str = ' '.join(commands)
+    logging.info(commands_str)
+    return_to_caller = status_to_caller or output_to_caller
+    status = -1
+    output = ''
+    hanging_timer = Timer(_HANGING_SEC, _hanging_msg,
+                          kwargs={"working_directory": cwd,
+                                  "command": commands_str})
+    hanging_timer.start()
     try:
-        ret_value = _poll_subprocess(
-            commands, status_to_caller, output_to_caller)
+        output = subprocess.check_output(commands, stderr=subprocess.STDOUT,
+                                         universal_newlines=True)
+        log_process_output(output)
+        status = 0
     except OSError as error:
         msg = failed_command_msg(
             'Command execution failed. Does the executable exist?',
@@ -319,20 +276,27 @@ def execute_subprocess(commands, status_to_caller=False,
         # simple status check. If returning, it is the callers
         # responsibility determine if an error occurred and handle it
         # appropriately.
-        if status_to_caller and output_to_caller:
-            ret_value = (error.returncode, error.output)
-        elif status_to_caller:
-            ret_value = error.returncode
-        elif output_to_caller:
-            ret_value = error.output
-        else:
+        if not return_to_caller:
             msg_context = ('Process did not run successfully; '
                            'returned status {0}'.format(error.returncode))
             msg = failed_command_msg(msg_context, commands,
                                      output=error.output)
             logging.error(error)
+            logging.error(msg)
             log_process_output(error.output)
             fatal_error(msg)
+        status = error.returncode
+    finally:
+        hanging_timer.cancel()
+
+    if status_to_caller and output_to_caller:
+        ret_value = (status, output)
+    elif status_to_caller:
+        ret_value = status
+    elif output_to_caller:
+        ret_value = output
+    else:
+        ret_value = None
 
     return ret_value
 
@@ -363,7 +327,4 @@ def failed_command_msg(msg_context, command, output=None):
     if output:
         errmsg += 'See above for output from failed command.\n'
 
-    errmsg += 'Please check the log file {log} for more details.'.format(
-        log=LOG_FILE_NAME)
-
     return errmsg

test/.pylint.rc

@@ -50,7 +50,7 @@ confidence=
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use"--disable=all --enable=classes
 # --disable=W"
-disable=bad-continuation
+disable=bad-continuation,useless-object-inheritance
 
 
 # Enable the message, report, category or checker with the given id(s). You can