Merge branch 'ks/branch-cleanup'
authorJunio C Hamano <gitster@pobox.com>
Wed, 27 Dec 2017 19:16:25 +0000 (11:16 -0800)
committerJunio C Hamano <gitster@pobox.com>
Wed, 27 Dec 2017 19:16:25 +0000 (11:16 -0800)
Code clean-up.

* ks/branch-cleanup:
builtin/branch: strip refs/heads/ using skip_prefix
branch: update warning message shown when copying a misnamed branch
branch: group related arguments of create_branch()
branch: improve documentation and naming of create_branch() parameters

1  2 
branch.c
branch.h
builtin/branch.c
builtin/checkout.c
diff --combined branch.c
index fe1e1c3676b26a4f35feaf8137700b25916ad5ee,bd607ae979f6c0a1b93ee89a0facdd818ea64e64..2672054f0b5423cd09010302285935650435f1dc
+++ b/branch.c
@@@ -178,40 -178,24 +178,40 @@@ int read_branch_desc(struct strbuf *buf
        return 0;
  }
  
 -int validate_new_branchname(const char *name, struct strbuf *ref,
 -                          int force, int attr_only)
 +/*
 + * Check if 'name' can be a valid name for a branch; die otherwise.
 + * Return 1 if the named branch already exists; return 0 otherwise.
 + * Fill ref with the full refname for the branch.
 + */
 +int validate_branchname(const char *name, struct strbuf *ref)
  {
        if (strbuf_check_branch_ref(ref, name))
                die(_("'%s' is not a valid branch name."), name);
  
 -      if (!ref_exists(ref->buf))
 +      return ref_exists(ref->buf);
 +}
 +
 +/*
 + * Check if a branch 'name' can be created as a new branch; die otherwise.
 + * 'force' can be used when it is OK for the named branch already exists.
 + * Return 1 if the named branch already exists; return 0 otherwise.
 + * Fill ref with the full refname for the branch.
 + */
 +int validate_new_branchname(const char *name, struct strbuf *ref, int force)
 +{
 +      const char *head;
 +
 +      if (!validate_branchname(name, ref))
                return 0;
 -      else if (!force && !attr_only)
 -              die(_("A branch named '%s' already exists."), ref->buf + strlen("refs/heads/"));
  
 -      if (!attr_only) {
 -              const char *head;
 +      if (!force)
 +              die(_("A branch named '%s' already exists."),
 +                  ref->buf + strlen("refs/heads/"));
 +
 +      head = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
 +      if (!is_bare_repository() && head && !strcmp(head, ref->buf))
 +              die(_("Cannot force update the current branch."));
  
 -              head = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
 -              if (!is_bare_repository() && head && !strcmp(head, ref->buf))
 -                      die(_("Cannot force update the current branch."));
 -      }
        return 1;
  }
  
