Jens Lehmann <Jens.Lehmann@xxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html