Squashed 'manage_externals/' changes from 82d3b247f..7b6d92ef6

7b6d92ef6 Merge pull request ESCOMP#198 from johnpaulalex/gitdir
927ce3a98 Merge pull request ESCOMP#197 from johnpaulalex/testpath
a04f1148f Merge pull request ESCOMP#196 from johnpaulalex/readmod
d9c14bf25 Change the rest of the methods to use -C. Still some usage of getcwd in test_unit_repository_git.
332b10640 Fix incorrect logged path of checkout_externals in test_sys_checkout: it was basically the parent of the current directory, which varies throughout the test. (it called abspath with '{0}/../../', which adds arbitrary and not-interpolated subdir '{0}' to the path, then removes it and removes one more level).
932a7499b Remove printlog from read_gitmodules_file since read_externals_description_file() already has a nearly-the-same printlog (but add it to the other caller).
5d13719ed Merge pull request ESCOMP#195 from johnpaulalex/check_repo
423395449 Update utest to mock _git_remote_verbose in a new way, since it is now called via the GitRepository class rather than on the specific GitRepository instance.
d7a42ae96 Check that desired repo was actually checked out.
71596bbc1 Merge pull request ESCOMP#194 from johnpaulalex/manic2
4c96e824e Make the MANIC_TEST_BARE_REPO_ROOT env var special - give it a constant for easy tracking, and automatically tear it down after each test.
259bfc04d test_sys_checkout: use actual paths in on-the-fly configs rather than MANIC_TEST_BARE_REPO_ROOT env var. This will make it easier to test (in the near future) that checkout_externals actually checked out the desired repo dir.
557bbd6eb Merge pull request ESCOMP#193 from johnpaulalex/manic
5314eede1 Remove MANIC_TEST_TMP_REPO_ROOT environment variable in favor of module-level variable.
345fc1e14 Merge pull request ESCOMP#191 from johnpaulalex/test_doc12
2117b843c test_sys_checkout: verify that basic by-tag/branch/hash tests actually take us to the correct git tag/branch/hash.
94d6e5f2b Merge pull request ESCOMP#190 from johnpaulalex/test_doc11
3ff33a6a8 Inline local-path-creation methods
47dea7f64 Merge pull request ESCOMP#189 from johnpaulalex/test_doc10
9ea75cbf8 Grab-bag of renamings: Remove redundant _NAME from repo constants, and consistently add _REPO suffix (This causes the majority of diffs).
c0c847ec8 Merge pull request ESCOMP#188 from johnpaulalex/test_doc9
2dd5ce0f7 test_sys_checkout.py: only check for correct 'required' or 'optional' state in the test that exercises required vs optional behavior. Removed a lot of boilerplate.
eb3085984 Merge pull request ESCOMP#187 from johnpaulalex/test_doc8
1832e1f84 test_sys_checkout: Simplify many tests to only use a single external.
8689d61ec Merge pull request ESCOMP#186 from johnpaulalex/test_doc7
fbee4253e Grab bag of test_sys_checkout cleanups:    Doc inside of each test more clearly/consistently.    TestSysCheckoutSVN didn’t get the inlining-of-helper-methods treatment, now it has that.    Move various standalone repo helper methods (like create_branch) into a RepoUtils class.    README.md was missing newlines when rendered as markdown.    Doc the return value of checkout.main    Fix test_container_exclude_component - it was looking for the wrong key (which is never present); now it looks for the correct key.
f0ed44a6e Merge pull request ESCOMP#185 from johnpaulalex/test_doc6
a3d59f5f2 Merge pull request ESCOMP#184 from johnpaulalex/test_doc5
5329c8ba7 test_sys_checkout: Inline config generation functions that are only called once.
464f2c7a7 test_sys_checkout: Inline another layer (per-config-file checks). Rename the 4 methods that are used multiple times, to reflect what they do rather than what they're called.
8872c0df6 Merge pull request ESCOMP#183 from johnpaulalex/doc_test4
c045335f6 Merge pull request ESCOMP#182 from johnpaulalex/doc_test3
c583b956e Merge pull request ESCOMP#181 from johnpaulalex/doc_test2
e01cfe278 test_sys_checkout: less confusing handling of return values from checkout_externals. Specifically, when doing a checkout, don't return tree_status from _before_ the checkout. Make a new wrapper to call checkout_externals a second time, to calculate the new status after a checkout (very frequent pattern).
23286818c test_sys_checkout: Remove another layer (which generates test component names)
c3717b6bc Merge pull request ESCOMP#180 from johnpaulalex/doc_test
36d7a4434 test_sys_checkout.py: remove one layer of functions (that check for local status enums). No-op.
2c4584bf7 More documentation about tests: * contents of test repositories (n a new README.md) * various constants in test_sys_checkout.py that point to those contents, and terminology like container/simple/mixed. * in each test method, the scenarios being tested. * The coupling between test methods.
55e74bd0a Merge pull request ESCOMP#179 from johnpaulalex/circ
66be84290 Remove circular dependency by making _External stop doing tricky things with sourcetrees.

