{"thread":{"id":"17600","subject":"[PATCH] submodule: handle trailing slash, warn about non-submodules","startedAt":"2009-02-06T04:00:22Z","lastAt":"2009-02-07T21:45:51Z","messageCount":8,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103419","messageId":"b14e34d2529721c24437ec9da3f8f15c916d4051.1233892769u.git.johannes.schindelin@gmx.de","threadId":"17600","inReplyTo":"cover.1233892769u.git.johannes.schindelin@gmx.de","subject":"[PATCH] submodule: handle trailing slash, warn about non-submodules","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-06T04:00:22Z","receivedAt":"2009-02-06T04:00:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Earlier, when you called\n\n\tgit submodule path/to/submodule/\n\n(which happens easily if you are a heavy user of tab-completion), Git\nwould silently ignore the given path, as \"git ls-files path/to/submodule/\"\ndoes not return anything due to the trailing slash.\n\nGit would also silently ignore paths that do not point to submodules at\nall, without warning the user about the likely mistake.\n\nNow, \"git submodule\" strips trailing slashes, and it warns about parameters\nthat are no submodules.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n git-submodule.sh |   14 +++++++++++++-\n 1 files changed, 13 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2f47e06..b878909 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -59,7 +59,19 @@ resolve_relative_url ()\n #\n module_list()\n {\n-\tgit ls-files --stage -- \"$@\" | grep '^160000 '\n+\twhile test $# -gt 0\n+\tdo\n+\t\tline=$(git ls-files --stage ${1%/} | grep '^160000 ')\n+\t\tcase \"$line\" in\n+\t\t'')\n+\t\t\techo \"Warning: ignoring non-submodule '$1'\" >&2\n+\t\t\t;;\n+\t\t*)\n+\t\t\techo \"$line\"\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n }\n \n #\n-- \n1.6.1.2.630.g01a7e\n"},{"id":"103428","messageId":"7vzlh0875l.fsf@gitster.siamese.dyndns.org","threadId":"17600","inReplyTo":"b14e34d2529721c24437ec9da3f8f15c916d4051.1233892769u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] submodule: handle trailing slash, warn about non-submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-06T05:29:42Z","receivedAt":"2009-02-06T05:29:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> Earlier, when you called\n>\n> \tgit submodule path/to/submodule/\n>\n> (which happens easily if you are a heavy user of tab-completion), Git\n> would silently ignore the given path, as \"git ls-files path/to/submodule/\"\n> does not return anything due to the trailing slash.\n\nDoes --error-unmatch help in such a case to avoid shell loops?\n"},{"id":"103461","messageId":"alpine.DEB.1.00.0902061238060.7377@intel-tinevez-2-302","threadId":"17600","inReplyTo":"7vzlh0875l.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] submodule: handle trailing slash, warn about non-submodules","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-06T11:41:40Z","receivedAt":"2009-02-06T11:41:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 5 Feb 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n> \n> > Earlier, when you called\n> >\n> > \tgit submodule path/to/submodule/\n> >\n> > (which happens easily if you are a heavy user of tab-completion), Git\n> > would silently ignore the given path, as \"git ls-files path/to/submodule/\"\n> > does not return anything due to the trailing slash.\n> \n> Does --error-unmatch help in such a case to avoid shell loops?\n\nNo, as the second issue solved by the shell loop still holds true:\n\n\t$ git ls-files --error-unmatch submodule/\n\terror: pathspec 'submodule/' did not match any file(s) known to git.\n\tDid you forget to 'git add'?\n\nThat would be really awkward, don't you think?\n\nBTW I have been asked about this behavior _easily_ a 50 times.  I just \nfinally broke down and went for the easiest solution.\n\nA proper solution would most probably involve a rewrite as a builtin.\n\nCiao,\nDscho\n"},{"id":"103600","messageId":"7vfxiqy76k.fsf@gitster.siamese.dyndns.org","threadId":"17600","inReplyTo":"b14e34d2529721c24437ec9da3f8f15c916d4051.1233892769u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] submodule: handle trailing slash, warn about non-submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-07T08:36:51Z","receivedAt":"2009-02-07T08:36:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> Earlier, when you called\n>\n> \tgit submodule path/to/submodule/\n>\n> (which happens easily if you are a heavy user of tab-completion), Git\n> would silently ignore the given path, as \"git ls-files path/to/submodule/\"\n> does not return anything due to the trailing slash.\n\nI see a sign of concentrating too much on the topic you were interested in\nto blind yourself here in this patch.\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 2f47e06..b878909 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -59,7 +59,19 @@ resolve_relative_url ()\n>  #\n>  module_list()\n>  {\n> -\tgit ls-files --stage -- \"$@\" | grep '^160000 '\n> +\twhile test $# -gt 0\n> +\tdo\n> +\t\tline=$(git ls-files --stage \"${1%/}\" | grep '^160000 ')\n> +\t\tcase \"$line\" in\n> +\t\t'')\n> +\t\t\techo \"Warning: ignoring non-submodule '$1'\" >&2\n> +\t\t\t;;\n> +\t\t*)\n> +\t\t\techo \"$line\"\n> +\t\t\t;;\n> +\t\tesac\n> +\t\tshift\n> +\tdone\n\nAlmost everybody seems to call module_list with the arguments it received\nfrom the caller, but that \"$@\" could be none.  Worse, cmd_foreach always\ncalls module_list with no argument.  Earlier you got all submodules, now\nyou get none.\n\nSo you would need:\n\n        case $# in\n        0)\n                git ls-files --stage -- | grep '^160000 '\n                ;;\n        *)\n                your while loop\n                ;;\n        esac\n\nor something, I think.\n"},{"id":"103619","messageId":"cover.1234013924u.git.johannes.schindelin@gmx.de","threadId":"17600","inReplyTo":"7vfxiqy76k.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 0/2] submodule path handling fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-07T13:42:43Z","receivedAt":"2009-02-07T13:42:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"My earlier attempt at this was misguided by my laziness, and I was willing \nto sacrifice correctness for the ability to move on to other problems.\n\nSo now, I taught plumbing to handle the trailing slash problem, which is \nnot only much cleaner, but helps other users of ls-files, too.\n\nNote that at this moment, it is beyond my reach to fix the issue that \"git \nsubmodule no-such-submodule\" will _only_ warn and not _fail_.\n\nThat would take a builtinification, as ls-files is used in a pipe in \ngit-submodule.sh, and AFAIR there is no really portable way to catch \nerrors in a pipe, short of using temporary files.\n\nJohannes Schindelin (2):\n  Let ls-files strip trailing slashes in submodules' paths\n  submodule: warn about non-submodules\n\n builtin-ls-files.c         |   21 ++++++++++++++++++++-\n git-submodule.sh           |    2 +-\n t/t7400-submodule-basic.sh |   13 +++++++++++++\n 3 files changed, 34 insertions(+), 2 deletions(-)\n"},{"id":"103620","messageId":"2bff02915466796c36d93ac17435f4256c46403f.1234013924u.git.johannes.schindelin@gmx.de","threadId":"17600","inReplyTo":"7vfxiqy76k.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 1/2] Let ls-files strip trailing slashes in submodules' paths","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-07T13:43:03Z","receivedAt":"2009-02-07T13:43:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Tab completion makes it easy to add a trailing slash to a submodule path.\nAs it is completely clear what the user actually wanted to say, be nice\nand strip that slash at the end.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin-ls-files.c         |   21 ++++++++++++++++++++-\n t/t7400-submodule-basic.sh |    6 ++++++\n 2 files changed, 26 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-ls-files.c b/builtin-ls-files.c\nindex 3434031..9dec282 100644\n--- a/builtin-ls-files.c\n+++ b/builtin-ls-files.c\n@@ -262,6 +262,21 @@ static const char *verify_pathspec(const char *prefix)\n \treturn max ? xmemdupz(prev, max) : NULL;\n }\n \n+static void strip_trailing_slash_from_submodules(void)\n+{\n+\tconst char **p;\n+\n+\tfor (p = pathspec; *p != NULL; p++) {\n+\t\tint len = strlen(*p), pos;\n+\n+\t\tif (len < 1 || (*p)[len - 1] != '/')\n+\t\t\tcontinue;\n+\t\tpos = cache_name_pos(*p, len - 1);\n+\t\tif (pos >= 0 && S_ISGITLINK(active_cache[pos]->ce_mode))\n+\t\t\t*p = xstrndup(*p, len - 1);\n+\t}\n+}\n+\n /*\n  * Read the tree specified with --with-tree option\n  * (typically, HEAD) into stage #1 and then\n@@ -510,6 +525,11 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)\n \n \tpathspec = get_pathspec(prefix, argv + i);\n \n+\t/* be nice with submodule patsh ending in a slash */\n+\tread_cache();\n+\tif (pathspec)\n+\t\tstrip_trailing_slash_from_submodules();\n+\n \t/* Verify that the pathspec matches the prefix */\n \tif (pathspec)\n \t\tprefix = verify_pathspec(prefix);\n@@ -533,7 +553,6 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)\n \t      show_killed | show_modified))\n \t\tshow_cached = 1;\n \n-\tread_cache();\n \tif (prefix)\n \t\tprune_cache(prefix);\n \tif (with_tree) {\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 2ec7ac6..a74f24c 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -234,4 +234,10 @@ test_expect_success 'gracefully add submodule with a trailing slash' '\n \n '\n \n+test_expect_success 'ls-files gracefully handles trailing slash' '\n+\n+\ttest \"init\" = \"$(git ls-files init/)\"\n+\n+'\n+\n test_done\n-- \n1.6.1.2.630.g01a7e\n"},{"id":"103621","messageId":"4392cb2a9f5f3283d6505e17ddcb36a6b427fbcf.1234013924u.git.johannes.schindelin@gmx.de","threadId":"17600","inReplyTo":"7vfxiqy76k.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 2/2] submodule: warn about non-submodules","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-07T13:43:15Z","receivedAt":"2009-02-07T13:43:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Earlier, when you called\n\n        git submodule some/bogus/path\n\nGit would silently ignore the path, without warning the user about the\nlikely mistake.  Now it does.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n git-submodule.sh           |    2 +-\n t/t7400-submodule-basic.sh |    7 +++++++\n 2 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2f47e06..6cc2d33 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -59,7 +59,7 @@ resolve_relative_url ()\n #\n module_list()\n {\n-\tgit ls-files --stage -- \"$@\" | grep '^160000 '\n+\tgit ls-files --error-unmatch --stage -- \"$@\" | grep '^160000 '\n }\n \n #\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex a74f24c..b8cb2df 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -240,4 +240,11 @@ test_expect_success 'ls-files gracefully handles trailing slash' '\n \n '\n \n+test_expect_success 'submodule <invalid-path> warns' '\n+\n+\tgit submodule no-such-submodule 2> output.err &&\n+\tgrep \"^error: .*no-such-submodule\" output.err\n+\n+'\n+\n test_done\n-- \n1.6.1.2.630.g01a7e\n"},{"id":"103656","messageId":"7vd4dtudio.fsf@gitster.siamese.dyndns.org","threadId":"17600","inReplyTo":"cover.1234013924u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 0/2] submodule path handling fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-07T21:45:51Z","receivedAt":"2009-02-07T21:45:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Both patches look trivially correct.\n"}]}