Skip to content

Commit fa79937

Browse files
committed
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 <[email protected]>
1 parent 587a9ee commit fa79937

File tree

5 files changed

+39
-12
lines changed

5 files changed

+39
-12
lines changed

branch.c

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -135,23 +135,25 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
135135
return 0;
136136
}
137137

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)
139140
{
140-
const char *head;
141-
unsigned char sha1[20];
142-
143141
if (strbuf_check_branch_ref(ref, name))
144142
die("'%s' is not a valid branch name.", name);
145143

146144
if (!ref_exists(ref->buf))
147145
return 0;
148-
else if (!force)
146+
else if (!force && !attr_only)
149147
die("A branch named '%s' already exists.", ref->buf + strlen("refs/heads/"));
150148

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];
154152

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+
}
155157
return 1;
156158
}
157159

@@ -171,7 +173,8 @@ void create_branch(const char *head,
171173
if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
172174
explicit_tracking = 1;
173175

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)) {
175178
if (!force)
176179
dont_change_ref = 1;
177180
else

branch.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,18 @@ void create_branch(const char *head, const char *name, const char *start_name,
2020
* interpreted ref in ref, force indicates whether (non-head) branches
2121
* may be overwritten. A non-zero return value indicates that the force
2222
* 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+
*
2333
*/
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);
2535

2636
/*
2737
* Remove information about the state of working on the current

builtin/branch.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
566566
die(_("Invalid branch name: '%s'"), oldname);
567567
}
568568

569-
validate_new_branchname(newname, &newref, force);
569+
validate_new_branchname(newname, &newref, force, 0);
570570

571571
strbuf_addf(&logmsg, "Branch: renamed %s to %s",
572572
oldref.buf, newref.buf);

builtin/checkout.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
10721072
if (opts.new_branch) {
10731073
struct strbuf buf = STRBUF_INIT;
10741074

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);
10761077

10771078
strbuf_release(&buf);
10781079
}

t/t3200-branch.sh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,4 +554,17 @@ test_expect_success 'attempt to delete a branch merged to its base' '
554554
test_must_fail git branch -d my10
555555
'
556556

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+
557570
test_done

0 commit comments

Comments
 (0)