git-subtree-dir: manage_externals
git-subtree-split: 7b6d92ef689e2f65733e27f8635ab91fb341356b

manic/checkout.py

@@ -378,9 +378,9 @@ def main(args):
     the --all option is passed.
 
     Returns a tuple (overall_status, tree_status). overall_status is 0
-    on success, non-zero on failure. tree_status gives the full status
-    *before* executing the checkout command - i.e., the status that it
-    used to determine if it's safe to proceed with the checkout.
+    on success, non-zero on failure. tree_status is a dict mapping local path
+    to ExternalStatus -- if no checkout is happening. If checkout is happening, tree_status
+    is None.
     """
     if args.do_logging:
         logging.basicConfig(filename=LOG_FILE_NAME,
@@ -438,6 +438,8 @@ def main(args):
             for comp in args.components:
                 source_tree.checkout(args.verbose, load_all, load_comp=comp)
             printlog('')
+            # New tree status is unknown, don't return anything.
+            tree_status = None
 
     logging.info('%s completed without exceptions.', program_name)
     # NOTE(bja, 2017-11) tree status is used by the systems tests

manic/externals_description.py

@@ -88,7 +88,7 @@ def read_externals_description_file(root_dir, file_name):
 
     externals_description = None
     if file_name == ExternalsDescription.GIT_SUBMODULES_FILENAME:
-        externals_description = read_gitmodules_file(root_dir, file_name)
+        externals_description = _read_gitmodules_file(root_dir, file_name)
     else:
         try:
             config = config_parser()
@@ -151,9 +151,8 @@ def git_submodule_status(repo_dir):
     """Run the git submodule status command to obtain submodule hashes.
         """
     # This function is here instead of GitRepository to avoid a dependency loop
-    cwd = os.getcwd()
-    os.chdir(repo_dir)
-    cmd = ['git', 'submodule', 'status']
+    cmd = 'git -C {repo_dir} submodule status'.format(
+        repo_dir=repo_dir).split()
     git_output = execute_subprocess(cmd, output_to_caller=True)
     submodules = {}
     submods = git_output.split('\n')
@@ -168,7 +167,6 @@ def git_submodule_status(repo_dir):
 
             submodules[items[1]] = {'hash':items[0], 'status':status, 'tag':tag}
 
-    os.chdir(cwd)
     return submodules
 
 def parse_submodules_desc_section(section_items, file_path):
@@ -191,7 +189,7 @@ def parse_submodules_desc_section(section_items, file_path):
 
     return path, url
 
-def read_gitmodules_file(root_dir, file_name):
+def _read_gitmodules_file(root_dir, file_name):
     # pylint: disable=deprecated-method
     # Disabling this check because the method is only used for python2
     # pylint: disable=too-many-locals
@@ -203,12 +201,11 @@ def read_gitmodules_file(root_dir, file_name):
     root_dir = os.path.abspath(root_dir)
     msg = 'In directory : {0}'.format(root_dir)
     logging.info(msg)
-    printlog('Processing submodules description file : {0}'.format(file_name))
 
     file_path = os.path.join(root_dir, file_name)
     if not os.path.exists(file_name):
         msg = ('ERROR: submodules description file, "{0}", does not '
-               'exist at path:\n    {1}'.format(file_name, file_path))
+               'exist in dir:\n    {1}'.format(file_name, root_dir))
         fatal_error(msg)
 
     submodules_description = None
@@ -640,8 +637,11 @@ def _repo_config_from_submodule(self, field, submod_desc):
                        '       Parent repo, "{1}" does not have submodules')
                 fatal_error(msg.format(field, self._parent_repo.name()))
 
-            submod_file = read_gitmodules_file(repo_path, submod_file)
-            submod_desc = create_externals_description(submod_file)
+            printlog(
+                'Processing submodules description file : {0} ({1})'.format(
+                    submod_file, repo_path))
+            submod_model_data= _read_gitmodules_file(repo_path, submod_file)
+            submod_desc = create_externals_description(submod_model_data)
 
         # Can we find our external?
         repo_url = None

manic/externals_status.py

@@ -30,12 +30,12 @@ class ExternalStatus(object):
 
     """
     # sync_state and clean_state can be one of the following:
-    DEFAULT = '-'  # aka not set yet.
+    DEFAULT = '-'  # not set yet (sync_state).  clean_state can be this if sync_state is EMPTY.
     UNKNOWN = '?'
     EMPTY = 'e'
     MODEL_MODIFIED = 's'  # repo version != externals (sync_state only)
     DIRTY = 'M'       # repo is dirty (clean_state only)
-    STATUS_OK = ' '   # repo is clean/matches externals.
+    STATUS_OK = ' '   # repo is clean (clean_state) or matches externals version (sync_state)
     STATUS_ERROR = '!'
 
     # source_type can be one of the following: