{"thread":{"id":"33422","subject":"[PATCH 1/2] rev-parse: add --filename-prefix option","startedAt":"2013-04-07T19:55:19Z","lastAt":"2013-04-24T19:12:39Z","messageCount":39,"participants":["John Keeping","Jens Lehmann","Jonathan Nieder","Junio C Hamano","Ramkumar Ramachandra","Eric Sunshine","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"213459","messageId":"cover.1365364193.git.john@keeping.me.uk","threadId":"33422","inReplyTo":null,"subject":"[RFC/PATCH 0/2] submodule: drop the top-level requirement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-07T19:55:19Z","receivedAt":"2013-04-07T19:55:19Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"With the recent discussion, I wondered how hard it would be to add\nSUBDIRECTORY_OK=Yes to git-submodule.sh and it doesn't seem that bad.\n\nNote that this series currently lacks both tests and documentation\nupdates. Also I have made no attempt to change the output from any\ncommands, so submodule paths are always printed relative to the\ntop-level of the repository.\n\nConsider this series a proof-of-concept, to see whether I'm on\ncompletely the wrong course.\n\nJohn Keeping (2):\n rev-parse: add --filename-prefix option\n submodule: drop the top-level requirement\n\n builtin/rev-parse.c | 17 ++++++++++++-----\n git-submodule.sh | 7 +++++++\n 2 files changed, 19 insertions(+), 5 deletions(-)\n\n-- \n1.8.2.694.ga76e9c3.dirty\n"},{"id":"213458","messageId":"ba2c7aa9eaa982306f1d5ad5ff2d26a6e2b8df85.1365364193.git.john@keeping.me.uk","threadId":"33422","inReplyTo":"cover.1365364193.git.john@keeping.me.uk","subject":"[PATCH 1/2] rev-parse: add --filename-prefix option","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-07T19:55:20Z","receivedAt":"2013-04-07T19:55:20Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"This adds a prefix string to any filename arguments encountered after it\nhas been specified.\n\nSigned-off-by: John Keeping \n---\n builtin/rev-parse.c | 17 ++++++++++++-----\n 1 file changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex f267a1d..11501a4 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -212,11 +212,12 @@ static void show_datestring(const char *flag, const char *datestr)\n \tshow(buffer);\n }\n \n-static int show_file(const char *arg)\n+static int show_file(const char *arg, const char *prefix)\n {\n \tshow_default();\n \tif ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {\n-\t\tshow(arg);\n+\t\tsize_t prefixlen = prefix ? strlen(prefix) : 0;\n+\t\tshow(prefix_filename(prefix, prefixlen, arg));\n \t\treturn 1;\n \t}\n \treturn 0;\n@@ -470,6 +471,7 @@ N_(\"git rev-parse --parseopt [options] -- [...]\\n\"\n int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n {\n \tint i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;\n+\tconst char *filename_prefix = NULL;\n \tunsigned char sha1[20];\n \tconst char *name = NULL;\n \n@@ -503,7 +505,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\tconst char *arg = argv[i];\n \n \t\tif (as_is) {\n-\t\t\tif (show_file(arg) && as_is < 2)\n+\t\t\tif (show_file(arg, filename_prefix) && as_is < 2)\n \t\t\t\tverify_filename(prefix, arg, 0);\n \t\t\tcontinue;\n \t\t}\n@@ -527,7 +529,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tas_is = 2;\n \t\t\t\t/* Pass on the \"--\" if we show anything but files.. */\n \t\t\t\tif (filter & (DO_FLAGS | DO_REVS))\n-\t\t\t\t\tshow_file(arg);\n+\t\t\t\t\tshow_file(arg, NULL);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--default\")) {\n@@ -535,6 +537,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\ti++;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--filename-prefix\")) {\n+\t\t\t\tfilename_prefix = argv[i+1];\n+\t\t\t\ti++;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--revs-only\")) {\n \t\t\t\tfilter &= ~DO_NOREV;\n \t\t\t\tcontinue;\n@@ -754,7 +761,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\tif (verify)\n \t\t\tdie_no_single_rev(quiet);\n \t\tas_is = 1;\n-\t\tif (!show_file(arg))\n+\t\tif (!show_file(arg, filename_prefix))\n \t\t\tcontinue;\n \t\tverify_filename(prefix, arg, 1);\n \t}\n-- \n1.8.2.694.ga76e9c3.dirty\n"},{"id":"213460","messageId":"da121bcc33e63b7f33318ef920e85a77bb304d44.1365364193.git.john@keeping.me.uk","threadId":"33422","inReplyTo":"cover.1365364193.git.john@keeping.me.uk","subject":"[PATCH 2/2] submodule: drop the top-level requirement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-07T19:55:21Z","receivedAt":"2013-04-07T19:55:21Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Use the new rev-parse --filename-prefix option to process all paths\ngiven to the submodule command, dropping the requirement that it be run\nfrom the top-level of the repository.\n\nSigned-off-by: John Keeping \n---\n git-submodule.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 79bfaac..10216aa 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -14,10 +14,13 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] [--name ] [--reference \n or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n OPTIONS_SPEC=\n+SUBDIRECTORY_OK=Yes\n . git-sh-setup\n . git-sh-i18n\n . git-parse-remote\n require_work_tree\n+wt_prefix=$(git rev-parse --show-prefix)\n+cd_to_toplevel\n \n command=\n branch=\n@@ -112,6 +115,7 @@ resolve_relative_url ()\n #\n module_list()\n {\n+\teval \"set $(git rev-parse --sq --filename-prefix \"$wt_prefix\" -- \"$@\")\"\n \t(\n \t\tgit ls-files --error-unmatch --stage -- \"$@\" ||\n \t\techo \"unmatched pathspec exists\"\n@@ -335,6 +339,8 @@ cmd_add()\n \t\tusage\n \tfi\n \n+\tsm_path=\"$wt_prefix$sm_path\"\n+\n \t# assure repo is absolute or relative to parent\n \tcase \"$repo\" in\n \t./*|../*)\n@@ -942,6 +948,7 @@ cmd_summary() {\n \tfi\n \n \tcd_to_toplevel\n+\teval \"set $(git rev-parse --sq --filename-prefix \"$wt_prefix\" -- \"$@\")\"\n \t# Get modified modules cared by user\n \tmodules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- \"$@\" |\n \t\tsane_egrep '^:([0-7]* )?160000' |\n-- \n1.8.2.694.ga76e9c3.dirty\n"},{"id":"213462","messageId":"5161D3CB.8090101@web.de","threadId":"33422","inReplyTo":"cover.1365364193.git.john@keeping.me.uk","subject":"Re: [RFC/PATCH 0/2] submodule: drop the top-level requirement","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-04-07T20:15:07Z","receivedAt":"2013-04-07T20:15:07Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 07.04.2013 21:55, schrieb John Keeping:\n> With the recent discussion, I wondered how hard it would be to add\n> SUBDIRECTORY_OK=Yes to git-submodule.sh and it doesn't seem that bad.\n> \n> Note that this series currently lacks both tests and documentation\n> updates. Also I have made no attempt to change the output from any\n> commands, so submodule paths are always printed relative to the\n> top-level of the repository.\n> \n> Consider this series a proof-of-concept, to see whether I'm on\n> completely the wrong course.\n\nCool! Thanks for working on that, I'll take a deeper look in my next\nGit timeslot.\n\n> John Keeping (2):\n> rev-parse: add --filename-prefix option\n> submodule: drop the top-level requirement\n> \n> builtin/rev-parse.c | 17 ++++++++++++-----\n> git-submodule.sh | 7 +++++++\n> 2 files changed, 19 insertions(+), 5 deletions(-)\n> \n"},{"id":"213469","messageId":"20130407221458.GE19857@elie.Belkin","threadId":"33422","inReplyTo":"ba2c7aa9eaa982306f1d5ad5ff2d26a6e2b8df85.1365364193.git.john@keeping.me.uk","subject":"Re: [PATCH 1/2] rev-parse: add --filename-prefix option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-07T22:14:58Z","receivedAt":"2013-04-07T22:14:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJohn Keeping wrote:\n\n> This adds a prefix string to any filename arguments encountered after it\n> has been specified.\n\nI assume this is a way of passing the prefix in? In that case, I\nthink a good UI would be\n\n\tgit rev-parse --prefix=Documentation/ \n\nThat sounds like a useful thing and would make the meaning very clear.\n\nHow does this interact with the following options?\n\n * --resolve-git-dir some/relative/path\n * master:./path\n\nAs for the patch itself, I haven't looked at it closely. My only\nimmediate reaction is that I wish it touched Documentation/ and t/. :)\n\nHope that helps,\nJonathan\n"},{"id":"213500","messageId":"20130408083123.GN2222@serenity.lan","threadId":"33422","inReplyTo":"20130407221458.GE19857@elie.Belkin","subject":"Re: [PATCH 1/2] rev-parse: add --filename-prefix option","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-08T08:31:23Z","receivedAt":"2013-04-08T08:31:23Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Apr 07, 2013 at 03:14:58PM -0700, Jonathan Nieder wrote:\n> John Keeping wrote:\n> \n> > This adds a prefix string to any filename arguments encountered after it\n> > has been specified.\n> \n> I assume this is a way of passing the prefix in? In that case, I\n> think a good UI would be\n> \n> \tgit rev-parse --prefix=Documentation/ \n> \n> That sounds like a useful thing and would make the meaning very clear.\n\nYes (ish), the intended usage is something like this:\n\n prefix=$(git rev-parse --show-prefix)\n cd_to_toplevel\n ... parse options here ...\n # Convert remaining arguments (filenames) into top-level paths:\n eval \"set $(git rev-parse --prefix \"$prefix\" --sq -- \"$@\")\"\n\nThe \"ish\" is that my current implementation introduced a new variable\ninstead of simply resetting the existing \"prefix\" variable, which I\nassume is what you mean. That is probably simpler than my\nimplementation, but loses the ability to be at an intermediate level,\nfor example:\n\n cd Documentation/\n eval \"set $(git rev-parse --prefix technical/ --sq -- api-strbuf.txt)\"\n\n> How does this interact with the following options?\n> \n> * --resolve-git-dir some/relative/path\n\nIt doesn't change this since --resolve-git-dir is handled separately\nfrom the other argument parsing at the moment and you cannot specify\nany other options with it.\n\n> * master:./path\n\nI hadn't considered this case, but I think it should be inserting the\nprefix into the path. I suspect the easiest thing to do is simply make\nthe path part of that absolute, by combining both the prefix based on\n$PWD and the prefix specified on the command line, but I haven't looked\nat doing this yet.\n\nThe other think that's missing at the moment is that the prefix passed\nto verify_filename should be modified by the one specified on the\ncommand line.\n\n> As for the patch itself, I haven't looked at it closely. My only\n> immediate reaction is that I wish it touched Documentation/ and t/. :)\n\nI'll make sure the next version does. This version was doing the\nminimum required to make patch 2/2 possible, it certainly needs some\npolish before it's more than a proof-of-concept.\n"},{"id":"213522","messageId":"7vmwt9ysaz.fsf@alter.siamese.dyndns.org","threadId":"33422","inReplyTo":"20130408083123.GN2222@serenity.lan","subject":"Re: [PATCH 1/2] rev-parse: add --filename-prefix option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T15:07:32Z","receivedAt":"2013-04-08T15:07:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> Yes (ish), the intended usage is something like this:\n>\n> prefix=$(git rev-parse --show-prefix)\n> cd_to_toplevel\n> ... parse options here ...\n> # Convert remaining arguments (filenames) into top-level paths:\n> eval \"set $(git rev-parse --prefix \"$prefix\" --sq -- \"$@\")\"\n>\n> The \"ish\" is that my current implementation introduced a new variable\n> instead of simply resetting the existing \"prefix\" variable, which I\n> assume is what you mean.\n\nThis is very sensible.\n\n> That is probably simpler than my\n> implementation, but loses the ability to be at an intermediate level,\n> for example:\n>\n> cd Documentation/\n> eval \"set $(git rev-parse --prefix technical/ --sq -- api-strbuf.txt)\"\n\nI am not sure in what situation it makes sense to do this. Where\ndoes \"technical/\" come from? For a script that wants to work from a\nsubdirectory, end-user input would come in the form relative to the\ncurrent directory, i.e. \"Documentation/\" from the top-level, so...\n"},{"id":"213563","messageId":"20130408173604.GP2222@serenity.lan","threadId":"33422","inReplyTo":"7vmwt9ysaz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] rev-parse: add --filename-prefix option","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-08T17:36:04Z","receivedAt":"2013-04-08T17:36:04Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Apr 08, 2013 at 08:07:32AM -0700, Junio C Hamano wrote:\n> John Keeping writes:\n> \n> > Yes (ish), the intended usage is something like this:\n> >\n> > prefix=$(git rev-parse --show-prefix)\n> > cd_to_toplevel\n> > ... parse options here ...\n> > # Convert remaining arguments (filenames) into top-level paths:\n> > eval \"set $(git rev-parse --prefix \"$prefix\" --sq -- \"$@\")\"\n> >\n> > The \"ish\" is that my current implementation introduced a new variable\n> > instead of simply resetting the existing \"prefix\" variable, which I\n> > assume is what you mean.\n> \n> This is very sensible.\n\nWhich bit specifically? I assume you agree with the intended usage, but\ndo you also mean that resetting the prefix returned from\nsetup_git_directory is the right way to approach this?\n"},{"id":"213569","messageId":"7v61zwyjs8.fsf@alter.siamese.dyndns.org","threadId":"33422","inReplyTo":"20130408173604.GP2222@serenity.lan","subject":"Re: [PATCH 1/2] rev-parse: add --filename-prefix option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T18:11:35Z","receivedAt":"2013-04-08T18:11:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> On Mon, Apr 08, 2013 at 08:07:32AM -0700, Junio C Hamano wrote:\n>> John Keeping writes:\n>> \n>> > Yes (ish), the intended usage is something like this:\n>> >\n>> > prefix=$(git rev-parse --show-prefix)\n>> > cd_to_toplevel\n>> > ... parse options here ...\n>> > # Convert remaining arguments (filenames) into top-level paths:\n>> > eval \"set $(git rev-parse --prefix \"$prefix\" --sq -- \"$@\")\"\n>> >\n>> > The \"ish\" is that my current implementation introduced a new variable\n>> > instead of simply resetting the existing \"prefix\" variable, which I\n>> > assume is what you mean.\n>> \n>> This is very sensible.\n>\n> Which bit specifically? I assume you agree with the intended usage, but\n> do you also mean that resetting the prefix returned from\n> setup_git_directory is the right way to approach this?\n\nMy gut feeling says yes, but you can persuade me easily why it is a\nbad idea if you have an example of why it would not work well.\n"},{"id":"213713","messageId":"cover.1365539059.git.john@keeping.me.uk","threadId":"33422","inReplyTo":"cover.1365364193.git.john@keeping.me.uk","subject":"[PATCH v2 0/2] submodule: drop the top-level requirement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-09T20:29:20Z","receivedAt":"2013-04-09T20:29:20Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Since version 1, patch 1 has been completely re-written using the\napproach proposed by Jonathan and Junio.\n\nAlso, there's now a documentation update and some tests.\n\nJohn Keeping (2):\n rev-parse: add --filename-prefix option\n submodule: drop the top-level requirement\n\n Documentation/git-rev-parse.txt | 16 ++++++++\n builtin/rev-parse.c | 24 ++++++++---\n git-submodule.sh | 7 ++++\n t/t1513-rev-parse-prefix.sh | 90 +++++++++++++++++++++++++++++++++++++++++\n t/t7400-submodule-basic.sh | 26 ++++++++++++\n t/t7401-submodule-summary.sh | 9 +++++\n 6 files changed, 167 insertions(+), 5 deletions(-)\n create mode 100755 t/t1513-rev-parse-prefix.sh\n\n-- \n1.8.2.694.ga76e9c3.dirty\n"},{"id":"213714","messageId":"0d570e110dbf714310f9cbc4fa47e711630707f2.1365539059.git.john@keeping.me.uk","threadId":"33422","inReplyTo":"cover.1365539059.git.john@keeping.me.uk","subject":"[PATCH v2 1/2] rev-parse: add --filename-prefix option","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-09T20:29:21Z","receivedAt":"2013-04-09T20:29:21Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"This adds a prefix string to any filename arguments encountered after it\nhas been specified.\n\nSigned-off-by: John Keeping \n---\n Documentation/git-rev-parse.txt | 16 ++++++++\n builtin/rev-parse.c | 24 ++++++++---\n t/t1513-rev-parse-prefix.sh | 90 +++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 125 insertions(+), 5 deletions(-)\n create mode 100755 t/t1513-rev-parse-prefix.sh\n\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex 1f9ed6c..0ab2b77 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -59,6 +59,22 @@ OPTIONS\n \tIf there is no parameter given by the user, use ``\n \tinstead.\n \n+--prefix ::\n+\tBehave as if 'git rev-parse' was invoked from the ``\n+\tsubdirectory of the working tree. Any relative filenames are\n+\tresolved as if they are prefixed by `` and will be printed\n+\tin that form.\n++\n+This can be used to convert arguments to a command run in a subdirectory\n+so that they can still be used after moving to the top-level of the\n+repository. For example:\n++\n+----\n+prefix=$(git rev-parse --show-prefix)\n+cd \"$(git rev-parse --show-toplevel)\"\n+eval \"set -- $(git rev-parse --sq --prefix \"$prefix\" \"$@\")\"\n+----\n+\n --verify::\n \tVerify that exactly one parameter is provided, and that it\n \tcan be turned into a raw 20-byte SHA-1 that can be used to\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex f267a1d..de894c7 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char *datestr)\n \tshow(buffer);\n }\n \n-static int show_file(const char *arg)\n+static int show_file(const char *arg, int output_prefix)\n {\n \tshow_default();\n \tif ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {\n-\t\tshow(arg);\n+\t\tif (output_prefix) {\n+\t\t\tconst char *prefix = startup_info->prefix;\n+\t\t\tshow(prefix_filename(prefix,\n+\t\t\t\t\t prefix ? strlen(prefix) : 0,\n+\t\t\t\t\t arg));\n+\t\t} else\n+\t\t\tshow(arg);\n \t\treturn 1;\n \t}\n \treturn 0;\n@@ -470,6 +476,7 @@ N_(\"git rev-parse --parseopt [options] -- [...]\\n\"\n int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n {\n \tint i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;\n+\tint output_prefix = 0;\n \tunsigned char sha1[20];\n \tconst char *name = NULL;\n \n@@ -503,7 +510,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\tconst char *arg = argv[i];\n \n \t\tif (as_is) {\n-\t\t\tif (show_file(arg) && as_is < 2)\n+\t\t\tif (show_file(arg, output_prefix) && as_is < 2)\n \t\t\t\tverify_filename(prefix, arg, 0);\n \t\t\tcontinue;\n \t\t}\n@@ -527,7 +534,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tas_is = 2;\n \t\t\t\t/* Pass on the \"--\" if we show anything but files.. */\n \t\t\t\tif (filter & (DO_FLAGS | DO_REVS))\n-\t\t\t\t\tshow_file(arg);\n+\t\t\t\t\tshow_file(arg, 0);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--default\")) {\n@@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\ti++;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--prefix\")) {\n+\t\t\t\tprefix = argv[i+1];\n+\t\t\t\tstartup_info->prefix = prefix;\n+\t\t\t\toutput_prefix = 1;\n+\t\t\t\ti++;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--revs-only\")) {\n \t\t\t\tfilter &= ~DO_NOREV;\n \t\t\t\tcontinue;\n@@ -754,7 +768,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\tif (verify)\n \t\t\tdie_no_single_rev(quiet);\n \t\tas_is = 1;\n-\t\tif (!show_file(arg))\n+\t\tif (!show_file(arg, output_prefix))\n \t\t\tcontinue;\n \t\tverify_filename(prefix, arg, 1);\n \t}\ndiff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh\nnew file mode 100755\nindex 0000000..5ef48d2\n--- /dev/null\n+++ b/t/t1513-rev-parse-prefix.sh\n@@ -0,0 +1,90 @@\n+#!/bin/sh\n+\n+test_description='Tests for rev-parse --prefix'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\tmkdir -p sub1/sub2 &&\n+\techo top >top &&\n+\techo file1 >sub1/file1 &&\n+\techo file2 >sub1/sub2/file2 &&\n+\tgit add top sub1/file1 sub1/sub2/file2 &&\n+\tgit commit -m commit\n+'\n+\n+test_expect_success 'empty prefix -- file' '\n+\tgit rev-parse --prefix \"\" -- top sub1/file1 >actual &&\n+\tcat <<-EOF >expected &&\n+\t--\n+\ttop\n+\tsub1/file1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'valid prefix -- file' '\n+\tgit rev-parse --prefix sub1/ -- file1 sub2/file2 >actual &&\n+\tcat <<-EOF >expected &&\n+\t--\n+\tsub1/file1\n+\tsub1/sub2/file2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'valid prefix -- ../file' '\n+\tgit rev-parse --prefix sub1/ -- ../top sub2/file2 >actual &&\n+\tcat <<-EOF >expected &&\n+\t--\n+\tsub1/../top\n+\tsub1/sub2/file2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'empty prefix HEAD:./path' '\n+\tgit rev-parse --prefix \"\" HEAD:./top >actual &&\n+\tgit rev-parse HEAD:top >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'valid prefix HEAD:./path' '\n+\tgit rev-parse --prefix sub1/ HEAD:./file1 >actual &&\n+\tgit rev-parse HEAD:sub1/file1 >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'valid prefix HEAD:../path' '\n+\tgit rev-parse --prefix sub1/ HEAD:../top >actual &&\n+\tgit rev-parse HEAD:top >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'disambiguate path with valid prefix' '\n+\tgit rev-parse --prefix sub1/ file1 >actual &&\n+\tcat <<-EOF >expected &&\n+\tsub1/file1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'file and refs with prefix' '\n+\tgit rev-parse --prefix sub1/ master file1 >actual &&\n+\tcat <<-EOF >expected &&\n+\t$(git rev-parse master)\n+\tsub1/file1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'two-levels deep' '\n+\tgit rev-parse --prefix sub1/sub2/ -- file2 >actual &&\n+\tcat <<-EOF >expected &&\n+\t--\n+\tsub1/sub2/file2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n1.8.2.694.ga76e9c3.dirty\n"},{"id":"213715","messageId":"4a6394e9e7124c2ad5a3ce232dc746e85cb1c600.1365539059.git.john@keeping.me.uk","threadId":"33422","inReplyTo":"cover.1365539059.git.john@keeping.me.uk","subject":"[PATCH v2 2/2] submodule: drop the top-level requirement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-09T20:29:22Z","receivedAt":"2013-04-09T20:29:22Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Use the new rev-parse --prefix option to process all paths given to the\nsubmodule command, dropping the requirement that it be run from the\ntop-level of the repository.\n\nSigned-off-by: John Keeping \n---\n git-submodule.sh | 7 +++++++\n t/t7400-submodule-basic.sh | 26 ++++++++++++++++++++++++++\n t/t7401-submodule-summary.sh | 9 +++++++++\n 3 files changed, 42 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 79bfaac..bbf7983 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -14,10 +14,13 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] [--name ] [--reference \n or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n OPTIONS_SPEC=\n+SUBDIRECTORY_OK=Yes\n . git-sh-setup\n . git-sh-i18n\n . git-parse-remote\n require_work_tree\n+wt_prefix=$(git rev-parse --show-prefix)\n+cd_to_toplevel\n \n command=\n branch=\n@@ -112,6 +115,7 @@ resolve_relative_url ()\n #\n module_list()\n {\n+\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n \t(\n \t\tgit ls-files --error-unmatch --stage -- \"$@\" ||\n \t\techo \"unmatched pathspec exists\"\n@@ -335,6 +339,8 @@ cmd_add()\n \t\tusage\n \tfi\n \n+\tsm_path=\"$wt_prefix$sm_path\"\n+\n \t# assure repo is absolute or relative to parent\n \tcase \"$repo\" in\n \t./*|../*)\n@@ -942,6 +948,7 @@ cmd_summary() {\n \tfi\n \n \tcd_to_toplevel\n+\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n \t# Get modified modules cared by user\n \tmodules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- \"$@\" |\n \t\tsane_egrep '^:([0-7]* )?160000' |\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex ff26535..7795f21 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -212,6 +212,23 @@ test_expect_success 'submodule add with ./, /.. and // in path' '\n \ttest_cmp empty untracked\n '\n \n+test_expect_success 'submodule add in subdir' '\n+\techo \"refs/heads/master\" >expect &&\n+\t>empty &&\n+\t(\n+\t\tmkdir addtest/sub &&\n+\t\tcd addtest/sub &&\n+\t\tgit submodule add \"$submodurl\" ../realsubmod3 &&\n+\t\tgit submodule init\n+\t) &&\n+\n+\trm -f heads head untracked &&\n+\tinspect addtest/realsubmod3 ../.. &&\n+\ttest_cmp expect heads &&\n+\ttest_cmp expect head &&\n+\ttest_cmp empty untracked\n+'\n+\n test_expect_success 'setup - add an example entry to .gitmodules' '\n \tGIT_CONFIG=.gitmodules \\\n \tgit config submodule.example.url git://example.com/init.git\n@@ -319,6 +336,15 @@ test_expect_success 'status should be \"up-to-date\" after update' '\n \tgrep \"^ $rev1\" list\n '\n \n+test_expect_success 'status works correctly from a subdirectory' '\n+\tmkdir sub &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit submodule status >../list\n+\t) &&\n+\tgrep \"^ $rev1\" list\n+'\n+\n test_expect_success 'status should be \"modified\" after submodule commit' '\n \t(\n \t\tcd init &&\ndiff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\nindex 30b429e..8f5c1e0 100755\n--- a/t/t7401-submodule-summary.sh\n+++ b/t/t7401-submodule-summary.sh\n@@ -45,6 +45,15 @@ EOF\n \ttest_cmp expected actual\n \"\n \n+test_expect_success 'run summary from subdir' '\n+\tmkdir sub &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit submodule summary >../actual\n+\t) &&\n+\ttest_cmp expected actual\n+'\n+\n commit_file sm1 &&\n head2=$(add_file sm1 foo3)\n \n-- \n1.8.2.694.ga76e9c3.dirty\n"},{"id":"213718","messageId":"7vtxnfpglq.fsf@alter.siamese.dyndns.org","threadId":"33422","inReplyTo":"0d570e110dbf714310f9cbc4fa47e711630707f2.1365539059.git.john@keeping.me.uk","subject":"Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-09T20:57:21Z","receivedAt":"2013-04-09T20:57:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> This adds a prefix string to any filename arguments encountered after it\n> has been specified.\n>\n> Signed-off-by: John Keeping \n> ---\n\nStale subject?\n\n> +--prefix ::\n> +\tBehave as if 'git rev-parse' was invoked from the ``\n> +\tsubdirectory of the working tree. Any relative filenames are\n> +\tresolved as if they are prefixed by `` and will be printed\n> +\tin that form.\n> ++\n> +This can be used to convert arguments to a command run in a subdirectory\n> +so that they can still be used after moving to the top-level of the\n> +repository. For example:\n> ++\n> +----\n> +prefix=$(git rev-parse --show-prefix)\n> +cd \"$(git rev-parse --show-toplevel)\"\n> +eval \"set -- $(git rev-parse --sq --prefix \"$prefix\" \"$@\")\"\n\nI think you should tighten rev-parse parameter to reject options and\nrevisions, especially as an example to teach how to use it. When\nthe user said\n\n\tgit mylog -U20 master..next -- README\n\ninside Documentation/ directory, \"git-mylog\" script would want to\nsee README prefixed with Documentation/ but want to see -U20 or\nmaster..next untouched. Historically, rev-parse was a way to sift\noptions and args meant for rev-list from those mant for diff-tree\nso that a variant of\n\n\tgit rev-list $(git rev-parse --revs) \"$@\" |\n git diff-tree --stdin $(git rev-parse --no-revs)\n\ncan be used to implement such \"git mylog\" script.\n\nI think \"--no-revs --no-flags\" is how you ask it to give you only\nthe paths, but I am writing from memory so please double check.\n\nHaving said all that.\n\nExisting scripts (e.g. \"git am\") do this kind of things without such\nan option added to rev-parse. They first do:\n\n prefix=$(git rev-parse --show-prefix)\n\nand refer to \"$prefix/$1\", \"$prefix/$2\", etc., I think.\n\nIs this option really necessary to update \"git submodule\"? Don't we\nhave a much better idea which parameter holds user-supplied path in the\nscript than having rev-parse make a guess on the entire \"$@\"?\n"},{"id":"213719","messageId":"7vobdnpgfv.fsf@alter.siamese.dyndns.org","threadId":"33422","inReplyTo":"4a6394e9e7124c2ad5a3ce232dc746e85cb1c600.1365539059.git.john@keeping.me.uk","subject":"Re: [PATCH v2 2/2] submodule: drop the top-level requirement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-09T21:00:52Z","receivedAt":"2013-04-09T21:00:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> +\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n\nThis may be handier than having to do the \"for arg\" loop git-am uses\nyourself.\n\n> \t(\n> \t\tgit ls-files --error-unmatch --stage -- \"$@\" ||\n> \t\techo \"unmatched pathspec exists\"\n> @@ -335,6 +339,8 @@ cmd_add()\n> \t\tusage\n> \tfi\n> \n> +\tsm_path=\"$wt_prefix$sm_path\"\n\nBut this is doing fine without \"rev-parse --prefix\" at all.\n"},{"id":"213721","messageId":"20130409212827.GG2222@serenity.lan","threadId":"33422","inReplyTo":"7vtxnfpglq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-09T21:28:27Z","receivedAt":"2013-04-09T21:28:27Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Apr 09, 2013 at 01:57:21PM -0700, Junio C Hamano wrote:\n> John Keeping writes:\n> \n> > This adds a prefix string to any filename arguments encountered after it\n> > has been specified.\n> >\n> > Signed-off-by: John Keeping \n> > ---\n> \n> Stale subject?\n\nYep. Sorry.\n\n> > +--prefix ::\n> > +\tBehave as if 'git rev-parse' was invoked from the ``\n> > +\tsubdirectory of the working tree. Any relative filenames are\n> > +\tresolved as if they are prefixed by `` and will be printed\n> > +\tin that form.\n> > ++\n> > +This can be used to convert arguments to a command run in a subdirectory\n> > +so that they can still be used after moving to the top-level of the\n> > +repository. For example:\n> > ++\n> > +----\n> > +prefix=$(git rev-parse --show-prefix)\n> > +cd \"$(git rev-parse --show-toplevel)\"\n> > +eval \"set -- $(git rev-parse --sq --prefix \"$prefix\" \"$@\")\"\n> \n> I think you should tighten rev-parse parameter to reject options and\n> revisions, especially as an example to teach how to use it. When\n> the user said\n> \n> \tgit mylog -U20 master..next -- README\n> \n> inside Documentation/ directory, \"git-mylog\" script would want to\n> see README prefixed with Documentation/ but want to see -U20 or\n> master..next untouched.\n\nAnd it will if it runs:\n\n\tgit rev-parse --prefix Documentation/ -U20 master..next -- README\n\nWhich gives:\n\n\t-U20\n\tf131fb6eb2a1e09f7ce53d148e21ce6960f42422\n\t^fa7285dc3dce8bd01fd8c665b032603ed55348e5\n\t--\n\tDocumentation/README\n\n> Historically, rev-parse was a way to sift\n> options and args meant for rev-list from those mant for diff-tree\n> so that a variant of\n> \n> \tgit rev-list $(git rev-parse --revs) \"$@\" |\n> git diff-tree --stdin $(git rev-parse --no-revs)\n> \n> can be used to implement such \"git mylog\" script.\n> \n> I think \"--no-revs --no-flags\" is how you ask it to give you only\n> the paths, but I am writing from memory so please double check.\n> \n> Having said all that.\n> \n> Existing scripts (e.g. \"git am\") do this kind of things without such\n> an option added to rev-parse. They first do:\n> \n> prefix=$(git rev-parse --show-prefix)\n> \n> and refer to \"$prefix/$1\", \"$prefix/$2\", etc., I think.\n> \n> Is this option really necessary to update \"git submodule\"? Don't we\n> have a much better idea which parameter holds user-supplied path in the\n> script than having rev-parse make a guess on the entire \"$@\"?\n\nIt's not guessing on all of \"$@\" in git-submodule - we know that\neverything left is a path.\n\nI've looked at git-am and hadn't thought of doing that, just thought\nthis was a reasonably elegant way of processing the arguments. I'm\nhappy to try another approach if that's going to be more acceptable.\n"},{"id":"213722","messageId":"20130409212946.GH2222@serenity.lan","threadId":"33422","inReplyTo":"7vobdnpgfv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/2] submodule: drop the top-level requirement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-09T21:29:46Z","receivedAt":"2013-04-09T21:29:46Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Apr 09, 2013 at 02:00:52PM -0700, Junio C Hamano wrote:\n> John Keeping writes:\n> \n> > +\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n> \n> This may be handier than having to do the \"for arg\" loop git-am uses\n> yourself.\n> \n> > \t(\n> > \t\tgit ls-files --error-unmatch --stage -- \"$@\" ||\n> > \t\techo \"unmatched pathspec exists\"\n> > @@ -335,6 +339,8 @@ cmd_add()\n> > \t\tusage\n> > \tfi\n> > \n> > +\tsm_path=\"$wt_prefix$sm_path\"\n> \n> But this is doing fine without \"rev-parse --prefix\" at all.\n\nIn this case we only have a single argument (and it must have a value).\nIn the cases using \"rev-parse --prefix\" we can have any number of\narguments (including zero).\n"},{"id":"213723","messageId":"7vfvyzpey1.fsf@alter.siamese.dyndns.org","threadId":"33422","inReplyTo":"20130409212827.GG2222@serenity.lan","subject":"Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-09T21:33:10Z","receivedAt":"2013-04-09T21:33:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> It's not guessing on all of \"$@\" in git-submodule - we know that\n> everything left is a path.\n\nOK, then.\n"},{"id":"214727","messageId":"CALkWK0nir7pJF-7YLRQF0jCR4dbb-JNBheD-4zKeQR0K9F9nZg@mail.gmail.com","threadId":"33422","inReplyTo":"0d570e110dbf714310f9cbc4fa47e711630707f2.1365539059.git.john@keeping.me.uk","subject":"Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-18T14:28:25Z","receivedAt":"2013-04-18T14:28:25Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"John Keeping wrote:\n> This adds a prefix string to any filename arguments encountered after it\n> has been specified.\n\nVery nice. I thought we'd have to resort to path mangling in shell to\nfix git-submodule.sh. Glad to see that we can go with something\ncleaner.\n\nPerhaps pull some bits from your nice Documentation into the commit message?\n> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\n> index f267a1d..de894c7 100644\n> --- a/builtin/rev-parse.c\n> +++ b/builtin/rev-parse.c\n> @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char *datestr)\n> show(buffer);\n> }\n>\n> -static int show_file(const char *arg)\n> +static int show_file(const char *arg, int output_prefix)\n\nOkay, so you've essentially patched show_file() to accept an\nadditional argument, and modified callers to call with this additional\nargument. I suppose\nshow_(rev|reference|default|flag|rev|with_type|datestring|abbrev)\ndon't need to be patched, as they are path-independent.\n\n> {\n> show_default();\n> if ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {\n> - show(arg);\n> + if (output_prefix) {\n> + const char *prefix = startup_info->prefix;\n> + show(prefix_filename(prefix,\n> + prefix ? strlen(prefix) : 0,\n> + arg));\n> + } else\n> + show(arg);\n\nUh, why do you need output_prefix? If startup_info->prefix is set,\nuse it. Is startup_info->prefix set by anyone by cmd_rev_parse()?\n\n> @@ -470,6 +476,7 @@ N_(\"git rev-parse --parseopt [options] -- [...]\\n\"\n> int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> i++;\n> continue;\n> }\n> + if (!strcmp(arg, \"--prefix\")) {\n> + prefix = argv[i+1];\n> + startup_info->prefix = prefix;\n> + output_prefix = 1;\n> + i++;\n> + continue;\n> + }\n\nWait, why isn't prefix filled in when run_builtin() calls this? Oh,\nright: because we didn't mark this builtin with RUN_SETUP or\nRUN_SETUP_GENTLY. Okay, now why didn't we change that? Because it\nwould be a major problem (all our scripts would break) if rev-parse\ndid cd-to-toplevel.\n\nWhy are you setting prefix to argv[i+1], and then setting\nstartup_info->prefix to that? Is anyone else in cmd_rev_parse() going\nto use it?\n\n> +prefix=$(git rev-parse --show-prefix)\n> +cd \"$(git rev-parse --show-toplevel)\"\n> +eval \"set -- $(git rev-parse --sq --prefix \"$prefix\" \"$@\")\"\n\nI'm wondering if you need such a convoluted usage though. Will you\never need to specify a prefix by hand that is different from what git\nrev-parse --show-toplevel returns? If not, why don't you just\nrev-parse --emulate-toplevel, and get rid of specifying prefix by hand\naltogether? Then again, this is a plumbing command, so the simplicity\nis probably more valuable.\n\n> diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh\n> new file mode 100755\n> index 0000000..5ef48d2\n> --- /dev/null\n> +++ b/t/t1513-rev-parse-prefix.sh\n> +test_expect_success 'empty prefix -- file' '\n> + git rev-parse --prefix \"\" -- top sub1/file1 >actual &&\n> + cat <<-EOF >expected &&\n\nNit: when you're not putting in variables, you can cat <<-\\EOF.\n\n> +test_expect_success 'empty prefix HEAD:./path' '\n> + git rev-parse --prefix \"\" HEAD:./top >actual &&\n> + git rev-parse HEAD:top >expected &&\n\nNit: why did you change \"./top\" to \"top\"? Your --prefix option\ndoesn't require you to change your arguments accordingly, does it?\n"},{"id":"214728","messageId":"20130418144257.GZ2278@serenity.lan","threadId":"33422","inReplyTo":"CALkWK0nir7pJF-7YLRQF0jCR4dbb-JNBheD-4zKeQR0K9F9nZg@mail.gmail.com","subject":"Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-18T14:42:57Z","receivedAt":"2013-04-18T14:42:57Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Thu, Apr 18, 2013 at 07:58:25PM +0530, Ramkumar Ramachandra wrote:\n> John Keeping wrote:\n> > This adds a prefix string to any filename arguments encountered after it\n> > has been specified.\n> \n> Very nice. I thought we'd have to resort to path mangling in shell to\n> fix git-submodule.sh. Glad to see that we can go with something\n> cleaner.\n> \n> Perhaps pull some bits from your nice Documentation into the commit message?\n\nGood idea. I intended to re-write the commit message for v2 since the\npatch was completely re-written but forgot by the time I'd sorted out\npatch 2 as well. I will do for v3.\n\n> > diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\n> > index f267a1d..de894c7 100644\n> > --- a/builtin/rev-parse.c\n> > +++ b/builtin/rev-parse.c\n> > @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char *datestr)\n> > show(buffer);\n> > }\n> >\n> > -static int show_file(const char *arg)\n> > +static int show_file(const char *arg, int output_prefix)\n> \n> Okay, so you've essentially patched show_file() to accept an\n> additional argument, and modified callers to call with this additional\n> argument. I suppose\n> show_(rev|reference|default|flag|rev|with_type|datestring|abbrev)\n> don't need to be patched, as they are path-independent.\n> \n> > {\n> > show_default();\n> > if ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {\n> > - show(arg);\n> > + if (output_prefix) {\n> > + const char *prefix = startup_info->prefix;\n> > + show(prefix_filename(prefix,\n> > + prefix ? strlen(prefix) : 0,\n> > + arg));\n> > + } else\n> > + show(arg);\n> \n> Uh, why do you need output_prefix? If startup_info->prefix is set,\n> use it. Is startup_info->prefix set by anyone by cmd_rev_parse()?\n\noutput_prefix is a flag to say \"do we want to show the prefix\". We need\nit because show_file is used for the \"--\" argument separator as well as\nfile paths. Without a separate flag we end up prefixing \"--\" with the\nprefix path.\n\n> > @@ -470,6 +476,7 @@ N_(\"git rev-parse --parseopt [options] -- [...]\\n\"\n> > int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> > @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> > i++;\n> > continue;\n> > }\n> > + if (!strcmp(arg, \"--prefix\")) {\n> > + prefix = argv[i+1];\n> > + startup_info->prefix = prefix;\n> > + output_prefix = 1;\n> > + i++;\n> > + continue;\n> > + }\n> \n> Wait, why isn't prefix filled in when run_builtin() calls this? Oh,\n> right: because we didn't mark this builtin with RUN_SETUP or\n> RUN_SETUP_GENTLY. Okay, now why didn't we change that? Because it\n> would be a major problem (all our scripts would break) if rev-parse\n> did cd-to-toplevel.\n\nprefix is already set, by setup_git_git_directory. The point is that we\njust change the values set in setup_git_directory so that the command\nbehaves as if it were run from a subdirectory.\n\n> Why are you setting prefix to argv[i+1], and then setting\n> startup_info->prefix to that? Is anyone else in cmd_rev_parse() going\n> to use it?\n> \n> > +prefix=$(git rev-parse --show-prefix)\n> > +cd \"$(git rev-parse --show-toplevel)\"\n> > +eval \"set -- $(git rev-parse --sq --prefix \"$prefix\" \"$@\")\"\n> \n> I'm wondering if you need such a convoluted usage though. Will you\n> ever need to specify a prefix by hand that is different from what git\n> rev-parse --show-toplevel returns? If not, why don't you just\n> rev-parse --emulate-toplevel, and get rid of specifying prefix by hand\n> altogether? Then again, this is a plumbing command, so the simplicity\n> is probably more valuable.\n\nHow does that work? When we run rev-parse with the --prefix argument\nwe're no longer in the subdirectory.\n\nWhile this may look convoluted here, I don't think it is in normal usage\ninside a script. If you look at the way it's used in patch 2 we're\ncareful not to just remap all the arguments but to extract the flags\nbefore remapping file paths when we know that everything we have is a\nfile path.\n\n> > diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh\n> > new file mode 100755\n> > index 0000000..5ef48d2\n> > --- /dev/null\n> > +++ b/t/t1513-rev-parse-prefix.sh\n> > +test_expect_success 'empty prefix -- file' '\n> > + git rev-parse --prefix \"\" -- top sub1/file1 >actual &&\n> > + cat <<-EOF >expected &&\n> \n> Nit: when you're not putting in variables, you can cat <<-\\EOF.\n> \n> > +test_expect_success 'empty prefix HEAD:./path' '\n> > + git rev-parse --prefix \"\" HEAD:./top >actual &&\n> > + git rev-parse HEAD:top >expected &&\n> \n> Nit: why did you change \"./top\" to \"top\"? Your --prefix option\n> doesn't require you to change your arguments accordingly, does it?\n\nThe point is to show that the case where a prefix is applied\n(\"HEAD:./top\") is the same as the canonical form (\"HEAD:top\").\n\nI should probably add a test for \"HEAD:top\" with a prefix to ensure that\nwe don't modify arguments like that.\n"},{"id":"214729","messageId":"CALkWK0==bnrRJDk1daANoZOOVLwZOPM+YhwOZs7yimoYBoyXyA@mail.gmail.com","threadId":"33422","inReplyTo":"4a6394e9e7124c2ad5a3ce232dc746e85cb1c600.1365539059.git.john@keeping.me.uk","subject":"Re: [PATCH v2 2/2] submodule: drop the top-level requirement","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-18T14:46:42Z","receivedAt":"2013-04-18T14:46:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"John Keeping wrote:\n> Use the new rev-parse --prefix option to process all paths given to the\n> submodule command, dropping the requirement that it be run from the\n> top-level of the repository.\n\nYay!\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 79bfaac..bbf7983 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -112,6 +115,7 @@ resolve_relative_url ()\n> #\n> module_list()\n> {\n> + eval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n\nNit: Why not use \"--\" to disambiguate between options and arguments,\nlike you showed in your intended usage?\n\nSo, this essentially converts all the paths specified in \"$@\" to\ntoplevel-relative paths. Beautiful, as this is practically the only\nchange you require in each function.\n\n> + sm_path=\"$wt_prefix$sm_path\"\n\nWait, isn't sm_path the value of submodule..path in .gitmodules?\n Why does it need to be prefixed?\n\n> @@ -942,6 +948,7 @@ cmd_summary() {\n> fi\n>\n> cd_to_toplevel\n> + eval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n\nUm, what about other functions? Yeah, it looks like all of them\nexcept this one call module_list().\n\n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index ff26535..7795f21 100755\n> --- a/t/t7400-submodule-basic.sh\n> +++ b/t/t7400-submodule-basic.sh\n> @@ -212,6 +212,23 @@ test_expect_success 'submodule add with ./, /.. and // in path' '\n> test_cmp empty untracked\n> '\n>\n> +test_expect_success 'submodule add in subdir' '\n> + echo \"refs/heads/master\" >expect &&\n> + >empty &&\n\nNit: Use : >empty. It's clearer.\n\n> + (\n> + mkdir addtest/sub &&\n\nWhy is the mkdir inside the subshell? The next statement (cd) onwards\nshould be in the subshell, to save you cd'ing back.\n\n> + cd addtest/sub &&\n> + git submodule add \"$submodurl\" ../realsubmod3 &&\n> + git submodule init\n> + ) &&\n> +\n> + rm -f heads head untracked &&\n\nHuh? What is this in the middle? The next statement (call to\ninspect) write-truncates them anyway, so this is unnecessary.\n"},{"id":"214730","messageId":"20130418145617.GA2278@serenity.lan","threadId":"33422","inReplyTo":"CALkWK0==bnrRJDk1daANoZOOVLwZOPM+YhwOZs7yimoYBoyXyA@mail.gmail.com","subject":"Re: [PATCH v2 2/2] submodule: drop the top-level requirement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-18T14:56:18Z","receivedAt":"2013-04-18T14:56:18Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Thu, Apr 18, 2013 at 08:16:42PM +0530, Ramkumar Ramachandra wrote:\n> John Keeping wrote:\n> > Use the new rev-parse --prefix option to process all paths given to the\n> > submodule command, dropping the requirement that it be run from the\n> > top-level of the repository.\n> \n> Yay!\n> \n> > diff --git a/git-submodule.sh b/git-submodule.sh\n> > index 79bfaac..bbf7983 100755\n> > --- a/git-submodule.sh\n> > +++ b/git-submodule.sh\n> > @@ -112,6 +115,7 @@ resolve_relative_url ()\n> > #\n> > module_list()\n> > {\n> > + eval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n> \n> Nit: Why not use \"--\" to disambiguate between options and arguments,\n> like you showed in your intended usage?\n\nErm... it does. What's before \"$@\"?\n\nAh, you mean after \"set\". In this case, rev-parse will keep the \"--\" we\nsupply to it, so we get that from there and do not want to give set an\nextra one.\n\n> So, this essentially converts all the paths specified in \"$@\" to\n> toplevel-relative paths. Beautiful, as this is practically the only\n> change you require in each function.\n> \n> > + sm_path=\"$wt_prefix$sm_path\"\n> \n> Wait, isn't sm_path the value of submodule..path in .gitmodules?\n> Why does it need to be prefixed?\n\nI think you've lost too much context here. This is in cmd_add and at\nthis point sm_path holds the argument supplied by the user, which we\nmust adjust as it will be relative to the current directory.\n\nWe should probably be testing for an absolute path here though.\n\n> > @@ -942,6 +948,7 @@ cmd_summary() {\n> > fi\n> >\n> > cd_to_toplevel\n> > + eval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n> \n> Um, what about other functions? Yeah, it looks like all of them\n> except this one call module_list().\n\nExactly. Apart from this and cmd_add everything uses module_list.\n\n> > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> > index ff26535..7795f21 100755\n> > --- a/t/t7400-submodule-basic.sh\n> > +++ b/t/t7400-submodule-basic.sh\n> > @@ -212,6 +212,23 @@ test_expect_success 'submodule add with ./, /.. and // in path' '\n> > test_cmp empty untracked\n> > '\n> >\n> > +test_expect_success 'submodule add in subdir' '\n> > + echo \"refs/heads/master\" >expect &&\n> > + >empty &&\n> \n> Nit: Use : >empty. It's clearer.\n> \n> > + (\n> > + mkdir addtest/sub &&\n> \n> Why is the mkdir inside the subshell? The next statement (cd) onwards\n> should be in the subshell, to save you cd'ing back.\n>\n> > + cd addtest/sub &&\n> > + git submodule add \"$submodurl\" ../realsubmod3 &&\n> > + git submodule init\n> > + ) &&\n> > +\n> > + rm -f heads head untracked &&\n> \n> Huh? What is this in the middle? The next statement (call to\n> inspect) write-truncates them anyway, so this is unnecessary.\n\nI just followed the surrounding tests here. I think it's better for\nfollow the pattern of the surrounding code here than get this one test\nperfect. A cleanup can follow on top if someone wants to do it.\n"},{"id":"214754","messageId":"cover.1366314439.git.john@keeping.me.uk","threadId":"33422","inReplyTo":"cover.1365539059.git.john@keeping.me.uk","subject":"[PATCH v3 0/2] submodule: drop the top-level requirement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-18T19:50:40Z","receivedAt":"2013-04-18T19:50:40Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Thanks to Ram and Jens for the feedback on v2. I've addressed most of\ntheir comments in this version, but I've ignored a couple of Ram's nits\nabout test cases where leaving it how it is makes the added tests\nconsistent with those already in the same file.\n\nSince v2, the main improvement is that the output from git-submodule now\ncontains paths relative to the directory in which the command is run,\nnot the top-level of the working tree.\n\nJohn Keeping (2):\n rev-parse: add --prefix option\n submodule: drop the top-level requirement\n\n Documentation/git-rev-parse.txt | 16 ++++++\n builtin/rev-parse.c | 24 +++++++--\n git-submodule.sh | 113 ++++++++++++++++++++++++++++++----------\n t/t1513-rev-parse-prefix.sh | 96 ++++++++++++++++++++++++++++++++++\n t/t7400-submodule-basic.sh | 27 ++++++++++\n t/t7401-submodule-summary.sh | 14 +++++\n 6 files changed, 258 insertions(+), 32 deletions(-)\n create mode 100755 t/t1513-rev-parse-prefix.sh\n\n-- \n1.8.2.1.424.g1899e5e.dirty\n"},{"id":"214755","messageId":"82316c445320b3cec3a35cbc87ad94a04ed26d1e.1366314439.git.john@keeping.me.uk","threadId":"33422","inReplyTo":"cover.1366314439.git.john@keeping.me.uk","subject":"[PATCH v3 1/2] rev-parse: add --prefix option","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-18T19:50:41Z","receivedAt":"2013-04-18T19:50:41Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"This makes 'git rev-parse' behave as if it were invoked from the\nspecified subdirectory of a repository, with the difference that any\nfile paths which it prints are prefixed with the full path from the top\nof the working tree.\n\nThis is useful for shell scripts where we may want to cd to the top of\nthe working tree but need to handle relative paths given by the user on\nthe command line.\n\nSigned-off-by: John Keeping \n---\nChanges since v2:\n - Rewrite commit message\n - Add a new test for 'prefix ignored with HEAD:top'\n - Use '<<-\\EOF' where appropriate in t1513\n\n Documentation/git-rev-parse.txt | 16 +++++++\n builtin/rev-parse.c | 24 ++++++++---\n t/t1513-rev-parse-prefix.sh | 96 +++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 131 insertions(+), 5 deletions(-)\n create mode 100755 t/t1513-rev-parse-prefix.sh\n\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex 1f9ed6c..0ab2b77 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -59,6 +59,22 @@ OPTIONS\n \tIf there is no parameter given by the user, use ``\n \tinstead.\n \n+--prefix ::\n+\tBehave as if 'git rev-parse' was invoked from the ``\n+\tsubdirectory of the working tree. Any relative filenames are\n+\tresolved as if they are prefixed by `` and will be printed\n+\tin that form.\n++\n+This can be used to convert arguments to a command run in a subdirectory\n+so that they can still be used after moving to the top-level of the\n+repository. For example:\n++\n+----\n+prefix=$(git rev-parse --show-prefix)\n+cd \"$(git rev-parse --show-toplevel)\"\n+eval \"set -- $(git rev-parse --sq --prefix \"$prefix\" \"$@\")\"\n+----\n+\n --verify::\n \tVerify that exactly one parameter is provided, and that it\n \tcan be turned into a raw 20-byte SHA-1 that can be used to\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex f267a1d..de894c7 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char *datestr)\n \tshow(buffer);\n }\n \n-static int show_file(const char *arg)\n+static int show_file(const char *arg, int output_prefix)\n {\n \tshow_default();\n \tif ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {\n-\t\tshow(arg);\n+\t\tif (output_prefix) {\n+\t\t\tconst char *prefix = startup_info->prefix;\n+\t\t\tshow(prefix_filename(prefix,\n+\t\t\t\t\t prefix ? strlen(prefix) : 0,\n+\t\t\t\t\t arg));\n+\t\t} else\n+\t\t\tshow(arg);\n \t\treturn 1;\n \t}\n \treturn 0;\n@@ -470,6 +476,7 @@ N_(\"git rev-parse --parseopt [options] -- [...]\\n\"\n int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n {\n \tint i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;\n+\tint output_prefix = 0;\n \tunsigned char sha1[20];\n \tconst char *name = NULL;\n \n@@ -503,7 +510,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\tconst char *arg = argv[i];\n \n \t\tif (as_is) {\n-\t\t\tif (show_file(arg) && as_is < 2)\n+\t\t\tif (show_file(arg, output_prefix) && as_is < 2)\n \t\t\t\tverify_filename(prefix, arg, 0);\n \t\t\tcontinue;\n \t\t}\n@@ -527,7 +534,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tas_is = 2;\n \t\t\t\t/* Pass on the \"--\" if we show anything but files.. */\n \t\t\t\tif (filter & (DO_FLAGS | DO_REVS))\n-\t\t\t\t\tshow_file(arg);\n+\t\t\t\t\tshow_file(arg, 0);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--default\")) {\n@@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\ti++;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--prefix\")) {\n+\t\t\t\tprefix = argv[i+1];\n+\t\t\t\tstartup_info->prefix = prefix;\n+\t\t\t\toutput_prefix = 1;\n+\t\t\t\ti++;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--revs-only\")) {\n \t\t\t\tfilter &= ~DO_NOREV;\n \t\t\t\tcontinue;\n@@ -754,7 +768,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\tif (verify)\n \t\t\tdie_no_single_rev(quiet);\n \t\tas_is = 1;\n-\t\tif (!show_file(arg))\n+\t\tif (!show_file(arg, output_prefix))\n \t\t\tcontinue;\n \t\tverify_filename(prefix, arg, 1);\n \t}\ndiff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh\nnew file mode 100755\nindex 0000000..87ec3ae\n--- /dev/null\n+++ b/t/t1513-rev-parse-prefix.sh\n@@ -0,0 +1,96 @@\n+#!/bin/sh\n+\n+test_description='Tests for rev-parse --prefix'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\tmkdir -p sub1/sub2 &&\n+\techo top >top &&\n+\techo file1 >sub1/file1 &&\n+\techo file2 >sub1/sub2/file2 &&\n+\tgit add top sub1/file1 sub1/sub2/file2 &&\n+\tgit commit -m commit\n+'\n+\n+test_expect_success 'empty prefix -- file' '\n+\tgit rev-parse --prefix \"\" -- top sub1/file1 >actual &&\n+\tcat <<-\\EOF >expected &&\n+\t--\n+\ttop\n+\tsub1/file1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'valid prefix -- file' '\n+\tgit rev-parse --prefix sub1/ -- file1 sub2/file2 >actual &&\n+\tcat <<-\\EOF >expected &&\n+\t--\n+\tsub1/file1\n+\tsub1/sub2/file2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'valid prefix -- ../file' '\n+\tgit rev-parse --prefix sub1/ -- ../top sub2/file2 >actual &&\n+\tcat <<-\\EOF >expected &&\n+\t--\n+\tsub1/../top\n+\tsub1/sub2/file2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'empty prefix HEAD:./path' '\n+\tgit rev-parse --prefix \"\" HEAD:./top >actual &&\n+\tgit rev-parse HEAD:top >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'valid prefix HEAD:./path' '\n+\tgit rev-parse --prefix sub1/ HEAD:./file1 >actual &&\n+\tgit rev-parse HEAD:sub1/file1 >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'valid prefix HEAD:../path' '\n+\tgit rev-parse --prefix sub1/ HEAD:../top >actual &&\n+\tgit rev-parse HEAD:top >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'prefix ignored with HEAD:top' '\n+\tgit rev-parse --prefix sub1/ HEAD:top >actual &&\n+\tgit rev-parse HEAD:top >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'disambiguate path with valid prefix' '\n+\tgit rev-parse --prefix sub1/ file1 >actual &&\n+\tcat <<-\\EOF >expected &&\n+\tsub1/file1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'file and refs with prefix' '\n+\tgit rev-parse --prefix sub1/ master file1 >actual &&\n+\tcat <<-EOF >expected &&\n+\t$(git rev-parse master)\n+\tsub1/file1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'two-levels deep' '\n+\tgit rev-parse --prefix sub1/sub2/ -- file2 >actual &&\n+\tcat <<-\\EOF >expected &&\n+\t--\n+\tsub1/sub2/file2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n1.8.2.1.424.g1899e5e.dirty\n"},{"id":"214756","messageId":"6e4122f3eedec3f520028b5598b78e0d59e5d12b.1366314439.git.john@keeping.me.uk","threadId":"33422","inReplyTo":"cover.1366314439.git.john@keeping.me.uk","subject":"[PATCH v3 2/2] submodule: drop the top-level requirement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-18T19:50:42Z","receivedAt":"2013-04-18T19:50:42Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Use the new rev-parse --prefix option to process all paths given to the\nsubmodule command, dropping the requirement that it be run from the\ntop-level of the repository.\n\nSigned-off-by: John Keeping \n---\nChanges since v2:\n - Handle relative paths given to \"submodule add --reference=./...\"\n - Check for absolute path before prepending $wt_prefix in cmd_add\n - Export relative sm_path in cmd_foreach\n - Change displayed sm_path to be relative\n - Move a mkdir out of a subshell in t7400\n\nI'm not sure if the relative_path function here is sufficient on\nWindows. We should be okay with the \"target\" argument since that comes\nfrom git-ls-files but I think we're in trouble if \"git rev-parse\n--show-prefix\" output the prefix with '\\' instead of '/'.\n\n git-submodule.sh | 113 ++++++++++++++++++++++++++++++++-----------\n t/t7400-submodule-basic.sh | 27 +++++++++++\n t/t7401-submodule-summary.sh | 14 ++++++\n 3 files changed, 127 insertions(+), 27 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 79bfaac..0eee703 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -14,10 +14,13 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] [--name ] [--reference \n or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n OPTIONS_SPEC=\n+SUBDIRECTORY_OK=Yes\n . git-sh-setup\n . git-sh-i18n\n . git-parse-remote\n require_work_tree\n+wt_prefix=$(git rev-parse --show-prefix)\n+cd_to_toplevel\n \n command=\n branch=\n@@ -106,12 +109,48 @@ resolve_relative_url ()\n \techo \"${is_relative:+${up_path}}${remoteurl#./}\"\n }\n \n+# Resolve a path to be relative to another path. This is intended for\n+# converting submodule paths when git-submodule is run in a subdirectory\n+# and only handles paths where the directory separator is '/'.\n+#\n+# The output is the first argument as a path relative to the second argument,\n+# which defaults to $wt_prefix if it is omitted.\n+relative_path ()\n+{\n+\tlocal target curdir result\n+\ttarget=$1\n+\tcurdir=${2-$wt_prefix}\n+\tcurdir=${curdir%/}\n+\tresult=\n+\n+\twhile test -n \"$curdir\"\n+\tdo\n+\t\tcase \"$target\" in\n+\t\t\"$curdir/\"*)\n+\t\t\ttarget=${target#$curdir/}\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\n+\t\tresult=\"${result}../\"\n+\t\tif test \"$curdir\" = \"${curdir%/*}\"\n+\t\tthen\n+\t\t\tcurdir=\n+\t\telse\n+\t\t\tcurdir=\"${curdir%/*}\"\n+\t\tfi\n+\tdone\n+\n+\techo \"$result$target\"\n+}\n+\n #\n # Get submodule info for registered submodules\n # $@ = path to limit submodule list\n #\n module_list()\n {\n+\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n \t(\n \t\tgit ls-files --error-unmatch --stage -- \"$@\" ||\n \t\techo \"unmatched pathspec exists\"\n@@ -282,6 +321,7 @@ isnumber()\n cmd_add()\n {\n \t# parse $args after \"submodule ... add\".\n+\treference_path=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -298,11 +338,11 @@ cmd_add()\n \t\t\t;;\n \t\t--reference)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\treference=\"--reference=$2\"\n+\t\t\treference_path=$2\n \t\t\tshift\n \t\t\t;;\n \t\t--reference=*)\n-\t\t\treference=\"$1\"\n+\t\t\treference_path=\"${1#--reference=}\"\n \t\t\t;;\n \t\t--name)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -323,6 +363,14 @@ cmd_add()\n \t\tshift\n \tdone\n \n+\tif test -n \"$reference_path\"\n+\tthen\n+\t\tis_absolute_path \"$reference_path\" ||\n+\t\treference_path=\"$wt_prefix$reference_path\"\n+\n+\t\treference=\"--reference=$reference_path\"\n+\tfi\n+\n \trepo=$1\n \tsm_path=$2\n \n@@ -335,6 +383,8 @@ cmd_add()\n \t\tusage\n \tfi\n \n+\tis_absolute_path \"$sm_path\" || sm_path=\"$wt_prefix$sm_path\"\n+\n \t# assure repo is absolute or relative to parent\n \tcase \"$repo\" in\n \t./*|../*)\n@@ -479,6 +529,7 @@ cmd_foreach()\n \t\t\t\t# we make $path available to scripts ...\n \t\t\t\tpath=$sm_path\n \t\t\t\tcd \"$sm_path\" &&\n+\t\t\t\tsm_path=$(relative_path \"$sm_path\") &&\n \t\t\t\teval \"$@\" &&\n \t\t\t\tif test -n \"$recursive\"\n \t\t\t\tthen\n@@ -594,27 +645,29 @@ cmd_deinit()\n \t\tdie_if_unmatched \"$mode\"\n \t\tname=$(module_name \"$sm_path\") || exit\n \n+\t\tdisplaypath=$(relative_path \"$sm_path\")\n+\n \t\t# Remove the submodule work tree (unless the user already did it)\n \t\tif test -d \"$sm_path\"\n \t\tthen\n \t\t\t# Protect submodules containing a .git directory\n \t\t\tif test -d \"$sm_path/.git\"\n \t\t\tthen\n-\t\t\t\techo >&2 \"$(eval_gettext \"Submodule work tree '\\$sm_path' contains a .git directory\")\"\n+\t\t\t\techo >&2 \"$(eval_gettext \"Submodule work tree '\\$displaypath' contains a .git directory\")\"\n \t\t\t\tdie \"$(eval_gettext \"(use 'rm -rf' if you really want to remove it including all of its history)\")\"\n \t\t\tfi\n \n \t\t\tif test -z \"$force\"\n \t\t\tthen\n \t\t\t\tgit rm -qn \"$sm_path\" ||\n-\t\t\t\tdie \"$(eval_gettext \"Submodule work tree '\\$sm_path' contains local modifications; use '-f' to discard them\")\"\n+\t\t\t\tdie \"$(eval_gettext \"Submodule work tree '\\$displaypath' contains local modifications; use '-f' to discard them\")\"\n \t\t\tfi\n \t\t\trm -rf \"$sm_path\" &&\n-\t\t\tsay \"$(eval_gettext \"Cleared directory '\\$sm_path'\")\" ||\n-\t\t\tsay \"$(eval_gettext \"Could not remove submodule work tree '\\$sm_path'\")\"\n+\t\t\tsay \"$(eval_gettext \"Cleared directory '\\$displaypath'\")\" ||\n+\t\t\tsay \"$(eval_gettext \"Could not remove submodule work tree '\\$displaypath'\")\"\n \t\tfi\n \n-\t\tmkdir \"$sm_path\" || say \"$(eval_gettext \"Could not create empty submodule directory '\\$sm_path'\")\"\n+\t\tmkdir \"$sm_path\" || say \"$(eval_gettext \"Could not create empty submodule directory '\\$displaypath'\")\"\n \n \t\t# Remove the .git/config entries (unless the user already did it)\n \t\tif test -n \"$(git config --get-regexp submodule.\"$name\\.\")\"\n@@ -623,7 +676,7 @@ cmd_deinit()\n \t\t\t# the user later decides to init this submodule again\n \t\t\turl=$(git config submodule.\"$name\".url)\n \t\t\tgit config --remove-section submodule.\"$name\" 2>/dev/null &&\n-\t\t\tsay \"$(eval_gettext \"Submodule '\\$name' (\\$url) unregistered for path '\\$sm_path'\")\"\n+\t\t\tsay \"$(eval_gettext \"Submodule '\\$name' (\\$url) unregistered for path '\\$displaypath'\")\"\n \t\tfi\n \tdone\n }\n@@ -717,9 +770,11 @@ cmd_update()\n \t\t\tupdate_module=$(git config submodule.\"$name\".update)\n \t\tfi\n \n+\t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n+\n \t\tif test \"$update_module\" = \"none\"\n \t\tthen\n-\t\t\techo \"Skipping submodule '$prefix$sm_path'\"\n+\t\t\techo \"Skipping submodule '$displaypath'\"\n \t\t\tcontinue\n \t\tfi\n \n@@ -728,7 +783,7 @@ cmd_update()\n \t\t\t# Only mention uninitialized submodules when its\n \t\t\t# path have been specified\n \t\t\ttest \"$#\" != \"0\" &&\n-\t\t\tsay \"$(eval_gettext \"Submodule path '\\$prefix\\$sm_path' not initialized\n+\t\t\tsay \"$(eval_gettext \"Submodule path '\\$displaypath' not initialized\n Maybe you want to use 'update --init'?\")\"\n \t\t\tcontinue\n \t\tfi\n@@ -741,7 +796,7 @@ Maybe you want to use 'update --init'?\")\"\n \t\telse\n \t\t\tsubsha1=$(clear_local_git_env; cd \"$sm_path\" &&\n \t\t\t\tgit rev-parse --verify HEAD) ||\n-\t\t\tdie \"$(eval_gettext \"Unable to find current revision in submodule path '\\$prefix\\$sm_path'\")\"\n+\t\t\tdie \"$(eval_gettext \"Unable to find current revision in submodule path '\\$displaypath'\")\"\n \t\tfi\n \n \t\tif test -n \"$remote\"\n@@ -774,7 +829,7 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\t\t(clear_local_git_env; cd \"$sm_path\" &&\n \t\t\t\t\t( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&\n \t\t\t\t\t test -z \"$rev\") || git-fetch)) ||\n-\t\t\t\tdie \"$(eval_gettext \"Unable to fetch in submodule path '\\$prefix\\$sm_path'\")\"\n+\t\t\t\tdie \"$(eval_gettext \"Unable to fetch in submodule path '\\$displaypath'\")\"\n \t\t\tfi\n \n \t\t\t# Is this something we just cloned?\n@@ -788,20 +843,20 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\tcase \"$update_module\" in\n \t\t\trebase)\n \t\t\t\tcommand=\"git rebase\"\n-\t\t\t\tdie_msg=\"$(eval_gettext \"Unable to rebase '\\$sha1' in submodule path '\\$prefix\\$sm_path'\")\"\n-\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$prefix\\$sm_path': rebased into '\\$sha1'\")\"\n+\t\t\t\tdie_msg=\"$(eval_gettext \"Unable to rebase '\\$sha1' in submodule path '\\$displaypath'\")\"\n+\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$displaypath': rebased into '\\$sha1'\")\"\n \t\t\t\tmust_die_on_failure=yes\n \t\t\t\t;;\n \t\t\tmerge)\n \t\t\t\tcommand=\"git merge\"\n-\t\t\t\tdie_msg=\"$(eval_gettext \"Unable to merge '\\$sha1' in submodule path '\\$prefix\\$sm_path'\")\"\n-\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$prefix\\$sm_path': merged in '\\$sha1'\")\"\n+\t\t\t\tdie_msg=\"$(eval_gettext \"Unable to merge '\\$sha1' in submodule path '\\$displaypath'\")\"\n+\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$displaypath': merged in '\\$sha1'\")\"\n \t\t\t\tmust_die_on_failure=yes\n \t\t\t\t;;\n \t\t\t*)\n \t\t\t\tcommand=\"git checkout $subforce -q\"\n-\t\t\t\tdie_msg=\"$(eval_gettext \"Unable to checkout '\\$sha1' in submodule path '\\$prefix\\$sm_path'\")\"\n-\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$prefix\\$sm_path': checked out '\\$sha1'\")\"\n+\t\t\t\tdie_msg=\"$(eval_gettext \"Unable to checkout '\\$sha1' in submodule path '\\$displaypath'\")\"\n+\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$displaypath': checked out '\\$sha1'\")\"\n \t\t\t\t;;\n \t\t\tesac\n \n@@ -828,7 +883,7 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\tres=$?\n \t\t\tif test $res -gt 0\n \t\t\tthen\n-\t\t\t\tdie_msg=\"$(eval_gettext \"Failed to recurse into submodule path '\\$prefix\\$sm_path'\")\"\n+\t\t\t\tdie_msg=\"$(eval_gettext \"Failed to recurse into submodule path '\\$displaypath'\")\"\n \t\t\t\tif test $res -eq 1\n \t\t\t\tthen\n \t\t\t\t\terr=\"${err};$die_msg\"\n@@ -942,6 +997,7 @@ cmd_summary() {\n \tfi\n \n \tcd_to_toplevel\n+\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n \t# Get modified modules cared by user\n \tmodules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- \"$@\" |\n \t\tsane_egrep '^:([0-7]* )?160000' |\n@@ -991,16 +1047,18 @@ cmd_summary() {\n \t\t! GIT_DIR=\"$name/.git\" git-rev-parse -q --verify $sha1_dst^0 >/dev/null &&\n \t\tmissing_dst=t\n \n+\t\tdisplay_name=$(relative_path \"$name\")\n+\n \t\ttotal_commits=\n \t\tcase \"$missing_src,$missing_dst\" in\n \t\tt,)\n-\t\t\terrmsg=\"$(eval_gettext \" Warn: \\$name doesn't contain commit \\$sha1_src\")\"\n+\t\t\terrmsg=\"$(eval_gettext \" Warn: \\$display_name doesn't contain commit \\$sha1_src\")\"\n \t\t\t;;\n \t\t,t)\n-\t\t\terrmsg=\"$(eval_gettext \" Warn: \\$name doesn't contain commit \\$sha1_dst\")\"\n+\t\t\terrmsg=\"$(eval_gettext \" Warn: \\$display_name doesn't contain commit \\$sha1_dst\")\"\n \t\t\t;;\n \t\tt,t)\n-\t\t\terrmsg=\"$(eval_gettext \" Warn: \\$name doesn't contain commits \\$sha1_src and \\$sha1_dst\")\"\n+\t\t\terrmsg=\"$(eval_gettext \" Warn: \\$display_name doesn't contain commits \\$sha1_src and \\$sha1_dst\")\"\n \t\t\t;;\n \t\t*)\n \t\t\terrmsg=\n@@ -1029,12 +1087,12 @@ cmd_summary() {\n \t\t\tsubmodule=\"$(gettext \"submodule\")\"\n \t\t\tif test $mod_dst = 160000\n \t\t\tthen\n-\t\t\t\techo \"* $name $sha1_abbr_src($blob)->$sha1_abbr_dst($submodule)$total_commits:\"\n+\t\t\t\techo \"* $display_name $sha1_abbr_src($blob)->$sha1_abbr_dst($submodule)$total_commits:\"\n \t\t\telse\n-\t\t\t\techo \"* $name $sha1_abbr_src($submodule)->$sha1_abbr_dst($blob)$total_commits:\"\n+\t\t\t\techo \"* $display_name $sha1_abbr_src($submodule)->$sha1_abbr_dst($blob)$total_commits:\"\n \t\t\tfi\n \t\telse\n-\t\t\techo \"* $name $sha1_abbr_src...$sha1_abbr_dst$total_commits:\"\n+\t\t\techo \"* $display_name $sha1_abbr_src...$sha1_abbr_dst$total_commits:\"\n \t\tfi\n \t\tif test -n \"$errmsg\"\n \t\tthen\n@@ -1118,7 +1176,7 @@ cmd_status()\n \t\tdie_if_unmatched \"$mode\"\n \t\tname=$(module_name \"$sm_path\") || exit\n \t\turl=$(git config submodule.\"$name\".url)\n-\t\tdisplaypath=\"$prefix$sm_path\"\n+\t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n \t\tif test \"$stage\" = U\n \t\tthen\n \t\t\tsay \"U$sha1 $displaypath\"\n@@ -1213,7 +1271,8 @@ cmd_sync()\n \n \t\tif git config \"submodule.$name.url\" >/dev/null 2>/dev/null\n \t\tthen\n-\t\t\tsay \"$(eval_gettext \"Synchronizing submodule url for '\\$prefix\\$sm_path'\")\"\n+\t\t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n+\t\t\tsay \"$(eval_gettext \"Synchronizing submodule url for '\\$displaypath'\")\"\n \t\t\tgit config submodule.\"$name\".url \"$super_config_url\"\n \n \t\t\tif test -e \"$sm_path\"/.git\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex ff26535..ca0a6ab 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -212,6 +212,24 @@ test_expect_success 'submodule add with ./, /.. and // in path' '\n \ttest_cmp empty untracked\n '\n \n+test_expect_success 'submodule add in subdir' '\n+\techo \"refs/heads/master\" >expect &&\n+\t>empty &&\n+\n+\tmkdir addtest/sub &&\n+\t(\n+\t\tcd addtest/sub &&\n+\t\tgit submodule add \"$submodurl\" ../realsubmod3 &&\n+\t\tgit submodule init\n+\t) &&\n+\n+\trm -f heads head untracked &&\n+\tinspect addtest/realsubmod3 ../.. &&\n+\ttest_cmp expect heads &&\n+\ttest_cmp expect head &&\n+\ttest_cmp empty untracked\n+'\n+\n test_expect_success 'setup - add an example entry to .gitmodules' '\n \tGIT_CONFIG=.gitmodules \\\n \tgit config submodule.example.url git://example.com/init.git\n@@ -319,6 +337,15 @@ test_expect_success 'status should be \"up-to-date\" after update' '\n \tgrep \"^ $rev1\" list\n '\n \n+test_expect_success 'status works correctly from a subdirectory' '\n+\tmkdir sub &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit submodule status >../list\n+\t) &&\n+\tgrep \"^ $rev1\" list\n+'\n+\n test_expect_success 'status should be \"modified\" after submodule commit' '\n \t(\n \t\tcd init &&\ndiff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\nindex 30b429e..992b66b 100755\n--- a/t/t7401-submodule-summary.sh\n+++ b/t/t7401-submodule-summary.sh\n@@ -45,6 +45,20 @@ EOF\n \ttest_cmp expected actual\n \"\n \n+test_expect_success 'run summary from subdir' '\n+\tmkdir sub &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit submodule summary >../actual\n+\t) &&\n+\tcat >expected <<-EOF &&\n+* ../sm1 0000000...$head1 (2):\n+ > Add foo2\n+\n+EOF\n+\ttest_cmp expected actual\n+'\n+\n commit_file sm1 &&\n head2=$(add_file sm1 foo3)\n \n-- \n1.8.2.1.424.g1899e5e.dirty\n"},{"id":"214767","messageId":"7vfvyn4g46.fsf@alter.siamese.dyndns.org","threadId":"33422","inReplyTo":"6e4122f3eedec3f520028b5598b78e0d59e5d12b.1366314439.git.john@keeping.me.uk","subject":"Re: [PATCH v3 2/2] submodule: drop the top-level requirement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-18T22:40:41Z","receivedAt":"2013-04-18T22:40:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> +relative_path ()\n> +{\n> +\tlocal target curdir result\n> +\ttarget=$1\n> +\tcurdir=${2-$wt_prefix}\n> +\tcurdir=${curdir%/}\n> +\tresult=\n> +\n> +\twhile test -n \"$curdir\"\n> +\tdo\n> +\t\tcase \"$target\" in\n> +\t\t\"$curdir/\"*)\n> +\t\t\ttarget=${target#$curdir/}\n> +\t\t\tbreak\n> +\t\t\t;;\n> +\t\tesac\n\nCould $curdir have glob wildcard to throw this part of the logic\noff? It is OK to have limitations like \"you cannot have a glob\ncharacters in your path to submodule working tree\" (at least until\nwe start rewriting these in C or Perl or Python), but we need to be\naware of them.\n\n> module_list()\n> {\n> +\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n\nAn efficient reuse of \"--\" ;-)\n\n> +test_expect_success 'run summary from subdir' '\n> +\tmkdir sub &&\n> +\t(\n> +\t\tcd sub &&\n> +\t\tgit submodule summary >../actual\n> +\t) &&\n> +\tcat >expected <<-EOF &&\n> +* ../sm1 0000000...$head1 (2):\n> + > Add foo2\n> +\n> +EOF\n\nIt somewhat looks strange to start with \"<<-EOF\" and then not to\nindent the body nor EOF.\n"},{"id":"214786","messageId":"CAPig+cR7TS-sahgdZPkWkeE2Sy=GimGUVMSekgqtomWKfdiatA@mail.gmail.com","threadId":"33422","inReplyTo":"6e4122f3eedec3f520028b5598b78e0d59e5d12b.1366314439.git.john@keeping.me.uk","subject":"Re: [PATCH v3 2/2] submodule: drop the top-level requirement","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-04-18T23:54:24Z","receivedAt":"2013-04-18T23:54:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 18, 2013 at 3:50 PM, John Keeping wrote:\n> Use the new rev-parse --prefix option to process all paths given to the\n> submodule command, dropping the requirement that it be run from the\n> top-level of the repository.\n>\n> Signed-off-by: John Keeping \n> ---\n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index ff26535..ca0a6ab 100755\n> --- a/t/t7400-submodule-basic.sh\n> +++ b/t/t7400-submodule-basic.sh\n> @@ -212,6 +212,24 @@ test_expect_success 'submodule add with ./, /.. and // in path' '\n> test_cmp empty untracked\n> '\n>\n> +test_expect_success 'submodule add in subdir' '\n\nA particularly minor nit. Existing subdirectory-related tests in t7400\nspell out \"subdirectory\" fully, so perhaps for consistency:\ns/subdir/subdirectory/\n\n> + echo \"refs/heads/master\" >expect &&\n> + >empty &&\n> +\n> + mkdir addtest/sub &&\n> + (\n> + cd addtest/sub &&\n> + git submodule add \"$submodurl\" ../realsubmod3 &&\n> + git submodule init\n> + ) &&\n> +\n> + rm -f heads head untracked &&\n> + inspect addtest/realsubmod3 ../.. &&\n> + test_cmp expect heads &&\n> + test_cmp expect head &&\n> + test_cmp empty untracked\n> +'\n> +\n> test_expect_success 'setup - add an example entry to .gitmodules' '\n> GIT_CONFIG=.gitmodules \\\n> git config submodule.example.url git://example.com/init.git\n> @@ -319,6 +337,15 @@ test_expect_success 'status should be \"up-to-date\" after update' '\n> grep \"^ $rev1\" list\n> '\n>\n> +test_expect_success 'status works correctly from a subdirectory' '\n\nGood: \"subdirectory\"\n\n> + mkdir sub &&\n> + (\n> + cd sub &&\n> + git submodule status >../list\n> + ) &&\n> + grep \"^ $rev1\" list\n> +'\n> +\n> test_expect_success 'status should be \"modified\" after submodule commit' '\n> (\n> cd init &&\n> diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\n> index 30b429e..992b66b 100755\n> --- a/t/t7401-submodule-summary.sh\n> +++ b/t/t7401-submodule-summary.sh\n> @@ -45,6 +45,20 @@ EOF\n> test_cmp expected actual\n> \"\n>\n> +test_expect_success 'run summary from subdir' '\n\nt7401 does not have any existing subdirectory-related tests, but for\nconsistency with t7400, perhaps: s/subdir/subdirectory/\n\n> + mkdir sub &&\n> + (\n> + cd sub &&\n> + git submodule summary >../actual\n> + ) &&\n> + cat >expected <<-EOF &&\n> +* ../sm1 0000000...$head1 (2):\n> + > Add foo2\n> +\n> +EOF\n> + test_cmp expected actual\n> +'\n> +\n> commit_file sm1 &&\n> head2=$(add_file sm1 foo3)\n"},{"id":"214818","messageId":"20130419074632.GC2278@serenity.lan","threadId":"33422","inReplyTo":"7vfvyn4g46.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 2/2] submodule: drop the top-level requirement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-19T07:46:32Z","receivedAt":"2013-04-19T07:46:32Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:\n> John Keeping writes:\n> \n> > +relative_path ()\n> > +{\n> > +\tlocal target curdir result\n> > +\ttarget=$1\n> > +\tcurdir=${2-$wt_prefix}\n> > +\tcurdir=${curdir%/}\n> > +\tresult=\n> > +\n> > +\twhile test -n \"$curdir\"\n> > +\tdo\n> > +\t\tcase \"$target\" in\n> > +\t\t\"$curdir/\"*)\n> > +\t\t\ttarget=${target#$curdir/}\n> > +\t\t\tbreak\n> > +\t\t\t;;\n> > +\t\tesac\n> \n> Could $curdir have glob wildcard to throw this part of the logic\n> off? It is OK to have limitations like \"you cannot have a glob\n> characters in your path to submodule working tree\" (at least until\n> we start rewriting these in C or Perl or Python), but we need to be\n> aware of them.\n\nI think the use of \"#\" instead of \"##\" saves us here because even with a\nwildcard in $curdir the case statement matches literally, so we know\nthat \"$target\" starts with \"$curdir/\", so \"${target#$curdir/}\" can't\nmatch anything longer than the literal \"$curdir\" prefix.\n\n> > module_list()\n> > {\n> > +\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n> \n> An efficient reuse of \"--\" ;-)\n> \n> > +test_expect_success 'run summary from subdir' '\n> > +\tmkdir sub &&\n> > +\t(\n> > +\t\tcd sub &&\n> > +\t\tgit submodule summary >../actual\n> > +\t) &&\n> > +\tcat >expected <<-EOF &&\n> > +* ../sm1 0000000...$head1 (2):\n> > + > Add foo2\n> > +\n> > +EOF\n> \n> It somewhat looks strange to start with \"<<-EOF\" and then not to\n> indent the body nor EOF.\n\nYes, but I copied this from the preceding test. I'd rather leave this\nas it is for consistency and let later style fixes change all of the\ntests in this file.\n"},{"id":"214821","messageId":"1366365217-17674-1-git-send-email-artagnon@gmail.com","threadId":"33422","inReplyTo":"82316c445320b3cec3a35cbc87ad94a04ed26d1e.1366314439.git.john@keeping.me.uk","subject":"Re: [PATCH v3 1/2] rev-parse: add --prefix option","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-19T09:53:37Z","receivedAt":"2013-04-19T09:53:37Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"John Keeping wrote:\n> Changes since v2:\n> - Rewrite commit message\n> - Add a new test for 'prefix ignored with HEAD:top'\n> - Use '<<-\\EOF' where appropriate in t1513\n\nThanks for the re-roll.\n\nIn the previous iteration, I wasn't sure this was the right approach\nbecause I thought it would be better to pass RUN_SETUP and let\nrun_builtin_command() take care of the prefix-setting. Unfortunately,\nas 5410a02 (git-rev-parse --parseopt, 2007-11-06) indicates, we have\nto run setup_git_directory() in cmd_rev_parse() after parsing\n--parseopt, as 'git rev-parse --parseopt' can be run outside a git\nrepository. You might want to include this note in your commit\nmessage for the benefit of other readers.\n\nOther than that, I just have one small suggestion: it's possible to\navoid passing output_prefix around, and simplify show_file() a bit if\nwe acknowledge that printing \"--\" is not the same as printing a file\n(although the condition is the same). Would you like to squash this\nin?\n\nSigned-off-by: Ramkumar Ramachandra \n-- 8< --\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex de894c7..7e69b3f 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -29,6 +29,7 @@ static int abbrev;\n static int abbrev_ref;\n static int abbrev_ref_strict;\n static int output_sq;\n+static int output_prefix;\n \n /*\n * Some arguments are relevant \"revision\" arguments,\n@@ -212,15 +213,13 @@ static void show_datestring(const char *flag, const char *datestr)\n \tshow(buffer);\n }\n \n-static int show_file(const char *arg, int output_prefix)\n+static int show_file(const char *arg)\n {\n \tshow_default();\n \tif ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {\n \t\tif (output_prefix) {\n \t\t\tconst char *prefix = startup_info->prefix;\n-\t\t\tshow(prefix_filename(prefix,\n-\t\t\t\t\t prefix ? strlen(prefix) : 0,\n-\t\t\t\t\t arg));\n+\t\t\tshow(prefix_filename(prefix, strlen(prefix), arg));\n \t\t} else\n \t\t\tshow(arg);\n \t\treturn 1;\n@@ -228,6 +227,16 @@ static int show_file(const char *arg, int output_prefix)\n \treturn 0;\n }\n \n+static int show_dashdash()\n+{\n+\tshow_default();\n+\tif ((filter & (DO_NONFLAGS | DO_NOREV)) == (DO_NONFLAGS | DO_NOREV)) {\n+\t\tshow(\"--\");\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n static int try_difference(const char *arg)\n {\n \tchar *dotdot;\n@@ -476,7 +485,6 @@ N_(\"git rev-parse --parseopt [options] -- [...]\\n\"\n int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n {\n \tint i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;\n-\tint output_prefix = 0;\n \tunsigned char sha1[20];\n \tconst char *name = NULL;\n \n@@ -510,7 +518,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\tconst char *arg = argv[i];\n \n \t\tif (as_is) {\n-\t\t\tif (show_file(arg, output_prefix) && as_is < 2)\n+\t\t\tif (show_file(arg) && as_is < 2)\n \t\t\t\tverify_filename(prefix, arg, 0);\n \t\t\tcontinue;\n \t\t}\n@@ -534,7 +542,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tas_is = 2;\n \t\t\t\t/* Pass on the \"--\" if we show anything but files.. */\n \t\t\t\tif (filter & (DO_FLAGS | DO_REVS))\n-\t\t\t\t\tshow_file(arg, 0);\n+\t\t\t\t\tshow_dashdash();\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--default\")) {\n@@ -543,7 +551,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--prefix\")) {\n-\t\t\t\tprefix = argv[i+1];\n+\t\t\t\tprefix = argv[i + 1];\n \t\t\t\tstartup_info->prefix = prefix;\n \t\t\t\toutput_prefix = 1;\n \t\t\t\ti++;\n@@ -768,7 +776,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\tif (verify)\n \t\t\tdie_no_single_rev(quiet);\n \t\tas_is = 1;\n-\t\tif (!show_file(arg, output_prefix))\n+\t\tif (!show_file(arg))\n \t\t\tcontinue;\n \t\tverify_filename(prefix, arg, 1);\n \t}\n"},{"id":"214822","messageId":"20130419102207.GF2278@serenity.lan","threadId":"33422","inReplyTo":"1366365217-17674-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v3 1/2] rev-parse: add --prefix option","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-19T10:22:07Z","receivedAt":"2013-04-19T10:22:07Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Fri, Apr 19, 2013 at 03:23:37PM +0530, Ramkumar Ramachandra wrote:\n> John Keeping wrote:\n> > Changes since v2:\n> > - Rewrite commit message\n> > - Add a new test for 'prefix ignored with HEAD:top'\n> > - Use '<<-\\EOF' where appropriate in t1513\n> \n> Thanks for the re-roll.\n> \n> In the previous iteration, I wasn't sure this was the right approach\n> because I thought it would be better to pass RUN_SETUP and let\n> run_builtin_command() take care of the prefix-setting. Unfortunately,\n> as 5410a02 (git-rev-parse --parseopt, 2007-11-06) indicates, we have\n> to run setup_git_directory() in cmd_rev_parse() after parsing\n> --parseopt, as 'git rev-parse --parseopt' can be run outside a git\n> repository. You might want to include this note in your commit\n> message for the benefit of other readers.\n> \n> Other than that, I just have one small suggestion: it's possible to\n> avoid passing output_prefix around, and simplify show_file() a bit if\n> we acknowledge that printing \"--\" is not the same as printing a file\n> (although the condition is the same). Would you like to squash this\n> in?\n\nI'm not actually sure this is better. I'm more afraid of the condition\nfor outputting files changing in the future than of passing\noutput_prefix around, so I'd much rather keep that condition in one\nplace.\n\nIf you really feel strongly about it, I'd prefer to extract the if\ncondition to a function and use that directly when deciding whether to\nprint \"--\".\n\n[Also, you introduce a potential segfault via passing a NULL prefix to\nstrlen.]\n\n> Signed-off-by: Ramkumar Ramachandra \n> -- 8< --\n> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\n> index de894c7..7e69b3f 100644\n> --- a/builtin/rev-parse.c\n> +++ b/builtin/rev-parse.c\n> @@ -29,6 +29,7 @@ static int abbrev;\n> static int abbrev_ref;\n> static int abbrev_ref_strict;\n> static int output_sq;\n> +static int output_prefix;\n> \n> /*\n> * Some arguments are relevant \"revision\" arguments,\n> @@ -212,15 +213,13 @@ static void show_datestring(const char *flag, const char *datestr)\n> \tshow(buffer);\n> }\n> \n> -static int show_file(const char *arg, int output_prefix)\n> +static int show_file(const char *arg)\n> {\n> \tshow_default();\n> \tif ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {\n> \t\tif (output_prefix) {\n> \t\t\tconst char *prefix = startup_info->prefix;\n> -\t\t\tshow(prefix_filename(prefix,\n> -\t\t\t\t\t prefix ? strlen(prefix) : 0,\n> -\t\t\t\t\t arg));\n> +\t\t\tshow(prefix_filename(prefix, strlen(prefix), arg));\n> \t\t} else\n> \t\t\tshow(arg);\n> \t\treturn 1;\n> @@ -228,6 +227,16 @@ static int show_file(const char *arg, int output_prefix)\n> \treturn 0;\n> }\n> \n> +static int show_dashdash()\n> +{\n> +\tshow_default();\n> +\tif ((filter & (DO_NONFLAGS | DO_NOREV)) == (DO_NONFLAGS | DO_NOREV)) {\n> +\t\tshow(\"--\");\n> +\t\treturn 1;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> static int try_difference(const char *arg)\n> {\n> \tchar *dotdot;\n> @@ -476,7 +485,6 @@ N_(\"git rev-parse --parseopt [options] -- [...]\\n\"\n> int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> {\n> \tint i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;\n> -\tint output_prefix = 0;\n> \tunsigned char sha1[20];\n> \tconst char *name = NULL;\n> \n> @@ -510,7 +518,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> \t\tconst char *arg = argv[i];\n> \n> \t\tif (as_is) {\n> -\t\t\tif (show_file(arg, output_prefix) && as_is < 2)\n> +\t\t\tif (show_file(arg) && as_is < 2)\n> \t\t\t\tverify_filename(prefix, arg, 0);\n> \t\t\tcontinue;\n> \t\t}\n> @@ -534,7 +542,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> \t\t\t\tas_is = 2;\n> \t\t\t\t/* Pass on the \"--\" if we show anything but files.. */\n> \t\t\t\tif (filter & (DO_FLAGS | DO_REVS))\n> -\t\t\t\t\tshow_file(arg, 0);\n> +\t\t\t\t\tshow_dashdash();\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \t\t\tif (!strcmp(arg, \"--default\")) {\n> @@ -543,7 +551,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \t\t\tif (!strcmp(arg, \"--prefix\")) {\n> -\t\t\t\tprefix = argv[i+1];\n> +\t\t\t\tprefix = argv[i + 1];\n> \t\t\t\tstartup_info->prefix = prefix;\n> \t\t\t\toutput_prefix = 1;\n> \t\t\t\ti++;\n> @@ -768,7 +776,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> \t\tif (verify)\n> \t\t\tdie_no_single_rev(quiet);\n> \t\tas_is = 1;\n> -\t\tif (!show_file(arg, output_prefix))\n> +\t\tif (!show_file(arg))\n> \t\t\tcontinue;\n> \t\tverify_filename(prefix, arg, 1);\n> \t}\n"},{"id":"214823","messageId":"CALkWK0kbXSfghZ69qypfaFF8orWy-bZLRPXcV5WMc5YQ=-18GA@mail.gmail.com","threadId":"33422","inReplyTo":"20130419102207.GF2278@serenity.lan","subject":"Re: [PATCH v3 1/2] rev-parse: add --prefix option","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-19T11:15:38Z","receivedAt":"2013-04-19T11:15:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"John Keeping wrote:\n> I'm not actually sure this is better. I'm more afraid of the condition\n> for outputting files changing in the future than of passing\n> output_prefix around, so I'd much rather keep that condition in one\n> place.\n>\n> If you really feel strongly about it, I'd prefer to extract the if\n> condition to a function and use that directly when deciding whether to\n> print \"--\".\n\nYeah, it would probably make more sense to extract the conditional. I\njust thought it was unnecessary to pass the argument around, but feel\nfree to go either way on this one.\n\n> [Also, you introduce a potential segfault via passing a NULL prefix to\n> strlen.]\n\nIsn't prefix guaranteed to be set by setup_git_directory()? If I\nwanted to check it nevertheless, I'd probably put in a die(\"internal\nerror\") before this line. Feel free to go either way though.\n"},{"id":"214825","messageId":"20130419112524.GH2278@serenity.lan","threadId":"33422","inReplyTo":"CALkWK0kbXSfghZ69qypfaFF8orWy-bZLRPXcV5WMc5YQ=-18GA@mail.gmail.com","subject":"Re: [PATCH v3 1/2] rev-parse: add --prefix option","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-19T11:25:24Z","receivedAt":"2013-04-19T11:25:24Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Fri, Apr 19, 2013 at 04:45:38PM +0530, Ramkumar Ramachandra wrote:\n> John Keeping wrote:\n> > I'm not actually sure this is better. I'm more afraid of the condition\n> > for outputting files changing in the future than of passing\n> > output_prefix around, so I'd much rather keep that condition in one\n> > place.\n> >\n> > If you really feel strongly about it, I'd prefer to extract the if\n> > condition to a function and use that directly when deciding whether to\n> > print \"--\".\n> \n> Yeah, it would probably make more sense to extract the conditional. I\n> just thought it was unnecessary to pass the argument around, but feel\n> free to go either way on this one.\n> \n> > [Also, you introduce a potential segfault via passing a NULL prefix to\n> > strlen.]\n> \n> Isn't prefix guaranteed to be set by setup_git_directory()? If I\n> wanted to check it nevertheless, I'd probably put in a die(\"internal\n> error\") before this line. Feel free to go either way though.\n\nIt looks to me like setup_git_directory() returns NULL if we're at the\ntop of the working tree so we do need that check. I suspect that's so\nthat \"!prefix\" does the right thing.\n"},{"id":"214826","messageId":"CALkWK0=nw7fR2EB6YT=Cx4mUxmBjJCAzAszmey0K+QD55NqwyQ@mail.gmail.com","threadId":"33422","inReplyTo":"20130419112524.GH2278@serenity.lan","subject":"Re: [PATCH v3 1/2] rev-parse: add --prefix option","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-19T11:29:46Z","receivedAt":"2013-04-19T11:29:46Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"John Keeping wrote:\n> It looks to me like setup_git_directory() returns NULL if we're at the\n> top of the working tree so we do need that check. I suspect that's so\n> that \"!prefix\" does the right thing.\n\nAh, yes. My bad. I expected it to return \"/\" on the toplevel\ndirectory actually.\n"},{"id":"214845","messageId":"7vmwsu31vh.fsf@alter.siamese.dyndns.org","threadId":"33422","inReplyTo":"20130419074632.GC2278@serenity.lan","subject":"Re: [PATCH v3 2/2] submodule: drop the top-level requirement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-19T16:45:54Z","receivedAt":"2013-04-19T16:45:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:\n>> John Keeping writes:\n>> \n>> > +relative_path ()\n>> > +{\n>> > +\tlocal target curdir result\n>> > +\ttarget=$1\n>> > +\tcurdir=${2-$wt_prefix}\n>> > +\tcurdir=${curdir%/}\n>> > +\tresult=\n>> > +\n>> > +\twhile test -n \"$curdir\"\n>> > +\tdo\n>> > +\t\tcase \"$target\" in\n>> > +\t\t\"$curdir/\"*)\n>> > +\t\t\ttarget=${target#$curdir/}\n>> > +\t\t\tbreak\n>> > +\t\t\t;;\n>> > +\t\tesac\n>> \n>> Could $curdir have glob wildcard to throw this part of the logic\n>> off? It is OK to have limitations like \"you cannot have a glob\n>> characters in your path to submodule working tree\" (at least until\n>> we start rewriting these in C or Perl or Python), but we need to be\n>> aware of them.\n>\n> I think the use of \"#\" instead of \"##\" saves us here because even with a\n> wildcard in $curdir the case statement matches literally, \n\nIf you have curdir=a*b and target=adropb/c/d/e, the chopping itself\n\n\ttarget=${target#$curdir/}\n\nwould happily chop \"adropb/\" from the target, but because the dq\naround \"$curdir/\"* in the case arm label enforces that target must\nliterally match curdir followed by a slash, we do not even come to\nthe chomping part.\n\nI still have not convinced myself that it is impossible for somebody\nmore clever than I to craft a pair of target and curdir that breaks\nit, though. (target=\"a*b/c/d\", curdir=\"a*b\") is correctly chopped,\nso that is not it.\n"},{"id":"214863","messageId":"517199AB.50109@kdbg.org","threadId":"33422","inReplyTo":"7vmwsu31vh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 2/2] submodule: drop the top-level requirement","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-04-19T19:23:23Z","receivedAt":"2013-04-19T19:23:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.04.2013 18:45, schrieb Junio C Hamano:\n> John Keeping writes:\n> \n>> On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:\n>>> John Keeping writes:\n>>>\n>>>> +relative_path ()\n>>>> +{\n>>>> +\tlocal target curdir result\n>>>> +\ttarget=$1\n>>>> +\tcurdir=${2-$wt_prefix}\n>>>> +\tcurdir=${curdir%/}\n>>>> +\tresult=\n>>>> +\n>>>> +\twhile test -n \"$curdir\"\n>>>> +\tdo\n>>>> +\t\tcase \"$target\" in\n>>>> +\t\t\"$curdir/\"*)\n>>>> +\t\t\ttarget=${target#$curdir/}\n>>>> +\t\t\tbreak\n>>>> +\t\t\t;;\n>>>> +\t\tesac\n>>>\n>>> Could $curdir have glob wildcard to throw this part of the logic\n>>> off? It is OK to have limitations like \"you cannot have a glob\n>>> characters in your path to submodule working tree\" (at least until\n>>> we start rewriting these in C or Perl or Python), but we need to be\n>>> aware of them.\n>>\n>> I think the use of \"#\" instead of \"##\" saves us here because even with a\n>> wildcard in $curdir the case statement matches literally, \n> \n> If you have curdir=a*b and target=adropb/c/d/e, the chopping itself\n> \n> \ttarget=${target#$curdir/}\n> \n> would happily chop \"adropb/\" from the target, but because the dq\n> around \"$curdir/\"* in the case arm label enforces that target must\n> literally match curdir followed by a slash, we do not even come to\n> the chomping part.\n> \n> I still have not convinced myself that it is impossible for somebody\n> more clever than I to craft a pair of target and curdir that breaks\n> it, though. (target=\"a*b/c/d\", curdir=\"a*b\") is correctly chopped,\n> so that is not it.\n\nWhy not just replace the six-liner by this one-liner:\n\n\t\ttarget=${target#\"$curdir\"/}\n\n-- Hannes\n"},{"id":"214887","messageId":"7vehe6z10t.fsf@alter.siamese.dyndns.org","threadId":"33422","inReplyTo":"517199AB.50109@kdbg.org","subject":"Re: [PATCH v3 2/2] submodule: drop the top-level requirement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-19T21:03:14Z","receivedAt":"2013-04-19T21:03:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Why not just replace the six-liner by this one-liner:\n>\n> \t\ttarget=${target#\"$curdir\"/}\n\nSimple enough ;-)\n"},{"id":"215298","messageId":"20130424081536.GT2278@serenity.lan","threadId":"33422","inReplyTo":"7vehe6z10t.fsf@alter.siamese.dyndns.org","subject":"[PATCH] submodule: fix quoting in relative_path()","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-24T08:15:36Z","receivedAt":"2013-04-24T08:15:36Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Commit caca2c1 (submodule: drop the top-level requirement) introduced a\nrelative_path helper but does not quote $curdir when it is stripped from\nthe front of a target path. In this particular case this should be safe\neven with special characters because we only do this after checking that\n$target begins with \"$curdir/\" which is quoted correctly, but we should\nquote the variable to be certain that there is not some obscure case\nwhere we this could strip more or less than we want.\n\nSigned-off-by: John Keeping \n---\nOn Fri, Apr 19, 2013 at 02:03:14PM -0700, Junio C Hamano wrote:\n> Johannes Sixt writes:\n> \n> > Why not just replace the six-liner by this one-liner:\n> >\n> > \t\ttarget=${target#\"$curdir\"/}\n> \n> Simple enough ;-)\n\nThis seems to have arrived on next without this fix, so here's a patch\non top.\n\n git-submodule.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 0eee703..db9f260 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -127,7 +127,7 @@ relative_path ()\n \tdo\n \t\tcase \"$target\" in\n \t\t\"$curdir/\"*)\n-\t\t\ttarget=${target#$curdir/}\n+\t\t\ttarget=${target#\"$curdir\"/}\n \t\t\tbreak\n \t\t\t;;\n \t\tesac\n-- \n1.8.2.1.715.gb260f47\n"},{"id":"215319","messageId":"7vehdzgar1.fsf@alter.siamese.dyndns.org","threadId":"33422","inReplyTo":"20130424081536.GT2278@serenity.lan","subject":"Re: [PATCH] submodule: fix quoting in relative_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-24T16:21:38Z","receivedAt":"2013-04-24T16:21:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n>> > Why not just replace the six-liner by this one-liner:\n>> >\n>> > \t\ttarget=${target#\"$curdir\"/}\n>> \n>> Simple enough ;-)\n>\n> This seems to have arrived on next without this fix, so here's a patch\n> on top.\n>\n> git-submodule.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 0eee703..db9f260 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -127,7 +127,7 @@ relative_path ()\n> \tdo\n> \t\tcase \"$target\" in\n> \t\t\"$curdir/\"*)\n> -\t\t\ttarget=${target#$curdir/}\n> +\t\t\ttarget=${target#\"$curdir\"/}\n> \t\t\tbreak\n> \t\t\t;;\n> \t\tesac\n\nJ6t meant a patch to remove the entire case...esac and replace it\nwith a single liner (target=${target#\"$curdir\"/}).\n"},{"id":"215320","messageId":"20130424162850.GU2278@serenity.lan","threadId":"33422","inReplyTo":"7vehdzgar1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] submodule: fix quoting in relative_path()","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-24T16:28:50Z","receivedAt":"2013-04-24T16:28:50Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Wed, Apr 24, 2013 at 09:21:38AM -0700, Junio C Hamano wrote:\n> John Keeping writes:\n> \n> >> > Why not just replace the six-liner by this one-liner:\n> >> >\n> >> > \t\ttarget=${target#\"$curdir\"/}\n> >> \n> >> Simple enough ;-)\n> >\n> > This seems to have arrived on next without this fix, so here's a patch\n> > on top.\n> >\n> > git-submodule.sh | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/git-submodule.sh b/git-submodule.sh\n> > index 0eee703..db9f260 100755\n> > --- a/git-submodule.sh\n> > +++ b/git-submodule.sh\n> > @@ -127,7 +127,7 @@ relative_path ()\n> > \tdo\n> > \t\tcase \"$target\" in\n> > \t\t\"$curdir/\"*)\n> > -\t\t\ttarget=${target#$curdir/}\n> > +\t\t\ttarget=${target#\"$curdir\"/}\n> > \t\t\tbreak\n> > \t\t\t;;\n> > \t\tesac\n> \n> J6t meant a patch to remove the entire case...esac and replace it\n> with a single liner (target=${target#\"$curdir\"/}).\n\nAh, I missed the \"six-liner\" part. But that doesn't work because we\nbreak out of the do...while loop when \"$target\" starts with \"$curdir/\"\nand replacing the case statement will remove that.\n"},{"id":"215361","messageId":"51782EA7.8000500@kdbg.org","threadId":"33422","inReplyTo":"20130424162850.GU2278@serenity.lan","subject":"Re: [PATCH] submodule: fix quoting in relative_path()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-04-24T19:12:39Z","receivedAt":"2013-04-24T19:12:39Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.04.2013 18:28, schrieb John Keeping:\n> On Wed, Apr 24, 2013 at 09:21:38AM -0700, Junio C Hamano wrote:\n>> J6t meant a patch to remove the entire case...esac and replace it\n>> with a single liner (target=${target#\"$curdir\"/}).\n> \n> Ah, I missed the \"six-liner\" part. But that doesn't work because we\n> break out of the do...while loop when \"$target\" starts with \"$curdir/\"\n> and replacing the case statement will remove that.\n\nYeah, right, _that_ is what I missed ;-)\n\n-- Hannes\n"}]}