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.

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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff 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
Original file line number Diff line number Diff 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
Original file line number Diff line number Diff 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 Down Expand 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!!
Copy link
Contributor Author

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)?

Copy link
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