Skip to content

Commit

Permalink
config, gitpython-developers#525: polish more config-urls
Browse files Browse the repository at this point in the history
  • Loading branch information
ankostis committed Oct 12, 2016
1 parent 6d441c9 commit 3358136
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 39 deletions.
4 changes: 4 additions & 0 deletions git/objects/submodule/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions git/test/lib/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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):
Expand Down
74 changes: 38 additions & 36 deletions git/test/test_submodule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -354,15 +356,15 @@ 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
osm = Submodule.add(rwrepo, osmid, csm_repopath, new_csmclone_path, Submodule.k_head_default)
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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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])
Expand All @@ -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")
Expand Down Expand Up @@ -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):
Expand All @@ -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")

Expand All @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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")
Expand All @@ -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'
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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")
Expand All @@ -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

Expand All @@ -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')
Expand All @@ -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")

Expand All @@ -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
Expand All @@ -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)
Expand Down

0 comments on commit 3358136

Please sign in to comment.