[RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Jonathan Nieder]

	git submodule add -b $branch $repository

fails when HEAD already points to $branch in $repository.

Reported-by: Klaus Ethgen <Klaus@Ethgen.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi Klaus,

Klaus Ethgen wrote at <http://bugs.debian.org/605600>:

> Strange problem, if I create a submodule of an other repository giving
> the currently used HEAD branch I get the error: »fatal: git checkout:
> branch myimbabranch already exists« while when giving other branch
> work well.

Interesting.  The problem is in cmd_add of git-submodule.sh; this
patch demonstrates a quick fix.  Jens, any idea why git submodule
is not using "clone --branch" directly?

diff --git a/git-submodule.sh b/git-submodule.sh
index 33bc41f..6242d7f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -241,7 +241,7 @@ cmd_add()
 			# ash fails to wordsplit ${branch:+-b "$branch"...}
 			case "$branch" in
 			'') git checkout -f -q ;;
-			?*) git checkout -f -q -b "$branch" "origin/$branch" ;;
+			?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
 			esac
 		) || die "Unable to checkout submodule '$path'"
 	fi

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Jens Lehmann]

Am 01.12.2010 19:50, schrieb Jonathan Nieder:
> 	git submodule add -b $branch $repository
> 
> fails when HEAD already points to $branch in $repository.
> 
> Reported-by: Klaus Ethgen <Klaus@Ethgen.de>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Hi Klaus,
> 
> Klaus Ethgen wrote at <http://bugs.debian.org/605600>:
> 
>> Strange problem, if I create a submodule of an other repository giving
>> the currently used HEAD branch I get the error: »fatal: git checkout:
>> branch myimbabranch already exists« while when giving other branch
>> work well.
> 
> Interesting.  The problem is in cmd_add of git-submodule.sh; this
> patch demonstrates a quick fix.  Jens, any idea why git submodule
> is not using "clone --branch" directly?

Nope, these lines date back to the time before I got involved in the
submodule business ... Seems like this "git checkout" was added in
March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
that.

But to me your change looks good, so feel free to add:
Acked-by: Jens Lehmann <Jens.Lehmann@web.de>


> diff --git a/git-submodule.sh b/git-submodule.sh
> index 33bc41f..6242d7f 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -241,7 +241,7 @@ cmd_add()
>  			# ash fails to wordsplit ${branch:+-b "$branch"...}
>  			case "$branch" in
>  			'') git checkout -f -q ;;
> -			?*) git checkout -f -q -b "$branch" "origin/$branch" ;;
> +			?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
>  			esac
>  		) || die "Unable to checkout submodule '$path'"
>  	fi
> 

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Mark Levedahl]

On 12/02/2010 04:11 PM, Jens Lehmann wrote:
> Nope, these lines date back to the time before I got involved in the
> submodule business ... Seems like this "git checkout" was added in
> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
> that.
>
> But to me your change looks good, so feel free to add:
> Acked-by: Jens Lehmann<Jens.Lehmann@web.de>
>
>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 33bc41f..6242d7f 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -241,7 +241,7 @@ cmd_add()
>>   			# ash fails to wordsplit ${branch:+-b "$branch"...}
>>   			case "$branch" in
>>   			'') git checkout -f -q ;;
>> -			?*) git checkout -f -q -b "$branch" "origin/$branch" ;;
>> +			?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
>>   			esac
>>   		) || die "Unable to checkout submodule '$path'"
>>   	fi
>>
These lines were actually added by Ben Jackson in commit ea10b60c91 in 
2009, long after I last touched that module.

Mark

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Ben Jackson]

On Thu, Dec 02, 2010 at 08:16:21PM -0500, Mark Levedahl wrote:
> On 12/02/2010 04:11 PM, Jens Lehmann wrote:
> > Nope, these lines date back to the time before I got involved in the
> > submodule business ... Seems like this "git checkout" was added in
> > March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
> > that.
> >
> > But to me your change looks good, so feel free to add:
> > Acked-by: Jens Lehmann<Jens.Lehmann@web.de>
> >
> >
> >> diff --git a/git-submodule.sh b/git-submodule.sh
> >> index 33bc41f..6242d7f 100755
> >> --- a/git-submodule.sh
> >> +++ b/git-submodule.sh
> >> @@ -241,7 +241,7 @@ cmd_add()
> >>   			# ash fails to wordsplit ${branch:+-b "$branch"...}
> >>   			case "$branch" in
> >>   			'') git checkout -f -q ;;
> >> -			?*) git checkout -f -q -b "$branch" "origin/$branch" ;;
> >> +			?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
> >>   			esac
> >>   		) || die "Unable to checkout submodule '$path'"
> >>   	fi
> >>
> These lines were actually added by Ben Jackson in commit ea10b60c91 in 
> 2009, long after I last touched that module.

