{"thread":{"id":"41788","subject":"[PATCH] submodule: Fix regression for deinit without submodules","startedAt":"2016-03-22T23:42:14Z","lastAt":"2016-03-23T02:47:37Z","messageCount":2,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"281511","messageId":"1458690134-538-1-git-send-email-sbeller@google.com","threadId":"41788","inReplyTo":null,"subject":"[PATCH] submodule: Fix regression for deinit without submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-22T23:42:14Z","receivedAt":"2016-03-22T23:42:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Per Cederqvist wrote:\n> It used to be possible to run\n>\n>    git submodule deinit -f .\n>\n> to remove any submodules, no matter how many submodules you had.  That\n> is no longer possible in projects that don't have any submodules at\n> all.  The command will fail with:\n>\n>     error: pathspec '.' did not match any file(s) known to git.\n\nThis regression was introduced in 74703a1e4dfc (submodule: rewrite\n`module_list` shell function in C, 2015-09-02), as we optimized\nthe new module listing for performance by first checking whether it is\na git link instead of feeding it to match_pathspec.\n\nBy switching these two commands we get the same behavior as before,\ni.e. in an empty repository this still breaks, but if there are files\nbut no submodules this works.\n\nBy reversing the order we also revert an accidental feature. If you had\na README file in a repository, you could do\n\n    git submodule deinit \"READM*\"\n\nand it would report that there is no submodule matching the \"READM*\"\npath spec.\n\nReported-by: Per Cederqvist <cederp@opera.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n Ok, let's try again. This is what I understand is the best bugfix for\n 2.8 and below. (This patch was developed on top of 2.7.4)\n \n As discussed we want to have a better solution in the future, which\n may included having the --unmatch-ok flag, but that is not in the scope\n for now.\n \n Thanks,\n Stefan\n\n\n builtin/submodule--helper.c |  6 +++---\n t/t7400-submodule-basic.sh  | 13 +++++++++++++\n 2 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex ed764c9..5295b72 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -37,9 +37,9 @@ static int module_list_compute(int argc, const char **argv,\n \tfor (i = 0; i < active_nr; i++) {\n \t\tconst struct cache_entry *ce = active_cache[i];\n \n-\t\tif (!S_ISGITLINK(ce->ce_mode) ||\n-\t\t    !match_pathspec(pathspec, ce->name, ce_namelen(ce),\n-\t\t\t\t    0, ps_matched, 1))\n+\t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce),\n+\t\t\t\t    0, ps_matched, 1) ||\n+\t\t    !S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n \n \t\tALLOC_GROW(list->entries, list->nr + 1, list->alloc);\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex be82a75..e1abd19 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -849,6 +849,19 @@ test_expect_success 'set up a second submodule' '\n \tgit commit -m \"submodule example2 added\"\n '\n \n+test_expect_success 'submodule deinit works on repository without submodules' '\n+\ttest_when_finished \"rm -rf newdirectory\" &&\n+\tmkdir newdirectory &&\n+\t(\n+\t\tcd newdirectory &&\n+\t\tgit init &&\n+\t\t>file &&\n+\t\tgit add file &&\n+\t\tgit commit -m \"repo should not be empty\"\n+\t\tgit submodule deinit .\n+\t)\n+'\n+\n test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '\n \tgit config submodule.example.foo bar &&\n \tgit config submodule.example2.frotz nitfol &&\n-- \n2.7.4.dirty\n"},{"id":"281518","messageId":"xmqqlh59ewqe.fsf@gitster.mtv.corp.google.com","threadId":"41788","inReplyTo":"1458690134-538-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] submodule: Fix regression for deinit without submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-23T02:47:37Z","receivedAt":"2016-03-23T02:47:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> Per Cederqvist wrote:\n>> It used to be possible to run\n>>\n>>    git submodule deinit -f .\n>>\n>> to remove any submodules, no matter how many submodules you had.  That\n>> is no longer possible in projects that don't have any submodules at\n>> all.  The command will fail with:\n>>\n>>     error: pathspec '.' did not match any file(s) known to git.\n>\n> This regression was introduced in 74703a1e4dfc (submodule: rewrite\n> `module_list` shell function in C, 2015-09-02), as we optimized\n> ...\n>\n> Reported-by: Per Cederqvist <cederp@opera.com>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>\n>  Ok, let's try again. This is what I understand is the best bugfix for\n>  2.8 and below. (This patch was developed on top of 2.7.4)\n\nThanks, let's aim to merge this for 2.8 final.\n"}]}