From d487b0ba50884fb1fd1767ac8c9adc58066ad999 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 6 Aug 2014 14:26:24 -0700 Subject: [PATCH 1/3] apply: use the right attribute for paths in non-Git patches We parse each patchfile and find the name of the path the patch applies to, and then use that name to consult the attribute system to find the whitespace rules to be used, and also the target file (either in the working tree or in the index) to replay the changes against. Unlike a Git-generated patch, a non-Git patch is taken to have the pathnames relative to the current working directory. The names found in such a patch are modified by prepending the prefix by the prefix_patches() helper function introduced in 56185f49 (git-apply: require -p when working in a subdirectory., 2007-02-19). However, this prefixing is done after the patch is fully parsed and affects only what target files are patched. Because the attributes are checked against the names found in the patch during the parsing, not against the final pathname, the whitespace check that is done during parsing ends up using attributes for a wrong path for non-Git patches. Fix this by doing the prefix much earlier, immediately after the header part of each patch is parsed and we learn the name of the path the patch affects. Signed-off-by: Junio C Hamano --- builtin/apply.c | 41 +++++++++++++++++++---------------------- t/t4119-apply-config.sh | 17 +++++++++++++++++ 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 6013e1913c..4270cded0b 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -1920,6 +1920,23 @@ static int parse_binary(char *buffer, unsigned long size, struct patch *patch) 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); +} + /* * 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). @@ -1935,6 +1952,8 @@ static int parse_chunk(char *buffer, unsigned long size, struct patch *patch) if (offset < 0) return offset; + prefix_patch(patch); + patch->ws_rule = whitespace_rule(patch->new_name ? patch->new_name : patch->old_name); @@ -4164,26 +4183,6 @@ static int use_patch(struct patch *p) 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) @@ -4209,8 +4208,6 @@ static int apply_patch(int fd, const char *filename, int options) break; if (apply_in_reverse) reverse_patches(patch); - if (prefix) - prefix_patches(patch); if (use_patch(patch)) { patch_stats(patch); *listp = patch; diff --git a/t/t4119-apply-config.sh b/t/t4119-apply-config.sh index 3d0384daa8..be325fa1ae 100755 --- a/t/t4119-apply-config.sh +++ b/t/t4119-apply-config.sh @@ -159,4 +159,21 @@ test_expect_success 'same but with traditional patch input of depth 2' ' check_result sub/file1 ' +test_expect_success 'in subdir with traditional patch input' ' + cd "$D" && + git config apply.whitespace strip && + cat >.gitattributes <<-EOF && + /* whitespace=blank-at-eol + sub/* whitespace=-blank-at-eol + EOF + rm -f sub/file1 && + cp saved sub/file1 && + git update-index --refresh && + + cd sub && + git apply ../gpatch.file && + echo "B " >expect && + test_cmp expect file1 +' + test_done From 3ee2ad14c6a6823d16eeda2aa48496d4b6e16f70 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 6 Aug 2014 13:11:17 -0700 Subject: [PATCH 2/3] apply: hoist use_patch() helper for path exclusion up We will be adding a caller to the function a bit earlier in this file in a later patch. Signed-off-by: Junio C Hamano --- builtin/apply.c | 81 ++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 4270cded0b..bf075cc29d 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -1937,6 +1937,49 @@ static void prefix_patch(struct patch *p) 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 (!fnmatch(it->string, pathname, 0)) + 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). @@ -4145,44 +4188,6 @@ static int write_out_results(struct patch *list) 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 (!fnmatch(it->string, pathname, 0)) - 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; -} - #define INACCURATE_EOF (1<<0) #define RECOUNT (1<<1) From 477a08af04c227064860ce99197c501037f7f39c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 6 Aug 2014 13:09:05 -0700 Subject: [PATCH 3/3] apply: omit ws check for excluded paths Whitespace breakages are checked while the patch is being parsed. Disable them at the beginning of parse_chunk(), where each individual patch is parsed, immediately after we learn the name of the file the patch applies to and before we start parsing the diff contained in the patch. One may naively think that we should be able to not just skip the whitespace checks but simply fast-forward to the next patch without doing anything once use_patch() tells us that this patch is not going to be used. But in reality we cannot really skip much of the parsing in order to do such a "fast-forward", primarily because parsing "@@ -k,l +m,n @@" lines and counting the input lines is how we determine the boundaries of individual patches. Signed-off-by: Junio C Hamano --- builtin/apply.c | 9 ++++++--- t/t4124-apply-ws-rule.sh | 11 +++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index bf075cc29d..13319e8f2f 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -1997,9 +1997,12 @@ static int parse_chunk(char *buffer, unsigned long size, struct patch *patch) prefix_patch(patch); - patch->ws_rule = whitespace_rule(patch->new_name - ? patch->new_name - : patch->old_name); + 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); diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh index 5d0c598338..c6474de4c8 100755 --- a/t/t4124-apply-ws-rule.sh +++ b/t/t4124-apply-ws-rule.sh @@ -512,4 +512,15 @@ test_expect_success 'whitespace=fix to expand' ' git -c core.whitespace=tab-in-indent apply --whitespace=fix patch ' +test_expect_success 'whitespace check skipped for excluded paths' ' + git config core.whitespace blank-at-eol && + >used && + >unused && + git add used unused && + echo "used" >used && + echo "unused " >unused && + git diff-files -p used unused >patch && + git apply --include=used --stat --whitespace=error