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

Jens Lehmann [off-list ref] 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