From dc67cc682ea872bc5f5a206eaaa8708dc6ccd814 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Thu, 17 Sep 2020 11:31:15 -0600 Subject: [PATCH 01/10] simpler solution --- manic/sourcetree.py | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/manic/sourcetree.py b/manic/sourcetree.py index b9c9c21082..b67992c090 100644 --- a/manic/sourcetree.py +++ b/manic/sourcetree.py @@ -331,11 +331,13 @@ def checkout(self, verbosity, load_all, load_comp=None): printlog('Checking out externals: ', end='') if load_all: - load_comps = self._all_components.keys() + tmp_comps = self._all_components.keys() elif load_comp is not None: - load_comps = [load_comp] + tmp_comps = [load_comp] else: - load_comps = self._required_compnames + tmp_comps = self._required_compnames + + load_comps = self.order_comps_by_local_path(self, tmp_comps) # checkout the primary externals for comp in load_comps: @@ -351,3 +353,20 @@ def checkout(self, verbosity, load_all, load_comp=None): # now give each external an opportunitity to checkout it's externals. for comp in load_comps: self._all_components[comp].checkout_externals(verbosity, load_all) + + def order_comps_by_local_path(self, comps_in): + """ + put the comps into an order so that comp local_paths + that are nested are checked out in correct order + """ + comps_out = [] + local_paths = [] + for comp in comps_in: + local_paths.append(self._all_components[comp].get_local_path()) + local_paths = list(set(local_paths)) + for path in sorted(local_paths, key=len): + for comp in comps_in: + if self._all_components[comp].get_local_path() == path: + comps_out.append(comp) + comps_out = list(set(comps_out)) + return comps_out From 36c56dbaca3e653f568555230c1e1af2c4398d71 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Thu, 17 Sep 2020 11:37:34 -0600 Subject: [PATCH 02/10] simplier fix for issue --- manic/sourcetree.py | 2 +- manic/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/manic/sourcetree.py b/manic/sourcetree.py index b67992c090..4796d3d007 100644 --- a/manic/sourcetree.py +++ b/manic/sourcetree.py @@ -337,7 +337,7 @@ def checkout(self, verbosity, load_all, load_comp=None): else: tmp_comps = self._required_compnames - load_comps = self.order_comps_by_local_path(self, tmp_comps) + load_comps = self.order_comps_by_local_path(tmp_comps) # checkout the primary externals for comp in load_comps: diff --git a/manic/utils.py b/manic/utils.py index f57f43930c..dfe1f4de34 100644 --- a/manic/utils.py +++ b/manic/utils.py @@ -255,7 +255,7 @@ def execute_subprocess(commands, status_to_caller=False, hanging_timer.start() try: output = subprocess.check_output(commands, stderr=subprocess.STDOUT, - universal_newlines=True) + universal_newlines=True).decode('utf-8') log_process_output(output) status = 0 except OSError as error: From 1c53be854200d418f3434f2bea8bded70b649861 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Thu, 17 Sep 2020 11:53:39 -0600 Subject: [PATCH 03/10] no need for set call --- manic/sourcetree.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/manic/sourcetree.py b/manic/sourcetree.py index 4796d3d007..65d6e2f112 100644 --- a/manic/sourcetree.py +++ b/manic/sourcetree.py @@ -338,7 +338,6 @@ def checkout(self, verbosity, load_all, load_comp=None): tmp_comps = self._required_compnames load_comps = self.order_comps_by_local_path(tmp_comps) - # checkout the primary externals for comp in load_comps: if verbosity < VERBOSITY_VERBOSE: @@ -363,10 +362,8 @@ def order_comps_by_local_path(self, comps_in): local_paths = [] for comp in comps_in: local_paths.append(self._all_components[comp].get_local_path()) - local_paths = list(set(local_paths)) for path in sorted(local_paths, key=len): for comp in comps_in: if self._all_components[comp].get_local_path() == path: comps_out.append(comp) - comps_out = list(set(comps_out)) return comps_out From 880a4e76578312d1c17a6b048b01cfab70b776c7 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Thu, 17 Sep 2020 11:59:38 -0600 Subject: [PATCH 04/10] remove decode --- manic/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manic/utils.py b/manic/utils.py index dfe1f4de34..f57f43930c 100644 --- a/manic/utils.py +++ b/manic/utils.py @@ -255,7 +255,7 @@ def execute_subprocess(commands, status_to_caller=False, hanging_timer.start() try: output = subprocess.check_output(commands, stderr=subprocess.STDOUT, - universal_newlines=True).decode('utf-8') + universal_newlines=True) log_process_output(output) status = 0 except OSError as error: From 29f45b086d24c8ff35630b6854b00f4fc55623a6 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Thu, 17 Sep 2020 12:43:56 -0600 Subject: [PATCH 05/10] remove py2 test and fix super call --- .travis.yml | 3 +-- test/test_sys_checkout.py | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1990cb9604..d9b24c584d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ language: python os: linux -python: - - "2.7" +python: - "3.4" - "3.5" - "3.6" diff --git a/test/test_sys_checkout.py b/test/test_sys_checkout.py index 007f5e4897..ff9d5024df 100644 --- a/test/test_sys_checkout.py +++ b/test/test_sys_checkout.py @@ -1591,7 +1591,8 @@ def setUp(self): """ # Run the basic setup - super(TestSubrepoCheckout, self).setUp() + #super(TestSubrepoCheckout, self).setUp() + super().setUp() # create test repo # We need to do this here (rather than have a static repo) because # git submodules do not allow for variables in .gitmodules files From 24a3726a17182047bee4f41b0c2d222ab23779cc Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 18 Sep 2020 08:05:31 -0600 Subject: [PATCH 06/10] improve sorting, checkout externals with each comp --- manic/sourcetree.py | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/manic/sourcetree.py b/manic/sourcetree.py index 65d6e2f112..d22de77688 100644 --- a/manic/sourcetree.py +++ b/manic/sourcetree.py @@ -336,8 +336,9 @@ def checkout(self, verbosity, load_all, load_comp=None): tmp_comps = [load_comp] else: tmp_comps = self._required_compnames - - load_comps = self.order_comps_by_local_path(tmp_comps) + # Sort by path so that if paths are nested the + # parent repo is checked out first. + load_comps = sorted(tmp_comps,key=lambda comp: self._all_components[comp].get_local_path()) # checkout the primary externals for comp in load_comps: if verbosity < VERBOSITY_VERBOSE: @@ -347,23 +348,6 @@ def checkout(self, verbosity, load_all, load_comp=None): # output a newline printlog(EMPTY_STR) self._all_components[comp].checkout(verbosity, load_all) - printlog('') - - # now give each external an opportunitity to checkout it's externals. - for comp in load_comps: + # now give each external an opportunitity to checkout it's externals. self._all_components[comp].checkout_externals(verbosity, load_all) - - def order_comps_by_local_path(self, comps_in): - """ - put the comps into an order so that comp local_paths - that are nested are checked out in correct order - """ - comps_out = [] - local_paths = [] - for comp in comps_in: - local_paths.append(self._all_components[comp].get_local_path()) - for path in sorted(local_paths, key=len): - for comp in comps_in: - if self._all_components[comp].get_local_path() == path: - comps_out.append(comp) - return comps_out + printlog('') From 75c5353d2a39149fd7388c8213b4e179690a8d4c Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 18 Sep 2020 08:12:12 -0600 Subject: [PATCH 07/10] fix spacing --- manic/sourcetree.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manic/sourcetree.py b/manic/sourcetree.py index d22de77688..54de763c30 100644 --- a/manic/sourcetree.py +++ b/manic/sourcetree.py @@ -338,7 +338,7 @@ def checkout(self, verbosity, load_all, load_comp=None): tmp_comps = self._required_compnames # Sort by path so that if paths are nested the # parent repo is checked out first. - load_comps = sorted(tmp_comps,key=lambda comp: self._all_components[comp].get_local_path()) + load_comps = sorted(tmp_comps, key=lambda comp: self._all_components[comp].get_local_path()) # checkout the primary externals for comp in load_comps: if verbosity < VERBOSITY_VERBOSE: From 7c9f3c613b6612af54f340e4f0e3266a83f35044 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 18 Sep 2020 09:39:50 -0600 Subject: [PATCH 08/10] add a test for nested repo checkout --- test/test_sys_checkout.py | 95 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 3 deletions(-) diff --git a/test/test_sys_checkout.py b/test/test_sys_checkout.py index ff9d5024df..c9e07cbdb5 100644 --- a/test/test_sys_checkout.py +++ b/test/test_sys_checkout.py @@ -85,6 +85,8 @@ CFG_SUB_NAME = 'sub-externals.cfg' README_NAME = 'readme.txt' REMOTE_BRANCH_FEATURE2 = 'feature2' +NESTED_NAME = ['./fred', './fred/wilma', './fred/wilma/barney'] + SVN_TEST_REPO = 'https://github.com/escomp/cesm' @@ -160,6 +162,23 @@ def container_simple_required(self, dest_dir): self.write_config(dest_dir) + def container_nested_required(self, dest_dir, order=[0,1,2]): + """Create a container externals file with only simple externals. + + """ + self.create_config() + self.create_section(SIMPLE_REPO_NAME, 'simp_tag', nested=True, + tag='tag1', path=NESTED_NAME[order[0]]) + + self.create_section(SIMPLE_REPO_NAME, 'simp_branch', nested=True, + branch=REMOTE_BRANCH_FEATURE2, path=NESTED_NAME[order[1]]) + + self.create_section(SIMPLE_REPO_NAME, 'simp_hash', nested=True, + ref_hash='60b1cc1a38d63',path=NESTED_NAME[order[2]]) + + self.write_config(dest_dir) + + def container_simple_optional(self, dest_dir): """Create a container externals file with optional simple externals @@ -261,7 +280,7 @@ def create_metadata(self): def create_section(self, repo_type, name, tag='', branch='', ref_hash='', required=True, path=EXTERNALS_NAME, externals='', repo_path=None, from_submodule=False, - sparse=''): + sparse='', nested=False): # pylint: disable=too-many-branches """Create a config section with autofilling some items and handling optional items. @@ -270,8 +289,11 @@ def create_section(self, repo_type, name, tag='', branch='', # pylint: disable=R0913 self._config.add_section(name) if not from_submodule: - self._config.set(name, ExternalsDescription.PATH, - os.path.join(path, name)) + if nested: + self._config.set(name, ExternalsDescription.PATH, path) + else: + self._config.set(name, ExternalsDescription.PATH, + os.path.join(path, name)) self._config.set(name, ExternalsDescription.PROTOCOL, ExternalsDescription.PROTOCOL_GIT) @@ -671,10 +693,16 @@ def _check_simple_tag_empty(self, tree, directory=EXTERNALS_NAME): name = './{0}/simp_tag'.format(directory) self._check_generic_empty_default_required(tree, name) + def _check_nested_tag_empty(self, tree, name=EXTERNALS_NAME): + self._check_generic_empty_default_required(tree, name) + def _check_simple_tag_ok(self, tree, directory=EXTERNALS_NAME): name = './{0}/simp_tag'.format(directory) self._check_generic_ok_clean_required(tree, name) + def _check_nested_tag_ok(self, tree, name=EXTERNALS_NAME): + self._check_generic_ok_clean_required(tree, name) + def _check_simple_tag_dirty(self, tree, directory=EXTERNALS_NAME): name = './{0}/simp_tag'.format(directory) self._check_generic_ok_dirty_required(tree, name) @@ -687,10 +715,16 @@ def _check_simple_branch_empty(self, tree, directory=EXTERNALS_NAME): name = './{0}/simp_branch'.format(directory) self._check_generic_empty_default_required(tree, name) + def _check_nested_branch_empty(self, tree, name=EXTERNALS_NAME): + self._check_generic_empty_default_required(tree, name) + def _check_simple_branch_ok(self, tree, directory=EXTERNALS_NAME): name = './{0}/simp_branch'.format(directory) self._check_generic_ok_clean_required(tree, name) + def _check_nested_branch_ok(self, tree, name=EXTERNALS_NAME): + self._check_generic_ok_clean_required(tree, name) + def _check_simple_branch_modified(self, tree, directory=EXTERNALS_NAME): name = './{0}/simp_branch'.format(directory) self._check_generic_modified_ok_required(tree, name) @@ -699,10 +733,16 @@ def _check_simple_hash_empty(self, tree, directory=EXTERNALS_NAME): name = './{0}/simp_hash'.format(directory) self._check_generic_empty_default_required(tree, name) + def _check_nested_hash_empty(self, tree, name=EXTERNALS_NAME): + self._check_generic_empty_default_required(tree, name) + def _check_simple_hash_ok(self, tree, directory=EXTERNALS_NAME): name = './{0}/simp_hash'.format(directory) self._check_generic_ok_clean_required(tree, name) + def _check_nested_hash_ok(self, tree, name=EXTERNALS_NAME): + self._check_generic_ok_clean_required(tree, name) + def _check_simple_hash_modified(self, tree, directory=EXTERNALS_NAME): name = './{0}/simp_hash'.format(directory) self._check_generic_modified_ok_required(tree, name) @@ -754,6 +794,12 @@ def _check_container_simple_required_pre_checkout(self, overall, tree): self._check_simple_branch_empty(tree) self._check_simple_hash_empty(tree) + def _check_container_nested_required_pre_checkout(self, overall, tree, order=[0,1,2]): + self.assertEqual(overall, 0) + self._check_nested_tag_empty(tree, name=NESTED_NAME[order[0]]) + self._check_nested_branch_empty(tree, name=NESTED_NAME[order[1]]) + self._check_nested_hash_empty(tree, name=NESTED_NAME[order[2]]) + def _check_container_simple_required_checkout(self, overall, tree): # Note, this is the internal tree status just before checkout self.assertEqual(overall, 0) @@ -761,12 +807,25 @@ def _check_container_simple_required_checkout(self, overall, tree): self._check_simple_branch_empty(tree) self._check_simple_hash_empty(tree) + def _check_container_nested_required_checkout(self, overall, tree, order=[0,1,2]): + # Note, this is the internal tree status just before checkout + self.assertEqual(overall, 0) + self._check_nested_tag_empty(tree, name=NESTED_NAME[order[0]]) + self._check_nested_branch_empty(tree, name=NESTED_NAME[order[1]]) + self._check_nested_hash_empty(tree, name=NESTED_NAME[order[2]]) + def _check_container_simple_required_post_checkout(self, overall, tree): self.assertEqual(overall, 0) self._check_simple_tag_ok(tree) self._check_simple_branch_ok(tree) self._check_simple_hash_ok(tree) + def _check_container_nested_required_post_checkout(self, overall, tree, order=[0,1,2]): + self.assertEqual(overall, 0) + self._check_nested_tag_ok(tree, name=NESTED_NAME[order[0]]) + self._check_nested_branch_ok(tree, name=NESTED_NAME[order[1]]) + self._check_nested_hash_ok(tree, name=NESTED_NAME[order[2]]) + def _check_container_simple_required_out_of_sync(self, overall, tree): self.assertEqual(overall, 0) self._check_simple_tag_modified(tree) @@ -964,6 +1023,36 @@ def test_container_simple_required(self): self.status_args) self._check_container_simple_required_post_checkout(overall, tree) + def test_container_nested_required(self): + """Verify that a container with nested subrepos + generates the correct initial status. + Tests over all possible permutations + """ + + orders = [[0,1,2], [1,2,0], [2,0,1], [0,2,1], [2,1,0], [1,0,2]] + for n, order in enumerate(orders): + # create repo + dest_dir=os.path.join(os.environ[MANIC_TEST_TMP_REPO_ROOT], + self._test_id,"test"+str(n)) + under_test_dir = self.setup_test_repo(CONTAINER_REPO_NAME, + dest_dir_in=dest_dir) + self._generator.container_nested_required(under_test_dir, order=order) + + # status of empty repo + overall, tree = self.execute_cmd_in_dir(under_test_dir, + self.status_args) + self._check_container_nested_required_pre_checkout(overall, tree, order=order) + + # checkout + overall, tree = self.execute_cmd_in_dir(under_test_dir, + self.checkout_args) + self._check_container_nested_required_checkout(overall, tree, order=order) + + # status clean checked out + overall, tree = self.execute_cmd_in_dir(under_test_dir, + self.status_args) + self._check_container_nested_required_post_checkout(overall, tree, order=order) + def test_container_simple_optional(self): """Verify that container with an optional simple subrepos generates the correct initial status. From 29e26af811c5f6db342614c8aa67ffbbfa68ced9 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 18 Sep 2020 09:54:46 -0600 Subject: [PATCH 09/10] fix pylint issues --- manic/checkout.py | 4 ++-- test/test_sys_checkout.py | 25 +++++++++++++------------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/manic/checkout.py b/manic/checkout.py index 222065553e..8dd1798d7a 100755 --- a/manic/checkout.py +++ b/manic/checkout.py @@ -380,14 +380,14 @@ def main(args): if args.status: # user requested status-only - for comp in sorted(tree_status.keys()): + for comp in sorted(tree_status): tree_status[comp].log_status_message(args.verbose) else: # checkout / update the external repositories. safe_to_update = check_safe_to_update_repos(tree_status) if not safe_to_update: # print status - for comp in sorted(tree_status.keys()): + for comp in sorted(tree_status): tree_status[comp].log_status_message(args.verbose) # exit gracefully msg = """The external repositories labeled with 'M' above are not in a clean state. diff --git a/test/test_sys_checkout.py b/test/test_sys_checkout.py index c9e07cbdb5..1be7cba104 100644 --- a/test/test_sys_checkout.py +++ b/test/test_sys_checkout.py @@ -162,7 +162,7 @@ def container_simple_required(self, dest_dir): self.write_config(dest_dir) - def container_nested_required(self, dest_dir, order=[0,1,2]): + def container_nested_required(self, dest_dir, order): """Create a container externals file with only simple externals. """ @@ -174,7 +174,7 @@ def container_nested_required(self, dest_dir, order=[0,1,2]): branch=REMOTE_BRANCH_FEATURE2, path=NESTED_NAME[order[1]]) self.create_section(SIMPLE_REPO_NAME, 'simp_hash', nested=True, - ref_hash='60b1cc1a38d63',path=NESTED_NAME[order[2]]) + ref_hash='60b1cc1a38d63', path=NESTED_NAME[order[2]]) self.write_config(dest_dir) @@ -794,7 +794,7 @@ def _check_container_simple_required_pre_checkout(self, overall, tree): self._check_simple_branch_empty(tree) self._check_simple_hash_empty(tree) - def _check_container_nested_required_pre_checkout(self, overall, tree, order=[0,1,2]): + def _check_container_nested_required_pre_checkout(self, overall, tree, order): self.assertEqual(overall, 0) self._check_nested_tag_empty(tree, name=NESTED_NAME[order[0]]) self._check_nested_branch_empty(tree, name=NESTED_NAME[order[1]]) @@ -807,7 +807,7 @@ def _check_container_simple_required_checkout(self, overall, tree): self._check_simple_branch_empty(tree) self._check_simple_hash_empty(tree) - def _check_container_nested_required_checkout(self, overall, tree, order=[0,1,2]): + def _check_container_nested_required_checkout(self, overall, tree, order): # Note, this is the internal tree status just before checkout self.assertEqual(overall, 0) self._check_nested_tag_empty(tree, name=NESTED_NAME[order[0]]) @@ -820,7 +820,7 @@ def _check_container_simple_required_post_checkout(self, overall, tree): self._check_simple_branch_ok(tree) self._check_simple_hash_ok(tree) - def _check_container_nested_required_post_checkout(self, overall, tree, order=[0,1,2]): + def _check_container_nested_required_post_checkout(self, overall, tree, order): self.assertEqual(overall, 0) self._check_nested_tag_ok(tree, name=NESTED_NAME[order[0]]) self._check_nested_branch_ok(tree, name=NESTED_NAME[order[1]]) @@ -1029,29 +1029,30 @@ def test_container_nested_required(self): Tests over all possible permutations """ - orders = [[0,1,2], [1,2,0], [2,0,1], [0,2,1], [2,1,0], [1,0,2]] + orders = [[0, 1, 2], [1, 2, 0], [2, 0, 1], + [0, 2, 1], [2, 1, 0], [1, 0, 2]] for n, order in enumerate(orders): # create repo - dest_dir=os.path.join(os.environ[MANIC_TEST_TMP_REPO_ROOT], - self._test_id,"test"+str(n)) + dest_dir = os.path.join(os.environ[MANIC_TEST_TMP_REPO_ROOT], + self._test_id, "test"+str(n)) under_test_dir = self.setup_test_repo(CONTAINER_REPO_NAME, dest_dir_in=dest_dir) - self._generator.container_nested_required(under_test_dir, order=order) + self._generator.container_nested_required(under_test_dir, order) # status of empty repo overall, tree = self.execute_cmd_in_dir(under_test_dir, self.status_args) - self._check_container_nested_required_pre_checkout(overall, tree, order=order) + self._check_container_nested_required_pre_checkout(overall, tree, order) # checkout overall, tree = self.execute_cmd_in_dir(under_test_dir, self.checkout_args) - self._check_container_nested_required_checkout(overall, tree, order=order) + self._check_container_nested_required_checkout(overall, tree, order) # status clean checked out overall, tree = self.execute_cmd_in_dir(under_test_dir, self.status_args) - self._check_container_nested_required_post_checkout(overall, tree, order=order) + self._check_container_nested_required_post_checkout(overall, tree, order) def test_container_simple_optional(self): """Verify that container with an optional simple subrepos From 42687bd537ffa51fd0c99de78fdf70884c1b3b4c Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 18 Sep 2020 14:22:08 -0600 Subject: [PATCH 10/10] remove commented code --- test/test_sys_checkout.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_sys_checkout.py b/test/test_sys_checkout.py index 1be7cba104..d978ba6fc9 100644 --- a/test/test_sys_checkout.py +++ b/test/test_sys_checkout.py @@ -1681,7 +1681,6 @@ def setUp(self): """ # Run the basic setup - #super(TestSubrepoCheckout, self).setUp() super().setUp() # create test repo # We need to do this here (rather than have a static repo) because