{"thread":{"id":"26522","subject":"[PATCH] submodule: no [--merge|--rebase] when newly cloned","startedAt":"2011-02-16T12:46:57Z","lastAt":"2011-02-18T00:59:02Z","messageCount":9,"participants":["Spencer E. Olson","Junio C Hamano","Jens Lehmann","Johannes Sixt","Martin von Zweigbergk"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"161299","messageId":"1297860417-21895-1-git-send-email-olsonse@umich.edu","threadId":"26522","inReplyTo":null,"subject":"[PATCH] submodule: no [--merge|--rebase] when newly cloned","fromName":"Spencer E. Olson","fromEmail":"olsonse@umich.edu","sentAt":"2011-02-16T12:46:57Z","receivedAt":"2011-02-16T12:46:57Z","isPatch":true,"sender":{"key":"olsonse@umich.edu","avatar":"https://avatars.githubusercontent.com/u/148647?v=4"},"body":"Previously, when a submodule was cloned in the same command execution, --rebase\nor --merge would attempt to run (instead of and) before a checkout, resulting in\nan empty working directory.  This patch ignores --rebase or --merge for a\nparticular submodule when that submodule is newly cloned and instead simply\nchecks out the appropriate revision.\n\nSigned-off-by: Spencer E. Olson <olsonse@umich.edu>\n---\n git-submodule.sh |   28 ++++++++++++++++++++++++++++\n 1 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 8b90589..44d8741 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -365,6 +365,27 @@ cmd_init()\n }\n \n #\n+# Test whether an element of the $3(=IFS) separated list $2 matches $1\n+#\n+str_contains()\n+{\n+\tif test -n \"$3\"\n+\tthen\n+\t\tlocal IFS=\"$3\"\n+\tfi\n+\tfor i in $2\n+\tdo\n+\t\tif test \"$i\" = \"$1\"\n+\t\tthen\n+\t\t\techo \"yes\"\n+\t\t\treturn\n+\t\tfi\n+\tdone\n+\techo \"no\"\n+\treturn\n+}\n+\n+#\n # Update each submodule path to correct revision, using clone and checkout as needed\n #\n # $@ = requested paths (default to all)\n@@ -423,6 +444,7 @@ cmd_update()\n \t\tcmd_init \"--\" \"$@\" || return\n \tfi\n \n+\tcloned_modules=\n \tmodule_list \"$@\" |\n \twhile read mode sha1 stage path\n \tdo\n@@ -442,6 +464,7 @@ cmd_update()\n \t\tif ! test -d \"$path\"/.git -o -f \"$path\"/.git\n \t\tthen\n \t\t\tmodule_clone \"$path\" \"$url\" \"$reference\"|| exit\n+\t\t\tcloned_modules=\"$cloned_modules;$name\"\n \t\t\tsubsha1=\n \t\telse\n \t\t\tsubsha1=$(clear_local_git_env; cd \"$path\" &&\n@@ -469,6 +492,11 @@ cmd_update()\n \t\t\t\tdie \"Unable to fetch in submodule path '$path'\"\n \t\t\tfi\n \n+\t\t\tif test \"$(str_contains \"$name\" \"$cloned_modules\" '\\;')\" = \"yes\"\n+\t\t\tthen\n+\t\t\t\tupdate_module=\n+\t\t\tfi\n+\n \t\t\tcase \"$update_module\" in\n \t\t\trebase)\n \t\t\t\tcommand=\"git rebase\"\n-- \n1.7.4.1.42.g43f9f\n"},{"id":"161330","messageId":"7v62sjkbbi.fsf@alter.siamese.dyndns.org","threadId":"26522","inReplyTo":"1297860417-21895-1-git-send-email-olsonse@umich.edu","subject":"Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-16T19:51:13Z","receivedAt":"2011-02-16T19:51:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Spencer E. Olson\" <olsonse@umich.edu> writes:\n\n> Previously, when a submodule was cloned in the same command execution, --rebase\n> or --merge would attempt to run (instead of and) before a checkout, resulting in\n> an empty working directory.  This patch ignores --rebase or --merge for a\n> particular submodule when that submodule is newly cloned and instead simply\n> checks out the appropriate revision.\n\nSorry, but I cannot parse the problem description, \"(instead of and)\" part.\n\nWhy is it a better thing to do to ignore these options, instead of\ndetecting the situation and error it out, saying \"you are initially\ncloning, don't say --rebase\"?\n\n> +# Test whether an element of the $3(=IFS) separated list $2 matches $1\n> +#\n> +str_contains()\n> +{\n> +\tif test -n \"$3\"\n> +\tthen\n> +\t\tlocal IFS=\"$3\"\n> +\tfi\n> +\tfor i in $2\n> +\tdo\n> +\t\tif test \"$i\" = \"$1\"\n> +\t\tthen\n> +\t\t\techo \"yes\"\n> +\t\t\treturn\n> +\t\tfi\n> +\tdone\n> +\techo \"no\"\n> +\treturn\n> +}\n\nWe don't allow bash-ism outside POSIX shell in our primary Porcelain set\n(the rule is looser for stuff in contrib/), so \"local\" is out.\n\nHere is how to write the above more concisely, efficiently and portably.\n\n\tcase \"$2;\" in\n        *\";$1;\"*)\n        \techo yes ;;\n        *)\n        \techo no ;;\n\tesac\n\nThe trailing ';' takes care of the case where cloned_modules has only one\nelement, in which case you have \";name\" in \"$2\".  No need for a loop.\n\n> +\tcloned_modules=\n> ...\n>  \twhile read mode sha1 stage path\n>  \tdo\n> @@ -442,6 +464,7 @@ cmd_update()\n>  \t\tif ! test -d \"$path\"/.git -o -f \"$path\"/.git\n>  \t\tthen\n>  \t\t\tmodule_clone \"$path\" \"$url\" \"$reference\"|| exit\n> +\t\t\tcloned_modules=\"$cloned_modules;$name\"\n>  \t\t\tsubsha1=\n"},{"id":"161334","messageId":"4D5C2F33.8020309@web.de","threadId":"26522","inReplyTo":"7v62sjkbbi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-16T20:10:27Z","receivedAt":"2011-02-16T20:10:27Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 16.02.2011 20:51, schrieb Junio C Hamano:\n> \"Spencer E. Olson\" <olsonse@umich.edu> writes:\n> \n>> Previously, when a submodule was cloned in the same command execution, --rebase\n>> or --merge would attempt to run (instead of and) before a checkout, resulting in\n>> an empty working directory.  This patch ignores --rebase or --merge for a\n>> particular submodule when that submodule is newly cloned and instead simply\n>> checks out the appropriate revision.\n> \n> Sorry, but I cannot parse the problem description, \"(instead of and)\" part.\n\nI think what he wanted to say was that after a new submodule is cloned\nby running \"git submodule update\" it will attempt to do a rebase or\nmerge without having checked out the submodule at all, which obviously\ncan't work. A plain checkout looks like the right thing to do, as there\naren't any local commits to rebase or merge yet.\n\n> Why is it a better thing to do to ignore these options, instead of\n> detecting the situation and error it out, saying \"you are initially\n> cloning, don't say --rebase\"?\n\nIt should be fine to use these options when new submodules appear.\n(And even without explicitly specifing these command line options\nthis bug can also be triggered by having the \"submodule.<name>.update\"\noption set in .gitmodules to either \"rebase\" or \"merge\", which is easy\nto miss)\n\nSo this looks like a worthwhile fix. The commit message and POSIX issue\nneed to be addressed, tests would be a good thing to add too, but apart\nfrom that it looks sane.\n"},{"id":"161342","messageId":"7v7hczirnz.fsf@alter.siamese.dyndns.org","threadId":"26522","inReplyTo":"4D5C2F33.8020309@web.de","subject":"Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-16T21:41:04Z","receivedAt":"2011-02-16T21:41:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> So this looks like a worthwhile fix. The commit message and POSIX issue\n> need to be addressed, tests would be a good thing to add too, but apart\n> from that it looks sane.\n\nThanks for a translation ;-).  I agree everthing you said above.\n"},{"id":"161360","messageId":"201102161721.37389.olsonse@umich.edu","threadId":"26522","inReplyTo":"7v7hczirnz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned","fromName":"Spencer E. Olson","fromEmail":"olsonse@umich.edu","sentAt":"2011-02-17T00:21:37Z","receivedAt":"2011-02-17T00:21:37Z","isPatch":true,"sender":{"key":"olsonse@umich.edu","avatar":"https://avatars.githubusercontent.com/u/148647?v=4"},"body":"I'll fix the POSIX issue and commit message and re-submit.  (I wasn't aware \nthat for loops were from a bash-ism).\n\nOn Wednesday 16 February 2011 14:41, Junio C Hamano wrote:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n> > So this looks like a worthwhile fix. The commit message and POSIX issue\n> > need to be addressed, tests would be a good thing to add too, but apart\n> > from that it looks sane.\n>\n> Thanks for a translation ;-).  I agree everthing you said above.\n"},{"id":"161383","messageId":"4D5CC9C9.60705@viscovery.net","threadId":"26522","inReplyTo":"7v62sjkbbi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-02-17T07:10:01Z","receivedAt":"2011-02-17T07:10:01Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 2/16/2011 20:51, schrieb Junio C Hamano:\n> Here is how to write the above more concisely, efficiently and portably.\n> \n> \tcase \"$2;\" in\n>         *\";$1;\"*)\n>         \techo yes ;;\n>         *)\n>         \techo no ;;\n> \tesac\n> \n> The trailing ';' takes care of the case where cloned_modules has only one\n> element, in which case you have \";name\" in \"$2\".  No need for a loop.\n\nAnd while you are here, you could make this:\n\nlist_contains()\n{\n\tcase \"$2;\" in\n\t*\";$1;\"*)\n\t\t: yes ;;\n\t*)\n\t\t! : no ;;\n\tesac\n}\n\nand test for the exit code of this function rather than its output at the\ncall site.\n\n-- Hannes\n"},{"id":"161476","messageId":"alpine.DEB.2.00.1102171929200.14950@debian","threadId":"26522","inReplyTo":"4D5CC9C9.60705@viscovery.net","subject":"Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-18T00:34:39Z","receivedAt":"2011-02-18T00:34:39Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Thu, 17 Feb 2011, Johannes Sixt wrote:\n\n> Am 2/16/2011 20:51, schrieb Junio C Hamano:\n> > Here is how to write the above more concisely, efficiently and portably.\n> > \n> > \tcase \"$2;\" in\n> >         *\";$1;\"*)\n> >         \techo yes ;;\n> >         *)\n> >         \techo no ;;\n> > \tesac\n> > \n> > The trailing ';' takes care of the case where cloned_modules has only one\n> > element, in which case you have \";name\" in \"$2\".  No need for a loop.\n> \n> And while you are here, you could make this:\n> \n> list_contains()\n> {\n> \tcase \"$2;\" in\n> \t*\";$1;\"*)\n> \t\t: yes ;;\n> \t*)\n> \t\t! : no ;;\n> \tesac\n> }\n> \n> and test for the exit code of this function rather than its output at the\n> call site.\n\nAccording to Brandon Casey: \"Some platforms (IRIX 6.5, Solaris 7) do\nnot provide the 'yes' utility.\" See 8648732 (t/test-lib.sh: provide a\nshell implementation of the 'yes' utility, 2009-08-28).\n\n\n/Martin\n"},{"id":"161479","messageId":"201102171748.15516.olsonse@umich.edu","threadId":"26522","inReplyTo":"alpine.DEB.2.00.1102171929200.14950@debian","subject":"Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned","fromName":"Spencer E. Olson","fromEmail":"olsonse@umich.edu","sentAt":"2011-02-18T00:48:15Z","receivedAt":"2011-02-18T00:48:15Z","isPatch":true,"sender":{"key":"olsonse@umich.edu","avatar":"https://avatars.githubusercontent.com/u/148647?v=4"},"body":"We are not actually using the yes utility (think y\\ny\\ny\\ny\\ny\\ny\\n...)\n\nAs far as I understand, in this particular use, \"yes\" and \"no\" are just \ncomments.  We could also write\n: I like banannas ;;\nand\n! : I do not like anchovies ;;\n\nOn Thursday 17 February 2011 17:34, Martin von Zweigbergk wrote:\n> On Thu, 17 Feb 2011, Johannes Sixt wrote:\n> > Am 2/16/2011 20:51, schrieb Junio C Hamano:\n> > > Here is how to write the above more concisely, efficiently and\n> > > portably.\n> > >\n> > > \tcase \"$2;\" in\n> > >         *\";$1;\"*)\n> > >         \techo yes ;;\n> > >         *)\n> > >         \techo no ;;\n> > > \tesac\n> > >\n> > > The trailing ';' takes care of the case where cloned_modules has only\n> > > one element, in which case you have \";name\" in \"$2\".  No need for a\n> > > loop.\n> >\n> > And while you are here, you could make this:\n> >\n> > list_contains()\n> > {\n> > \tcase \"$2;\" in\n> > \t*\";$1;\"*)\n> >\n> > \t\t: yes ;;\n> >\n> > \t*)\n> > \t\t! : no ;;\n> > \tesac\n> > }\n> >\n> > and test for the exit code of this function rather than its output at the\n> > call site.\n>\n> According to Brandon Casey: \"Some platforms (IRIX 6.5, Solaris 7) do\n> not provide the 'yes' utility.\" See 8648732 (t/test-lib.sh: provide a\n> shell implementation of the 'yes' utility, 2009-08-28).\n>\n>\n> /Martin\n"},{"id":"161483","messageId":"alpine.DEB.2.00.1102171957350.14950@debian","threadId":"26522","inReplyTo":"201102171748.15516.olsonse@umich.edu","subject":"Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-18T00:59:02Z","receivedAt":"2011-02-18T00:59:02Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Thu, 17 Feb 2011, Spencer E. Olson wrote:\n\n> We are not actually using the yes utility (think y\\ny\\ny\\ny\\ny\\ny\\n...)\n> \n> As far as I understand, in this particular use, \"yes\" and \"no\" are just \n> comments.  We could also write\n> : I like banannas ;;\n> and\n> ! : I do not like anchovies ;;\n\nAh, I'm sure you're right. Need to read up on that colon operator. Thanks.\n\n> \n> On Thursday 17 February 2011 17:34, Martin von Zweigbergk wrote:\n> > On Thu, 17 Feb 2011, Johannes Sixt wrote:\n> > > Am 2/16/2011 20:51, schrieb Junio C Hamano:\n> > > > Here is how to write the above more concisely, efficiently and\n> > > > portably.\n> > > >\n> > > > \tcase \"$2;\" in\n> > > >         *\";$1;\"*)\n> > > >         \techo yes ;;\n> > > >         *)\n> > > >         \techo no ;;\n> > > > \tesac\n> > > >\n> > > > The trailing ';' takes care of the case where cloned_modules has only\n> > > > one element, in which case you have \";name\" in \"$2\".  No need for a\n> > > > loop.\n> > >\n> > > And while you are here, you could make this:\n> > >\n> > > list_contains()\n> > > {\n> > > \tcase \"$2;\" in\n> > > \t*\";$1;\"*)\n> > >\n> > > \t\t: yes ;;\n> > >\n> > > \t*)\n> > > \t\t! : no ;;\n> > > \tesac\n> > > }\n> > >\n> > > and test for the exit code of this function rather than its output at the\n> > > call site.\n> >\n> > According to Brandon Casey: \"Some platforms (IRIX 6.5, Solaris 7) do\n> > not provide the 'yes' utility.\" See 8648732 (t/test-lib.sh: provide a\n> > shell implementation of the 'yes' utility, 2009-08-28).\n> >\n> >\n> > /Martin\n> \n"}]}