{"thread":{"id":"25896","subject":"[RFC/PATCH] Re: git submodule -b ... of current HEAD fails","startedAt":"2010-12-01T18:50:46Z","lastAt":"2010-12-29T22:23:25Z","messageCount":16,"participants":["Jonathan Nieder","Jens Lehmann","Mark Levedahl","Ben Jackson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"156981","messageId":"20101201185046.GB27024@burratino","threadId":"25896","inReplyTo":"20101201171814.GC6439@ikki.ethgen.de","subject":"[RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-01T18:50:46Z","receivedAt":"2010-12-01T18:50:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"\tgit submodule add -b $branch $repository\n\nfails when HEAD already points to $branch in $repository.\n\nReported-by: Klaus Ethgen <Klaus@Ethgen.de>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nHi Klaus,\n\nKlaus Ethgen wrote at <http://bugs.debian.org/605600>:\n\n> Strange problem, if I create a submodule of an other repository giving\n> the currently used HEAD branch I get the error: »fatal: git checkout:\n> branch myimbabranch already exists« while when giving other branch\n> work well.\n\nInteresting.  The problem is in cmd_add of git-submodule.sh; this\npatch demonstrates a quick fix.  Jens, any idea why git submodule\nis not using \"clone --branch\" directly?\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 33bc41f..6242d7f 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -241,7 +241,7 @@ cmd_add()\n \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n \t\t\tcase \"$branch\" in\n \t\t\t'') git checkout -f -q ;;\n-\t\t\t?*) git checkout -f -q -b \"$branch\" \"origin/$branch\" ;;\n+\t\t\t?*) git checkout -f -q -B \"$branch\" \"origin/$branch\" ;;\n \t\t\tesac\n \t\t) || die \"Unable to checkout submodule '$path'\"\n \tfi\n"},{"id":"157156","messageId":"4CF80B71.3010309@web.de","threadId":"25896","inReplyTo":"20101201185046.GB27024@burratino","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-12-02T21:11:13Z","receivedAt":"2010-12-02T21:11:13Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 01.12.2010 19:50, schrieb Jonathan Nieder:\n> \tgit submodule add -b $branch $repository\n> \n> fails when HEAD already points to $branch in $repository.\n> \n> Reported-by: Klaus Ethgen <Klaus@Ethgen.de>\n> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n> ---\n> Hi Klaus,\n> \n> Klaus Ethgen wrote at <http://bugs.debian.org/605600>:\n> \n>> Strange problem, if I create a submodule of an other repository giving\n>> the currently used HEAD branch I get the error: »fatal: git checkout:\n>> branch myimbabranch already exists« while when giving other branch\n>> work well.\n> \n> Interesting.  The problem is in cmd_add of git-submodule.sh; this\n> patch demonstrates a quick fix.  Jens, any idea why git submodule\n> is not using \"clone --branch\" directly?\n\nNope, these lines date back to the time before I got involved in the\nsubmodule business ... Seems like this \"git checkout\" was added in\nMarch 2008 by Mark Levedahl (CCed), maybe he can shed some light on\nthat.\n\nBut to me your change looks good, so feel free to add:\nAcked-by: Jens Lehmann <Jens.Lehmann@web.de>\n\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 33bc41f..6242d7f 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -241,7 +241,7 @@ cmd_add()\n>  \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n>  \t\t\tcase \"$branch\" in\n>  \t\t\t'') git checkout -f -q ;;\n> -\t\t\t?*) git checkout -f -q -b \"$branch\" \"origin/$branch\" ;;\n> +\t\t\t?*) git checkout -f -q -B \"$branch\" \"origin/$branch\" ;;\n>  \t\t\tesac\n>  \t\t) || die \"Unable to checkout submodule '$path'\"\n>  \tfi\n> \n"},{"id":"157165","messageId":"4CF844E5.5010808@gmail.com","threadId":"25896","inReplyTo":"4CF80B71.3010309@web.de","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2010-12-03T01:16:21Z","receivedAt":"2010-12-03T01:16:21Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"On 12/02/2010 04:11 PM, Jens Lehmann wrote:\n> Nope, these lines date back to the time before I got involved in the\n> submodule business ... Seems like this \"git checkout\" was added in\n> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on\n> that.\n>\n> But to me your change looks good, so feel free to add:\n> Acked-by: Jens Lehmann<Jens.Lehmann@web.de>\n>\n>\n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 33bc41f..6242d7f 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -241,7 +241,7 @@ cmd_add()\n>>   \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n>>   \t\t\tcase \"$branch\" in\n>>   \t\t\t'') git checkout -f -q ;;\n>> -\t\t\t?*) git checkout -f -q -b \"$branch\" \"origin/$branch\" ;;\n>> +\t\t\t?*) git checkout -f -q -B \"$branch\" \"origin/$branch\" ;;\n>>   \t\t\tesac\n>>   \t\t) || die \"Unable to checkout submodule '$path'\"\n>>   \tfi\n>>\nThese lines were actually added by Ben Jackson in commit ea10b60c91 in \n2009, long after I last touched that module.\n\nMark\n"},{"id":"157167","messageId":"20101203012155.GA30999@kronos.home.ben.com","threadId":"25896","inReplyTo":"4CF844E5.5010808@gmail.com","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Ben Jackson","fromEmail":"ben@ben.com","sentAt":"2010-12-03T01:21:56Z","receivedAt":"2010-12-03T01:21:56Z","isPatch":true,"sender":{"key":"ben@ben.com","avatar":"https://gravatar.com/avatar/df49904dd23b03a5f57d9d53c0bf9fb6f69a14fac075c98f54f26cf1ce960794?d=mp&s=160"},"body":"On Thu, Dec 02, 2010 at 08:16:21PM -0500, Mark Levedahl wrote:\n> On 12/02/2010 04:11 PM, Jens Lehmann wrote:\n> > Nope, these lines date back to the time before I got involved in the\n> > submodule business ... Seems like this \"git checkout\" was added in\n> > March 2008 by Mark Levedahl (CCed), maybe he can shed some light on\n> > that.\n> >\n> > But to me your change looks good, so feel free to add:\n> > Acked-by: Jens Lehmann<Jens.Lehmann@web.de>\n> >\n> >\n> >> diff --git a/git-submodule.sh b/git-submodule.sh\n> >> index 33bc41f..6242d7f 100755\n> >> --- a/git-submodule.sh\n> >> +++ b/git-submodule.sh\n> >> @@ -241,7 +241,7 @@ cmd_add()\n> >>   \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n> >>   \t\t\tcase \"$branch\" in\n> >>   \t\t\t'') git checkout -f -q ;;\n> >> -\t\t\t?*) git checkout -f -q -b \"$branch\" \"origin/$branch\" ;;\n> >> +\t\t\t?*) git checkout -f -q -B \"$branch\" \"origin/$branch\" ;;\n> >>   \t\t\tesac\n> >>   \t\t) || die \"Unable to checkout submodule '$path'\"\n> >>   \tfi\n> >>\n> These lines were actually added by Ben Jackson in commit ea10b60c91 in \n> 2009, long after I last touched that module.\n\nI didn't mean to change any functionality -- I just wanted to fix a\nportability problem (/bin/sh is ash on FreeBSD, hence the comment at the\ntop of the context).  Looks like `checkout -B' (capital B) didn't even\nexist at that time.  Seems reasonable, though.\n\n-- \nBen Jackson AD7GD\n<ben@ben.com>\nhttp://www.ben.com/\n"},{"id":"157172","messageId":"20101203071037.GA18202@burratino","threadId":"25896","inReplyTo":"4CF80B71.3010309@web.de","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-03T07:10:37Z","receivedAt":"2010-12-03T07:10:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jens Lehmann wrote:\n> Am 01.12.2010 19:50, schrieb Jonathan Nieder:\n\n>>                                  Jens, any idea why git submodule\n>> is not using \"clone --branch\" directly?\n>\n> Nope, these lines date back to the time before I got involved in the\n> submodule business ... Seems like this \"git checkout\" was added in\n> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on\n> that.\n\nAh, so the problem is that \"clone --branch\" did not exist.  Sorry for\nthe noise.\n\nAnother question can be also be easily answered by history examination:\nthe series of checks in module_clone are because 70c7ac22d:git-clone.sh\ndid not have checks of its own for the target directory.\n\nSo there is some simplification within grasp.\n\n'night,\nJonathan\n"},{"id":"157331","messageId":"4CFACE67.1080206@web.de","threadId":"25896","inReplyTo":"20101203071037.GA18202@burratino","subject":"[PATCH] git submodule: Remove now obsolete tests before cloning a repo","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-12-04T23:27:35Z","receivedAt":"2010-12-04T23:27:35Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Since 55892d23 \"git clone\" itself checks that the destination path is not\na file but an empty directory if it exists, so there is no need anymore\nfor module_clone() to check that too.\n\nTwo tests have been added to test the behavior of \"git submodule add\" when\npath is a file or a directory (A subshell had to be added to the former\nlast test to stay in the right directory).\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nAm 03.12.2010 08:10, schrieb Jonathan Nieder:\n> Jens Lehmann wrote:\n>> Am 01.12.2010 19:50, schrieb Jonathan Nieder:\n> \n>>>                                  Jens, any idea why git submodule\n>>> is not using \"clone --branch\" directly?\n>>\n>> Nope, these lines date back to the time before I got involved in the\n>> submodule business ... Seems like this \"git checkout\" was added in\n>> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on\n>> that.\n> \n> Ah, so the problem is that \"clone --branch\" did not exist.  Sorry for\n> the noise.\n> \n> Another question can be also be easily answered by history examination:\n> the series of checks in module_clone are because 70c7ac22d:git-clone.sh\n> did not have checks of its own for the target directory.\n> \n> So there is some simplification within grasp.\n\nMaybe something like this?\n\n\n git-submodule.sh           |   14 --------------\n t/t7400-submodule-basic.sh |   28 +++++++++++++++++++++++-----\n 2 files changed, 23 insertions(+), 19 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 33bc41f..8085876 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -93,20 +93,6 @@ module_clone()\n \turl=$2\n \treference=\"$3\"\n\n-\t# If there already is a directory at the submodule path,\n-\t# expect it to be empty (since that is the default checkout\n-\t# action) and try to remove it.\n-\t# Note: if $path is a symlink to a directory the test will\n-\t# succeed but the rmdir will fail. We might want to fix this.\n-\tif test -d \"$path\"\n-\tthen\n-\t\trmdir \"$path\" 2>/dev/null ||\n-\t\tdie \"Directory '$path' exists, but is neither empty nor a git repository\"\n-\tfi\n-\n-\ttest -e \"$path\" &&\n-\tdie \"A file already exist at path '$path'\"\n-\n \tif test -n \"$reference\"\n \tthen\n \t\tgit-clone \"$reference\" -n \"$url\" \"$path\"\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 782b0a3..2c49db9 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -421,11 +421,29 @@ test_expect_success 'add submodules without specifying an explicit path' '\n \t\tgit commit -m \"repo commit 1\"\n \t) &&\n \tgit clone --bare repo/ bare.git &&\n-\tcd addtest &&\n-\tgit submodule add \"$submodurl/repo\" &&\n-\tgit config -f .gitmodules submodule.repo.path repo &&\n-\tgit submodule add \"$submodurl/bare.git\" &&\n-\tgit config -f .gitmodules submodule.bare.path bare\n+\t(\n+\t\tcd addtest &&\n+\t\tgit submodule add \"$submodurl/repo\" &&\n+\t\tgit config -f .gitmodules submodule.repo.path repo &&\n+\t\tgit submodule add \"$submodurl/bare.git\" &&\n+\t\tgit config -f .gitmodules submodule.bare.path bare\n+\t)\n+'\n+\n+test_expect_success 'add should fail when path is used by a file' '\n+\t(\n+\t\tcd addtest &&\n+\t\ttouch file &&\n+\t\ttest_must_fail\tgit submodule add \"$submodurl/repo\" file\n+\t)\n+'\n+\n+test_expect_success 'add should fail when path is used by an existing directory' '\n+\t(\n+\t\tcd addtest &&\n+\t\tmkdir empty-dir &&\n+\t\ttest_must_fail git submodule add \"$submodurl/repo\" empty-dir\n+\t)\n '\n\n test_done\n-- \n1.7.3.2.657.g92628.dirty\n"},{"id":"157552","messageId":"7vipz5nqd0.fsf@alter.siamese.dyndns.org","threadId":"25896","inReplyTo":"4CF80B71.3010309@web.de","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-07T22:57:15Z","receivedAt":"2010-12-07T22:57:15Z","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> Nope, these lines date back to the time before I got involved in the\n> submodule business ... Seems like this \"git checkout\" was added in\n> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on\n> that.\n>\n> But to me your change looks good, so feel free to add:\n> Acked-by: Jens Lehmann <Jens.Lehmann@web.de>\n\nDoes either of you want to add a test for this?\n"},{"id":"157630","messageId":"4CFFFA05.6070609@web.de","threadId":"25896","inReplyTo":"7vipz5nqd0.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-12-08T21:35:01Z","receivedAt":"2010-12-08T21:35:01Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 07.12.2010 23:57, schrieb Junio C Hamano:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n> \n>> Nope, these lines date back to the time before I got involved in the\n>> submodule business ... Seems like this \"git checkout\" was added in\n>> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on\n>> that.\n>>\n>> But to me your change looks good, so feel free to add:\n>> Acked-by: Jens Lehmann <Jens.Lehmann@web.de>\n> \n> Does either of you want to add a test for this?\n\nWill do.\n"},{"id":"157648","messageId":"4D001292.3020503@web.de","threadId":"25896","inReplyTo":"4CFFFA05.6070609@web.de","subject":"[PATCH] git submodule -b ... of current HEAD fails","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-12-08T23:19:46Z","receivedAt":"2010-12-08T23:19:46Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"\tgit submodule add -b $branch $repository\n\nfails when HEAD already points to $branch in $repository.\n\nWhen the freshly cloned submodules HEAD is the same as the checked out\nbranch, it doesn't make sense to update it again as \"git checkout -b\"\nwould fail with »fatal: git checkout: branch $branch already exists«.\n\nReported-by: Klaus Ethgen <Klaus@Ethgen.de>\nThanks-to: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nAm 08.12.2010 22:35, schrieb Jens Lehmann:\n> Am 07.12.2010 23:57, schrieb Junio C Hamano:\n>> Jens Lehmann <Jens.Lehmann@web.de> writes:\n>>\n>>> Nope, these lines date back to the time before I got involved in the\n>>> submodule business ... Seems like this \"git checkout\" was added in\n>>> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on\n>>> that.\n>>>\n>>> But to me your change looks good, so feel free to add:\n>>> Acked-by: Jens Lehmann <Jens.Lehmann@web.de>\n>>\n>> Does either of you want to add a test for this?\n> \n> Will do.\n\nAnd as it happens from time to time, while writing the test you find\nout that the first attempt to fix the bug didn't work as expected ...\n\n git-submodule.sh           |    4 +++-\n t/t7400-submodule-basic.sh |    7 +++++++\n 2 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 33bc41f..bf2803f 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -241,7 +241,9 @@ cmd_add()\n \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n \t\t\tcase \"$branch\" in\n \t\t\t'') git checkout -f -q ;;\n-\t\t\t?*) git checkout -f -q -b \"$branch\" \"origin/$branch\" ;;\n+\t\t\t?*) if [ \"$(git branch)\" != \"* $branch\"  ]; then\n+\t\t\t\tgit checkout -f -q -b \"$branch\" \"origin/$branch\"\n+\t\t\tfi ;;\n \t\t\tesac\n \t\t) || die \"Unable to checkout submodule '$path'\"\n \tfi\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 782b0a3..e224da4 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -131,6 +131,13 @@ test_expect_success 'submodule add --branch' '\n \ttest_cmp empty untracked\n '\n\n+test_expect_success 'submodule add --branch succeeds even when branch is at HEAD' '\n+\t(\n+\t\tcd addtest &&\n+\t\tgit submodule add -b master \"$submodurl\" submod-existing-branch\n+\t)\n+'\n+\n test_expect_success 'submodule add with ./ in path' '\n \techo \"refs/heads/master\" >expect &&\n \t>empty &&\n-- \n1.7.3.3.580.ged75d\n"},{"id":"157650","messageId":"20101208234508.GB8865@burratino","threadId":"25896","inReplyTo":"4D001292.3020503@web.de","subject":"Re: [PATCH] git submodule -b ... of current HEAD fails","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-08T23:45:08Z","receivedAt":"2010-12-08T23:45:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jens Lehmann wrote:\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -241,7 +241,9 @@ cmd_add()\n>  \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n>  \t\t\tcase \"$branch\" in\n>  \t\t\t'') git checkout -f -q ;;\n> -\t\t\t?*) git checkout -f -q -b \"$branch\" \"origin/$branch\" ;;\n> +\t\t\t?*) if [ \"$(git branch)\" != \"* $branch\"  ]; then\n\nAgh.  The command to use is \"git symbolic-ref -q HEAD\", I suppose.\n\nMaybe we can simplify by relying on \"git clone\".\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n git-submodule.sh           |   16 ++++++----------\n t/t7400-submodule-basic.sh |    8 +++++++-\n 2 files changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex d937f0b..6fd09e7 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -219,17 +219,13 @@ cmd_add()\n \t\tesac\n \t\tgit config submodule.\"$path\".url \"$url\"\n \telse\n-\n-\t\tmodule_clone \"$path\" \"$realrepo\" \"$reference\" || exit\n+\t\tbrancharg=${branch:+\"-b $(git rev-parse --sq-quote \"$branch\")\"}\n+\t\trefarg=${reference:+\"$(git rev-parse --sq-quote \"$reference\")\"}\n \t\t(\n-\t\t\tclear_local_git_env\n-\t\t\tcd \"$path\" &&\n-\t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n-\t\t\tcase \"$branch\" in\n-\t\t\t'') git checkout -f -q ;;\n-\t\t\t?*) git checkout -f -q -b \"$branch\" \"origin/$branch\" ;;\n-\t\t\tesac\n-\t\t) || die \"Unable to checkout submodule '$path'\"\n+\t\t\texport realrepo path &&\n+\t\t\teval \"git clone $brancharg $refarg\" '\"$realrepo\" \"$path\"'\n+\t\t) ||\n+\t\tdie \"Clone of '$realrepo' into submodule path '$path' failed\"\n \tfi\n \n \tgit add $force \"$path\" ||\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 2c49db9..77088cc 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -114,7 +114,6 @@ test_expect_success 'submodule add --branch' '\n \techo \"refs/heads/initial\" >expect-head &&\n \tcat <<-\\EOF >expect-heads &&\n \trefs/heads/initial\n-\trefs/heads/master\n \tEOF\n \t>empty &&\n \n@@ -131,6 +130,13 @@ test_expect_success 'submodule add --branch' '\n \ttest_cmp empty untracked\n '\n \n+test_expect_success 'submodule add --branch succeeds even when branch is at HEAD' '\n+\t(\n+\t\tcd addtest &&\n+\t\tgit submodule add -b master \"$submodurl\" submod-existing-branch\n+\t)\n+'\n+\n test_expect_success 'submodule add with ./ in path' '\n \techo \"refs/heads/master\" >expect &&\n \t>empty &&\n"},{"id":"158688","messageId":"7vipydwp50.fsf@alter.siamese.dyndns.org","threadId":"25896","inReplyTo":"20101201185046.GB27024@burratino","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-28T21:42:19Z","receivedAt":"2010-12-28T21:42:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> \tgit submodule add -b $branch $repository\n>\n> fails when HEAD already points to $branch in $repository.\n\nI was reviewing the overall picture before tagging 1.7.4-rc0 and started\nwondering if this was a good change.  If repository already had branch\nchecked out, and if it was pointing at a commit that was different from\nwhatever was taken from origin, shouldn't the command _fail_ to prevent\nthe divergent commits from getting lost?\n"},{"id":"158697","messageId":"4D1A7B42.1050907@web.de","threadId":"25896","inReplyTo":"7vipydwp50.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-12-29T00:05:22Z","receivedAt":"2010-12-29T00:05:22Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 28.12.2010 22:42, schrieb Junio C Hamano:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n> \n>> \tgit submodule add -b $branch $repository\n>>\n>> fails when HEAD already points to $branch in $repository.\n> \n> I was reviewing the overall picture before tagging 1.7.4-rc0 and started\n> wondering if this was a good change.  If repository already had branch\n> checked out, and if it was pointing at a commit that was different from\n> whatever was taken from origin, shouldn't the command _fail_ to prevent\n> the divergent commits from getting lost?\n\nBut doesn't this change only affect the case where the submodule is\nfreshly cloned, so there is no chance of losing local changes?\n\n(But AFAIK the patch doesn't really fix the issue, please see [1] and\nJonathan's followup)\n\n[1] http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/772659/focus=163242\n"},{"id":"158698","messageId":"7vlj39to1t.fsf@alter.siamese.dyndns.org","threadId":"25896","inReplyTo":"4D1A7B42.1050907@web.de","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-29T00:34:06Z","receivedAt":"2010-12-29T00:34:06Z","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> But doesn't this change only affect the case where the submodule is\n> freshly cloned, so there is no chance of losing local changes?\n\nMy fault, for not looking at the surrounding context.\n\n> (But AFAIK the patch doesn't really fix the issue, please see [1] and\n> Jonathan's followup)\n>\n> [1] http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/772659/focus=163242\n\nI think we queued the later round just uses \"checkout -B\"; shouldn't that\nwork?\n"},{"id":"158704","messageId":"4D1AF989.3000105@web.de","threadId":"25896","inReplyTo":"7vlj39to1t.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-12-29T09:04:09Z","receivedAt":"2010-12-29T09:04:09Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 29.12.2010 01:34, schrieb Junio C Hamano:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n>> (But AFAIK the patch doesn't really fix the issue, please see [1] and\n>> Jonathan's followup)\n>>\n>> [1] http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/772659/focus=163242\n> \n> I think we queued the later round just uses \"checkout -B\"; shouldn't that\n> work?\n\nThat's what I thought too (and that is what I based my ack upon, the\npatch hit the right spot and used the - according to the documentation\n- better suited -B option). But while writing the test you rightfully\nrequested I noticed that using -B didn't change much. The reason is\nthat the following commands both fail in a freshly cloned repo:\n\n$ git checkout -f -q -b master origin/master\nfatal: git checkout: branch master already exists\n$ git checkout -f -q -B master origin/master\nfatal: Cannot force update the current branch.\n\nSo maybe the real problem here is that \"git checkout -B\" barfs when it\ndoesn't have anything to do instead of silently doing nothing?\n\nSo we maybe want to fix this issue in \"git checkout\"? Then the patch\nwill start working (and the test for it can be added in a later patch).\n"},{"id":"158727","messageId":"7voc84s3ks.fsf_-_@alter.siamese.dyndns.org","threadId":"25896","inReplyTo":"4D1AF989.3000105@web.de","subject":"Re* [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-29T20:53:55Z","receivedAt":"2010-12-29T20:53:55Z","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> $ git checkout -f -q -b master origin/master\n> fatal: git checkout: branch master already exists\n> $ git checkout -f -q -B master origin/master\n> fatal: Cannot force update the current branch.\n>\n> So maybe the real problem here is that \"git checkout -B\" barfs when it\n> doesn't have anything to do instead of silently doing nothing?\n>\n> So we maybe want to fix this issue in \"git checkout\"? Then the patch\n> will start working (and the test for it can be added in a later patch).\n\nLet me think aloud, as it may be a good time to summarize the current\nsemantics of what \"checking out a branch\" means.\n\n 0. \"git checkout $branch\" to check out a branch is to start preparing to\n    commit on that branch while keeping the tentative changes you have in\n    your working tree and your index.\n\n 1. You might not have the $branch yet to check out in such a way.  You\n    could do\n\n    $ git branch $branch $start && git checkout $branch\n\n    and\n\n    $ git checkout -b $branch $start\n\n    is conceptually a short-hand of the above two command sequence.\n\n 3. When $branch exists, 2. should fail, as it involves resetting the tip\n    of $branch to $start, potentially losing commits near its old tip.\n    If you really mean it, you can force it by\n\n    $ git checkout -B $branch $start\n\n    which is conceptually a short-hand for:\n\n    $ git branch -f $branch $start && git checkout $branch\n\n 4. \"git checkout $branch\" refuses to discard your work when the\n    tentative changes you made conflict with the difference between your\n    current HEAD and $branch.  You can give \"-m\" to force it to attempt a\n    three-way merge with possible conflicts, or you can give \"-f\" to force\n    it to discard all tentative changes.\n\n 5. The logical consequence of all of the above is that \"git checkout [-f]\n    -B $branch $start\" should conceptually be a short-hand for:\n\n    $ git branch -f $branch $start && git checkout [-f] $branch\n\nAnd \"branch -f $branch\" refuses to reset the tip of the current branch.\n\nAn end-user who runs \"git checkout -f -B $branch $start\" is telling us\nthe following things:\n\n - The $branch may or may not exist;\n\n - The tip it currently may point at is immaterial and the user is willing\n   to lose it (this is what capital-ness of -B means);\n\n - The tentative changes in the working tree and the index is immaterial\n   (this is what -f given to checkout means).\n\nIt seems to me that \"checkout -f -B $branch $start\" when $branch is the\ncurrent one ought to do an equivalent of \"reset --hard $start\" and nothing\nelse.\n\nWhat happens if -f is not given?  The user wants to keep the tentative\nchanges, and that is done by comparing a commit in HEAD (i.e. the current\nbranch) and the target $branch.  The two step approach that can be\nillustrated by the short-hand definition of \"checkout -B\" however breaks\ndown in this case, even if you change the variant of \"git branch -f\" it\ninternally runs:\n\n    $ git branch --allow-updating-current -f $branch $start &&\n      git checkout $branch\n\nThe first step already resets $branch to its new value, and the second\nstep does not have a way to know that your tentative changes are based on\nthe previous value.\n\nBUT ;-)\n\nI think it is Ok after all. The actual implementation of \"checkout -[b|B]\"\nis more like:\n\n    $ git read-tree -m -u HEAD $start\n    $ git update-ref -f refs/heads/$branch $start\n    $ git symbolic-ref HEAD refs/heads/$branch\n\nThat is, when we update the working tree and the index, we still haven't\nupdated the HEAD yet (switch_branches() calls merge_working_tree() to run\nthis two-way merge, and then calls update_refs_for_switch() which does the\nupdate-ref part including the branch creation).\n\nThe next question is if we want to enable \"git branch -f $branch $start\"\nfor the current branch from the end user, which presumably would look as\nif the user ran \"git reset $start\".  I am actually indifferent about this,\nbut as the first step we probably should be conservative to forbid it as\nwe do now.\n\nSo in conclusion, here is a patch that is not even compile tested ;-)\n\nThis was made on top of 'maint' only because that was the branch I\nhappened to have checked out in my development working tree.\n\nNote that the test script does not even test the case I was initially\nworried about (i.e. keeping local changes).  I'll leave it to interested\nothers ;-)\n\n branch.c                   |   12 ++++++++----\n branch.h                   |   23 +++++++++++++++--------\n builtin/branch.c           |    3 ++-\n builtin/checkout.c         |    4 +++-\n t/t2018-checkout-branch.sh |   13 +++++++++++++\n 5 files changed, 41 insertions(+), 14 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 93dc866..74eb866 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -137,7 +137,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,\n \n void create_branch(const char *head,\n \t\t   const char *name, const char *start_name,\n-\t\t   int force, int reflog, enum branch_track track)\n+\t\t   int flags, int reflog, enum branch_track track)\n {\n \tstruct ref_lock *lock = NULL;\n \tstruct commit *commit;\n@@ -147,6 +147,9 @@ void create_branch(const char *head,\n \tint forcing = 0;\n \tint dont_change_ref = 0;\n \tint explicit_tracking = 0;\n+\tint ok_to_update = flags & (CREATE_BRANCH_UPDATE_OK |\n+\t\t\t\t    CREATE_BRANCH_UPDATE_CURRENT_OK);\n+\tint ok_to_update_current = flags & CREATE_BRANCH_UPDATE_CURRENT_OK;\n \n \tif (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)\n \t\texplicit_tracking = 1;\n@@ -155,11 +158,12 @@ void create_branch(const char *head,\n \t\tdie(\"'%s' is not a valid branch name.\", name);\n \n \tif (resolve_ref(ref.buf, sha1, 1, NULL)) {\n-\t\tif (!force && track == BRANCH_TRACK_OVERRIDE)\n+\t\tif (!ok_to_update && track == BRANCH_TRACK_OVERRIDE)\n \t\t\tdont_change_ref = 1;\n-\t\telse if (!force)\n+\t\telse if (!ok_to_update)\n \t\t\tdie(\"A branch named '%s' already exists.\", name);\n-\t\telse if (!is_bare_repository() && head && !strcmp(head, name))\n+\t\telse if (!ok_to_update_current &&\n+\t\t\t !is_bare_repository() && head && !strcmp(head, name))\n \t\t\tdie(\"Cannot force update the current branch.\");\n \t\tforcing = 1;\n \t}\ndiff --git a/branch.h b/branch.h\nindex eed817a..0afbeae 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -4,16 +4,23 @@\n /* Functions for acting on the information about branches. */\n \n /*\n- * Creates a new branch, where head is the branch currently checked\n- * out, name is the new branch name, start_name is the name of the\n- * existing branch that the new branch should start from, force\n- * enables overwriting an existing (non-head) branch, reflog creates a\n- * reflog for the branch, and track causes the new branch to be\n- * configured to merge the remote branch that start_name is a tracking\n- * branch for (if any).\n+ * Creates a new branch, where:\n+ *\n+ * - head is the branch currently checked out;\n+ * - name is the new branch name;\n+ * - start_name is the name of a commit that the new branch should start at\n+ *   (could be another branch or a remote-tracking branch, in which case\n+ *   track---see below---may also trigger);\n+ * - flags indicates overwriting an existing branch and/or overwriting the\n+ *   current branch is allowed;\n+ * - reflog creates a reflog for the branch; and\n+ * - track causes the new branch to be configured to merge the remote branch\n+ *   that start_name is a tracking branch for (if any).\n  */\n+#define CREATE_BRANCH_UPDATE_OK 01\n+#define CREATE_BRANCH_UPDATE_CURRENT_OK 02\n void create_branch(const char *head, const char *name, const char *start_name,\n-\t\t   int force, int reflog, enum branch_track track);\n+\t\t   int flags, int reflog, enum branch_track track);\n \n /*\n  * Remove information about the state of working on the current\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 87976f0..ea5b411 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -651,7 +651,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('m', NULL, &rename, \"move/rename a branch and its reflog\", 1),\n \t\tOPT_BIT('M', NULL, &rename, \"move/rename a branch, even if target exists\", 2),\n \t\tOPT_BOOLEAN('l', NULL, &reflog, \"create the branch's reflog\"),\n-\t\tOPT_BOOLEAN('f', \"force\", &force_create, \"force creation (when already exists)\"),\n+\t\tOPT_SET_INT('f', \"force\", &force_create, \"force creation (when already exists)\",\n+\t\t\t    CREATE_BRANCH_UPDATE_OK),\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"no-merged\", &merge_filter_ref,\n \t\t\t\"commit\", \"print only not merged branches\",\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex a54583b..bf5b43a 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -529,7 +529,9 @@ static void update_refs_for_switch(struct checkout_opts *opts,\n \t\t}\n \t\telse\n \t\t\tcreate_branch(old->name, opts->new_branch, new->name,\n-\t\t\t\t      opts->new_branch_force ? 1 : 0,\n+\t\t\t\t      opts->new_branch_force\n+\t\t\t\t      ? CREATE_BRANCH_UPDATE_CURRENT_OK\n+\t\t\t\t      : 0,\n \t\t\t\t      opts->new_branch_log, opts->track);\n \t\tnew->name = opts->new_branch;\n \t\tsetup_branch_path(new);\ndiff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh\nindex fa69016..37e4fdb 100755\n--- a/t/t2018-checkout-branch.sh\n+++ b/t/t2018-checkout-branch.sh\n@@ -169,4 +169,17 @@ test_expect_success 'checkout -f -B to an existing branch with mergeable changes\n \ttest_must_fail test_dirty_mergeable\n '\n \n+test_expect_success 'checkout -b/B the current branch' '\n+\tgit reset --hard &&\n+\tgit checkout branch1 &&\n+\tit=$(git rev-parse --verify HEAD) &&\n+\ttest_must_fail git checkout -b branch1 HEAD &&\n+\tgit checkout -B branch1 $it &&\n+\ttest \"$it\" = \"$(git rev-parse --verify HEAD)\" &&\n+\ttest \"refs/heads/branch1\" = \"$(git symbolic-ref HEAD)\" &&\n+\tgit checkout -B branch1 HEAD &&\n+\ttest \"$it\" = \"$(git rev-parse --verify HEAD)\" &&\n+\ttest \"refs/heads/branch1\" = \"$(git symbolic-ref HEAD)\"\n+'\n+\n test_done\n"},{"id":"158734","messageId":"4D1BB4DD.2000800@web.de","threadId":"25896","inReplyTo":"4D1BB26D.1010502@web.de","subject":"Re: Re* [RFC/PATCH] Re: git submodule -b ... of current HEAD fails","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-12-29T22:23:25Z","receivedAt":"2010-12-29T22:23:25Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 29.12.2010 23:13, schrieb Jens Lehmann:\n> Am 29.12.2010 21:53, schrieb Junio C Hamano:\n>> Jens Lehmann <Jens.Lehmann@web.de> writes:\n>>> So we maybe want to fix this issue in \"git checkout\"? Then the patch\n>>> will start working (and the test for it can be added in a later patch).\n>>\n>> So in conclusion, here is a patch that is not even compile tested ;-)\n\nJust for the record: It does compile (at least for me ;-) and together\nwith Jonathan's patch referenced in the the subject passes all tests,\nincluding the new 'submodule add --branch succeeds even when branch is\nat HEAD' test I came up with to verify this issue.\n"}]}