From 2117b843c2afc0efc5ec237e25180d057b573a6d Mon Sep 17 00:00:00 2001 From: John Alex Date: Sun, 12 Feb 2023 18:49:41 +0000 Subject: [PATCH] test_sys_checkout: verify that basic by-tag/branch/hash tests actually take us to the correct git tag/branch/hash. Add GitRepository._git_current_remote_branch() method --- manic/repository_git.py | 78 +++++++++++++++++++++++++++++++++++---- test/test_sys_checkout.py | 69 ++++++++++++++++++++++------------ 2 files changed, 117 insertions(+), 30 deletions(-) diff --git a/manic/repository_git.py b/manic/repository_git.py index c99ea7e4b..6633ea157 100644 --- a/manic/repository_git.py +++ b/manic/repository_git.py @@ -340,7 +340,11 @@ def _checkout_local_ref(self, verbosity, submodules): def _checkout_external_ref(self, verbosity, submodules): """Checkout the reference from a remote repository if is True, recursively initialize and update - the repo's submodules + the repo's submodules. + Note that this results in a 'detached HEAD' state if checking out + a branch, because we check out the remote branch rather than the + local. See https://github.com/ESMCI/manage_externals/issues/34 for + more discussion. """ if self._tag: ref = self._tag @@ -361,6 +365,10 @@ def _checkout_external_ref(self, verbosity, submodules): self._check_for_valid_ref(ref, remote_name) if self._branch: + # Prepend remote name to branch. This means we avoid various + # special cases if the local branch is not tracking the remote or + # cannot be trivially fast-forwarded to match; but, it also + # means we end up in a 'detached HEAD' state. ref = '{0}/{1}'.format(remote_name, ref) self._git_checkout_ref(ref, verbosity, submodules) @@ -601,28 +609,75 @@ def _status_v1z_is_dirty(git_output): # # ---------------------------------------------------------------- @staticmethod - def _git_current_hash(): + def _git_current_hash(dir=None): """Return the full hash of the currently checked-out version. + if dir is None, uses the cwd. + Returns a tuple, (hash_found, hash), where hash_found is a logical specifying whether a hash was found for HEAD (False could mean we're not in a git repository at all). (If hash_found is False, then hash is ''.) """ + if dir: + cwd = os.getcwd() + os.chdir(dir) + status, git_output = GitRepository._git_revparse_commit("HEAD") hash_found = not status if not hash_found: git_output = '' + if dir: + os.chdir(cwd) return hash_found, git_output @staticmethod - def _git_current_branch(): - """Determines the name of the current branch. + def _git_current_remote_branch(dir=None): + """Determines the name of the current remote branch, if any. + + if dir is None, uses the cwd. Returns a tuple, (branch_found, branch_name), where branch_found - is a logical specifying whether a branch name was found for + is a bool specifying whether a branch name was found for + HEAD. (If branch_found is False, then branch_name is ''). + branch_name is in the format '$remote/$branch', e.g. 'origin/foo'. + """ + if dir: + cwd = os.getcwd() + os.chdir(dir) + + branch_found = False + branch_name = '' + + cmd = 'git log -n 1 --pretty=%d HEAD'.split() + status, git_output = execute_subprocess(cmd, + output_to_caller=True, + status_to_caller=True) + branch_found = 'HEAD,' in git_output + if branch_found: + # git_output is of the form " (HEAD, origin/blah)" + branch_name = git_output.split(',')[1].strip()[:-1] + if dir: + os.chdir(cwd) + return branch_found, branch_name + + @staticmethod + def _git_current_branch(dir=None): + """Determines the name of the current local branch. + + if dir is None, uses the cwd. + + Returns a tuple, (branch_found, branch_name), where branch_found + is a bool specifying whether a branch name was found for HEAD. (If branch_found is False, then branch_name is ''.) + Note that currently we check out the remote branch rather than + the local, so this command does not return the just-checked-out + branch. See _git_current_remote_branch. """ + if dir: + cwd = os.getcwd() + os.chdir(dir) + cmd = ['git', 'symbolic-ref', '--short', '-q', 'HEAD'] status, git_output = execute_subprocess(cmd, output_to_caller=True, @@ -632,16 +687,23 @@ def _git_current_branch(): git_output = git_output.strip() else: git_output = '' + if dir: + os.chdir(cwd) return branch_found, git_output @staticmethod - def _git_current_tag(): + def _git_current_tag(dir=None): """Determines the name tag corresponding to HEAD (if any). + if dir is None, uses the cwd. + Returns a tuple, (tag_found, tag_name), where tag_found is a - logical specifying whether we found a tag name corresponding to + bool specifying whether we found a tag name corresponding to HEAD. (If tag_found is False, then tag_name is ''.) """ + if dir: + cwd = os.getcwd() + os.chdir(dir) # git describe --exact-match --tags HEAD cmd = ['git', 'describe', '--exact-match', '--tags', 'HEAD'] status, git_output = execute_subprocess(cmd, @@ -652,6 +714,8 @@ def _git_current_tag(): git_output = git_output.strip() else: git_output = '' + if dir: + os.chdir(cwd) return tag_found, git_output @staticmethod diff --git a/test/test_sys_checkout.py b/test/test_sys_checkout.py index a902f7cf4..14669e040 100644 --- a/test/test_sys_checkout.py +++ b/test/test_sys_checkout.py @@ -624,69 +624,88 @@ def test_required_bytag(self): # externals start out 'empty' aka not checked out. tree = self.execute_checkout_in_dir(cloned_repo_dir, self.status_args) - self._check_sync_clean(tree[self._external_path(TAG_SECTION)], + local_path = self._external_path(TAG_SECTION) + self._check_sync_clean(tree[local_path], ExternalStatus.EMPTY, ExternalStatus.DEFAULT) + tag_full_path = os.path.join(cloned_repo_dir, local_path) + self.assertFalse(os.path.exists(tag_full_path)) # after checkout, the external is 'clean' aka at the correct version. tree = self.execute_checkout_with_status(cloned_repo_dir, self.checkout_args) - self._check_sync_clean(tree[self._external_path(TAG_SECTION)], + self._check_sync_clean(tree[local_path], ExternalStatus.STATUS_OK, ExternalStatus.STATUS_OK) + # Actually checked out the desired tag. + (tag_found, tag_name) = GitRepository._git_current_tag(tag_full_path) + self.assertEqual(tag_name, 'tag1') + # Check existence of some simp_tag files tag_path = os.path.join('externals', TAG_SECTION) self._check_file_exists(cloned_repo_dir, os.path.join(tag_path, README_NAME)) + # Subrepo should not exist (not referenced by configs). self._check_file_absent(cloned_repo_dir, os.path.join(tag_path, 'simple_subdir', 'subdir_file.txt')) - - def test_required_bybranch(self): """Check out a required external pointing to a git branch.""" cloned_repo_dir = self.clone_test_repo(CONTAINER_REPO) self._generator.create_config() self._generator.create_section(SIMPLE_REPO, BRANCH_SECTION, - branch=REMOTE_BRANCH_FEATURE2) + branch=REMOTE_BRANCH_FEATURE2) self._generator.write_config(cloned_repo_dir) # externals start out 'empty' aka not checked out. tree = self.execute_checkout_in_dir(cloned_repo_dir, self.status_args) - self._check_sync_clean(tree[self._external_path(BRANCH_SECTION)], + local_path = self._external_path(BRANCH_SECTION) + self._check_sync_clean(tree[local_path], ExternalStatus.EMPTY, ExternalStatus.DEFAULT) + branch_path = os.path.join(cloned_repo_dir, local_path) + self.assertFalse(os.path.exists(branch_path)) # after checkout, the external is 'clean' aka at the correct version. tree = self.execute_checkout_with_status(cloned_repo_dir, self.checkout_args) - self._check_sync_clean(tree[self._external_path(BRANCH_SECTION)], + self._check_sync_clean(tree[local_path], ExternalStatus.STATUS_OK, ExternalStatus.STATUS_OK) + self.assertTrue(os.path.exists(branch_path)) + (branch_found, branch_name) = GitRepository._git_current_remote_branch( + branch_path) + self.assertEquals(branch_name, 'origin/' + REMOTE_BRANCH_FEATURE2) def test_required_byhash(self): """Check out a required external pointing to a git hash.""" cloned_repo_dir = self.clone_test_repo(CONTAINER_REPO) self._generator.create_config() self._generator.create_section(SIMPLE_REPO, HASH_SECTION, - ref_hash='60b1cc1a38d63') + ref_hash='60b1cc1a38d63') self._generator.write_config(cloned_repo_dir) # externals start out 'empty' aka not checked out. tree = self.execute_checkout_in_dir(cloned_repo_dir, self.status_args) - self._check_sync_clean(tree[self._external_path(HASH_SECTION)], + local_path = self._external_path(HASH_SECTION) + self._check_sync_clean(tree[local_path], ExternalStatus.EMPTY, ExternalStatus.DEFAULT) + hash_path = os.path.join(cloned_repo_dir, local_path) + self.assertFalse(os.path.exists(hash_path)) # after checkout, the externals are 'clean' aka at their correct version. tree = self.execute_checkout_with_status(cloned_repo_dir, self.checkout_args) - self._check_sync_clean(tree[self._external_path(HASH_SECTION)], + self._check_sync_clean(tree[local_path], ExternalStatus.STATUS_OK, ExternalStatus.STATUS_OK) + (hash_found, hash_name) = GitRepository._git_current_hash(hash_path) + self.assertTrue(hash_name.startswith('60b1cc1a38d63'), + msg=hash_name) def test_container_nested_required(self): """Verify that a container with nested subrepos generates the correct initial status. @@ -709,14 +728,15 @@ def test_container_nested_required(self): # We happen to check out each section via a different reference (tag/branch/hash) but # those don't really matter, we just need to check out three repos into a nested set of # directories. - self._generator.create_section(SIMPLE_REPO, TAG_SECTION, nested=True, - tag='tag1', path=NESTED_SUBDIR[order[0]]) - - self._generator.create_section(SIMPLE_REPO, BRANCH_SECTION, nested=True, - branch=REMOTE_BRANCH_FEATURE2, path=NESTED_SUBDIR[order[1]]) - - self._generator.create_section(SIMPLE_REPO, HASH_SECTION, nested=True, - ref_hash='60b1cc1a38d63', path=NESTED_SUBDIR[order[2]]) + self._generator.create_section( + SIMPLE_REPO, TAG_SECTION, nested=True, + tag='tag1', path=NESTED_SUBDIR[order[0]]) + self._generator.create_section( + SIMPLE_REPO, BRANCH_SECTION, nested=True, + branch=REMOTE_BRANCH_FEATURE2, path=NESTED_SUBDIR[order[1]]) + self._generator.create_section( + SIMPLE_REPO, HASH_SECTION, nested=True, + ref_hash='60b1cc1a38d63', path=NESTED_SUBDIR[order[2]]) self._generator.write_config(cloned_repo_dir) # all externals start out 'empty' aka not checked out. @@ -746,8 +766,8 @@ def test_container_nested_required(self): ExternalStatus.STATUS_OK) def test_container_simple_optional(self): - """Verify that container with an optional simple subrepos generates the correct initial - status. + """Verify that container with an optional simple subrepos generates + the correct initial status. """ # create repo and externals config. @@ -845,7 +865,8 @@ def test_container_simple_dirty(self): ExternalStatus.STATUS_OK) # add a tracked file to the simp_tag external, should be dirty. - RepoUtils.add_file_to_repo(cloned_repo_dir, 'externals/{0}/tmp.txt'.format(TAG_SECTION), + RepoUtils.add_file_to_repo(cloned_repo_dir, + 'externals/{0}/tmp.txt'.format(TAG_SECTION), tracked=True) tree = self.execute_checkout_in_dir(cloned_repo_dir, self.status_args) self._check_sync_clean(tree[self._external_path(TAG_SECTION)], @@ -871,13 +892,15 @@ def test_container_simple_untracked(self): self._generator.write_config(cloned_repo_dir) # checkout, should start out clean. - tree = self.execute_checkout_with_status(cloned_repo_dir, self.checkout_args) + tree = self.execute_checkout_with_status(cloned_repo_dir, + self.checkout_args) self._check_sync_clean(tree[self._external_path(TAG_SECTION)], ExternalStatus.STATUS_OK, ExternalStatus.STATUS_OK) # add an untracked file to the simp_tag external, should stay clean. - RepoUtils.add_file_to_repo(cloned_repo_dir, 'externals/{0}/tmp.txt'.format(TAG_SECTION), + RepoUtils.add_file_to_repo(cloned_repo_dir, + 'externals/{0}/tmp.txt'.format(TAG_SECTION), tracked=False) tree = self.execute_checkout_in_dir(cloned_repo_dir, self.status_args) self._check_sync_clean(tree[self._external_path(TAG_SECTION)],