I didn't mean to change any functionality -- I just wanted to fix a
portability problem (/bin/sh is ash on FreeBSD, hence the comment at the
top of the context).  Looks like `checkout -B' (capital B) didn't even
exist at that time.  Seems reasonable, though.

-- 
Ben Jackson AD7GD
<ben@ben.com>
http://www.ben.com/

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Jonathan Nieder]

Jens Lehmann wrote:
> Am 01.12.2010 19:50, schrieb Jonathan Nieder:

>>                                  Jens, any idea why git submodule
>> is not using "clone --branch" directly?
>
> Nope, these lines date back to the time before I got involved in the
> submodule business ... Seems like this "git checkout" was added in
> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
> that.

Ah, so the problem is that "clone --branch" did not exist.  Sorry for
the noise.

Another question can be also be easily answered by history examination:
the series of checks in module_clone are because 70c7ac22d:git-clone.sh
did not have checks of its own for the target directory.

So there is some simplification within grasp.

'night,
Jonathan

[PATCH] git submodule: Remove now obsolete tests before cloning a repo

[Jens Lehmann]

Since 55892d23 "git clone" itself checks that the destination path is not
a file but an empty directory if it exists, so there is no need anymore
for module_clone() to check that too.

Two tests have been added to test the behavior of "git submodule add" when
path is a file or a directory (A subshell had to be added to the former
last test to stay in the right directory).

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 03.12.2010 08:10, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
>> Am 01.12.2010 19:50, schrieb Jonathan Nieder:
> 
>>>                                  Jens, any idea why git submodule
>>> is not using "clone --branch" directly?
>>
>> Nope, these lines date back to the time before I got involved in the
>> submodule business ... Seems like this "git checkout" was added in
>> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
>> that.
> 
> Ah, so the problem is that "clone --branch" did not exist.  Sorry for
> the noise.
> 
> Another question can be also be easily answered by history examination:
> the series of checks in module_clone are because 70c7ac22d:git-clone.sh
> did not have checks of its own for the target directory.
> 
> So there is some simplification within grasp.

Maybe something like this?


 git-submodule.sh           |   14 --------------
 t/t7400-submodule-basic.sh |   28 +++++++++++++++++++++++-----
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 33bc41f..8085876 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -93,20 +93,6 @@ module_clone()
 	url=$2
 	reference="$3"

-	# If there already is a directory at the submodule path,
-	# expect it to be empty (since that is the default checkout
-	# action) and try to remove it.
-	# Note: if $path is a symlink to a directory the test will
-	# succeed but the rmdir will fail. We might want to fix this.
-	if test -d "$path"
-	then
-		rmdir "$path" 2>/dev/null ||
-		die "Directory '$path' exists, but is neither empty nor a git repository"
-	fi
-
-	test -e "$path" &&
-	die "A file already exist at path '$path'"
-
 	if test -n "$reference"
 	then
 		git-clone "$reference" -n "$url" "$path"
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 782b0a3..2c49db9 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -421,11 +421,29 @@ test_expect_success 'add submodules without specifying an explicit path' '
 		git commit -m "repo commit 1"
 	) &&
 	git clone --bare repo/ bare.git &&
-	cd addtest &&
-	git submodule add "$submodurl/repo" &&
-	git config -f .gitmodules submodule.repo.path repo &&
-	git submodule add "$submodurl/bare.git" &&
-	git config -f .gitmodules submodule.bare.path bare
+	(
+		cd addtest &&
+		git submodule add "$submodurl/repo" &&
+		git config -f .gitmodules submodule.repo.path repo &&
+		git submodule add "$submodurl/bare.git" &&
+		git config -f .gitmodules submodule.bare.path bare
+	)
+'
+
+test_expect_success 'add should fail when path is used by a file' '
+	(
+		cd addtest &&
+		touch file &&
+		test_must_fail	git submodule add "$submodurl/repo" file
+	)
+'
+
+test_expect_success 'add should fail when path is used by an existing directory' '
+	(
+		cd addtest &&
+		mkdir empty-dir &&
+		test_must_fail git submodule add "$submodurl/repo" empty-dir
+	)
 '

 test_done
-- 
1.7.3.2.657.g92628.dirty

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Nope, these lines date back to the time before I got involved in the
> submodule business ... Seems like this "git checkout" was added in
> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
> that.
>
> But to me your change looks good, so feel free to add:
> Acked-by: Jens Lehmann <Jens.Lehmann@web.de>

Does either of you want to add a test for this?

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Jens Lehmann]

Am 07.12.2010 23:57, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Nope, these lines date back to the time before I got involved in the
>> submodule business ... Seems like this "git checkout" was added in
>> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
>> that.
>>
>> But to me your change looks good, so feel free to add:
>> Acked-by: Jens Lehmann <Jens.Lehmann@web.de>
> 
> Does either of you want to add a test for this?

Will do.

[PATCH] git submodule -b ... of current HEAD fails

[Jens Lehmann]

	git submodule add -b $branch $repository

fails when HEAD already points to $branch in $repository.

When the freshly cloned submodules HEAD is the same as the checked out
branch, it doesn't make sense to update it again as "git checkout -b"
would fail with »fatal: git checkout: branch $branch already exists«.

Reported-by: Klaus Ethgen <Klaus@Ethgen.de>
Thanks-to: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 08.12.2010 22:35, schrieb Jens Lehmann:
> Am 07.12.2010 23:57, schrieb Junio C Hamano:
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>
>>> Nope, these lines date back to the time before I got involved in the
>>> submodule business ... Seems like this "git checkout" was added in
>>> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
>>> that.
>>>
>>> But to me your change looks good, so feel free to add:
>>> Acked-by: Jens Lehmann <Jens.Lehmann@web.de>
>>
>> Does either of you want to add a test for this?
> 
> Will do.

And as it happens from time to time, while writing the test you find
out that the first attempt to fix the bug didn't work as expected ...

 git-submodule.sh           |    4 +++-
 t/t7400-submodule-basic.sh |    7 +++++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 33bc41f..bf2803f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -241,7 +241,9 @@ cmd_add()
 			# ash fails to wordsplit ${branch:+-b "$branch"...}
 			case "$branch" in
 			'') git checkout -f -q ;;
-			?*) git checkout -f -q -b "$branch" "origin/$branch" ;;
+			?*) if [ "$(git branch)" != "* $branch"  ]; then
+				git checkout -f -q -b "$branch" "origin/$branch"
+			fi ;;
 			esac
 		) || die "Unable to checkout submodule '$path'"
 	fi
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 782b0a3..e224da4 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -131,6 +131,13 @@ test_expect_success 'submodule add --branch' '
 	test_cmp empty untracked
 '

+test_expect_success 'submodule add --branch succeeds even when branch is at HEAD' '
+	(
+		cd addtest &&
+		git submodule add -b master "$submodurl" submod-existing-branch
+	)
+'
+
 test_expect_success 'submodule add with ./ in path' '
 	echo "refs/heads/master" >expect &&
 	>empty &&
-- 
1.7.3.3.580.ged75d

Re: [PATCH] git submodule -b ... of current HEAD fails

[Jonathan Nieder]

Jens Lehmann wrote:
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -241,7 +241,9 @@ cmd_add()
>  			# ash fails to wordsplit ${branch:+-b "$branch"...}
>  			case "$branch" in
>  			'') git checkout -f -q ;;
> -			?*) git checkout -f -q -b "$branch" "origin/$branch" ;;
> +			?*) if [ "$(git branch)" != "* $branch"  ]; then

Agh.  The command to use is "git symbolic-ref -q HEAD", I suppose.

Maybe we can simplify by relying on "git clone".

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 git-submodule.sh           |   16 ++++++----------
 t/t7400-submodule-basic.sh |    8 +++++++-
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index d937f0b..6fd09e7 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -219,17 +219,13 @@ cmd_add()
 		esac
 		git config submodule."$path".url "$url"
 	else
-
-		module_clone "$path" "$realrepo" "$reference" || exit
+		brancharg=${branch:+"-b $(git rev-parse --sq-quote "$branch")"}
+		refarg=${reference:+"$(git rev-parse --sq-quote "$reference")"}
 		(
-			clear_local_git_env
-			cd "$path" &&
-			# ash fails to wordsplit ${branch:+-b "$branch"...}
-			case "$branch" in
-			'') git checkout -f -q ;;
-			?*) git checkout -f -q -b "$branch" "origin/$branch" ;;
-			esac
-		) || die "Unable to checkout submodule '$path'"
+			export realrepo path &&
+			eval "git clone $brancharg $refarg" '"$realrepo" "$path"'
+		) ||
+		die "Clone of '$realrepo' into submodule path '$path' failed"
 	fi
 
 	git add $force "$path" ||
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 2c49db9..77088cc 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -114,7 +114,6 @@ test_expect_success 'submodule add --branch' '
 	echo "refs/heads/initial" >expect-head &&
 	cat <<-\EOF >expect-heads &&
 	refs/heads/initial
-	refs/heads/master
 	EOF
 	>empty &&
 
@@ -131,6 +130,13 @@ test_expect_success 'submodule add --branch' '
 	test_cmp empty untracked
 '
 
+test_expect_success 'submodule add --branch succeeds even when branch is at HEAD' '
+	(
+		cd addtest &&
+		git submodule add -b master "$submodurl" submod-existing-branch
+	)
+'
+
 test_expect_success 'submodule add with ./ in path' '
 	echo "refs/heads/master" >expect &&
 	>empty &&

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> 	git submodule add -b $branch $repository
>
> fails when HEAD already points to $branch in $repository.

I was reviewing the overall picture before tagging 1.7.4-rc0 and started
wondering if this was a good change.  If repository already had branch
checked out, and if it was pointing at a commit that was different from
whatever was taken from origin, shouldn't the command _fail_ to prevent
the divergent commits from getting lost?

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Jens Lehmann]

Am 28.12.2010 22:42, schrieb Junio C Hamano:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
>> 	git submodule add -b $branch $repository
>>
>> fails when HEAD already points to $branch in $repository.
> 
> I was reviewing the overall picture before tagging 1.7.4-rc0 and started
> wondering if this was a good change.  If repository already had branch
> checked out, and if it was pointing at a commit that was different from
> whatever was taken from origin, shouldn't the command _fail_ to prevent
> the divergent commits from getting lost?

But doesn't this change only affect the case where the submodule is
freshly cloned, so there is no chance of losing local changes?

(But AFAIK the patch doesn't really fix the issue, please see [1] and
Jonathan's followup)

[1] http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/772659/focus=163242

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> But doesn't this change only affect the case where the submodule is
> freshly cloned, so there is no chance of losing local changes?

My fault, for not looking at the surrounding context.

> (But AFAIK the patch doesn't really fix the issue, please see [1] and
> Jonathan's followup)
>
> [1] http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/772659/focus=163242

I think we queued the later round just uses "checkout -B"; shouldn't that
work?

Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Jens Lehmann]

Am 29.12.2010 01:34, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> (But AFAIK the patch doesn't really fix the issue, please see [1] and
>> Jonathan's followup)
>>
>> [1] http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/772659/focus=163242
> 
> I think we queued the later round just uses "checkout -B"; shouldn't that
> work?

That's what I thought too (and that is what I based my ack upon, the
patch hit the right spot and used the - according to the documentation
- better suited -B option). But while writing the test you rightfully
requested I noticed that using -B didn't change much. The reason is
that the following commands both fail in a freshly cloned repo:

$ git checkout -f -q -b master origin/master
fatal: git checkout: branch master already exists
$ git checkout -f -q -B master origin/master
fatal: Cannot force update the current branch.

So maybe the real problem here is that "git checkout -B" barfs when it
doesn't have anything to do instead of silently doing nothing?

So we maybe want to fix this issue in "git checkout"? Then the patch
will start working (and the test for it can be added in a later patch).

Re* [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> $ git checkout -f -q -b master origin/master
> fatal: git checkout: branch master already exists
> $ git checkout -f -q -B master origin/master
> fatal: Cannot force update the current branch.
>
> So maybe the real problem here is that "git checkout -B" barfs when it
> doesn't have anything to do instead of silently doing nothing?
>
> So we maybe want to fix this issue in "git checkout"? Then the patch
> will start working (and the test for it can be added in a later patch).

Let me think aloud, as it may be a good time to summarize the current
semantics of what "checking out a branch" means.

 0. "git checkout $branch" to check out a branch is to start preparing to
    commit on that branch while keeping the tentative changes you have in
    your working tree and your index.

 1. You might not have the $branch yet to check out in such a way.  You
    could do

    $ git branch $branch $start && git checkout $branch

    and

    $ git checkout -b $branch $start

    is conceptually a short-hand of the above two command sequence.

 3. When $branch exists, 2. should fail, as it involves resetting the tip
    of $branch to $start, potentially losing commits near its old tip.
    If you really mean it, you can force it by

    $ git checkout -B $branch $start

    which is conceptually a short-hand for:

    $ git branch -f $branch $start && git checkout $branch

 4. "git checkout $branch" refuses to discard your work when the
    tentative changes you made conflict with the difference between your
    current HEAD and $branch.  You can give "-m" to force it to attempt a
    three-way merge with possible conflicts, or you can give "-f" to force
    it to discard all tentative changes.

 5. The logical consequence of all of the above is that "git checkout [-f]
    -B $branch $start" should conceptually be a short-hand for:

    $ git branch -f $branch $start && git checkout [-f] $branch

And "branch -f $branch" refuses to reset the tip of the current branch.

An end-user who runs "git checkout -f -B $branch $start" is telling us
the following things:

 - The $branch may or may not exist;

 - The tip it currently may point at is immaterial and the user is willing
   to lose it (this is what capital-ness of -B means);

 - The tentative changes in the working tree and the index is immaterial
   (this is what -f given to checkout means).

It seems to me that "checkout -f -B $branch $start" when $branch is the
current one ought to do an equivalent of "reset --hard $start" and nothing
else.

What happens if -f is not given?  The user wants to keep the tentative
changes, and that is done by comparing a commit in HEAD (i.e. the current
branch) and the target $branch.  The two step approach that can be
illustrated by the short-hand definition of "checkout -B" however breaks
down in this case, even if you change the variant of "git branch -f" it
internally runs:

    $ git branch --allow-updating-current -f $branch $start &&
      git checkout $branch

The first step already resets $branch to its new value, and the second
step does not have a way to know that your tentative changes are based on
the previous value.

BUT ;-)

I think it is Ok after all. The actual implementation of "checkout -[b|B]"
is more like:

    $ git read-tree -m -u HEAD $start
    $ git update-ref -f refs/heads/$branch $start
    $ git symbolic-ref HEAD refs/heads/$branch

That is, when we update the working tree and the index, we still haven't
updated the HEAD yet (switch_branches() calls merge_working_tree() to run
this two-way merge, and then calls update_refs_for_switch() which does the
update-ref part including the branch creation).

The next question is if we want to enable "git branch -f $branch $start"
for the current branch from the end user, which presumably would look as
if the user ran "git reset $start".  I am actually indifferent about this,
but as the first step we probably should be conservative to forbid it as
we do now.

So in conclusion, here is a patch that is not even compile tested ;-)

This was made on top of 'maint' only because that was the branch I
happened to have checked out in my development working tree.

Note that the test script does not even test the case I was initially
worried about (i.e. keeping local changes).  I'll leave it to interested
others ;-)

 branch.c                   |   12 ++++++++----
 branch.h                   |   23 +++++++++++++++--------
 builtin/branch.c           |    3 ++-
 builtin/checkout.c         |    4 +++-
 t/t2018-checkout-branch.sh |   13 +++++++++++++
 5 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/branch.c b/branch.c
index 93dc866..74eb866 100644
--- a/branch.c
+++ b/branch.c
@@ -137,7 +137,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
 
 void create_branch(const char *head,
 		   const char *name, const char *start_name,
-		   int force, int reflog, enum branch_track track)
+		   int flags, int reflog, enum branch_track track)
 {
 	struct ref_lock *lock = NULL;
 	struct commit *commit;
@@ -147,6 +147,9 @@ void create_branch(const char *head,
 	int forcing = 0;
 	int dont_change_ref = 0;
 	int explicit_tracking = 0;
+	int ok_to_update = flags & (CREATE_BRANCH_UPDATE_OK |
+				    CREATE_BRANCH_UPDATE_CURRENT_OK);
+	int ok_to_update_current = flags & CREATE_BRANCH_UPDATE_CURRENT_OK;
 
 	if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
 		explicit_tracking = 1;
@@ -155,11 +158,12 @@ void create_branch(const char *head,
 		die("'%s' is not a valid branch name.", name);
 
 	if (resolve_ref(ref.buf, sha1, 1, NULL)) {
-		if (!force && track == BRANCH_TRACK_OVERRIDE)
+		if (!ok_to_update && track == BRANCH_TRACK_OVERRIDE)
 			dont_change_ref = 1;
-		else if (!force)
+		else if (!ok_to_update)
 			die("A branch named '%s' already exists.", name);
-		else if (!is_bare_repository() && head && !strcmp(head, name))
+		else if (!ok_to_update_current &&
+			 !is_bare_repository() && head && !strcmp(head, name))
 			die("Cannot force update the current branch.");
 		forcing = 1;
 	}
diff --git a/branch.h b/branch.h
index eed817a..0afbeae 100644
--- a/branch.h
+++ b/branch.h
@@ -4,16 +4,23 @@
 /* Functions for acting on the information about branches. */
 
 /*
- * Creates a new branch, where head is the branch currently checked
- * out, name is the new branch name, start_name is the name of the
- * existing branch that the new branch should start from, force
- * enables overwriting an existing (non-head) branch, reflog creates a
- * reflog for the branch, and track causes the new branch to be
- * configured to merge the remote branch that start_name is a tracking
- * branch for (if any).
+ * Creates a new branch, where:
+ *
+ * - head is the branch currently checked out;
+ * - name is the new branch name;
+ * - start_name is the name of a commit that the new branch should start at
+ *   (could be another branch or a remote-tracking branch, in which case
+ *   track---see below---may also trigger);
+ * - flags indicates overwriting an existing branch and/or overwriting the
+ *   current branch is allowed;
+ * - reflog creates a reflog for the branch; and
+ * - track causes the new branch to be configured to merge the remote branch
+ *   that start_name is a tracking branch for (if any).
  */
+#define CREATE_BRANCH_UPDATE_OK 01
+#define CREATE_BRANCH_UPDATE_CURRENT_OK 02
 void create_branch(const char *head, const char *name, const char *start_name,
-		   int force, int reflog, enum branch_track track);
+		   int flags, int reflog, enum branch_track track);
 
 /*
  * Remove information about the state of working on the current
diff --git a/builtin/branch.c b/builtin/branch.c
index 87976f0..ea5b411 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -651,7 +651,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('m', NULL, &rename, "move/rename a branch and its reflog", 1),
 		OPT_BIT('M', NULL, &rename, "move/rename a branch, even if target exists", 2),
 		OPT_BOOLEAN('l', NULL, &reflog, "create the branch's reflog"),
-		OPT_BOOLEAN('f', "force", &force_create, "force creation (when already exists)"),
+		OPT_SET_INT('f', "force", &force_create, "force creation (when already exists)",
+			    CREATE_BRANCH_UPDATE_OK),
 		{
 			OPTION_CALLBACK, 0, "no-merged", &merge_filter_ref,
 			"commit", "print only not merged branches",
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a54583b..bf5b43a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -529,7 +529,9 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 		}
 		else
 			create_branch(old->name, opts->new_branch, new->name,
-				      opts->new_branch_force ? 1 : 0,
+				      opts->new_branch_force
+				      ? CREATE_BRANCH_UPDATE_CURRENT_OK
+				      : 0,
 				      opts->new_branch_log, opts->track);
 		new->name = opts->new_branch;
 		setup_branch_path(new);
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index fa69016..37e4fdb 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -169,4 +169,17 @@ test_expect_success 'checkout -f -B to an existing branch with mergeable changes
 	test_must_fail test_dirty_mergeable
 '
 
+test_expect_success 'checkout -b/B the current branch' '
+	git reset --hard &&
+	git checkout branch1 &&
+	it=$(git rev-parse --verify HEAD) &&
+	test_must_fail git checkout -b branch1 HEAD &&
+	git checkout -B branch1 $it &&
+	test "$it" = "$(git rev-parse --verify HEAD)" &&
+	test "refs/heads/branch1" = "$(git symbolic-ref HEAD)" &&
+	git checkout -B branch1 HEAD &&
+	test "$it" = "$(git rev-parse --verify HEAD)" &&
+	test "refs/heads/branch1" = "$(git symbolic-ref HEAD)"
+'
+
 test_done

Re: Re* [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

[Jens Lehmann]

Am 29.12.2010 23:13, schrieb Jens Lehmann:
> Am 29.12.2010 21:53, schrieb Junio C Hamano:
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>> So we maybe want to fix this issue in "git checkout"? Then the patch
>>> will start working (and the test for it can be added in a later patch).
>>
>> So in conclusion, here is a patch that is not even compile tested ;-)

Just for the record: It does compile (at least for me ;-) and together
with Jonathan's patch referenced in the the subject passes all tests,
including the new 'submodule add --branch succeeds even when branch is
at HEAD' test I came up with to verify this issue.