@@@ -244,7 -228,7 +244,7 @@@ N_("\n
  "\"git push -u\" to set the upstream config as you push.");
  
  void create_branch(const char *name, const char *start_name,
-                  int force, int reflog, int clobber_head,
+                  int force, int clobber_head_ok, int reflog,
                   int quiet, enum branch_track track)
  {
        struct commit *commit;
        if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
                explicit_tracking = 1;
  
-       if ((track == BRANCH_TRACK_OVERRIDE || clobber_head)
 -      if (validate_new_branchname(name, &ref, force,
 -                                  track == BRANCH_TRACK_OVERRIDE ||
 -                                  clobber_head_ok)) {
++      if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok)
 +          ? validate_branchname(name, &ref)
 +          : validate_new_branchname(name, &ref, force)) {
                if (!force)
                        dont_change_ref = 1;
                else
diff --combined branch.h
index be5e5d13083a98e57e834d81afe5450f11202130,f66536a10c9ec3d8dd6c148968364a5f7e3996a3..473d0a93e910d8f053e6e8c919e430048a3033a0
+++ b/branch.h
   *
   *   - force enables overwriting an existing (non-head) branch
   *
+  *   - clobber_head_ok allows the currently checked out (hence existing)
+  *     branch to be overwritten; without 'force', it has no effect.
+  *
   *   - reflog creates a reflog for the branch
   *
+  *   - quiet suppresses tracking information
+  *
   *   - track causes the new branch to be configured to merge the remote branch
   *     that start_name is a tracking branch for (if any).
+  *
   */
  void create_branch(const char *name, const char *start_name,
-                  int force, int reflog,
-                  int clobber_head, int quiet, enum branch_track track);
+                  int force, int clobber_head_ok,
+                  int reflog, int quiet, enum branch_track track);
  
  /*
 - * Validates that the requested branch may be created, returning the
 - * 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.
 - *
 + * Check if 'name' can be a valid name for a branch; die otherwise.
 + * Return 1 if the named branch already exists; return 0 otherwise.
 + * Fill ref with the full refname for the branch.
 + */
 +extern int validate_branchname(const char *name, struct strbuf *ref);
 +
 +/*
 + * Check if a branch 'name' can be created as a new branch; die otherwise.
 + * 'force' can be used when it is OK for the named branch already exists.
 + * Return 1 if the named branch already exists; return 0 otherwise.
 + * Fill ref with the full refname for the branch.
   */
 -int validate_new_branchname(const char *name, struct strbuf *ref, int force, int attr_only);
 +extern int validate_new_branchname(const char *name, struct strbuf *ref, int force);
  
  /*
   * Remove information about the state of working on the current
diff --combined builtin/branch.c
index af95ad2192004ad3a712c15f1e50356daadb7c86,196d5fe9b5e2a9859d34fb2d55fcb11da68aa712..8dcc2ed058be6e653f885e48c9aa5f465a5f9749
@@@ -462,7 -462,10 +462,9 @@@ static void copy_or_rename_branch(cons
  {
        struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
        struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
+       const char *interpreted_oldname = NULL;
+       const char *interpreted_newname = NULL;
        int recovery = 0;
 -      int clobber_head_ok;
  
        if (!oldname) {
                if (copy)
         * A command like "git branch -M currentbranch currentbranch" cannot
         * cause the worktree to become inconsistent with HEAD, so allow it.
         */
 -      clobber_head_ok = !strcmp(oldname, newname);
 -
 -      validate_new_branchname(newname, &newref, force, clobber_head_ok);
 +      if (!strcmp(oldname, newname))
 +              validate_branchname(newname, &newref);
 +      else
 +              validate_new_branchname(newname, &newref, force);
  
        reject_rebase_or_bisect_branch(oldref.buf);
  
+       if (!skip_prefix(oldref.buf, "refs/heads/", &interpreted_oldname) ||
+           !skip_prefix(newref.buf, "refs/heads/", &interpreted_newname)) {
+               die("BUG: expected prefix missing for refs");
+       }
        if (copy)
                strbuf_addf(&logmsg, "Branch: copied %s to %s",
                            oldref.buf, newref.buf);
  
        if (recovery) {
                if (copy)
-                       warning(_("Copied a misnamed branch '%s' away"),
-                               oldref.buf + 11);
+                       warning(_("Created a copy of a misnamed branch '%s'"),
+                               interpreted_oldname);
                else
                        warning(_("Renamed a misnamed branch '%s' away"),
-                               oldref.buf + 11);
+                               interpreted_oldname);
        }
  
        if (!copy &&
  
        strbuf_release(&logmsg);
  
-       strbuf_addf(&oldsection, "branch.%s", oldref.buf + 11);
+       strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
        strbuf_release(&oldref);
-       strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
+       strbuf_addf(&newsection, "branch.%s", interpreted_newname);
        strbuf_release(&newref);
        if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
                die(_("Branch is renamed, but update of config-file failed"));
@@@ -675,9 -682,6 +682,9 @@@ int cmd_branch(int argc, const char **a
                copy *= 2;
        }
  
 +      if (list)
 +              setup_auto_pager("branch", 1);
 +
        if (delete) {
                if (!argc)
                        die(_("branch name required"));
        } else if (argc > 0 && argc <= 2) {
                struct branch *branch = branch_get(argv[0]);
  
 -              if (!strcmp(argv[0], "HEAD"))
 -                      die(_("it does not make sense to create 'HEAD' manually"));
 -
                if (!branch)
                        die(_("no such branch '%s'"), argv[0]);
  
                        die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
  
                create_branch(argv[0], (argc == 2) ? argv[1] : head,
-                             force, reflog, 0, quiet, track);
+                             force, 0, reflog, quiet, track);
  
        } else
                usage_with_options(builtin_branch_usage, options);
