{"thread":{"id":"15154","subject":"[PATCH 1/2] git-submodule: replace duplicated code with a module_list function","startedAt":"2008-08-22T07:30:50Z","lastAt":"2008-08-23T01:01:40Z","messageCount":8,"participants":["David Aguilar","Junio C Hamano","Mark Levedahl"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"88096","messageId":"edc9ece47ca86c3c3e4265e7f4222c8ea6917461.1219390139.git.davvid@gmail.com","threadId":"15154","inReplyTo":null,"subject":"[PATCH 1/2] git-submodule: replace duplicated code with a module_list function","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2008-08-22T07:30:50Z","receivedAt":"2008-08-22T07:30:50Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Several call sites in git-submodule.sh used the same idiom for getting\nsubmodule information:\n\n\tgit ls-files --stage -- \"$@\" | grep '^160000 '\n\nThis patch removes this duplication by introducing a module_list function.\n\nSigned-off-by: David Aguilar \n---\n git-submodule.sh | 17 +++++++++++++----\n 1 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2d57d60..2a3a197 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -54,6 +54,15 @@ resolve_relative_url ()\n }\n \n #\n+# Get submodule info for registered submodules\n+# $@ = path to limit submodule list\n+#\n+module_list()\n+{\n+\tgit ls-files --stage -- \"$@\" | grep '^160000 '\n+}\n+\n+#\n # Map submodule path to submodule name\n #\n # $1 = path\n@@ -206,7 +215,7 @@ cmd_add()\n #\n cmd_foreach()\n {\n-\tgit ls-files --stage | grep '^160000 ' |\n+\tmodule_list |\n \twhile read mode sha1 stage path\n \tdo\n \t\tif test -e \"$path\"/.git\n@@ -246,7 +255,7 @@ cmd_init()\n \t\tshift\n \tdone\n \n-\tgit ls-files --stage -- \"$@\" | grep '^160000 ' |\n+\tmodule_list \"$@\" |\n \twhile read mode sha1 stage path\n \tdo\n \t\t# Skip already registered paths\n@@ -304,7 +313,7 @@ cmd_update()\n \t\tesac\n \tdone\n \n-\tgit ls-files --stage -- \"$@\" | grep '^160000 ' |\n+\tmodule_list \"$@\" |\n \twhile read mode sha1 stage path\n \tdo\n \t\tname=$(module_name \"$path\") || exit\n@@ -569,7 +578,7 @@ cmd_status()\n \t\tshift\n \tdone\n \n-\tgit ls-files --stage -- \"$@\" | grep '^160000 ' |\n+\tmodule_list \"$@\" |\n \twhile read mode sha1 stage path\n \tdo\n \t\tname=$(module_name \"$path\") || exit\n-- \n1.6.0.90.g436ed\n"},{"id":"88097","messageId":"e1d3f1871f869a0161fb3f0d84777de9e0125023.1219390139.git.davvid@gmail.com","threadId":"15154","inReplyTo":"edc9ece47ca86c3c3e4265e7f4222c8ea6917461.1219390139.git.davvid@gmail.com","subject":"[PATCH 2/2] git-submodule: add \"sync\" command","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2008-08-22T07:30:51Z","receivedAt":"2008-08-22T07:30:51Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"When a submodule's URL changes upstream, existing submodules\nwill be out of sync since their remote.origin.url will still\nbe set to the old value.\n\nThis change adds a \"git submodule sync\" command that reads the\nsubmodule URLs from .gitmodules and updates any existing\nsubmodules accordingly.\n\nSigned-off-by: David Aguilar \n---\n Documentation/git-submodule.txt | 7 ++++++\n git-submodule.sh | 45 +++++++++++++++++++++++++++++++++++++-\n 2 files changed, 50 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex abbd5b7..a229032 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -15,6 +15,7 @@ SYNOPSIS\n 'git submodule' [--quiet] update [--init] [--] [...]\n 'git submodule' [--quiet] summary [--summary-limit ] [commit] [--] [...]\n 'git submodule' [--quiet] foreach \n+'git submodule' [--quiet] sync [--] [...]\n \n \n DESCRIPTION\n@@ -139,6 +140,12 @@ foreach::\n As an example, \"git submodule foreach 'echo $path `git rev-parse HEAD`' will\n show the path and currently checked out commit for each submodule.\n \n+sync::\n+\tSynchronizes each submodule's remote.origin.url configuration\n+\tsetting to match that of the corresponding submodule.*.url\n+\tvalue as specified in .gitmodules. This is useful if your\n+\tsubmodule URLs have changed upstream and you want to update your\n+\tlocal repositories accordingly.\n \n OPTIONS\n -------\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2a3a197..46739de 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -6,7 +6,7 @@\n \n USAGE=\"[--quiet] [--cached] \\\n [add [-b branch] ]|[status|init|update [-i|--init]|summary [-n|--summary-limit ] []] \\\n-[--] [...]|[foreach ]\"\n+[--] [...]|[foreach ]|[sync [...]]\"\n OPTIONS_SPEC=\n . git-sh-setup\n require_work_tree\n@@ -602,6 +602,47 @@ cmd_status()\n \t\tfi\n \tdone\n }\n+#\n+# Sync git urls for submodules\n+# This makes the value for remote.origin.url match the value\n+# specified in .gitmodules.\n+#\n+cmd_sync()\n+{\n+\twhile test $# -ne 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-q|--quiet)\n+\t\t\tquiet=1\n+\t\t\t;;\n+\t\t-*)\n+\t\t\tusage\n+\t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n+\tcd_to_toplevel\n+\n+\tmodule_list \"$@\" |\n+\twhile read mode sha1 stage path\n+\tdo\n+\t\t! test -f \"$path\"/.git/config &&\n+\t\techo \"Warn: submodule at path '$path' does not exist.\"\n+\t\ttest -f \"$path\"/.git/config || continue\n+\t\tname=$(module_name \"$path\")\n+\t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n+\t\tsay \"Synchronizing submodule url for '$name'\"\n+\t\tgit config -f \"$path\"/.git/config remote.origin.url \"$url\"\n+\tdone\n+}\n \n # This loop parses the command line arguments to find the\n # subcommand name to dispatch. Parsing of the subcommand specific\n@@ -612,7 +653,7 @@ cmd_status()\n while test $# != 0 && test -z \"$command\"\n do\n \tcase \"$1\" in\n-\tadd | foreach | init | update | status | summary)\n+\tadd | foreach | init | update | status | summary | sync)\n \t\tcommand=$1\n \t\t;;\n \t-q|--quiet)\n-- \n1.6.0.90.g436ed\n"},{"id":"88223","messageId":"7vpro064qz.fsf@gitster.siamese.dyndns.org","threadId":"15154","inReplyTo":"edc9ece47ca86c3c3e4265e7f4222c8ea6917461.1219390139.git.davvid@gmail.com","subject":"Re: [PATCH 1/2] git-submodule: replace duplicated code with a module_list function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-22T22:53:40Z","receivedAt":"2008-08-22T22:53:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> @@ -54,6 +54,15 @@ resolve_relative_url ()\n> }\n> \n> #\n> +# Get submodule info for registered submodules\n> +# $@ = path to limit submodule list\n> +#\n> +module_list()\n> +{\n> +\tgit ls-files --stage -- \"$@\" | grep '^160000 '\n> +}\n> +\n> +#\n> # Map submodule path to submodule name\n> #\n> # $1 = path\n> @@ -206,7 +215,7 @@ cmd_add()\n> #\n> cmd_foreach()\n> {\n> -\tgit ls-files --stage | grep '^160000 ' |\n> +\tmodule_list |\n\nThanks.\n\nI think the original \"foreach\" implementation does not pay attention to\n\"$@\" not by design but by mistake, and we should pass \"$@\" here as well.\n\nOther than that I do not see anything obviously wrong with the patch.\n\nMark?\n"},{"id":"88228","messageId":"7vhc9c63ua.fsf@gitster.siamese.dyndns.org","threadId":"15154","inReplyTo":"e1d3f1871f869a0161fb3f0d84777de9e0125023.1219390139.git.davvid@gmail.com","subject":"Re: [PATCH 2/2] git-submodule: add \"sync\" command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-22T23:13:17Z","receivedAt":"2008-08-22T23:13:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> When a submodule's URL changes upstream, existing submodules\n> will be out of sync since their remote.origin.url will still\n> be set to the old value.\n\nOk.\n\n> +#\n> +# Sync git urls for submodules\n> +# This makes the value for remote.origin.url match the value\n> +# specified in .gitmodules.\n> +#\n> +cmd_sync()\n> +{\n> +\twhile test $# -ne 0\n> +\tdo\n> +\t\tcase \"$1\" in\n> +\t\t-q|--quiet)\n> +\t\t\tquiet=1\n> +\t\t\t;;\n> +\t\t-*)\n> +\t\t\tusage\n> +\t\t\t;;\n> +\t\t--)\n> +\t\t\tshift\n> +\t\t\tbreak\n> +\t\t\t;;\n> +\t\t*)\n> +\t\t\tbreak\n> +\t\t\t;;\n> +\t\tesac\n> +\t\tshift\n> +\tdone\n> +\n> +\tcd_to_toplevel\n> +\n> +\tmodule_list \"$@\" |\n> +\twhile read mode sha1 stage path\n> +\tdo\n> +\t\t! test -f \"$path\"/.git/config &&\n> +\t\techo \"Warn: submodule at path '$path' does not exist.\"\n> +\t\ttest -f \"$path\"/.git/config || continue\n> +\t\tname=$(module_name \"$path\")\n> +\t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n> +\t\tsay \"Synchronizing submodule url for '$name'\"\n> +\t\tgit config -f \"$path\"/.git/config remote.origin.url \"$url\"\n> +\tdone\n> +}\n\nHmm. I do not quite like this.\n\nWe allow using a gitfile for submodule $GIT_DIR; checking for file\nexistence of \"$path/.git/config\" does not work for such a configuration.\n\nNeither does 'git config -f \"$path/.git/config\"' work. I think it should\nbe more like:\n\n ( unset GIT_DIR; cd \"$path\" && git config remote.origin.url \"$url\" )\n\nIt is a norm not to have any interest in a submodule that appears in the\nindex of a superproject. You shouldn't get as many warnings as you have\nsubmodules in such a repository. You should just skip them (which you\nalready do, but see above), or at least honor \"$quiet\".\n\nI also think the warning should be sent to the standard error stream.\n"},{"id":"88236","messageId":"48AF533E.9020102@gmail.com","threadId":"15154","inReplyTo":"7vpro064qz.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-submodule: replace duplicated code with a module_list function","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-08-23T00:01:02Z","receivedAt":"2008-08-23T00:01:02Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n>> cmd_foreach()\n>> {\n>> -\tgit ls-files --stage | grep '^160000 ' |\n>> +\tmodule_list |\n>> \n>\n> Thanks.\n>\n> I think the original \"foreach\" implementation does not pay attention to\n> \"$@\" not by design but by mistake, and we should pass \"$@\" here as well.\n>\n> Other than that I do not see anything obviously wrong with the patch.\n>\n> Mark?\n>\n> \n\nActually, this was by design, not mistake, though we did not discuss \nthis at all. I'm not sure what the semantics would / should be: first of \nall, some part of \"$@\" is the command to be executed in each submodule, \nand as written \"$@\" in its entirety is what is used. Also, as written \nand documented, foreach operates in each checked out submodule, not a \nsubset. I guess the basic questions are:\na) What specific option or options to git ls-files makes sense here?\nb) How do we distinguish the ls-files options from the command to be \nexecuted?\n\nMark\n"},{"id":"88239","messageId":"7v8wuo619x.fsf@gitster.siamese.dyndns.org","threadId":"15154","inReplyTo":"48AF533E.9020102@gmail.com","subject":"Re: [PATCH 1/2] git-submodule: replace duplicated code with a module_list function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-23T00:08:42Z","receivedAt":"2008-08-23T00:08:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> Actually, this was by design, not mistake, though we did not discuss\n> this at all. I'm not sure what the semantics would / should be: first\n> of all, some part of \"$@\" is the command to be executed in each\n> submodule, and as written \"$@\" in its entirety is what is used. Also,\n> as written and documented, foreach operates in each checked out\n> submodule, not a subset. I guess the basic questions are:\n> a) What specific option or options to git ls-files makes sense here?\n> b) How do we distinguish the ls-files options from the command to be\n> executed?\n\nAh, I was blind. For (a) I do not see any need for \"option\" but\npathspecs; and (b) I agree is a real problem. We of course could do\nsomething like:\n\n $ git submodule foreach -c 'your command here' your pathspec here\n"},{"id":"88243","messageId":"48AF5BA0.5050001@gmail.com","threadId":"15154","inReplyTo":"7v8wuo619x.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-submodule: replace duplicated code with a module_list function","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-08-23T00:36:48Z","receivedAt":"2008-08-23T00:36:48Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n>> a) What specific option or options to git ls-files makes sense here?\n>> b) How do we distinguish the ls-files options from the command to be\n>> executed?\n>> \n>\n> Ah, I was blind. For (a) I do not see any need for \"option\" but\n> pathspecs; and (b) I agree is a real problem. We of course could do\n> something like:\n>\n> $ git submodule foreach -c 'your command here' your pathspec here\n>\n> \nAs the command is required, while pathspec is optional, the latter \nshould require the option, not the former. How about...\n\n $ git submodule foreach [-l pathspec] 'command'\n\nor\n\n $ git submodule foreach [ --] 'command'\n\nMark\n"},{"id":"88245","messageId":"7vpro04k97.fsf@gitster.siamese.dyndns.org","threadId":"15154","inReplyTo":"48AF5BA0.5050001@gmail.com","subject":"Re: [PATCH 1/2] git-submodule: replace duplicated code with a module_list function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-23T01:01:40Z","receivedAt":"2008-08-23T01:01:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> As the command is required, while pathspec is optional, the latter\n> should require the option, not the former. How about...\n>\n> $ git submodule foreach [-l pathspec] 'command'\n>\n> or\n>\n> $ git submodule foreach [ --] 'command'\n\nNeither would fly well. For all normal git command, pathspec comes after\nthe double dash.\n\nAnd I think it is probably a mistake to think \"the command is required\" as\nanything holy. \"submodule foreach\" looks very much like an index aware\n\"find\". Perhaps we could even make the lack of 'command' to default to\n\"echo its name\", just like \"find .\" and \"find . -print\" are equivalent (I\nam not seriously suggesting this, though. Read on).\n\nHaving said all that, I think you can have best of both worlds by doing\nsomething like this:\n\n - If you do not have -c 'command' or any option, then everything is\n command and you cannot limit with pathspecs. This is the original\n syntax.\n\n - If you do have options to \"foreach\", then -c 'command' string is the\n command, and perhaps the foreach subcommand will gain other options\n over time, including possible -- pathspec separator. The remainder of\n the command line after options are processed are pathspecs that limit\n the ls-files.\n\nI do not think it is a defect that we do not allow pathspec limited\nforeach subcommand. Teaching foreach to take pathspecs is an independent\ntopic if somebody has such an itch.\n\nAs a conclusion of this discussion, I think David's [1/2] is fine as-is.\n\nThanks.\n"}]}