Skip to content

BF: remove a submodule with a remote without refs + misc fixes around#521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ankostis merged 4 commits into from
Oct 2, 2016
Merged

BF: remove a submodule with a remote without refs + misc fixes around #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f2b92c6
BF: Allow to remove a submodule with a remote without refs
yarikoptic Oct 2, 2016
31fd955
BF: @with_rw_directory must return decorated call
yarikoptic Oct 2, 2016
2528d11
BF: log.info is a function, just pass msg, no .write!
yarikoptic Oct 2, 2016
f284a4e
TST: finishing test for removing submodule with remotes without refs
yarikoptic Oct 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion git/objects/submodule/base.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -836,7 +836,7 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
num_branches_with_new_commits += len(mod.git.cherry(rref)) != 0
# END for each remote ref
# not a single remote branch contained all our commits
if num_branches_with_new_commits == len(rrefs):
if len(rrefs) and num_branches_with_new_commits == len(rrefs):
raise InvalidGitRepositoryError(
"Cannot delete module at %s as there are new commits" % mod.working_tree_dir)
# END handle new commits
Expand Down
6 changes: 4 additions & 2 deletions git/test/lib/helper.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -94,8 +94,8 @@ def wrapper(self):
try:
return func(self, path)
except Exception:
log.info.write("Test %s.%s failed, output is at %r\n",
type(self).__name__, func.__name__, path)
log.info("Test %s.%s failed, output is at %r\n",
type(self).__name__, func.__name__, path)
keep = True
raise
finally:
Expand All@@ -107,6 +107,8 @@ def wrapper(self):
gc.collect()
if not keep:
rmtree(path)
wrapper.__name__ = func.__name__
return wrapper


def with_rw_repo(working_tree_ref, bare=False):
Expand Down
22 changes: 20 additions & 2 deletions git/test/test_submodule.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -317,8 +317,8 @@ def _do_base_tests(self, rwrepo):
# forcibly delete the child repository
prev_count = len(sm.children())
self.failUnlessRaises(ValueError, csm.remove, force=True)
# We removed sm, which removed all submodules. Howver, the instance we have
# still points to the commit prior to that, where it still existed
# We removed sm, which removed all submodules. However, the instance we
# have still points to the commit prior to that, where it still existed
csm.set_parent_commit(csm.repo.commit(), check=False)
assert not csm.exists()
assert not csm.module_exists()
Expand DownExpand Up@@ -801,6 +801,24 @@ def assert_exists(sm, value=True):
assert os.path.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'))
sm_name = 'mymodules/myname'
sm = parent.create_submodule(sm_name, sm_name, url=self._small_repo_url())
assert sm.exists()

parent.index.commit("Added submodule")

assert sm.repo is parent # yoh was surprised since expected sm repo!!

@yarikopticyarikopticOct 2, 2016

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and I am not sure why that is the case -- I was expecting submodules repo to be the repo of the submodule ... but I guess it can make sense to refer to parent which contains the submodule, but then is there an attribute for submodules repo (if present)?

@ankostisankostisOct 2, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Byron any comment? I'm not familiar with git-modules.

# so created a new instance for submodule
smrepo = git.Repo(os.path.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
sm.remove()
assert not sm.exists()

@with_rw_directory
def test_rename(self, rwdir):
parent = git.Repo.init(os.path.join(rwdir, 'parent'))
Expand Down