Merge branch 'jc/apply-ws-fix-expands-report'
authorJunio C Hamano <gitster@pobox.com>
Tue, 3 Mar 2015 22:37:02 +0000 (14:37 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 3 Mar 2015 22:37:02 +0000 (14:37 -0800)
"git apply --whitespace=fix" fixed whitespace errors in the common
context lines but did so without reporting.

* jc/apply-ws-fix-expands-report:
apply: detect and mark whitespace errors in context lines when fixing

1  2 
builtin/apply.c
diff --combined builtin/apply.c
index 525db4a2a237fa0b12a254fd72b86dcd66738e93,a51a1b0872af6e6778e05079accfa8966f30f39a..65b97eee69d4814bf80f87b1fd6857b8ed80cb90
@@@ -7,7 -7,6 +7,7 @@@
   *
   */
  #include "cache.h"
 +#include "lockfile.h"
  #include "cache-tree.h"
  #include "quote.h"
  #include "blob.h"
@@@ -51,12 -50,11 +51,12 @@@ static int apply_verbosely
  static int allow_overlap;
  static int no_add;
  static int threeway;
 +static int unsafe_paths;
  static const char *fake_ancestor;
  static int line_termination = '\n';
  static unsigned int p_context = UINT_MAX;
  static const char * const apply_usage[] = {
 -      N_("git apply [options] [<patch>...]"),
 +      N_("git apply [<options>] [<patch>...]"),
        NULL
  };
  
@@@ -437,7 -435,7 +437,7 @@@ static unsigned long linelen(const cha
  
  static int is_dev_null(const char *str)
  {
 -      return !memcmp("/dev/null", str, 9) && isspace(str[9]);
 +      return skip_prefix(str, "/dev/null", &str) && isspace(*str);
  }
  
  #define TERM_SPACE    1
@@@ -658,6 -656,11 +658,6 @@@ static size_t diff_timestamp_len(const 
        return line + len - end;
  }
  
 -static char *null_strdup(const char *s)
 -{
 -      return s ? xstrdup(s) : NULL;
 -}
 -
  static char *find_name_common(const char *line, const char *def,
                              int p_value, const char *end, int terminate)
  {
                        start = line;
        }
        if (!start)
 -              return squash_slash(null_strdup(def));
 +              return squash_slash(xstrdup_or_null(def));
        len = line - start;
        if (!len)
 -              return squash_slash(null_strdup(def));
 +              return squash_slash(xstrdup_or_null(def));
  
        /*
         * Generally we prefer the shorter name, especially
@@@ -905,7 -908,7 +905,7 @@@ static void parse_traditional_patch(con
                        patch->old_name = name;
                } else {
                        patch->old_name = name;
 -                      patch->new_name = null_strdup(name);
 +                      patch->new_name = xstrdup_or_null(name);
                }
        }
        if (!name)
@@@ -994,7 -997,7 +994,7 @@@ static int gitdiff_delete(const char *l
  {
        patch->is_delete = 1;
        free(patch->old_name);
 -      patch->old_name = null_strdup(patch->def_name);
 +      patch->old_name = xstrdup_or_null(patch->def_name);
        return gitdiff_oldmode(line, patch);
  }
  
@@@ -1002,7 -1005,7 +1002,7 @@@ static int gitdiff_newfile(const char *
  {
        patch->is_new = 1;
        free(patch->new_name);
 -      patch->new_name = null_strdup(patch->def_name);
 +      patch->new_name = xstrdup_or_null(patch->def_name);
        return gitdiff_newmode(line, patch);
  }
  
@@@ -1072,7 -1075,7 +1072,7 @@@ static int gitdiff_index(const char *li
  
        line = ptr + 2;
        ptr = strchr(line, ' ');
 -      eol = strchr(line, '\n');
 +      eol = strchrnul(line, '\n');
  
        if (!ptr || eol < ptr)
                ptr = eol;
@@@ -1278,7 -1281,9 +1278,7 @@@ static int parse_git_header(const char 
         */
        patch->def_name = git_header_name(line, len);
        if (patch->def_name && root) {
 -              char *s = xmalloc(root_len + strlen(patch->def_name) + 1);
 -              strcpy(s, root);
 -              strcpy(s + root_len, patch->def_name);
 +              char *s = xstrfmt("%s%s", root, patch->def_name);
                free(patch->def_name);
                patch->def_name = s;
        }
@@@ -1601,6 -1606,9 +1601,9 @@@ static int parse_fragment(const char *l
                        if (!deleted && !added)
                                leading++;
                        trailing++;
+                       if (!apply_in_reverse &&
+                           ws_error_action == correct_ws_error)
+                               check_whitespace(line, len, patch->ws_rule);
                        break;
                case '-':
                        if (apply_in_reverse &&
@@@ -1917,66 -1925,6 +1920,66 @@@ static int parse_binary(char *buffer, u
        return used;
  }
  
 +static void prefix_one(char **name)
 +{
 +      char *old_name = *name;
 +      if (!old_name)
 +              return;
 +      *name = xstrdup(prefix_filename(prefix, prefix_length, *name));
 +      free(old_name);
 +}
 +
 +static void prefix_patch(struct patch *p)
 +{
 +      if (!prefix || p->is_toplevel_relative)
 +              return;
 +      prefix_one(&p->new_name);
 +      prefix_one(&p->old_name);
 +}
 +
 +/*
 + * include/exclude
 + */
 +
 +static struct string_list limit_by_name;
 +static int has_include;
 +static void add_name_limit(const char *name, int exclude)
 +{
 +      struct string_list_item *it;
 +
 +      it = string_list_append(&limit_by_name, name);
 +      it->util = exclude ? NULL : (void *) 1;
 +}
 +
 +static int use_patch(struct patch *p)
 +{
 +      const char *pathname = p->new_name ? p->new_name : p->old_name;
 +      int i;
 +
 +      /* Paths outside are not touched regardless of "--include" */
 +      if (0 < prefix_length) {
 +              int pathlen = strlen(pathname);
 +              if (pathlen <= prefix_length ||
 +                  memcmp(prefix, pathname, prefix_length))
 +                      return 0;
 +      }
 +
 +      /* See if it matches any of exclude/include rule */
 +      for (i = 0; i < limit_by_name.nr; i++) {
 +              struct string_list_item *it = &limit_by_name.items[i];
 +              if (!wildmatch(it->string, pathname, 0, NULL))
 +                      return (it->util != NULL);
 +      }
 +
 +      /*
 +       * If we had any include, a path that does not match any rule is
 +       * not used.  Otherwise, we saw bunch of exclude rules (or none)
 +       * and such a path is used.
 +       */
 +      return !has_include;
 +}
 +
 +
  /*
   * Read the patch text in "buffer" that extends for "size" bytes; stop
   * reading after seeing a single patch (i.e. changes to a single file).
@@@ -1992,14 -1940,9 +1995,14 @@@ static int parse_chunk(char *buffer, un
        if (offset < 0)
                return offset;
  
 -      patch->ws_rule = whitespace_rule(patch->new_name
 -                                       ? patch->new_name
 -                                       : patch->old_name);
 +      prefix_patch(patch);
 +
 +      if (!use_patch(patch))
 +              patch->ws_rule = 0;
 +      else
 +              patch->ws_rule = whitespace_rule(patch->new_name
 +                                               ? patch->new_name
 +                                               : patch->old_name);
  
        patchsize = parse_single_patch(buffer + offset + hdrsize,
                                       size - offset - hdrsize, patch);
@@@ -2648,7 -2591,7 +2651,7 @@@ static void update_image(struct image *
                 * NOTE: this knows that we never call remove_first_line()
                 * on anything other than pre/post image.
                 */
 -              img->line = xrealloc(img->line, nr * sizeof(*img->line));
 +              REALLOC_ARRAY(img->line, nr);
                img->line_allocated = img->line;
        }
        if (preimage_limit != postimage->nr)
@@@ -3169,15 -3112,13 +3172,15 @@@ static void prepare_fn_table(struct pat
        }
  }
  
 -static int checkout_target(struct cache_entry *ce, struct stat *st)
 +static int checkout_target(struct index_state *istate,
 +                         struct cache_entry *ce, struct stat *st)
  {
        struct checkout costate;
  
        memset(&costate, 0, sizeof(costate));
        costate.base_dir = "";
        costate.refresh_cache = 1;
 +      costate.istate = istate;
        if (checkout_entry(ce, &costate, NULL) || lstat(ce->name, st))
                return error(_("cannot checkout %s"), ce->name);
        return 0;
@@@ -3222,7 -3163,7 +3225,7 @@@ static int load_patch_target(struct str
                             const char *name,
                             unsigned expected_mode)
  {
 -      if (cached) {
 +      if (cached || check_index) {
                if (read_file_or_gitlink(ce, buf))
                        return error(_("read of %s failed"), name);
        } else if (name) {
                                return read_file_or_gitlink(ce, buf);
                        else
                                return SUBMODULE_PATCH_WITHOUT_INDEX;
 +              } else if (has_symlink_leading_path(name, strlen(name))) {
 +                      return error(_("reading from '%s' beyond a symbolic link"), name);
                } else {
                        if (read_old_data(st, name, buf))
                                return error(_("read of %s failed"), name);
@@@ -3346,7 -3285,7 +3349,7 @@@ static int load_current(struct image *i
        if (lstat(name, &st)) {
                if (errno != ENOENT)
                        return error(_("%s: %s"), name, strerror(errno));
 -              if (checkout_target(ce, &st))
 +              if (checkout_target(&the_index, ce, &st))
                        return -1;
        }
        if (verify_index_match(ce, &st))
@@@ -3500,7 -3439,7 +3503,7 @@@ static int check_preimage(struct patch 
                }
                *ce = active_cache[pos];
                if (stat_ret < 0) {
 -                      if (checkout_target(*ce, st))
 +                      if (checkout_target(&the_index, *ce, st))
                                return -1;
                }
                if (!cached && verify_index_match(*ce, st))
@@@ -3572,121 -3511,6 +3575,121 @@@ static int check_to_create(const char *
        return 0;
  }
  
 +/*
 + * We need to keep track of how symlinks in the preimage are
 + * manipulated by the patches.  A patch to add a/b/c where a/b
 + * is a symlink should not be allowed to affect the directory
 + * the symlink points at, but if the same patch removes a/b,
 + * it is perfectly fine, as the patch removes a/b to make room
 + * to create a directory a/b so that a/b/c can be created.
 + */
 +static struct string_list symlink_changes;
 +#define SYMLINK_GOES_AWAY 01
 +#define SYMLINK_IN_RESULT 02
 +
 +static uintptr_t register_symlink_changes(const char *path, uintptr_t what)
 +{
 +      struct string_list_item *ent;
 +
 +      ent = string_list_lookup(&symlink_changes, path);
 +      if (!ent) {
 +              ent = string_list_insert(&symlink_changes, path);
 +              ent->util = (void *)0;
 +      }
 +      ent->util = (void *)(what | ((uintptr_t)ent->util));
 +      return (uintptr_t)ent->util;
 +}
 +
 +static uintptr_t check_symlink_changes(const char *path)
 +{
 +      struct string_list_item *ent;
 +
 +      ent = string_list_lookup(&symlink_changes, path);
 +      if (!ent)
 +              return 0;
 +      return (uintptr_t)ent->util;
 +}
 +
 +static void prepare_symlink_changes(struct patch *patch)
 +{
 +      for ( ; patch; patch = patch->next) {
 +              if ((patch->old_name && S_ISLNK(patch->old_mode)) &&
 +                  (patch->is_rename || patch->is_delete))
 +                      /* the symlink at patch->old_name is removed */
 +                      register_symlink_changes(patch->old_name, SYMLINK_GOES_AWAY);
 +
 +              if (patch->new_name && S_ISLNK(patch->new_mode))
 +                      /* the symlink at patch->new_name is created or remains */
 +                      register_symlink_changes(patch->new_name, SYMLINK_IN_RESULT);
 +      }
 +}
 +
 +static int path_is_beyond_symlink_1(struct strbuf *name)
 +{
 +      do {
 +              unsigned int change;
 +
 +              while (--name->len && name->buf[name->len] != '/')
 +                      ; /* scan backwards */
 +              if (!name->len)
 +                      break;
 +              name->buf[name->len] = '\0';
 +              change = check_symlink_changes(name->buf);
 +              if (change & SYMLINK_IN_RESULT)
 +                      return 1;
 +              if (change & SYMLINK_GOES_AWAY)
 +                      /*
 +                       * This cannot be "return 0", because we may
 +                       * see a new one created at a higher level.
 +                       */
 +                      continue;
 +
 +              /* otherwise, check the preimage */
 +              if (check_index) {
 +                      struct cache_entry *ce;
 +
 +                      ce = cache_file_exists(name->buf, name->len, ignore_case);
 +                      if (ce && S_ISLNK(ce->ce_mode))
 +                              return 1;
 +              } else {
 +                      struct stat st;
 +                      if (!lstat(name->buf, &st) && S_ISLNK(st.st_mode))
 +                              return 1;
 +              }
 +      } while (1);
 +      return 0;
 +}
 +
 +static int path_is_beyond_symlink(const char *name_)
 +{
 +      int ret;
 +      struct strbuf name = STRBUF_INIT;
 +
 +      assert(*name_ != '\0');
 +      strbuf_addstr(&name, name_);
 +      ret = path_is_beyond_symlink_1(&name);
 +      strbuf_release(&name);
 +
 +      return ret;
 +}
 +
 +static void die_on_unsafe_path(struct patch *patch)
 +{
 +      const char *old_name = NULL;
 +      const char *new_name = NULL;
 +      if (patch->is_delete)
 +              old_name = patch->old_name;
 +      else if (!patch->is_new && !patch->is_copy)
 +              old_name = patch->old_name;
 +      if (!patch->is_delete)
 +              new_name = patch->new_name;
 +
 +      if (old_name && !verify_path(old_name))
 +              die(_("invalid path '%s'"), old_name);
 +      if (new_name && !verify_path(new_name))
 +              die(_("invalid path '%s'"), new_name);
 +}
 +
  /*
   * Check and apply the patch in-core; leave the result in patch->result
   * for the caller to write it out to the final destination.
@@@ -3774,22 -3598,6 +3777,22 @@@ static int check_patch(struct patch *pa
                }
        }
  
 +      if (!unsafe_paths)
 +              die_on_unsafe_path(patch);
 +
 +      /*
 +       * An attempt to read from or delete a path that is beyond a
 +       * symbolic link will be prevented by load_patch_target() that
 +       * is called at the beginning of apply_data() so we do not
 +       * have to worry about a patch marked with "is_delete" bit
 +       * here.  We however need to make sure that the patch result
 +       * is not deposited to a path that is beyond a symbolic link
 +       * here.
 +       */
 +      if (!patch->is_delete && path_is_beyond_symlink(patch->new_name))
 +              return error(_("affected file '%s' is beyond a symbolic link"),
 +                           patch->new_name);
 +
        if (apply_data(patch, &st, ce) < 0)
                return error(_("%s: patch does not apply"), name);
        patch->rejected = 0;
@@@ -3800,7 -3608,6 +3803,7 @@@ static int check_patch_list(struct patc
  {
        int err = 0;
  
 +      prepare_symlink_changes(patch);
        prepare_fn_table(patch);
        while (patch) {
                if (apply_verbosely)
@@@ -3865,7 -3672,7 +3868,7 @@@ static void build_fake_ancestor(struct 
  {
        struct patch *patch;
        struct index_state result = { NULL };
 -      int fd;
 +      static struct lock_file lock;
  
        /* Once we start supporting the reverse patch, it may be
         * worth showing the new sha1 prefix, but until then...
                        if (!preimage_sha1_in_gitlink_patch(patch, sha1))
                                ; /* ok, the textual part looks sane */
                        else
 -                              die("sha1 information is lacking or useless for submoule %s",
 +                              die("sha1 information is lacking or useless for submodule %s",
                                    name);
                } else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
                        ; /* ok */
                        die ("Could not add %s to temporary index", name);
        }
  
 -      fd = open(filename, O_WRONLY | O_CREAT, 0666);
 -      if (fd < 0 || write_index(&result, fd) || close(fd))
 +      hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
 +      if (write_locked_index(&result, &lock, COMMIT_LOCK))
                die ("Could not write temporary index to %s", filename);
  
        discard_index(&result);
@@@ -4066,10 -3873,9 +4069,10 @@@ static void add_index_file(const char *
        ce->ce_flags = create_ce_flags(0);
        ce->ce_namelen = namelen;
        if (S_ISGITLINK(mode)) {
 -              const char *s = buf;
 +              const char *s;
  
 -              if (get_sha1_hex(s + strlen("Subproject commit "), ce->sha1))
 +              if (!skip_prefix(buf, "Subproject commit ", &s) ||
 +                  get_sha1_hex(s, ce->sha1))
                        die(_("corrupt patch for submodule %s"), path);
        } else {
                if (!cached) {
@@@ -4335,7 -4141,7 +4338,7 @@@ static int write_out_results(struct pat
        if (cpath.nr) {
                struct string_list_item *item;
  
 -              sort_string_list(&cpath);
 +              string_list_sort(&cpath);
                for_each_string_list_item(item, &cpath)
                        fprintf(stderr, "U %s\n", item->string);
                string_list_clear(&cpath, 0);
  
  static struct lock_file lock_file;
  
 -static struct string_list limit_by_name;
 -static int has_include;
 -static void add_name_limit(const char *name, int exclude)
 -{
 -      struct string_list_item *it;
 -
 -      it = string_list_append(&limit_by_name, name);
 -      it->util = exclude ? NULL : (void *) 1;
 -}
 -
 -static int use_patch(struct patch *p)
 -{
 -      const char *pathname = p->new_name ? p->new_name : p->old_name;
 -      int i;
 -
 -      /* Paths outside are not touched regardless of "--include" */
 -      if (0 < prefix_length) {
 -              int pathlen = strlen(pathname);
 -              if (pathlen <= prefix_length ||
 -                  memcmp(prefix, pathname, prefix_length))
 -                      return 0;
 -      }
 -
 -      /* See if it matches any of exclude/include rule */
 -      for (i = 0; i < limit_by_name.nr; i++) {
 -              struct string_list_item *it = &limit_by_name.items[i];
 -              if (!wildmatch(it->string, pathname, 0, NULL))
 -                      return (it->util != NULL);
 -      }
 -
 -      /*
 -       * If we had any include, a path that does not match any rule is
 -       * not used.  Otherwise, we saw bunch of exclude rules (or none)
 -       * and such a path is used.
 -       */
 -      return !has_include;
 -}
 -
 -
 -static void prefix_one(char **name)
 -{
 -      char *old_name = *name;
 -      if (!old_name)
 -              return;
 -      *name = xstrdup(prefix_filename(prefix, prefix_length, *name));
 -      free(old_name);
 -}
 -
 -static void prefix_patches(struct patch *p)
 -{
 -      if (!prefix || p->is_toplevel_relative)
 -              return;
 -      for ( ; p; p = p->next) {
 -              prefix_one(&p->new_name);
 -              prefix_one(&p->old_name);
 -      }
 -}
 -
  #define INACCURATE_EOF        (1<<0)
  #define RECOUNT               (1<<1)
  
@@@ -4373,6 -4237,8 +4376,6 @@@ static int apply_patch(int fd, const ch
                        break;
                if (apply_in_reverse)
                        reverse_patches(patch);
 -              if (prefix)
 -                      prefix_patches(patch);
                if (use_patch(patch)) {
                        patch_stats(patch);
                        *listp = patch;
        return 0;
  }
  
 -static int git_apply_config(const char *var, const char *value, void *cb)
 +static void git_apply_config(void)
  {
 -      if (!strcmp(var, "apply.whitespace"))
 -              return git_config_string(&apply_default_whitespace, var, value);
 -      else if (!strcmp(var, "apply.ignorewhitespace"))
 -              return git_config_string(&apply_default_ignorewhitespace, var, value);
 -      return git_default_config(var, value, cb);
 +      git_config_get_string_const("apply.whitespace", &apply_default_whitespace);
 +      git_config_get_string_const("apply.ignorewhitespace", &apply_default_ignorewhitespace);
 +      git_config(git_default_config, NULL);
  }
  
  static int option_parse_exclude(const struct option *opt,
@@@ -4539,8 -4407,6 +4542,8 @@@ int cmd_apply(int argc, const char **ar
                        N_("make sure the patch is applicable to the current index")),
                OPT_BOOL(0, "cached", &cached,
                        N_("apply a patch without touching the working tree")),
 +              OPT_BOOL(0, "unsafe-paths", &unsafe_paths,
 +                      N_("accept a patch that touches outside the working area")),
                OPT_BOOL(0, "apply", &force_apply,
                        N_("also apply the patch (use with --stat/--summary/--check)")),
                OPT_BOOL('3', "3way", &threeway,
  
        prefix = prefix_;
        prefix_length = prefix ? strlen(prefix) : 0;
 -      git_config(git_apply_config, NULL);
 +      git_apply_config();
        if (apply_default_whitespace)
                parse_whitespace_option(apply_default_whitespace);
        if (apply_default_ignorewhitespace)
                        die(_("--cached outside a repository"));
                check_index = 1;
        }
 +      if (check_index)
 +              unsafe_paths = 0;
 +
        for (i = 0; i < argc; i++) {
                const char *arg = argv[i];
                int fd;
        }
  
        if (update_index) {
 -              if (write_cache(newfd, active_cache, active_nr) ||
 -                  commit_locked_index(&lock_file))
 +              if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
                        die(_("Unable to write new index file"));
        }