[PATCH] submodule: Fix regression for deinit without submodules

[PATCH] submodule: Fix regression for deinit without submodules

[Stefan Beller]

Per Cederqvist wrote:
> It used to be possible to run
>
>    git submodule deinit -f .
>
> to remove any submodules, no matter how many submodules you had.  That
> is no longer possible in projects that don't have any submodules at
> all.  The command will fail with:
>
>     error: pathspec '.' did not match any file(s) known to git.

This regression was introduced in 74703a1e4dfc (submodule: rewrite
`module_list` shell function in C, 2015-09-02), as we optimized
the new module listing for performance by first checking whether it is
a git link instead of feeding it to match_pathspec.

By switching these two commands we get the same behavior as before,
i.e. in an empty repository this still breaks, but if there are files
but no submodules this works.

By reversing the order we also revert an accidental feature. If you had
a README file in a repository, you could do

    git submodule deinit "READM*"

and it would report that there is no submodule matching the "READM*"
path spec.

Reported-by: Per Cederqvist <cederp@opera.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---

 Ok, let's try again. This is what I understand is the best bugfix for
 2.8 and below. (This patch was developed on top of 2.7.4)
 
 As discussed we want to have a better solution in the future, which
 may included having the --unmatch-ok flag, but that is not in the scope
 for now.
 
 Thanks,
 Stefan


 builtin/submodule--helper.c |  6 +++---
 t/t7400-submodule-basic.sh  | 13 +++++++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index ed764c9..5295b72 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -37,9 +37,9 @@ static int module_list_compute(int argc, const char **argv,
 	for (i = 0; i < active_nr; i++) {
 		const struct cache_entry *ce = active_cache[i];
 
-		if (!S_ISGITLINK(ce->ce_mode) ||
-		    !match_pathspec(pathspec, ce->name, ce_namelen(ce),
-				    0, ps_matched, 1))
+		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
+				    0, ps_matched, 1) ||
+		    !S_ISGITLINK(ce->ce_mode))
 			continue;
 
 		ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index be82a75..e1abd19 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -849,6 +849,19 @@ test_expect_success 'set up a second submodule' '
 	git commit -m "submodule example2 added"
 '
 
+test_expect_success 'submodule deinit works on repository without submodules' '
+	test_when_finished "rm -rf newdirectory" &&
+	mkdir newdirectory &&
+	(
+		cd newdirectory &&
+		git init &&
+		>file &&
+		git add file &&
+		git commit -m "repo should not be empty"
+		git submodule deinit .
+	)
+'
+
 test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
 	git config submodule.example.foo bar &&
 	git config submodule.example2.frotz nitfol &&
-- 
2.7.4.dirty

Re: [PATCH] submodule: Fix regression for deinit without submodules

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> Per Cederqvist wrote:
>> It used to be possible to run
>>
>>    git submodule deinit -f .
>>
>> to remove any submodules, no matter how many submodules you had.  That
>> is no longer possible in projects that don't have any submodules at
>> all.  The command will fail with:
>>
>>     error: pathspec '.' did not match any file(s) known to git.
>
> This regression was introduced in 74703a1e4dfc (submodule: rewrite
> `module_list` shell function in C, 2015-09-02), as we optimized
> ...
>
> Reported-by: Per Cederqvist <cederp@opera.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
>  Ok, let's try again. This is what I understand is the best bugfix for
>  2.8 and below. (This patch was developed on top of 2.7.4)

Thanks, let's aim to merge this for 2.8 final.