branch --set-upstream: regression fix

The "git branch" command, while not in listing mode, calls create_branch() even when the target branch already exists, and it does so even when it is not interested in updating the value of the branch (i.e. the name of the commit object that sits at the tip of the existing branch). This happens when the command is run with "--set-upstream" option. The earlier safety-measure to prevent "git branch -f $branch $commit" from updating the currently checked out branch did not take it into account, and we no longer can update the tracking information of the current branch. Minimally fix this regression by telling the validation code if it is called to really update the value of a potentially existing branch, or if the caller merely is interested in updating auxiliary aspects of a branch. Reported-and-Tested-by: Jay Soffian Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Sep 16, 2011 at 16:28 UTC fa79937675600ad5dda5031c532097a0461d843e
5 files changed +39 -12
branch.c
+12 -9
@@ -135,23 +135,25 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
135 return 0;
136 }
137
138 -int validate_new_branchname(const char *name, struct strbuf *ref, int force)
138 +int validate_new_branchname(const char *name, struct strbuf *ref,
139 + int force, int attr_only)
140 {
140 - const char *head;
141 - unsigned char sha1[20];
142 -
141 if (strbuf_check_branch_ref(ref, name))
142 die("'%s' is not a valid branch name.", name);
143
144 if (!ref_exists(ref->buf))
145 return 0;
148 - else if (!force)
146 + else if (!force && !attr_only)
147 die("A branch named '%s' already exists.", ref->buf + strlen("refs/heads/"));
148
151 - head = resolve_ref("HEAD", sha1, 0, NULL);
152 - if (!is_bare_repository() && head && !strcmp(head, ref->buf))
153 - die("Cannot force update the current branch.");
149 + if (!attr_only) {
150 + const char *head;
151 + unsigned char sha1[20];
152
153 + head = resolve_ref("HEAD", sha1, 0, NULL);
154 + if (!is_bare_repository() && head && !strcmp(head, ref->buf))
155 + die("Cannot force update the current branch.");
156 + }
157 return 1;
158 }
159
@@ -171,7 +173,8 @@ void create_branch(const char *head,
173 if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
174 explicit_tracking = 1;
175
174 - if (validate_new_branchname(name, &ref, force || track == BRANCH_TRACK_OVERRIDE)) {
176 + if (validate_new_branchname(name, &ref, force,
177 + track == BRANCH_TRACK_OVERRIDE)) {
178 if (!force)
179 dont_change_ref = 1;
180 else
branch.h
+11 -1
@@ -20,8 +20,18 @@ void create_branch(const char *head, const char *name, const char *start_name,
20 * interpreted ref in ref, force indicates whether (non-head) branches
21 * may be overwritten. A non-zero return value indicates that the force
22 * parameter was non-zero and the branch already exists.
23 + *
24 + * Contrary to all of the above, when attr_only is 1, the caller is
25 + * not interested in verifying if it is Ok to update the named
26 + * branch to point at a potentially different commit. It is merely
27 + * asking if it is OK to change some attribute for the named branch
28 + * (e.g. tracking upstream).
29 + *
30 + * NEEDSWORK: This needs to be split into two separate functions in the
31 + * longer run for sanity.
32 + *
33 */
24 -int validate_new_branchname(const char *name, struct strbuf *ref, int force);
34 +int validate_new_branchname(const char *name, struct strbuf *ref, int force, int attr_only);
35
36 /*
37 * Remove information about the state of working on the current
builtin/branch.c
+1 -1
@@ -566,7 +566,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
566 die(_("Invalid branch name: '%s'"), oldname);
567 }
568
569 - validate_new_branchname(newname, &newref, force);
569 + validate_new_branchname(newname, &newref, force, 0);
570
571 strbuf_addf(&logmsg, "Branch: renamed %s to %s",
572 oldref.buf, newref.buf);
builtin/checkout.c
+2 -1
@@ -1072,7 +1072,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
1072 if (opts.new_branch) {
1073 struct strbuf buf = STRBUF_INIT;
1074
1075 - opts.branch_exists = validate_new_branchname(opts.new_branch, &buf, !!opts.new_branch_force);
1075 + opts.branch_exists = validate_new_branchname(opts.new_branch, &buf,
1076 + !!opts.new_branch_force, 0);
1077
1078 strbuf_release(&buf);
1079 }
t/t3200-branch.sh
+13
@@ -554,4 +554,17 @@ test_expect_success 'attempt to delete a branch merged to its base' '
554 test_must_fail git branch -d my10
555 '
556
557 +test_expect_success 'use set-upstream on the current branch' '
558 + git checkout master &&
559 + git --bare init myupstream.git &&
560 + git push myupstream.git master:refs/heads/frotz &&
561 + git remote add origin myupstream.git &&
562 + git fetch &&
563 + git branch --set-upstream master origin/frotz &&
564 +
565 + test "z$(git config branch.master.remote)" = "zorigin" &&
566 + test "z$(git config branch.master.merge)" = "zrefs/heads/frotz"
567 +
568 +'
569 +
570 test_done