From: Junio C Hamano Date: Tue, 3 Mar 2015 22:37:02 +0000 (-0800) Subject: Merge branch 'jc/apply-ws-fix-expands-report' X-Git-Tag: v2.4.0-rc0~78 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/73b690a634b7065471144e0b97c74f30cef2acef?ds=inline;hp=-c Merge branch 'jc/apply-ws-fix-expands-report' "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 --- 73b690a634b7065471144e0b97c74f30cef2acef diff --combined builtin/apply.c index 525db4a2a2,a51a1b0872..65b97eee69 --- a/builtin/apply.c +++ b/builtin/apply.c @@@ -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] [...]"), + N_("git apply [] [...]"), 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) { @@@ -680,10 -683,10 +680,10 @@@ 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) { @@@ -3231,8 -3172,6 +3234,8 @@@ 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... @@@ -3883,7 -3690,7 +3886,7 @@@ 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 */ @@@ -3903,8 -3710,8 +3906,8 @@@ 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); @@@ -4348,6 -4154,64 +4351,6 @@@ 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; @@@ -4430,11 -4296,13 +4433,11 @@@ 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, @@@ -4584,7 -4450,7 +4587,7 @@@ 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) @@@ -4613,9 -4479,6 +4616,9 @@@ 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; @@@ -4666,7 -4529,8 +4669,7 @@@ } 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")); }