diff --combined builtin/checkout.c
index f9f3797e1191291c48609b01cf5c388ff86d0def,6068f8d8cf67b0c4c0880cf22be9aec706ea85bf..8bdc927d3f561e62802ab53be0b3029325e6ebcd
@@@ -1,6 -1,5 +1,6 @@@
  #include "builtin.h"
  #include "config.h"
 +#include "checkout.h"
  #include "lockfile.h"
  #include "parse-options.h"
  #include "refs.h"
@@@ -401,16 -400,10 +401,16 @@@ static void show_local_changes(struct o
  static void describe_detached_head(const char *msg, struct commit *commit)
  {
        struct strbuf sb = STRBUF_INIT;
 +
        if (!parse_commit(commit))
                pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
 -      fprintf(stderr, "%s %s... %s\n", msg,
 -              find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
 +      if (print_sha1_ellipsis()) {
 +              fprintf(stderr, "%s %s... %s\n", msg,
 +                      find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
 +      } else {
 +              fprintf(stderr, "%s %s %s\n", msg,
 +                      find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
 +      }
        strbuf_release(&sb);
  }
  
@@@ -521,7 -514,7 +521,7 @@@ static int merge_working_tree(const str
                }
                tree = parse_tree_indirect(old->commit ?
                                           &old->commit->object.oid :
 -                                         &empty_tree_oid);
 +                                         the_hash_algo->empty_tree);
                init_tree_desc(&trees[0], tree->buffer, tree->size);
                tree = parse_tree_indirect(&new->commit->object.oid);
                init_tree_desc(&trees[1], tree->buffer, tree->size);
@@@ -647,8 -640,8 +647,8 @@@ static void update_refs_for_switch(cons
                else
                        create_branch(opts->new_branch, new->name,
                                      opts->new_branch_force ? 1 : 0,
-                                     opts->new_branch_log,
                                      opts->new_branch_force ? 1 : 0,
+                                     opts->new_branch_log,
                                      opts->quiet,
                                      opts->track);
                new->name = opts->new_branch;
@@@ -879,6 -872,46 +879,6 @@@ static int git_checkout_config(const ch
        return git_xmerge_config(var, value, NULL);
  }
  
 -struct tracking_name_data {
 -      /* const */ char *src_ref;
 -      char *dst_ref;
 -      struct object_id *dst_oid;
 -      int unique;
 -};
 -
 -static int check_tracking_name(struct remote *remote, void *cb_data)
 -{
 -      struct tracking_name_data *cb = cb_data;
 -      struct refspec query;
 -      memset(&query, 0, sizeof(struct refspec));
 -      query.src = cb->src_ref;
 -      if (remote_find_tracking(remote, &query) ||
 -          get_oid(query.dst, cb->dst_oid)) {
 -              free(query.dst);
 -              return 0;
 -      }
 -      if (cb->dst_ref) {
 -              free(query.dst);
 -              cb->unique = 0;
 -              return 0;
 -      }
 -      cb->dst_ref = query.dst;
 -      return 0;
 -}
 -
 -static const char *unique_tracking_name(const char *name, struct object_id *oid)
 -{
 -      struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
 -      cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 -      cb_data.dst_oid = oid;
 -      for_each_remote(check_tracking_name, &cb_data);
 -      free(cb_data.src_ref);
 -      if (cb_data.unique)
 -              return cb_data.dst_ref;
 -      free(cb_data.dst_ref);
 -      return NULL;
 -}
 -
  static int parse_branchname_arg(int argc, const char **argv,
                                int dwim_new_local_branch_ok,
                                struct branch_info *new,
@@@ -1254,11 -1287,11 +1254,11 @@@ int cmd_checkout(int argc, const char *
        if (opts.new_branch) {
                struct strbuf buf = STRBUF_INIT;
  
 -              opts.branch_exists =
 -                      validate_new_branchname(opts.new_branch, &buf,
 -                                              !!opts.new_branch_force,
 -                                              !!opts.new_branch_force);
 -
 +              if (opts.new_branch_force)
 +                      opts.branch_exists = validate_branchname(opts.new_branch, &buf);
 +              else
 +                      opts.branch_exists =
 +                              validate_new_branchname(opts.new_branch, &buf, 0);
                strbuf_release(&buf);
        }