Re: git branch -M" regression in 1.7.7?
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Nov 26, 2011, 08:54 UTC
- Message-ID
- <20111126085455.GB22656@elie.hsd1.il.comcast.net>
- In-Reply-To
- <CAOTq_pv4dyAkbqye+diK9mTTsrTg9OKg0tExKcfDgs8RfiTwTQ@mail.gmail.com>
Conrad Irwin wrote:
Show 7 quoted lines
> I thought after reading your patch about making it just do: > > if (!strcmp(oldname, newname)) > exit(0); > > but I guess it would then not mark an entry in the reflog that people > could be relying on...
Ah, this deserves a comment.
I thought about doing the same thing, and then didn't do it because I wanted to make sure that
git branch -M nonexistent nonexistent
does not succeed. Which presumably warrants another test:
test_expect_success "rename-to-self dwimery doesn't hide nonexistent ref" ' test_must_fail git branch -M nonexistent nonexistent && test_must_fail git rev-parse --verify nonexistent '
Sloppy of me.
Show 5 quoted lines
>> + clobber_head_ok = !strcmp(oldname, newname); >> + >> + validate_new_branchname(newname, &newref, force, clobber_head_ok); > > This looks ok, and will be improvable if the NEEDSWORK in branch.h is done.
Thanks for looking it over.
> The other thing I wonder is whether "git checkout -B master HEAD" or > "git branch -f master master" should have the same short-cut?
I think "git branch -M" is the only one buildbot was relying on.
As an aside, I'm not convinced the check is needed for checkout -B at all. In an ideal world, the order of operations would be:
1. look up commit argument 2. merge working tree 3. update branch to match commit 4. update HEAD symref to point to branch
In other words, when on master, "git checkout -B master <commit>" would be another way to say "git reset --keep <commit>", except that it also sets up tracking.
Surprisingly, switch_branches() seems to match that pretty well already.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- branch.c | 6 ++++-- branch.h | 3 ++- builtin/branch.c | 2 +- builtin/checkout.c | 15 +++++++++++---- t/t2018-checkout-branch.sh | 9 +++++---- 5 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/branch.c b/branch.c index 025a97be..f85c4382 100644 --- a/branch.c +++ b/branch.c @@ -160,7 +160,8 @@ int validate_new_branchname(const char *name, struct strbuf *ref, void create_branch(const char *head, const char *name, const char *start_name, - int force, int reflog, enum branch_track track) + int force, int reflog, int clobber_head, + enum branch_track track) { struct ref_lock *lock = NULL; struct commit *commit; @@ -175,7 +176,8 @@ void create_branch(const char *head, explicit_tracking = 1; if (validate_new_branchname(name, &ref, force, - track == BRANCH_TRACK_OVERRIDE)) { + track == BRANCH_TRACK_OVERRIDE || + clobber_head)) { if (!force) dont_change_ref = 1; else diff --git a/branch.h b/branch.h index 1285158d..e125ff4c 100644 --- a/branch.h +++ b/branch.h @@ -13,7 +13,8 @@ * branch for (if any). */ void create_branch(const char *head, const char *name, const char *start_name, - int force, int reflog, enum branch_track track); + int force, int reflog, + int clobber_head, enum branch_track track); /* * Validates that the requested branch may be created, returning the diff --git a/builtin/branch.c b/builtin/branch.c index 51ca6a02..730f9139 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -730,7 +730,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) if (kinds != REF_LOCAL_BRANCH) die(_("-a and -r options to 'git branch' do not make sense with a branch name")); create_branch(head, argv[0], (argc == 2) ? argv[1] : head, - force_create, reflog, track); + force_create, reflog, 0, track); } else usage_with_options(builtin_branch_usage, options); diff --git a/builtin/checkout.c b/builtin/checkout.c index 2a807724..ca00a853 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -540,7 +540,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_log, opts->track); + opts->new_branch_log, + opts->new_branch_force ? 1 : 0, + opts->track); new->name = opts->new_branch; setup_branch_path(new); } @@ -565,8 +567,12 @@ static void update_refs_for_switch(struct checkout_opts *opts, create_symref("HEAD", new->path, msg.buf); if (!opts->quiet) { if (old->path && !strcmp(new->path, old->path)) { - fprintf(stderr, _("Already on '%s'\n"), - new->name); + if (opts->new_branch_force) + fprintf(stderr, _("Reset branch '%s'\n"), + new->name); + else + fprintf(stderr, _("Already on '%s'\n"), + new->name); } else if (opts->new_branch) { if (opts->branch_exists) fprintf(stderr, _("Switched to and reset branch '%s'\n"), new->name); @@ -1057,7 +1063,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) struct strbuf buf = STRBUF_INIT; opts.branch_exists = validate_new_branchname(opts.new_branch, &buf, - !!opts.new_branch_force, 0); + !!opts.new_branch_force, + !!opts.new_branch_force); strbuf_release(&buf); } diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index 75874e85..27412623 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -189,12 +189,13 @@ test_expect_success 'checkout -b <describe>' ' test_cmp expect actual ' -test_expect_success 'checkout -B to the current branch fails before merging' ' +test_expect_success 'checkout -B to the current branch works' ' git checkout branch1 && + git checkout -B branch1-scratch && + setup_dirty_mergeable && - git commit -mfooble && - test_must_fail git checkout -B branch1 initial && - test_must_fail test_dirty_mergeable + git checkout -B branch1-scratch initial && + test_dirty_mergeable ' test_done
-- 1.7.8.rc3