From d7a42ae961a91e1396ce613705170bd0975be2ea Mon Sep 17 00:00:00 2001 From: John Alex Date: Wed, 15 Feb 2023 02:50:27 +0000 Subject: [PATCH] Check that desired repo was actually checked out. Change GitRepository._determine_remote_name() into a classmethod _remote_name_for_url that takes in a url, for more clarity in how the remote name is being picked. --- manic/repository_git.py | 36 ++++++++------- test/test_sys_checkout.py | 75 ++++++++++++++++++++++---------- test/test_unit_repository_git.py | 4 +- 3 files changed, 75 insertions(+), 40 deletions(-) diff --git a/manic/repository_git.py b/manic/repository_git.py index 6633ea157..ef2c55920 100644 --- a/manic/repository_git.py +++ b/manic/repository_git.py @@ -194,7 +194,7 @@ def compare_refs(current_ref, expected_ref): if self._url == LOCAL_PATH_INDICATOR: expected_ref = self._branch else: - remote_name = self._determine_remote_name() + remote_name = self._remote_name_for_url(self._url) if not remote_name: # git doesn't know about this remote. by definition # this is a modified state. @@ -232,16 +232,13 @@ def compare_refs(current_ref, expected_ref): os.chdir(cwd) - def _determine_remote_name(self): - """Return the remote name. - - Note that this is for the *future* repo url and branch, not - the current working copy! + @classmethod + def _remote_name_for_url(cls, remote_url, dir=None): + """Return the remote name matching remote_url (or None) """ - git_output = self._git_remote_verbose() + git_output = cls._git_remote_verbose(dir) git_output = git_output.splitlines() - remote_name = '' for line in git_output: data = line.strip() if not data: @@ -249,10 +246,9 @@ def _determine_remote_name(self): data = data.split() name = data[0].strip() url = data[1].strip() - if self._url == url: - remote_name = name - break - return remote_name + if remote_url == url: + return name + return None def _create_remote_name(self): """The url specified in the externals description file was not known @@ -353,7 +349,7 @@ def _checkout_external_ref(self, verbosity, submodules): else: ref = self._hash - remote_name = self._determine_remote_name() + remote_name = self._remote_name_for_url(self._url) if not remote_name: remote_name = self._create_remote_name() self._git_remote_add(remote_name, self._url) @@ -612,7 +608,7 @@ def _status_v1z_is_dirty(git_output): def _git_current_hash(dir=None): """Return the full hash of the currently checked-out version. - if dir is None, uses the cwd. + 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 @@ -787,11 +783,21 @@ def _git_status_verbose(): return git_output @staticmethod - def _git_remote_verbose(): + def _git_remote_verbose(dir=None): """Run the git remote command to obtain repository information. + + If dir is None, uses the cwd. + Returned string is of the form: + myfork git@github.com:johnpaulalex/manage_externals_jp.git (fetch) + myfork git@github.com:johnpaulalex/manage_externals_jp.git (push) """ + if dir: + cwd = os.getcwd() + os.chdir(dir) cmd = ['git', 'remote', '--verbose'] git_output = execute_subprocess(cmd, output_to_caller=True) + if dir: + os.chdir(cwd) return git_output @staticmethod diff --git a/test/test_sys_checkout.py b/test/test_sys_checkout.py index 37a637ad7..806f0d8c6 100644 --- a/test/test_sys_checkout.py +++ b/test/test_sys_checkout.py @@ -269,6 +269,12 @@ def create_metadata(self): self._config.set(DESCRIPTION_SECTION, VERSION_ITEM, self._schema_version) + def url_for_repo_path(self, repo_path, repo_path_abs=None): + if repo_path_abs is not None: + return repo_path_abs + else: + return os.path.join(self._bare_root, repo_path) + def create_section(self, repo_path, name, tag='', branch='', ref_hash='', required=True, path=EXTERNALS_PATH, sub_externals='', repo_path_abs=None, from_submodule=False, @@ -304,10 +310,7 @@ def create_section(self, repo_path, name, tag='', branch='', ref_hash = '' branch = '' - if repo_path_abs is not None: - repo_url = repo_path_abs - else: - repo_url = os.path.join(self._bare_root, repo_path) + repo_url = self.url_for_repo_path(repo_path, repo_path_abs) if not from_submodule: self._config.set(name, ExternalsDescription.REPO_URL, repo_url) @@ -639,22 +642,29 @@ 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) - local_path = self._external_path(TAG_SECTION) - self._check_sync_clean(tree[local_path], + local_path_rel = self._external_path(TAG_SECTION) + self._check_sync_clean(tree[local_path_rel], ExternalStatus.EMPTY, ExternalStatus.DEFAULT) - tag_full_path = os.path.join(cloned_repo_dir, local_path) - self.assertFalse(os.path.exists(tag_full_path)) + local_path_abs = os.path.join(cloned_repo_dir, local_path_rel) + self.assertFalse(os.path.exists(local_path_abs)) # 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[local_path], + self._check_sync_clean(tree[local_path_rel], ExternalStatus.STATUS_OK, ExternalStatus.STATUS_OK) + # Actually checked out the desired repo. + self.assertEqual('origin', GitRepository._remote_name_for_url( + # Which url to look up + self._generator.url_for_repo_path(SIMPLE_REPO), + # Which directory has the local checked-out repo. + dir=local_path_abs)) + # Actually checked out the desired tag. - (tag_found, tag_name) = GitRepository._git_current_tag(tag_full_path) + (tag_found, tag_name) = GitRepository._git_current_tag(local_path_abs) self.assertEqual(tag_name, 'tag1') # Check existence of some simp_tag files @@ -677,22 +687,31 @@ def test_required_bybranch(self): # externals start out 'empty' aka not checked out. tree = self.execute_checkout_in_dir(cloned_repo_dir, self.status_args) - local_path = self._external_path(BRANCH_SECTION) - self._check_sync_clean(tree[local_path], + local_path_rel = self._external_path(BRANCH_SECTION) + self._check_sync_clean(tree[local_path_rel], ExternalStatus.EMPTY, ExternalStatus.DEFAULT) - branch_path = os.path.join(cloned_repo_dir, local_path) - self.assertFalse(os.path.exists(branch_path)) + local_path_abs = os.path.join(cloned_repo_dir, local_path_rel) + self.assertFalse(os.path.exists(local_path_abs)) # 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[local_path], + self._check_sync_clean(tree[local_path_rel], ExternalStatus.STATUS_OK, ExternalStatus.STATUS_OK) - self.assertTrue(os.path.exists(branch_path)) + self.assertTrue(os.path.exists(local_path_abs)) + + # Actually checked out the desired repo. + self.assertEqual('origin', GitRepository._remote_name_for_url( + # Which url to look up + self._generator.url_for_repo_path(SIMPLE_REPO), + # Which directory has the local checked-out repo. + dir=local_path_abs)) + + # Actually checked out the desired branch. (branch_found, branch_name) = GitRepository._git_current_remote_branch( - branch_path) + local_path_abs) self.assertEquals(branch_name, 'origin/' + REMOTE_BRANCH_FEATURE2) def test_required_byhash(self): @@ -706,20 +725,30 @@ def test_required_byhash(self): # externals start out 'empty' aka not checked out. tree = self.execute_checkout_in_dir(cloned_repo_dir, self.status_args) - local_path = self._external_path(HASH_SECTION) - self._check_sync_clean(tree[local_path], + local_path_rel = self._external_path(HASH_SECTION) + self._check_sync_clean(tree[local_path_rel], ExternalStatus.EMPTY, ExternalStatus.DEFAULT) - hash_path = os.path.join(cloned_repo_dir, local_path) - self.assertFalse(os.path.exists(hash_path)) + local_path_abs = os.path.join(cloned_repo_dir, local_path_rel) + self.assertFalse(os.path.exists(local_path_abs)) # 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[local_path], + self._check_sync_clean(tree[local_path_rel], ExternalStatus.STATUS_OK, ExternalStatus.STATUS_OK) - (hash_found, hash_name) = GitRepository._git_current_hash(hash_path) + + # Actually checked out the desired repo. + self.assertEqual('origin', GitRepository._remote_name_for_url( + # Which url to look up + self._generator.url_for_repo_path(SIMPLE_REPO), + # Which directory has the local checked-out repo. + dir=local_path_abs)) + + # Actually checked out the desired hash. + (hash_found, hash_name) = GitRepository._git_current_hash( + local_path_abs) self.assertTrue(hash_name.startswith('60b1cc1a38d63'), msg=hash_name) diff --git a/test/test_unit_repository_git.py b/test/test_unit_repository_git.py index a6ad9f100..ebc0fb7fa 100644 --- a/test/test_unit_repository_git.py +++ b/test/test_unit_repository_git.py @@ -433,7 +433,7 @@ def test_sync_branch_on_diff_hash(self): self.assertEqual(stat.clean_state, ExternalStatus.DEFAULT) def test_sync_branch_diff_remote(self): - """Test _determine_remote_name with a different remote + """Test _remote_name_for_url with a different remote """ stat = ExternalStatus() @@ -449,7 +449,7 @@ def test_sync_branch_diff_remote(self): # expected argument def test_sync_branch_diff_remote2(self): - """Test _determine_remote_name with a different remote + """Test _remote_name_for_url with a different remote """ stat = ExternalStatus()