From 335813647f9f71a91bcf874de3f759db3cf256c1 Mon Sep 17 00:00:00 2001 From: Kostis Anagnostopoulos Date: Wed, 12 Oct 2016 17:09:16 +0200 Subject: [PATCH] config, #525: polish more config-urls --- git/objects/submodule/base.py | 4 ++ git/test/lib/helper.py | 7 ++-- git/test/test_submodule.py | 74 ++++++++++++++++++----------------- 3 files changed, 46 insertions(+), 39 deletions(-) diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 999c452be..9bb563d7b 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -41,6 +41,7 @@ from unittest.case import SkipTest from git.util import HIDE_WINDOWS_KNOWN_ERRORS from git.objects.base import IndexObject, Object +from git.cmd import Git __all__ = ["Submodule", "UpdateProgress"] @@ -394,6 +395,9 @@ def add(cls, repo, name, path, url=None, branch=None, no_checkout=False): mrepo = cls._clone_repo(repo, url, path, name, **kwargs) # END verify url + ## See #525 for ensuring git urls in config-files valid under Windows. + url = Git.polish_url(url) + # It's important to add the URL to the parent config, to let `git submodule` know. # otherwise there is a '-' character in front of the submodule listing # a38efa84daef914e4de58d1905a500d8d14aaf45 mymodule (v0.9.0-1-ga38efa8) diff --git a/git/test/lib/helper.py b/git/test/lib/helper.py index e3e7020cd..80d1ae7a2 100644 --- a/git/test/lib/helper.py +++ b/git/test/lib/helper.py @@ -214,7 +214,7 @@ def case(self, rw_repo, rw_remote_repo) See working dir info in with_rw_repo :note: We attempt to launch our own invocation of git-daemon, which will be shutdown at the end of the test. """ - from git import Git, Remote + from git import Git, Remote # To avoid circular deps. assert isinstance(working_tree_ref, string_types), "Decorator requires ref name for working tree checkout" @@ -250,7 +250,7 @@ def remote_repo_creator(self): base_path, rel_repo_dir = osp.split(remote_repo_dir) - remote_repo_url = "git://localhost:%s/%s" % (GIT_DAEMON_PORT, rel_repo_dir) + remote_repo_url = Git.polish_url("git://localhost:%s/%s" % (GIT_DAEMON_PORT, rel_repo_dir)) with d_remote.config_writer as cw: cw.set('url', remote_repo_url) @@ -342,7 +342,8 @@ class TestBase(TestCase): def _small_repo_url(self): """:return" a path to a small, clonable repository""" - return osp.join(self.rorepo.working_tree_dir, 'git/ext/gitdb/gitdb/ext/smmap') + from git.cmd import Git + return Git.polish_url(osp.join(self.rorepo.working_tree_dir, 'git/ext/gitdb/gitdb/ext/smmap')) @classmethod def setUpClass(cls): diff --git a/git/test/test_submodule.py b/git/test/test_submodule.py index 902a96b55..9db4f9c90 100644 --- a/git/test/test_submodule.py +++ b/git/test/test_submodule.py @@ -5,6 +5,7 @@ from unittest.case import skipIf import git +from git.cmd import Git from git.compat import string_types, is_win from git.exc import ( InvalidGitRepositoryError, @@ -23,6 +24,7 @@ from git.test.lib import with_rw_directory from git.util import HIDE_WINDOWS_KNOWN_ERRORS from git.util import to_native_path_linux, join_path_native +import os.path as osp # Change the configuration if possible to prevent the underlying memory manager @@ -111,7 +113,7 @@ def _do_base_tests(self, rwrepo): else: with sm.config_writer() as writer: # for faster checkout, set the url to the local path - new_smclone_path = to_native_path_linux(join_path_native(self.rorepo.working_tree_dir, sm.path)) + new_smclone_path = Git.polish_url(osp.join(self.rorepo.working_tree_dir, sm.path)) writer.set_value('url', new_smclone_path) writer.release() assert sm.config_reader().get_value('url') == new_smclone_path @@ -168,7 +170,7 @@ def _do_base_tests(self, rwrepo): ################# # lets update it - its a recursive one too - newdir = os.path.join(sm.abspath, 'dir') + newdir = osp.join(sm.abspath, 'dir') os.makedirs(newdir) # update fails if the path already exists non-empty @@ -213,7 +215,7 @@ def _do_base_tests(self, rwrepo): csm_repopath = csm.path # adjust the path of the submodules module to point to the local destination - new_csmclone_path = to_native_path_linux(join_path_native(self.rorepo.working_tree_dir, sm.path, csm.path)) + new_csmclone_path = Git.polish_url(osp.join(self.rorepo.working_tree_dir, sm.path, csm.path)) with csm.config_writer() as writer: writer.set_value('url', new_csmclone_path) assert csm.url == new_csmclone_path @@ -301,7 +303,7 @@ def _do_base_tests(self, rwrepo): csm.update() assert csm.module_exists() assert csm.exists() - assert os.path.isdir(csm.module().working_tree_dir) + assert osp.isdir(csm.module().working_tree_dir) # this would work assert sm.remove(force=True, dry_run=True) is sm @@ -354,7 +356,7 @@ def _do_base_tests(self, rwrepo): assert nsm.module_exists() assert nsm.exists() # its not checked out - assert not os.path.isfile(join_path_native(nsm.module().working_tree_dir, Submodule.k_modules_file)) + assert not osp.isfile(join_path_native(nsm.module().working_tree_dir, Submodule.k_modules_file)) assert len(rwrepo.submodules) == 1 # add another submodule, but into the root, not as submodule @@ -362,7 +364,7 @@ def _do_base_tests(self, rwrepo): assert osm != nsm assert osm.module_exists() assert osm.exists() - assert os.path.isfile(join_path_native(osm.module().working_tree_dir, 'setup.py')) + assert osp.isfile(join_path_native(osm.module().working_tree_dir, 'setup.py')) assert len(rwrepo.submodules) == 2 @@ -479,7 +481,7 @@ def test_root_module(self, rwrepo): # assure we clone from a local source with sm.config_writer() as writer: - writer.set_value('url', to_native_path_linux(join_path_native(self.rorepo.working_tree_dir, sm.path))) + writer.set_value('url', Git.polish_url(osp.join(self.rorepo.working_tree_dir, sm.path))) # dry-run does nothing sm.update(recursive=False, dry_run=True, progress=prog) @@ -513,7 +515,7 @@ def test_root_module(self, rwrepo): #================ nsmn = "newsubmodule" nsmp = "submrepo" - subrepo_url = to_native_path_linux(join_path_native(self.rorepo.working_tree_dir, rsmsp[0], rsmsp[1])) + subrepo_url = Git.polish_url(osp.join(self.rorepo.working_tree_dir, rsmsp[0], rsmsp[1])) nsm = Submodule.add(rwrepo, nsmn, nsmp, url=subrepo_url) csmadded = rwrepo.index.commit("Added submodule").hexsha # make sure we don't keep the repo reference nsm.set_parent_commit(csmadded) @@ -535,24 +537,24 @@ def test_root_module(self, rwrepo): sm.set_parent_commit(csmadded) smp = sm.abspath assert not sm.remove(module=False).exists() - assert os.path.isdir(smp) # module still exists + assert osp.isdir(smp) # module still exists csmremoved = rwrepo.index.commit("Removed submodule") # an update will remove the module # not in dry_run rm.update(recursive=False, dry_run=True, force_remove=True) - assert os.path.isdir(smp) + assert osp.isdir(smp) # when removing submodules, we may get new commits as nested submodules are auto-committing changes # to allow deletions without force, as the index would be dirty otherwise. # QUESTION: Why does this seem to work in test_git_submodule_compatibility() ? self.failUnlessRaises(InvalidGitRepositoryError, rm.update, recursive=False, force_remove=False) rm.update(recursive=False, force_remove=True) - assert not os.path.isdir(smp) + assert not osp.isdir(smp) # 'apply work' to the nested submodule and assure this is not removed/altered during updates # Need to commit first, otherwise submodule.update wouldn't have a reason to change the head - touch(os.path.join(nsm.module().working_tree_dir, 'new-file')) + touch(osp.join(nsm.module().working_tree_dir, 'new-file')) # We cannot expect is_dirty to even run as we wouldn't reset a head to the same location assert nsm.module().head.commit.hexsha == nsm.hexsha nsm.module().index.add([nsm]) @@ -574,7 +576,7 @@ def test_root_module(self, rwrepo): # ... to the first repository, this way we have a fast checkout, and a completely different # repository at the different url nsm.set_parent_commit(csmremoved) - nsmurl = to_native_path_linux(join_path_native(self.rorepo.working_tree_dir, rsmsp[0])) + nsmurl = Git.polish_url(osp.join(self.rorepo.working_tree_dir, rsmsp[0])) with nsm.config_writer() as writer: writer.set_value('url', nsmurl) csmpathchange = rwrepo.index.commit("changed url") @@ -648,21 +650,21 @@ def test_first_submodule(self, rwrepo): assert len(list(rwrepo.iter_submodules())) == 0 for sm_name, sm_path in (('first', 'submodules/first'), - ('second', os.path.join(rwrepo.working_tree_dir, 'submodules/second'))): + ('second', osp.join(rwrepo.working_tree_dir, 'submodules/second'))): sm = rwrepo.create_submodule(sm_name, sm_path, rwrepo.git_dir, no_checkout=True) assert sm.exists() and sm.module_exists() rwrepo.index.commit("Added submodule " + sm_name) # end for each submodule path to add - self.failUnlessRaises(ValueError, rwrepo.create_submodule, 'fail', os.path.expanduser('~')) + self.failUnlessRaises(ValueError, rwrepo.create_submodule, 'fail', osp.expanduser('~')) self.failUnlessRaises(ValueError, rwrepo.create_submodule, 'fail-too', - rwrepo.working_tree_dir + os.path.sep) + rwrepo.working_tree_dir + osp.sep) @with_rw_directory def test_add_empty_repo(self, rwdir): - empty_repo_dir = os.path.join(rwdir, 'empty-repo') + empty_repo_dir = osp.join(rwdir, 'empty-repo') - parent = git.Repo.init(os.path.join(rwdir, 'parent')) + parent = git.Repo.init(osp.join(rwdir, 'parent')) git.Repo.init(empty_repo_dir) for checkout_mode in range(2): @@ -673,7 +675,7 @@ def test_add_empty_repo(self, rwdir): @with_rw_directory def test_git_submodules_and_add_sm_with_new_commit(self, rwdir): - parent = git.Repo.init(os.path.join(rwdir, 'parent')) + parent = git.Repo.init(osp.join(rwdir, 'parent')) parent.git.submodule('add', self._small_repo_url(), 'module') parent.index.commit("added submodule") @@ -683,7 +685,7 @@ def test_git_submodules_and_add_sm_with_new_commit(self, rwdir): assert sm.exists() and sm.module_exists() clone = git.Repo.clone_from(self._small_repo_url(), - os.path.join(parent.working_tree_dir, 'existing-subrepository')) + osp.join(parent.working_tree_dir, 'existing-subrepository')) sm2 = parent.create_submodule('nongit-file-submodule', clone.working_tree_dir) assert len(parent.submodules) == 2 @@ -700,7 +702,7 @@ def test_git_submodules_and_add_sm_with_new_commit(self, rwdir): parent.index.commit("moved submodules") smm = sm.module() - fp = os.path.join(smm.working_tree_dir, 'empty-file') + fp = osp.join(smm.working_tree_dir, 'empty-file') with open(fp, 'w'): pass smm.git.add(fp) @@ -733,7 +735,7 @@ def test_git_submodules_and_add_sm_with_new_commit(self, rwdir): # "'C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\test_work_tree_unsupportedryfa60di\\master_repo\\.git\\objects\\pack\\pack-bc9e0787aef9f69e1591ef38ea0a6f566ec66fe3.idx") # noqa E501 @with_rw_directory def test_git_submodule_compatibility(self, rwdir): - parent = git.Repo.init(os.path.join(rwdir, 'parent')) + parent = git.Repo.init(osp.join(rwdir, 'parent')) sm_path = join_path_native('submodules', 'intermediate', 'one') sm = parent.create_submodule('mymodules/myname', sm_path, url=self._small_repo_url()) parent.index.commit("added submodule") @@ -747,13 +749,13 @@ def assert_exists(sm, value=True): # muss it up. That's the only reason why the test is still here ... . assert len(parent.git.submodule().splitlines()) == 1 - module_repo_path = os.path.join(sm.module().working_tree_dir, '.git') - assert module_repo_path.startswith(os.path.join(parent.working_tree_dir, sm_path)) + module_repo_path = osp.join(sm.module().working_tree_dir, '.git') + assert module_repo_path.startswith(osp.join(parent.working_tree_dir, sm_path)) if not sm._need_gitfile_submodules(parent.git): - assert os.path.isdir(module_repo_path) + assert osp.isdir(module_repo_path) assert not sm.module().has_separate_working_tree() else: - assert os.path.isfile(module_repo_path) + assert osp.isfile(module_repo_path) assert sm.module().has_separate_working_tree() assert find_git_dir(module_repo_path) is not None, "module pointed to by .git file must be valid" # end verify submodule 'style' @@ -803,12 +805,12 @@ def assert_exists(sm, value=True): for dry_run in (True, False): sm.remove(dry_run=dry_run, force=True) assert_exists(sm, value=dry_run) - assert os.path.isdir(sm_module_path) == dry_run + assert osp.isdir(sm_module_path) == dry_run # end for each dry-run mode @with_rw_directory def test_remove_norefs(self, rwdir): - parent = git.Repo.init(os.path.join(rwdir, 'parent')) + parent = git.Repo.init(osp.join(rwdir, 'parent')) sm_name = 'mymodules/myname' sm = parent.create_submodule(sm_name, sm_name, url=self._small_repo_url()) assert sm.exists() @@ -817,7 +819,7 @@ def test_remove_norefs(self, rwdir): assert sm.repo is parent # yoh was surprised since expected sm repo!! # so created a new instance for submodule - smrepo = git.Repo(os.path.join(rwdir, 'parent', sm.path)) + smrepo = git.Repo(osp.join(rwdir, 'parent', sm.path)) # Adding a remote without fetching so would have no references smrepo.create_remote('special', 'git@server-shouldnotmatter:repo.git') # And we should be able to remove it just fine @@ -826,7 +828,7 @@ def test_remove_norefs(self, rwdir): @with_rw_directory def test_rename(self, rwdir): - parent = git.Repo.init(os.path.join(rwdir, 'parent')) + parent = git.Repo.init(osp.join(rwdir, 'parent')) sm_name = 'mymodules/myname' sm = parent.create_submodule(sm_name, sm_name, url=self._small_repo_url()) parent.index.commit("Added submodule") @@ -843,7 +845,7 @@ def test_rename(self, rwdir): assert sm.exists() sm_mod = sm.module() - if os.path.isfile(os.path.join(sm_mod.working_tree_dir, '.git')) == sm._need_gitfile_submodules(parent.git): + if osp.isfile(osp.join(sm_mod.working_tree_dir, '.git')) == sm._need_gitfile_submodules(parent.git): assert sm_mod.git_dir.endswith(join_path_native('.git', 'modules', new_sm_name)) # end @@ -852,8 +854,8 @@ def test_branch_renames(self, rw_dir): # Setup initial sandbox: # parent repo has one submodule, which has all the latest changes source_url = self._small_repo_url() - sm_source_repo = git.Repo.clone_from(source_url, os.path.join(rw_dir, 'sm-source'), b='master') - parent_repo = git.Repo.init(os.path.join(rw_dir, 'parent')) + sm_source_repo = git.Repo.clone_from(source_url, osp.join(rw_dir, 'sm-source'), b='master') + parent_repo = git.Repo.init(osp.join(rw_dir, 'parent')) sm = parent_repo.create_submodule('mysubmodule', 'subdir/submodule', sm_source_repo.working_tree_dir, branch='master') parent_repo.index.commit('added submodule') @@ -862,7 +864,7 @@ def test_branch_renames(self, rw_dir): # Create feature branch with one new commit in submodule source sm_fb = sm_source_repo.create_head('feature') sm_fb.checkout() - new_file = touch(os.path.join(sm_source_repo.working_tree_dir, 'new-file')) + new_file = touch(osp.join(sm_source_repo.working_tree_dir, 'new-file')) sm_source_repo.index.add([new_file]) sm.repo.index.commit("added new file") @@ -888,7 +890,7 @@ def test_branch_renames(self, rw_dir): # To make it even 'harder', we shall fork and create a new commit sm_pfb = sm_source_repo.create_head('past-feature', commit='HEAD~20') sm_pfb.checkout() - sm_source_repo.index.add([touch(os.path.join(sm_source_repo.working_tree_dir, 'new-file'))]) + sm_source_repo.index.add([touch(osp.join(sm_source_repo.working_tree_dir, 'new-file'))]) sm_source_repo.index.commit("new file added, to past of '%r'" % sm_fb) # Change designated submodule checkout branch to a new commit in its own past @@ -897,7 +899,7 @@ def test_branch_renames(self, rw_dir): sm.repo.index.commit("changed submodule branch to '%s'" % sm_pfb) # Test submodule updates - must fail if submodule is dirty - touch(os.path.join(sm_mod.working_tree_dir, 'unstaged file')) + touch(osp.join(sm_mod.working_tree_dir, 'unstaged file')) # This doesn't fail as our own submodule binsha didn't change, and the reset is only triggered if # to latest revision is True. parent_repo.submodule_update(to_latest_revision=False)