From: Junio C Hamano Date: Sun, 27 Aug 2017 05:55:05 +0000 (-0700) Subject: Merge branch 'tb/apply-with-crlf' X-Git-Tag: v2.15.0-rc0~125 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/a17483fcfe313e9ff5b9b0eb8245605fe7f66ea7?hp=-c Merge branch 'tb/apply-with-crlf' "git apply" that is used as a better "patch -p1" failed to apply a taken from a file with CRLF line endings to a file with CRLF line endings. The root cause was because it misused convert_to_git() that tried to do "safe-crlf" processing by looking at the index entry at the same path, which is a nonsense---in that mode, "apply" is not working on the data in (or derived from) the index at all. This has been fixed. * tb/apply-with-crlf: apply: file commited with CRLF should roundtrip diff and apply convert: add SAFE_CRLF_KEEP_CRLF --- a17483fcfe313e9ff5b9b0eb8245605fe7f66ea7 diff --combined apply.c index 956f56a927,66c68f193a..86666217d4 --- a/apply.c +++ b/apply.c @@@ -80,6 -80,7 +80,6 @@@ int init_apply_state(struct apply_stat { memset(state, 0, sizeof(*state)); state->prefix = prefix; - state->prefix_length = state->prefix ? strlen(state->prefix) : 0; state->lock_file = lock_file; state->newfd = -1; state->apply = 1; @@@ -219,6 -220,7 +219,7 @@@ struct patch unsigned int recount:1; unsigned int conflicted_threeway:1; unsigned int direct_to_threeway:1; + unsigned int crlf_in_old:1; struct fragment *fragments; char *result; size_t resultsize; @@@ -785,11 -787,11 +786,11 @@@ static int guess_p_value(struct apply_s * Does it begin with "a/$our-prefix" and such? Then this is * very likely to apply to our directory. */ - if (!strncmp(name, state->prefix, state->prefix_length)) + if (starts_with(name, state->prefix)) val = count_slashes(state->prefix); else { cp++; - if (!strncmp(cp, state->prefix, state->prefix_length)) + if (starts_with(cp, state->prefix)) val = count_slashes(state->prefix) + 1; } } @@@ -1661,6 -1663,19 +1662,19 @@@ static void check_whitespace(struct app record_ws_error(state, result, line + 1, len - 2, state->linenr); } + /* + * Check if the patch has context lines with CRLF or + * the patch wants to remove lines with CRLF. + */ + static void check_old_for_crlf(struct patch *patch, const char *line, int len) + { + if (len >= 2 && line[len-1] == '\n' && line[len-2] == '\r') { + patch->ws_rule |= WS_CR_AT_EOL; + patch->crlf_in_old = 1; + } + } + + /* * Parse a unified diff. Note that this really needs to parse each * fragment separately, since the only way to know the difference @@@ -1711,11 -1726,14 +1725,14 @@@ static int parse_fragment(struct apply_ if (!deleted && !added) leading++; trailing++; + check_old_for_crlf(patch, line, len); if (!state->apply_in_reverse && state->ws_error_action == correct_ws_error) check_whitespace(state, line, len, patch->ws_rule); break; case '-': + if (!state->apply_in_reverse) + check_old_for_crlf(patch, line, len); if (state->apply_in_reverse && state->ws_error_action != nowarn_ws_error) check_whitespace(state, line, len, patch->ws_rule); @@@ -1724,6 -1742,8 +1741,8 @@@ trailing = 0; break; case '+': + if (state->apply_in_reverse) + check_old_for_crlf(patch, line, len); if (!state->apply_in_reverse && state->ws_error_action != nowarn_ws_error) check_whitespace(state, line, len, patch->ws_rule); @@@ -2088,9 -2108,10 +2107,9 @@@ static int use_patch(struct apply_stat int i; /* Paths outside are not touched regardless of "--include" */ - if (0 < state->prefix_length) { - int pathlen = strlen(pathname); - if (pathlen <= state->prefix_length || - memcmp(state->prefix, pathname, state->prefix_length)) + if (state->prefix && *state->prefix) { + const char *rest; + if (!skip_prefix(pathname, state->prefix, &rest) || !*rest) return 0; } @@@ -2266,8 -2287,11 +2285,11 @@@ static void show_stats(struct apply_sta add, pluses, del, minuses); } - static int read_old_data(struct stat *st, const char *path, struct strbuf *buf) + static int read_old_data(struct stat *st, struct patch *patch, + const char *path, struct strbuf *buf) { + enum safe_crlf safe_crlf = patch->crlf_in_old ? + SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_RENORMALIZE; switch (st->st_mode & S_IFMT) { case S_IFLNK: if (strbuf_readlink(buf, path, st->st_size) < 0) @@@ -2276,7 -2300,15 +2298,15 @@@ case S_IFREG: if (strbuf_read_file(buf, path, st->st_size) != st->st_size) return error(_("unable to open or read %s"), path); - convert_to_git(&the_index, path, buf->buf, buf->len, buf, 0); + /* + * "git apply" without "--index/--cached" should never look + * at the index; the target file may not have been added to + * the index yet, and we may not even be in any Git repository. + * Pass NULL to convert_to_git() to stress this; the function + * should never look at the index when explicit crlf option + * is given. + */ + convert_to_git(NULL, path, buf->buf, buf->len, buf, safe_crlf); return 0; default: return -1; @@@ -2807,10 -2839,13 +2837,10 @@@ static void update_image(struct apply_s img->line_allocated = img->line; } if (preimage_limit != postimage->nr) - memmove(img->line + applied_pos + postimage->nr, - img->line + applied_pos + preimage_limit, - (img->nr - (applied_pos + preimage_limit)) * - sizeof(*img->line)); - memcpy(img->line + applied_pos, - postimage->line, - postimage->nr * sizeof(*img->line)); + MOVE_ARRAY(img->line + applied_pos + postimage->nr, + img->line + applied_pos + preimage_limit, + img->nr - (applied_pos + preimage_limit)); + COPY_ARRAY(img->line + applied_pos, postimage->line, postimage->nr); if (!state->allow_overlap) for (i = 0; i < postimage->nr; i++) img->line[applied_pos + i].flag |= LINE_PATCHED; @@@ -3379,6 -3414,7 +3409,7 @@@ static int load_patch_target(struct app struct strbuf *buf, const struct cache_entry *ce, struct stat *st, + struct patch *patch, const char *name, unsigned expected_mode) { @@@ -3394,7 -3430,7 +3425,7 @@@ } 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)) + if (read_old_data(st, patch, name, buf)) return error(_("failed to read %s"), name); } } @@@ -3427,7 -3463,7 +3458,7 @@@ static int load_preimage(struct apply_s /* We have a patched copy in memory; use that. */ strbuf_add(&buf, previous->result, previous->resultsize); } else { - status = load_patch_target(state, &buf, ce, st, + status = load_patch_target(state, &buf, ce, st, patch, patch->old_name, patch->old_mode); if (status < 0) return status; @@@ -3515,7 -3551,7 +3546,7 @@@ static int load_current(struct apply_st if (verify_index_match(ce, &st)) return error(_("%s: does not match index"), name); - status = load_patch_target(state, &buf, ce, &st, name, mode); + status = load_patch_target(state, &buf, ce, &st, patch, name, mode); if (status < 0) return status; else if (status) @@@ -3546,7 -3582,7 +3577,7 @@@ static int try_threeway(struct apply_st /* Preimage the patch was prepared for */ if (patch->is_new) write_sha1_file("", 0, blob_type, pre_oid.hash); - else if (get_sha1(patch->old_sha1_prefix, pre_oid.hash) || + else if (get_oid(patch->old_sha1_prefix, &pre_oid) || read_blob_object(&buf, &pre_oid, patch->old_mode)) return error(_("repository lacks the necessary blob to fall back on 3-way merge.")); @@@ -4070,7 -4106,7 +4101,7 @@@ static int build_fake_ancestor(struct a else return error(_("sha1 information is lacking or " "useless for submodule %s"), name); - } else if (!get_sha1_blob(patch->old_sha1_prefix, oid.hash)) { + } else if (!get_oid_blob(patch->old_sha1_prefix, &oid)) { ; /* ok */ } else if (!patch->lines_added && !patch->lines_deleted) { /* mode-only change: update the current */ diff --combined convert.c index 1012462e3c,040123b4fe..c5f0b21037 --- a/convert.c +++ b/convert.c @@@ -501,7 -501,6 +501,7 @@@ static int apply_single_file_filter(con #define CAP_CLEAN (1u<<0) #define CAP_SMUDGE (1u<<1) +#define CAP_DELAY (1u<<2) struct cmd2process { struct subprocess_entry subprocess; /* must be the first member! */ @@@ -513,49 -512,69 +513,49 @@@ static struct hashmap subprocess_map static int start_multi_file_filter_fn(struct subprocess_entry *subprocess) { - int err; + static int versions[] = {2, 0}; + static struct subprocess_capability capabilities[] = { + { "clean", CAP_CLEAN }, + { "smudge", CAP_SMUDGE }, + { "delay", CAP_DELAY }, + { NULL, 0 } + }; struct cmd2process *entry = (struct cmd2process *)subprocess; - struct string_list cap_list = STRING_LIST_INIT_NODUP; - char *cap_buf; - const char *cap_name; - struct child_process *process = &subprocess->process; - const char *cmd = subprocess->cmd; - - sigchain_push(SIGPIPE, SIG_IGN); - - err = packet_writel(process->in, "git-filter-client", "version=2", NULL); - if (err) - goto done; - - err = strcmp(packet_read_line(process->out, NULL), "git-filter-server"); - if (err) { - error("external filter '%s' does not support filter protocol version 2", cmd); - goto done; - } - err = strcmp(packet_read_line(process->out, NULL), "version=2"); - if (err) - goto done; - err = packet_read_line(process->out, NULL) != NULL; - if (err) - goto done; - - err = packet_writel(process->in, "capability=clean", "capability=smudge", NULL); - - for (;;) { - cap_buf = packet_read_line(process->out, NULL); - if (!cap_buf) - break; - string_list_split_in_place(&cap_list, cap_buf, '=', 1); - - if (cap_list.nr != 2 || strcmp(cap_list.items[0].string, "capability")) - continue; - - cap_name = cap_list.items[1].string; - if (!strcmp(cap_name, "clean")) { - entry->supported_capabilities |= CAP_CLEAN; - } else if (!strcmp(cap_name, "smudge")) { - entry->supported_capabilities |= CAP_SMUDGE; - } else { - warning( - "external filter '%s' requested unsupported filter capability '%s'", - cmd, cap_name - ); - } + return subprocess_handshake(subprocess, "git-filter", versions, NULL, + capabilities, + &entry->supported_capabilities); +} - string_list_clear(&cap_list, 0); +static void handle_filter_error(const struct strbuf *filter_status, + struct cmd2process *entry, + const unsigned int wanted_capability) { + if (!strcmp(filter_status->buf, "error")) + ; /* The filter signaled a problem with the file. */ + else if (!strcmp(filter_status->buf, "abort") && wanted_capability) { + /* + * The filter signaled a permanent problem. Don't try to filter + * files with the same command for the lifetime of the current + * Git process. + */ + entry->supported_capabilities &= ~wanted_capability; + } else { + /* + * Something went wrong with the protocol filter. + * Force shutdown and restart if another blob requires filtering. + */ + error("external filter '%s' failed", entry->subprocess.cmd); + subprocess_stop(&subprocess_map, &entry->subprocess); + free(entry); } - -done: - sigchain_pop(SIGPIPE); - - return err; } static int apply_multi_file_filter(const char *path, const char *src, size_t len, int fd, struct strbuf *dst, const char *cmd, - const unsigned int wanted_capability) + const unsigned int wanted_capability, + struct delayed_checkout *dco) { int err; + int can_delay = 0; struct cmd2process *entry; struct child_process *process; struct strbuf nbuf = STRBUF_INIT; @@@ -564,7 -583,8 +564,7 @@@ if (!subprocess_map_initialized) { subprocess_map_initialized = 1; - hashmap_init(&subprocess_map, (hashmap_cmp_fn) cmd2process_cmp, - NULL, 0); + hashmap_init(&subprocess_map, cmd2process_cmp, NULL, 0); entry = NULL; } else { entry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd); @@@ -583,12 -603,12 +583,12 @@@ } process = &entry->subprocess.process; - if (!(wanted_capability & entry->supported_capabilities)) + if (!(entry->supported_capabilities & wanted_capability)) return 0; - if (CAP_CLEAN & wanted_capability) + if (wanted_capability & CAP_CLEAN) filter_type = "clean"; - else if (CAP_SMUDGE & wanted_capability) + else if (wanted_capability & CAP_SMUDGE) filter_type = "smudge"; else die("unexpected filter type"); @@@ -610,14 -630,6 +610,14 @@@ if (err) goto done; + if ((entry->supported_capabilities & CAP_DELAY) && + dco && dco->state == CE_CAN_DELAY) { + can_delay = 1; + err = packet_write_fmt_gently(process->in, "can-delay=1\n"); + if (err) + goto done; + } + err = packet_flush_gently(process->in); if (err) goto done; @@@ -633,73 -645,14 +633,73 @@@ if (err) goto done; - err = strcmp(filter_status.buf, "success"); + if (can_delay && !strcmp(filter_status.buf, "delayed")) { + string_list_insert(&dco->filters, cmd); + string_list_insert(&dco->paths, path); + } else { + /* The filter got the blob and wants to send us a response. */ + err = strcmp(filter_status.buf, "success"); + if (err) + goto done; + + err = read_packetized_to_strbuf(process->out, &nbuf) < 0; + if (err) + goto done; + + err = subprocess_read_status(process->out, &filter_status); + if (err) + goto done; + + err = strcmp(filter_status.buf, "success"); + } + +done: + sigchain_pop(SIGPIPE); + + if (err) + handle_filter_error(&filter_status, entry, wanted_capability); + else + strbuf_swap(dst, &nbuf); + strbuf_release(&nbuf); + return !err; +} + + +int async_query_available_blobs(const char *cmd, struct string_list *available_paths) +{ + int err; + char *line; + struct cmd2process *entry; + struct child_process *process; + struct strbuf filter_status = STRBUF_INIT; + + assert(subprocess_map_initialized); + entry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd); + if (!entry) { + error("external filter '%s' is not available anymore although " + "not all paths have been filtered", cmd); + return 0; + } + process = &entry->subprocess.process; + sigchain_push(SIGPIPE, SIG_IGN); + + err = packet_write_fmt_gently( + process->in, "command=list_available_blobs\n"); if (err) goto done; - err = read_packetized_to_strbuf(process->out, &nbuf) < 0; + err = packet_flush_gently(process->in); if (err) goto done; + while ((line = packet_read_line(process->out, NULL))) { + const char *path; + if (skip_prefix(line, "pathname=", &path)) + string_list_insert(available_paths, xstrdup(path)); + else + ; /* ignore unknown keys */ + } + err = subprocess_read_status(process->out, &filter_status); if (err) goto done; @@@ -709,8 -662,29 +709,8 @@@ done: sigchain_pop(SIGPIPE); - if (err) { - if (!strcmp(filter_status.buf, "error")) { - /* The filter signaled a problem with the file. */ - } else if (!strcmp(filter_status.buf, "abort")) { - /* - * The filter signaled a permanent problem. Don't try to filter - * files with the same command for the lifetime of the current - * Git process. - */ - entry->supported_capabilities &= ~wanted_capability; - } else { - /* - * Something went wrong with the protocol filter. - * Force shutdown and restart if another blob requires filtering. - */ - error("external filter '%s' failed", cmd); - subprocess_stop(&subprocess_map, &entry->subprocess); - free(entry); - } - } else { - strbuf_swap(dst, &nbuf); - } - strbuf_release(&nbuf); + if (err) + handle_filter_error(&filter_status, entry, 0); return !err; } @@@ -725,8 -699,7 +725,8 @@@ static struct convert_driver static int apply_filter(const char *path, const char *src, size_t len, int fd, struct strbuf *dst, struct convert_driver *drv, - const unsigned int wanted_capability) + const unsigned int wanted_capability, + struct delayed_checkout *dco) { const char *cmd = NULL; @@@ -736,16 -709,15 +736,16 @@@ if (!dst) return 1; - if ((CAP_CLEAN & wanted_capability) && !drv->process && drv->clean) + if ((wanted_capability & CAP_CLEAN) && !drv->process && drv->clean) cmd = drv->clean; - else if ((CAP_SMUDGE & wanted_capability) && !drv->process && drv->smudge) + else if ((wanted_capability & CAP_SMUDGE) && !drv->process && drv->smudge) cmd = drv->smudge; if (cmd && *cmd) return apply_single_file_filter(path, src, len, fd, dst, cmd); else if (drv->process && *drv->process) - return apply_multi_file_filter(path, src, len, fd, dst, drv->process, wanted_capability); + return apply_multi_file_filter(path, src, len, fd, dst, + drv->process, wanted_capability, dco); return 0; } @@@ -1086,7 -1058,7 +1086,7 @@@ int would_convert_to_git_filter_fd(cons if (!ca.drv->required) return 0; - return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN); + return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN, NULL); } const char *get_convert_attr_ascii(const char *path) @@@ -1124,7 -1096,7 +1124,7 @@@ int convert_to_git(const struct index_s convert_attrs(&ca, path); - ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN); + ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN, NULL); if (!ret && ca.drv && ca.drv->required) die("%s: clean filter '%s' failed", path, ca.drv->name); @@@ -1132,10 -1104,12 +1132,12 @@@ src = dst->buf; len = dst->len; } - ret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe); - if (ret && dst) { - src = dst->buf; - len = dst->len; + if (checksafe != SAFE_CRLF_KEEP_CRLF) { + ret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe); + if (ret && dst) { + src = dst->buf; + len = dst->len; + } } return ret | ident_to_git(path, src, len, dst, ca.ident); } @@@ -1150,7 -1124,7 +1152,7 @@@ void convert_to_git_filter_fd(const str assert(ca.drv); assert(ca.drv->clean || ca.drv->process); - if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN)) + if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN, NULL)) die("%s: clean filter '%s' failed", path, ca.drv->name); crlf_to_git(istate, path, dst->buf, dst->len, dst, ca.crlf_action, checksafe); @@@ -1159,7 -1133,7 +1161,7 @@@ static int convert_to_working_tree_internal(const char *path, const char *src, size_t len, struct strbuf *dst, - int normalizing) + int normalizing, struct delayed_checkout *dco) { int ret = 0, ret_filter = 0; struct conv_attrs ca; @@@ -1184,30 -1158,22 +1186,30 @@@ } } - ret_filter = apply_filter(path, src, len, -1, dst, ca.drv, CAP_SMUDGE); + ret_filter = apply_filter( + path, src, len, -1, dst, ca.drv, CAP_SMUDGE, dco); if (!ret_filter && ca.drv && ca.drv->required) die("%s: smudge filter %s failed", path, ca.drv->name); return ret | ret_filter; } +int async_convert_to_working_tree(const char *path, const char *src, + size_t len, struct strbuf *dst, + void *dco) +{ + return convert_to_working_tree_internal(path, src, len, dst, 0, dco); +} + int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst) { - return convert_to_working_tree_internal(path, src, len, dst, 0); + return convert_to_working_tree_internal(path, src, len, dst, 0, NULL); } int renormalize_buffer(const struct index_state *istate, const char *path, const char *src, size_t len, struct strbuf *dst) { - int ret = convert_to_working_tree_internal(path, src, len, dst, 1); + int ret = convert_to_working_tree_internal(path, src, len, dst, 1, NULL); if (ret) { src = dst->buf; len = dst->len; diff --combined convert.h index 6b06144281,cabd5ed6dd..4f2da225a8 --- a/convert.h +++ b/convert.h @@@ -4,15 -4,14 +4,16 @@@ #ifndef CONVERT_H #define CONVERT_H +#include "string-list.h" + struct index_state; enum safe_crlf { SAFE_CRLF_FALSE = 0, SAFE_CRLF_FAIL = 1, SAFE_CRLF_WARN = 2, - SAFE_CRLF_RENORMALIZE = 3 + SAFE_CRLF_RENORMALIZE = 3, + SAFE_CRLF_KEEP_CRLF = 4 }; extern enum safe_crlf safe_crlf; @@@ -36,26 -35,6 +37,26 @@@ enum eol #endif }; +enum ce_delay_state { + CE_NO_DELAY = 0, + CE_CAN_DELAY = 1, + CE_RETRY = 2 +}; + +struct delayed_checkout { + /* + * State of the currently processed cache entry. If the state is + * CE_CAN_DELAY, then the filter can delay the current cache entry. + * If the state is CE_RETRY, then this signals the filter that the + * cache entry was requested before. + */ + enum ce_delay_state state; + /* List of filter drivers that signaled delayed blobs. */ + struct string_list filters; + /* List of delayed blobs identified by their path. */ + struct string_list paths; +}; + extern enum eol core_eol; extern const char *get_cached_convert_stats_ascii(const struct index_state *istate, const char *path); @@@ -68,10 -47,6 +69,10 @@@ extern int convert_to_git(const struct struct strbuf *dst, enum safe_crlf checksafe); extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst); +extern int async_convert_to_working_tree(const char *path, const char *src, + size_t len, struct strbuf *dst, + void *dco); +extern int async_query_available_blobs(const char *cmd, struct string_list *available_paths); extern int renormalize_buffer(const struct index_state *istate, const char *path, const char *src, size_t len, struct strbuf *dst);