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>

diff --git a/branch.c b/branch.c
index 1fe3078a6cade3c34c5dcd65977b286840bec6ee..4338a90b396351f7fd51ec2e07da6a4044b4bb3d 100644 (file)
--- a/branch.c
+++ b/branch.c
@@ -135,23 +135,25 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
        return 0;
 }
 
        return 0;
 }
 

-int validate_new_branchname(const char *name, struct strbuf *ref, int force)
+int validate_new_branchname(const char *name, struct strbuf *ref,
+                           int force, int attr_only)

 {
 {

-       const char *head;
-       unsigned char sha1[20];
-

        if (strbuf_check_branch_ref(ref, name))
                die("'%s' is not a valid branch name.", name);
 
        if (!ref_exists(ref->buf))
                return 0;
        if (strbuf_check_branch_ref(ref, name))
                die("'%s' is not a valid branch name.", name);
 
        if (!ref_exists(ref->buf))
                return 0;

-       else if (!force)
+       else if (!force && !attr_only)

                die("A branch named '%s' already exists.", ref->buf + strlen("refs/heads/"));
 
                die("A branch named '%s' already exists.", ref->buf + strlen("refs/heads/"));
 

-       head = resolve_ref("HEAD", sha1, 0, NULL);
-       if (!is_bare_repository() && head && !strcmp(head, ref->buf))
-               die("Cannot force update the current branch.");
+       if (!attr_only) {
+               const char *head;
+               unsigned char sha1[20];

 
 

+               head = resolve_ref("HEAD", sha1, 0, NULL);
+               if (!is_bare_repository() && head && !strcmp(head, ref->buf))
+                       die("Cannot force update the current branch.");
+       }

        return 1;
 }
 
        return 1;
 }
 


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

-       if (validate_new_branchname(name, &ref, force || track == BRANCH_TRACK_OVERRIDE)) {
+       if (validate_new_branchname(name, &ref, force,
+                                   track == BRANCH_TRACK_OVERRIDE)) {

                if (!force)
                        dont_change_ref = 1;
                else
                if (!force)
                        dont_change_ref = 1;
                else

diff --git a/branch.h b/branch.h
index 01544e25cbbf77c573da38c73f105a8b62c55f99..1285158dd4f26e5bbb0e0d7133055f168fee773f 100644 (file)
--- a/branch.h
+++ b/branch.h
@@ -20,8 +20,18 @@ void create_branch(const char *head, const char *name, const char *start_name,
  * interpreted ref in ref, force indicates whether (non-head) branches
  * may be overwritten. A non-zero return value indicates that the force
  * parameter was non-zero and the branch already exists.
  * interpreted ref in ref, force indicates whether (non-head) branches
  * may be overwritten. A non-zero return value indicates that the force
  * parameter was non-zero and the branch already exists.

+ *
+ * Contrary to all of the above, when attr_only is 1, the caller is
+ * not interested in verifying if it is Ok to update the named
+ * branch to point at a potentially different commit. It is merely
+ * asking if it is OK to change some attribute for the named branch
+ * (e.g. tracking upstream).
+ *
+ * NEEDSWORK: This needs to be split into two separate functions in the
+ * longer run for sanity.
+ *

  */
  */

-int validate_new_branchname(const char *name, struct strbuf *ref, int force);
+int validate_new_branchname(const char *name, struct strbuf *ref, int force, int attr_only);

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

diff --git a/builtin/branch.c b/builtin/branch.c
index 40f885c9fe9598f8e7c25f14c6e5aa01b45d1c82..5fb3d85ad317937a1d369665412e468151e6d9af 100644 (file)
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -566,7 +566,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
                        die(_("Invalid branch name: '%s'"), oldname);
        }
 
                        die(_("Invalid branch name: '%s'"), oldname);
        }
 

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

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

diff --git a/builtin/checkout.c b/builtin/checkout.c
index ddefec053e563dff0e16cd5adb2f8f6d26c77d87..909a334627ccc49c1fe525ef32d88bbc1f52ebdc 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1072,7 +1072,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
        if (opts.new_branch) {
                struct strbuf buf = STRBUF_INIT;
 
        if (opts.new_branch) {
                struct strbuf buf = STRBUF_INIT;
 

-               opts.branch_exists = validate_new_branchname(opts.new_branch, &buf, !!opts.new_branch_force);
+               opts.branch_exists = validate_new_branchname(opts.new_branch, &buf,
+                                                            !!opts.new_branch_force, 0);

 
                strbuf_release(&buf);
        }
 
                strbuf_release(&buf);
        }

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index cb6458d1c820bd131af4659fe878ff9406fbbcac..7633930bb472e1c9aebf2799f3eaf3bc53c7263c 100755 (executable)
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -554,4 +554,17 @@ test_expect_success 'attempt to delete a branch merged to its base' '
        test_must_fail git branch -d my10
 '
 
        test_must_fail git branch -d my10
 '
 

+test_expect_success 'use set-upstream on the current branch' '
+       git checkout master &&
+       git --bare init myupstream.git &&
+       git push myupstream.git master:refs/heads/frotz &&
+       git remote add origin myupstream.git &&
+       git fetch &&
+       git branch --set-upstream master origin/frotz &&
+
+       test "z$(git config branch.master.remote)" = "zorigin" &&
+       test "z$(git config branch.master.merge)" = "zrefs/heads/frotz"
+
+'
+

 test_done
 test_done