git-apply patches
Here is a series of patches which fix some bugs in and add some new command line options to git-apply.
[PATCH 1/9] Fix git patch header processing in git-apply.
[PATCH 2/9] Fix detection of files with only one line in git-apply. [PATCH 3/9] Fix processing of a patch file which modifies the same file in git-apply. [PATCH 4/9] Fix the procssing of multiple patch files with --check in git-apply. [PATCH 5/9] New option --force-delete for git-apply.
[PATCH 6/9] New option --ignore-whitespace for git-apply.
[PATCH 7/9] New option --ignore-applied for git-apply.
[PATCH 8/9] New git-apply test cases for patches with mulitple fragments. [PATCH 9/9] New git-apply test cases for scanning forwards and backwards.
Robert Fitzsimons
[email protected]
PATCH 1/9 Fix git patch header processing in git-apply.
Stop processing and return NULL if we encounter a '\n' character before we have two matching names in the git header.
---
apply.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
289e53f770e37dfe25c740788256d24c19c2e16d
apply.c +1 -1
@@ -387,7 +387,7 @@ static char *git_header_name(char *line)default :continue ;case '\n' : -break ; +return NULL ;case '\t' :case ' ' : second = name+len;for (;;) {
PATCH 2/9 Fix detection of files with only one line in git-apply.
A patch which added one line to an empty file or removed the only line from a one line file would be incorrectly detected as an invalid new or delete patch. Added a new test case.
---
apply.c | 4 ++-- t/t4103-apply-oneline.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 t/t4103-apply-oneline.sh
babd40afef54b06775b11a877b347685edbedcd2
apply.c +2 -2
@@ -638,9 +638,9 @@ static int parse_fragment(char *line, un patch->new_name =NULL ; } -if (patch->is_new != !oldlines) +if (patch->is_new && (patch->is_new != !oldlines))return error ("new file depends on old contents" ); -if (patch->is_delete != !newlines) { +if (patch->is_delete && (patch->is_delete != !newlines)) {if (newlines)return error ("deleted file still has contents" );fprintf (stderr,"** warning: file %s becomes empty but is not deleted\n" , patch->new_name);
t/t4103-apply-oneline.sh new file +46 -0
46 lines not on this page · raw message
PATCH 3/9 Fix processing of a patch file which modifies the same file in git-apply.
A patch file (or stdin) which modifies the same file more then once will fail to apply the patch correctly. In the worst case it will apply some of the patch and leave an invalid output file(s).
apply.c has been changed to search for previously processed files and use the in memory copy of the data instead of the on disk image. Added a series of test cases for the processing of complex single file patches.
---
apply.c | 64 +++++++++++++- t/t4104-apply-complex.sh | 211 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 5 deletions(-) create mode 100644 t/t4104-apply-complex.sh
be1277e3531a03441839fd1dc750dad640ebf218
apply.c +59 -5
@@ -67,6 +67,7 @@ struct patch {char *result;unsigned long resultsize;struct patch *next; +struct patch *prev; };#define CHUNKSIZE (8192)
@@ -974,7 +975,7 @@ static int apply_fragments(struct bufferreturn 0 ; } -static int apply_data (struct patch *patch,struct stat *st) +static int apply_data (struct patch *patch,struct patch *old_patch,struct stat *st) {char *buf;unsigned long size, alloc;
@@ -983,7 +984,13 @@ static int apply_data(struct patch *patc size =0 ; alloc =0 ; buf =NULL ; -if (patch->old_name) { + +if (patch->old_name && old_patch) { + size = old_patch->resultsize; + alloc = size +8192 ; + buf =xmalloc (alloc); +memcpy (buf, old_patch->result, size); + }else if (patch->old_name) { size = st->st_size; alloc = size +8192 ; buf =xmalloc (alloc);
@@ -1010,8 +1017,46 @@ static int check_patch(struct patch *patstruct stat st;const char *old_name = patch->old_name;const char *new_name = patch->new_name; +struct patch *old_patch =NULL ; +struct patch *new_patch =NULL ;if (old_name) { +for (old_patch = patch->prev; old_patch; old_patch = old_patch->prev) { +if (old_patch->new_name && !strcmp (old_name, old_patch->new_name)) { +break ; + } +if (old_patch->old_name && !strcmp (old_name, old_patch->old_name)) { +if (old_patch->is_delete || old_patch->is_rename) { +return error ("%s: file missing because of previous patch" , old_name); + } +break ; + } + } + } + +if (new_name) { +for (new_patch = patch->prev; new_patch; new_patch = new_patch->prev) { +if (new_patch->new_name && !strcmp (new_name, new_patch->new_name)) { +if (patch->is_new || patch->is_rename || patch->is_copy) +return error ("%s: file exists from previous patch (new)" , new_name); +break ; + } +if (new_patch->old_name && !strcmp (new_name, new_patch->old_name)) { +if (!(patch->is_new || patch->is_delete || patch->is_rename || patch->is_copy)) +break ; +if (new_patch->is_delete || new_patch->is_rename) +break ; +return error ("%s: file exists from previous patch (old)" , new_name); + } + } + } + +if (old_patch) { +if (patch->is_new <0 ) + patch->is_new =0 ; +if (!patch->old_mode) + patch->old_mode = old_patch->new_mode; + }else if (old_name) {int changed;if (lstat (old_name, &st) <0 )
@@ -1036,7 +1081,14 @@ static int check_patch(struct patch *pat old_name, st.st_mode, patch->old_mode); } -if (new_name && (patch->is_new | patch->is_rename | patch->is_copy)) { +if (new_patch) { +if (!patch->new_mode) { +if (patch->is_new) + patch->new_mode =S_IFREG |0644 ; +else + patch->new_mode = patch->old_mode; + } + }else if (new_name && (patch->is_new | patch->is_rename | patch->is_copy)) {if (check_index &&cache_name_pos (new_name,strlen (new_name)) >=0 )return error ("%s: already exists in index" , new_name);if (!lstat (new_name, &st))
@@ -1061,7 +1113,7 @@ static int check_patch(struct patch *pat same ? "" : " of ", same ? "" : old_name); } -if (apply_data (patch, &st) <0 ) +if (apply_data (patch, old_patch, &st) <0 )return error ("%s: patch does not apply" , old_name);return 0 ; }
@@ -1393,6 +1445,7 @@ static int apply_patch(int fd)unsigned long offset, size;char *buffer =read_patch_file (fd, &size);struct patch *list =NULL , **listp = &list; +struct patch *list_prev =NULL ;int skipped_patch =0 ;if (!buffer)
@@ -1409,7 +1462,8 @@ static int apply_patch(int fd)break ;if (use_patch (patch)) {patch_stats (patch); - *listp = patch; + patch->prev = list_prev; + list_prev = *listp = patch; listp = &patch->next; }else {/* perhaps free it a bit better? */
t/t4104-apply-complex.sh new file +211 -0
211 lines not on this page · raw message
PATCH 4/9 Fix the procssing of multiple patch files with --check in git-apply.
When the --check option was used with multiple patch files which modify the same files, the patch can fail because the previously modified contents aren't written to the disk.
So save the in memory patch contents across the processing of multiple patch files. Added a new test case for --check with multiple patch files.
---
apply.c | 10 ++++- t/t4105-apply-check.sh | 89 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 t/t4105-apply-check.sh
80ed0622e31bd52cedd073f54c9a9364b5289c89
apply.c +8 -2
@@ -1439,13 +1439,16 @@ static int use_patch(struct patch *p)return 1 ; } +struct patch *master_list =NULL ; +struct patch *master_list_prev =NULL ; +static int apply_patch (int fd) {int newfd;unsigned long offset, size;char *buffer =read_patch_file (fd, &size); -struct patch *list =NULL , **listp = &list; -struct patch *list_prev =NULL ; +struct patch *list = master_list, **listp = &list; +struct patch *list_prev = master_list_prev;int skipped_patch =0 ;if (!buffer)
@@ -1504,6 +1507,9 @@ static int apply_patch(int fd)if (summary)summary_patch_list (list); + master_list = list; + master_list_prev = list_prev; +free (buffer);return 0 ; }
t/t4105-apply-check.sh new file +89 -0
89 lines not on this page · raw message
PATCH 5/9 New option --force-delete for git-apply.
Allow the user to force a patch that deletes a file to succeed even though the file might still contain data. Added a test case.
---
Documentation/git-apply.txt | 8 +++++- apply.c | 12 ++++++++- t/t4106-apply-force-delete.h | 54 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 t/t4106-apply-force-delete.h
3fb13d4a86a1af7b545ce5f99904c1a5adae27e7
Documentation/git-apply.txt +7 -1
@@ -9,7 +9,9 @@ git-apply - Apply patch on a GIT index f SYNOPSIS -------- -'git-apply' [--no-merge] [--stat] [--summary] [--check] [--index] [--show-files] [--apply] [<patch>...] +'git-apply' [--no-merge] [--stat] [--summary] [--check] + [--index] [--show-files] [--apply] + [--force-delete] [<patch>...] DESCRIPTION -----------
@@ -61,6 +63,10 @@ OPTIONS patch. Give this flag after those flags to also apply the patch. +--force-delete:: + Force a patch which deletes a file to succeed even if + file contents would be left behind, lines added or the + patch will not apply. Author ------
apply.c +10 -2
@@ -32,8 +32,9 @@ static int summary = 0;static int check =0 ;static int apply =1 ;static int show_files =0 ; +static int force_delete =0 ;static const char apply_usage[] = -"git-apply [--no-merge] [--stat] [--summary] [--check] [--index] [--apply] [--show-files] <patch>..." ; +"git-apply [--no-merge] [--stat] [--summary] [--check] [--index] [--apply] [--show-files] [--force-delete] <patch>..." ;/* * For "diff-stat" like behaviour, we keep track of the biggest change
@@ -641,7 +642,7 @@ static int parse_fragment(char *line, unif (patch->is_new && (patch->is_new != !oldlines))return error ("new file depends on old contents" ); -if (patch->is_delete && (patch->is_delete != !newlines)) { +if (!force_delete && patch->is_delete && (patch->is_delete != !newlines)) {if (newlines)return error ("deleted file still has contents" );fprintf (stderr,"** warning: file %s becomes empty but is not deleted\n" , patch->new_name);
@@ -981,6 +982,9 @@ static int apply_data(struct patch *patcunsigned long size, alloc;struct buffer_desc desc; +if (force_delete && patch->is_delete) +return 0 ; + size =0 ; alloc =0 ; buf =NULL ;
@@ -1567,6 +1571,10 @@ int main(int argc, char **argv) show_files =1 ;continue ; } +if (!strcmp (arg,"--force-delete" )) { + force_delete =1 ; +continue ; + } fd =open (arg,O_RDONLY );if (fd <0 )usage (apply_usage);
t/t4106-apply-force-delete.h new file +54 -0
54 lines not on this page · raw message
PATCH 6/9 New option --ignore-whitespace for git-apply.
Allow the user to force a patch to be applied even though there might be whitespace differences. Added a test case for the new option.
---
Documentation/git-apply.txt | 7 +++- apply.c | 38 ++++++++++++++++++- t/t4107-apply-ignore-whitespace.sh | 72 ++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 t/t4107-apply-ignore-whitespace.sh
a928441d83ccc347b1d57fc561d413da56b93197
Documentation/git-apply.txt +6 -1
@@ -11,7 +11,8 @@ SYNOPSIS -------- 'git-apply' [--no-merge] [--stat] [--summary] [--check] [--index] [--show-files] [--apply] - [--force-delete] [<patch>...] + [--force-delete] [--ignore-whitespace] + [<patch>...] DESCRIPTION -----------
@@ -68,6 +69,10 @@ OPTIONS file contents would be left behind, lines added or the patch will not apply. +--ignore-whitespace:: + When matching the patch to the file contents ignore + sequences of tabs or spaces. + Author ------ Written by Linus Torvalds <[email protected]>
apply.c +35 -3
@@ -33,8 +33,9 @@ static int check = 0;static int apply =1 ;static int show_files =0 ;static int force_delete =0 ; +static int ignore_whitespace =0 ;static const char apply_usage[] = -"git-apply [--no-merge] [--stat] [--summary] [--check] [--index] [--apply] [--show-files] [--force-delete] <patch>..." ; +"git-apply [--no-merge] [--stat] [--summary] [--check] [--index] [--apply] [--show-files] [--force-delete] [--ignore-whitespace] <patch>..." ;/* * For "diff-stat" like behaviour, we keep track of the biggest change
@@ -109,6 +110,27 @@ static void *read_patch_file(int fd, unsreturn buffer; } +static int memcmp_ignore_whitespace (const char *s1,const char *s2,size_t n) +{ +const char *stop = s1 + n; +int result; + +if (!n) +return 0 ; + +do { +if (((*s1 ==' ' ) || (*s1 =='\t' )) && ((*s2 ==' ' ) || (*s2 =='\t' ))) { +while ((*s1 ==' ' ) || (*s1 =='\t' )) + s1++; +while ((*s2 ==' ' ) || (*s2 =='\t' )) + s2++; + } + result = *s1++ - *s2++; + }while (!result && (s1 < stop)); + +return result; +} +static unsigned long linelen (const char *buffer,unsigned long size) {unsigned long len =0 ;
@@ -839,7 +861,10 @@ static int find_offset(const char *buf, }/* Exact line number? */ -if (!memcmp (buf + start, fragment, fragsize)) +if (ignore_whitespace) { +if (!memcmp_ignore_whitespace (buf + start, fragment, fragsize)) +return start; + }else if (!memcmp (buf + start, fragment, fragsize))return start;/*
@@ -873,7 +898,10 @@ static int find_offset(const char *buf,if (try + fragsize > size)continue ; -if (memcmp (buf + try, fragment, fragsize)) +if (ignore_whitespace) { +if (memcmp_ignore_whitespace (buf + try, fragment, fragsize)) +continue ; + }else if (memcmp (buf + try, fragment, fragsize))continue ; n = (i >>1 )+1 ;if (i &1 )
@@ -1575,6 +1603,10 @@ int main(int argc, char **argv) force_delete =1 ;continue ; } +if (!strcmp (arg,"--ignore-whitespace" )) { + ignore_whitespace =1 ; +continue ; + } fd =open (arg,O_RDONLY );if (fd <0 )usage (apply_usage);
t/t4107-apply-ignore-whitespace.sh new file +72 -0
72 lines not on this page · raw message
PATCH 7/9 New option --ignore-applied for git-apply.
Allow the user to allow a patch which has some hunks (fragments) already applied to succeed. Added test case and documentation.
---
Documentation/git-apply.txt | 6 ++ apply.c | 12 +++++ t/t4108-apply-ignore-applied.sh | 94 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 t/t4108-apply-ignore-applied.sh
5e36f3b7c8ca0710096552ce3f20f16da04e601b
Documentation/git-apply.txt +5 -1
@@ -12,7 +12,7 @@ SYNOPSIS 'git-apply' [--no-merge] [--stat] [--summary] [--check] [--index] [--show-files] [--apply] [--force-delete] [--ignore-whitespace] - [<patch>...] + [--ignore-applied] [<patch>...] DESCRIPTION -----------
@@ -73,6 +73,10 @@ OPTIONS When matching the patch to the file contents ignore sequences of tabs or spaces. +--ignore-applied:: + If a patch hunk (fragment) fails to apply, reverse the + hunk and check if the hunk has already been applied. + Author ------ Written by Linus Torvalds <[email protected]>
apply.c +11 -1
@@ -34,8 +34,9 @@ static int apply = 1;static int show_files =0 ;static int force_delete =0 ;static int ignore_whitespace =0 ; +static int ignore_applied =0 ;static const char apply_usage[] = -"git-apply [--no-merge] [--stat] [--summary] [--check] [--index] [--apply] [--show-files] [--force-delete] [--ignore-whitespace] <patch>..." ; +"git-apply [--no-merge] [--stat] [--summary] [--check] [--index] [--apply] [--show-files] [--force-delete] [--ignore-whitespace] [--ignore-applied] <patch>..." ;/* * For "diff-stat" like behaviour, we keep track of the biggest change
@@ -985,6 +986,11 @@ static int apply_one_fragment(struct bufmemmove (buf + offset + newsize, buf + offset + oldsize, size - offset - newsize);memcpy (buf + offset, new, newsize); offset =0 ; + }else if (ignore_applied) { + offset =find_offset (buf, desc->size, new, newsize, frag->newpos); +if (offset >=0 ) { + offset =0 ; + } }free (old);
@@ -1607,6 +1613,10 @@ int main(int argc, char **argv) ignore_whitespace =1 ;continue ; } +if (!strcmp (arg,"--ignore-applied" )) { + ignore_applied =1 ; +continue ; + } fd =open (arg,O_RDONLY );if (fd <0 )usage (apply_usage);
t/t4108-apply-ignore-applied.sh new file +94 -0
94 lines not on this page · raw message
PATCH 8/9 New git-apply test cases for patches with mulitple fragments.
Added a test case for patches with multiple fragments.
---
t/t4109-apply-multifrag.sh | 176 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 176 insertions(+), 0 deletions(-) create mode 100644 t/t4109-apply-multifrag.sh
28b46271270ad79ba8720a3fde1f40724493fdfe
t/t4109-apply-multifrag.sh new file +176 -0
@@ -0,0 +1,176 @@ +#!/bin/sh +# +# Copyright (c) 2005 Junio C Hamano +# Copyright (c) 2005 Robert Fitzsimons +# + +test_description='git-apply test patches with multiple fragments. + +' +. ./test-lib.sh + +# setup + +cat > patch1.patch <<\EOF +diff --git a/main.c b/main.c +new file mode 100644 +--- /dev/null ++++ b/main.c +@@ -0,0 +1,23 @@ ++#include <stdio.h> ++ ++int func(int num); ++void print_int(int num); ++ ++int main() { ++ int i; ++ ++ for (i = 0; i < 10; i++) { ++ print_int(func(i)); ++ } ++ ++ return 0; ++} ++ ++int func(int num) { ++ return num * num; ++} ++ ++void print_int(int num) { ++ printf("%d", num); ++} ++ +EOF +cat > patch2.patch <<\EOF +diff --git a/main.c b/main.c +--- a/main.c ++++ b/main.c +@@ -1,7 +1,9 @@ ++#include <stdlib.h> + #include <stdio.h> + + int func(int num); + void print_int(int num); ++void print_ln(); + + int main() { + int i; +@@ -10,6 +12,8 @@ + print_int(func(i)); + } + ++ print_ln(); ++ + return 0; + } + +@@ -21,3 +25,7 @@ + printf("%d", num); + } + ++void print_ln() { ++ printf("\n"); ++} ++ +EOF +cat > patch3.patch <<\EOF +diff --git a/main.c b/main.c +--- a/main.c ++++ b/main.c +@@ -1,9 +1,7 @@ +-#include <stdlib.h> + #include <stdio.h> + + int func(int num); + void print_int(int num); +-void print_ln(); + + int main() { + int i; +@@ -12,8 +10,6 @@ + print_int(func(i)); + } + +- print_ln(); +- + return 0; + } + +@@ -25,7 +21,3 @@ + printf("%d", num); + } + +-void print_ln() { +- printf("\n"); +-} +- +EOF +cat > patch4.patch <<\EOF +diff --git a/main.c b/main.c +--- a/main.c ++++ b/main.c +@@ -1,13 +1,14 @@ + #include <stdio.h> + + int func(int num); +-void print_int(int num); ++int func2(int num); + + int main() { + int i; + + for (i = 0; i < 10; i++) { +- print_int(func(i)); ++ printf("%d", func(i)); ++ printf("%d", func3(i)); + } + + return 0; +@@ -17,7 +18,7 @@ + return num * num; + } + +-void print_int(int num) { +- printf("%d", num); ++int func2(int num) { ++ return num * num * num; + } + +EOF + +test_expect_success "S = git-apply (1)" \ + 'git-apply patch1.patch patch2.patch' +mv main.c main.c.git + +test_expect_success "S = patch (1)" \ + 'cat patch1.patch patch2.patch | patch -p1' + +test_expect_success "S = cmp (1)" \ + 'cmp main.c.git main.c' + +rm -f main.c main.c.git + +test_expect_success "S = git-apply (2)" \ + 'git-apply patch1.patch patch2.patch patch3.patch' +mv main.c main.c.git + +test_expect_success "S = patch (2)" \ + 'cat patch1.patch patch2.patch patch3.patch | patch -p1' + +test_expect_success "S = cmp (2)" \ + 'cmp main.c.git main.c' + +rm -f main.c main.c.git + +test_expect_success "S = git-apply (3)" \ + 'git-apply patch1.patch patch4.patch' +mv main.c main.c.git + +test_expect_success "S = patch (3)" \ + 'cat patch1.patch patch4.patch | patch -p1' + +test_expect_success "S = cmp (3)" \ + 'cmp main.c.git main.c' + +test_done +
PATCH 9/9 New git-apply test cases for scanning forwards and backwards.
Added a new test case for the scanning forwards and backwards for the correct location to apply a patch fragment.
---
t/t4110-apply-scan.sh | 101 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 101 insertions(+), 0 deletions(-) create mode 100644 t/t4110-apply-scan.sh
51e9797ee16edc9d6164f2c7afaf1f8245538255
t/t4110-apply-scan.sh new file +101 -0
@@ -0,0 +1,101 @@ +#!/bin/sh +# +# Copyright (c) 2005 Junio C Hamano +# Copyright (c) 2005 Robert Fitzsimons +# + +test_description='git-apply test for patches which require scanning forwards and backwards. + +' +. ./test-lib.sh + +# setup + +cat > patch1.patch <<\EOF +diff --git a/new.txt b/new.txt +new file mode 100644 +--- /dev/null ++++ b/new.txt +@@ -0,0 +1,12 @@ ++a1 ++a11 ++a111 ++a1111 ++b1 ++b11 ++b111 ++b1111 ++c1 ++c11 ++c111 ++c1111 +EOF +cat > patch2.patch <<\EOF +diff --git a/new.txt b/new.txt +--- a/new.txt ++++ b/new.txt +@@ -1,7 +1,3 @@ +-a1 +-a11 +-a111 +-a1111 + b1 + b11 + b111 +EOF +cat > patch3.patch <<\EOF +diff --git a/new.txt b/new.txt +--- a/new.txt ++++ b/new.txt +@@ -6,6 +6,10 @@ + b11 + b111 + b1111 ++b2 ++b22 ++b222 ++b2222 + c1 + c11 + c111 +EOF +cat > patch4.patch <<\EOF +diff --git a/new.txt b/new.txt +--- a/new.txt ++++ b/new.txt +@@ -1,3 +1,7 @@ ++a1 ++a11 ++a111 ++a1111 + b1 + b11 + b111 +EOF +cat > patch5.patch <<\EOF +diff --git a/new.txt b/new.txt +--- a/new.txt ++++ b/new.txt +@@ -10,3 +10,7 @@ + c11 + c111 + c1111 ++c2 ++c22 ++c222 ++c2222 +EOF + +test_expect_success "S = git-apply scan" \ + 'git-apply patch1.patch patch2.patch patch3.patch patch4.patch patch5.patch' +mv new.txt apply.txt + +test_expect_success "S = patch scan" \ + 'cat patch1.patch patch2.patch patch3.patch patch4.patch patch5.patch | patch' +mv new.txt patch.txt + +test_expect_success "S = cmp" \ + 'cmp apply.txt patch.txt' + +test_done +
Re: [PATCH 6/9] New option --ignore-whitespace for git-apply.
In reply to Robert FitzsimonsQuoted 2 lines · Robert Fitzsimons
Allow the user to force a patch to be applied even though there might be whitespace differences. Added a test case for the new option.
It might be worth doing fuzz in general, but treating whitespace-only fuzz specially is a good idea, since they are usually more benign.
Re: [PATCH 6/9] New option --ignore-whitespace for git-apply.
In reply to Robert FitzsimonsQuoted 3 lines · Robert Fitzsimons
Allow the user to force a patch to be applied even though there might be whitespace differences. Added a test case for the new option.
If you ignore whitespace, then you should probably accept patches that are whitespace corrupted in another way: lines missing the ' ' at the beginning. Especially lines that are empty. That's actually the most common form of whitespace corruption there is.
Linus
Re: [PATCH 6/9] New option --ignore-whitespace for git-apply.
Quoted 7 lines · Linus Torvalds
Quoted 2 lines · Robert Fitzsimons
Allow the user to force a patch to be applied even though there might be whitespace differences. Added a test case for the new option.
If you ignore whitespace, then you should probably accept patches that are
whitespace corrupted in another way: lines missing the ' ' at the
beginning. Especially lines that are empty. That's actually the most common form of whitespace corruption there is.
Actually, a more restricted from of --ignore-whitespace may be useful, --ignore-trailing-whitespace (needs a shorter name!). Ignore only trailing whitespace differences (including empty context lines without the initial space).
Re: [PATCH 5/9] New option --force-delete for git-apply.
In reply to Robert FitzsimonsQuoted 2 lines · Robert Fitzsimons
Allow the user to force a patch that deletes a file to succeed even though the file might still contain data. Added a test case.
Why?
I've seen cases where git-apply incorrectly thinks a patch leaves some data in a file to be deleted, when incomplete lines are involved. But I think that bug has been fixed, and the only case where you would need this new flag should be dealing with a corrupt/invalid patch.
Re: [PATCH 3/9] Fix processing of a patch file which modifies the same file in git-apply.
In reply to Robert FitzsimonsQuoted 3 lines · Robert Fitzsimons
A patch file (or stdin) which modifies the same file more then once will fail to apply the patch correctly. In the worst case it will apply some of the patch and leave an invalid output file(s).
I am not sure if this is solving the right problem. A patch that touches the same file more than once is hard to review and I think we should be discouraging the users to do so. The same comment goes for [PATCH 4/9].
Re: [PATCH 2/9] Fix detection of files with only one line in git-apply.
In reply to Robert FitzsimonsQuoted 4 lines · Robert Fitzsimons
A patch which added one line to an empty file or removed the only line from a one line file would be incorrectly detected as an invalid new or delete patch. Added a new test case.
This patch looks wrong or at least needs some more thought. The test was done the way it was done for a reason. For example:
if (patch->is_new != !oldlines)
return error("new file depends on old contents");actually has two error cases (admittedly it re-uses the same error message for both, which is a bit confusing):
- is_new, and oldlines != 0
This is the obvious error, and the one the message talk about.
- !is_new, and oldlines == 0
In kernel usage, empty files are not legal, and empty files get deleted. So this actually was intentional, even if the error message ends up being less than perfect.
The "is_delete" thing is the same. At least for the kernel, I want a patch that removes all content to also always _delete_ the file, because otherwise old-style patches are ambiguous. Is it a delete, or is it a "leave an empty file"?
Now, git patches aren't ambigious, so while git patches can clearly say "this is a patch to an existing empty file" vs "this patch creates this file", I wanted to have the behaviour where we maintain the principle that empty files are wrong.
Now, that's a kernel-specific thing, so it may be that enabling this behaviour this should be a command line flag, but the point is that I don't want to entirely lose the test.
Linus
Re: [PATCH 1/9] Fix git patch header processing in git-apply.
In reply to Robert FitzsimonsThanks for "apply" patches. I have merged some of them and pushed out.
I further wanted to apply the following to the proposed updates branch, but even with only the first one applied, it fails its own test, t4104-apply-complex.sh:
* Fix processing of a patch file which modifies the same file in git-apply.
* Fix the processing of multiple patch files with --check in git-apply.
* New option --ignore-whitespace for git-apply.
* New option --ignore-applied for git-apply.
*** t4104-apply-complex.sh ***
* FAIL 5: S = Nx Px1
cat Nx Px1 | git-apply -
* FAIL 6: S = Nx Px1 Px2
cat Nx Px1 Px2 | git-apply -
* FAIL 7: S = Nx Px1 Px2 Px3
cat Nx Px1 Px2 Px3 | git-apply -
* FAIL 8: S = Nx Px1 Px2 Px3 Dx
cat Nx Px1 Px2 Px3 Dx | git-apply -
* FAIL 13: S = Ny Ryx Px1 Px2 Cxy Px3 Dx
cat Ny Ryx Px1 Px2 Cxy Px3 Dx | git-apply -
* FAIL 14: S = Ny Ryx Px1 Px2 Cxy Px3 Dx Cyx
cat Ny Ryx Px1 Px2 Cxy Px3 Dx Cyx | git-apply -
* FAIL 15: S = Ny Cyx Px1 Px2 Px3 Dy Cxy Dx Cyx Dy Rxy Dy cat Ny Cyx Px1 Px2 Px3 Dy Cxy Dx Cyx Dy Rxy Dy| git-apply -
Re: [PATCH 1/9] Fix git patch header processing in git-apply.
I should have mentioned before, these changes are being used in a SourcePuller dump file to git import script I've been working on.
Quoted 3 lines
I further wanted to apply the following to the proposed updates branch, but even with only the first one applied, it fails its own test,
t4104-apply-complex.sh:
The test was the problem it tried to work with empty files.
Quoted 3 lines
* Fix processing of a patch file which modifies the same file in git-apply.
* Fix the processing of multiple patch files with --check in git-apply.
* New option --ignore-applied for git-apply.
I've fixed up the problems with these patches and rebased them.
Quoted 1 line
* New option --ignore-whitespace for git-apply.
This change was just wrong and could cause file corruption, I've got another version in the works which fixes the problem and adds some of the suggested features.
Robert Fitzsimons
[email protected]
PATCH 1/3 Fix the processing of a patch file which modifies the same file in git-apply.
A patch file (or stdin) which modifies the same file more then once will fail to apply the patch correctly. In the worst case it will apply some of the patch and leave an invalid output file(s).
apply.c has been changed to search for previously processed files and use the in memory copy of the data instead of the on disk image.
Added a series of test cases for the processing of complex single file patches. The test cases have been updated to not use empty files.
---
apply.c | 64 +++++++++++- t/t4104-apply-complex.sh | 241 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 300 insertions(+), 5 deletions(-) create mode 100644 t/t4104-apply-complex.sh
1c680d2768e248e4df3ef24591b7d53913199885
apply.c +59 -5
@@ -67,6 +67,7 @@ struct patch {char *result;unsigned long resultsize;struct patch *next; +struct patch *prev; };#define CHUNKSIZE (8192)
@@ -974,7 +975,7 @@ static int apply_fragments(struct bufferreturn 0 ; } -static int apply_data (struct patch *patch,struct stat *st) +static int apply_data (struct patch *patch,struct patch *old_patch,struct stat *st) {char *buf;unsigned long size, alloc;
@@ -983,7 +984,13 @@ static int apply_data(struct patch *patc size =0 ; alloc =0 ; buf =NULL ; -if (patch->old_name) { + +if (patch->old_name && old_patch) { + size = old_patch->resultsize; + alloc = size +8192 ; + buf =xmalloc (alloc); +memcpy (buf, old_patch->result, size); + }else if (patch->old_name) { size = st->st_size; alloc = size +8192 ; buf =xmalloc (alloc);
@@ -1010,8 +1017,46 @@ static int check_patch(struct patch *patstruct stat st;const char *old_name = patch->old_name;const char *new_name = patch->new_name; +struct patch *old_patch =NULL ; +struct patch *new_patch =NULL ;if (old_name) { +for (old_patch = patch->prev; old_patch; old_patch = old_patch->prev) { +if (old_patch->new_name && !strcmp (old_name, old_patch->new_name)) { +break ; + } +if (old_patch->old_name && !strcmp (old_name, old_patch->old_name)) { +if (old_patch->is_delete || old_patch->is_rename) { +return error ("%s: file missing because of previous patch" , old_name); + } +break ; + } + } + } + +if (new_name) { +for (new_patch = patch->prev; new_patch; new_patch = new_patch->prev) { +if (new_patch->new_name && !strcmp (new_name, new_patch->new_name)) { +if (patch->is_new || patch->is_rename || patch->is_copy) +return error ("%s: file exists from previous patch (new)" , new_name); +break ; + } +if (new_patch->old_name && !strcmp (new_name, new_patch->old_name)) { +if (!(patch->is_new || patch->is_delete || patch->is_rename || patch->is_copy)) +break ; +if (new_patch->is_delete || new_patch->is_rename) +break ; +return error ("%s: file exists from previous patch (old)" , new_name); + } + } + } + +if (old_patch) { +if (patch->is_new <0 ) + patch->is_new =0 ; +if (!patch->old_mode) + patch->old_mode = old_patch->new_mode; + }else if (old_name) {int changed;if (lstat (old_name, &st) <0 )
@@ -1036,7 +1081,14 @@ static int check_patch(struct patch *pat old_name, st.st_mode, patch->old_mode); } -if (new_name && (patch->is_new | patch->is_rename | patch->is_copy)) { +if (new_patch) { +if (!patch->new_mode) { +if (patch->is_new) + patch->new_mode =S_IFREG |0644 ; +else + patch->new_mode = patch->old_mode; + } + }else if (new_name && (patch->is_new | patch->is_rename | patch->is_copy)) {if (check_index &&cache_name_pos (new_name,strlen (new_name)) >=0 )return error ("%s: already exists in index" , new_name);if (!lstat (new_name, &st))
@@ -1061,7 +1113,7 @@ static int check_patch(struct patch *pat same ? "" : " of ", same ? "" : old_name); } -if (apply_data (patch, &st) <0 ) +if (apply_data (patch, old_patch, &st) <0 )return error ("%s: patch does not apply" , old_name);return 0 ; }
@@ -1393,6 +1445,7 @@ static int apply_patch(int fd)unsigned long offset, size;char *buffer =read_patch_file (fd, &size);struct patch *list =NULL , **listp = &list; +struct patch *list_prev =NULL ;int skipped_patch =0 ;if (!buffer)
@@ -1409,7 +1462,8 @@ static int apply_patch(int fd)break ;if (use_patch (patch)) {patch_stats (patch); - *listp = patch; + patch->prev = list_prev; + list_prev = *listp = patch; listp = &patch->next; }else {/* perhaps free it a bit better? */
t/t4104-apply-complex.sh new file +241 -0
241 lines not on this page · raw message
PATCH 2/3 Fix the processing of multiple patch files with --check in git-apply.
When the --check option was used with multiple patch files which modify the same files, the patch can fail because the previously modified contents aren't written to the disk.
So save the in memory patch contents across the processing of multiple patch files. Added a new test case for --check with multiple patch files.
---
apply.c | 10 ++++- t/t4105-apply-check.sh | 89 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 t/t4105-apply-check.sh
797cc1bdcc647b9fb744784969c814145803407d
apply.c +8 -2
@@ -1439,13 +1439,16 @@ static int use_patch(struct patch *p)return 1 ; } +struct patch *master_list =NULL ; +struct patch *master_list_prev =NULL ; +static int apply_patch (int fd) {int newfd;unsigned long offset, size;char *buffer =read_patch_file (fd, &size); -struct patch *list =NULL , **listp = &list; -struct patch *list_prev =NULL ; +struct patch *list = master_list, **listp = &list; +struct patch *list_prev = master_list_prev;int skipped_patch =0 ;if (!buffer)
@@ -1504,6 +1507,9 @@ static int apply_patch(int fd)if (summary)summary_patch_list (list); + master_list = list; + master_list_prev = list_prev; +free (buffer);return 0 ; }
t/t4105-apply-check.sh new file +89 -0
89 lines not on this page · raw message
PATCH 3/3 New option --ignore-applied for git-apply.
Allow the user to allow a patch which has some hunks (fragments) already applied to succeed. Added test case and documentation.
---
Documentation/git-apply.txt | 7 ++- apply.c | 12 +++++ t/t4108-apply-ignore-applied.sh | 94 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 t/t4108-apply-ignore-applied.sh
e8f2ff89f44b0c4d35dea3d86331a5e00fadf27e
Documentation/git-apply.txt +6 -1
@@ -9,7 +9,9 @@ git-apply - Apply patch on a GIT index f SYNOPSIS -------- -'git-apply' [--no-merge] [--stat] [--summary] [--check] [--index] [--show-files] [--apply] [<patch>...] +'git-apply' [--no-merge] [--stat] [--summary] [--check] + [--index] [--show-files] [--apply] + [--ignore-applied] [<patch>...] DESCRIPTION -----------
@@ -61,6 +63,9 @@ OPTIONS patch. Give this flag after those flags to also apply the patch. +--ignore-applied:: + If a patch hunk (fragment) fails to apply, reverse the + hunk and check if the hunk has already been applied. Author ------
apply.c +11 -1
@@ -32,8 +32,9 @@ static int summary = 0;static int check =0 ;static int apply =1 ;static int show_files =0 ; +static int ignore_applied =0 ;static const char apply_usage[] = -"git-apply [--no-merge] [--stat] [--summary] [--check] [--index] [--apply] [--show-files] <patch>..." ; +"git-apply [--no-merge] [--stat] [--summary] [--check] [--index] [--apply] [--show-files] [--ignore-applied] <patch>..." ;/* * For "diff-stat" like behaviour, we keep track of the biggest change
@@ -956,6 +957,11 @@ static int apply_one_fragment(struct bufmemmove (buf + offset + newsize, buf + offset + oldsize, size - offset - newsize);memcpy (buf + offset, new, newsize); offset =0 ; + }else if (ignore_applied) { + offset =find_offset (buf, desc->size, new, newsize, frag->newpos); +if (offset >=0 ) { + offset =0 ; + } }free (old);
@@ -1567,6 +1573,10 @@ int main(int argc, char **argv) show_files =1 ;continue ; } +if (!strcmp (arg,"--ignore-applied" )) { + ignore_applied =1 ; +continue ; + } fd =open (arg,O_RDONLY );if (fd <0 )usage (apply_usage);
t/t4108-apply-ignore-applied.sh new file +94 -0
94 lines not on this page · raw message
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): 20050905123445.GA27107@localhost
Re: [PATCH 2/3] Fix the processing of multiple patch files with --check in git-apply.
Quoted 1 line · Robert Fitzsimons
No problem I know it's a controversial change.
Not really controversial anymore, as long as --ignore-applied is used only when appropriate.
Quoted 2 lines
Quoted 1 line
Quoted 1 line
Yes my change has a bug with a copy patch which modifies the same file.
I'll send on a fix now.
Here is another one that the patch seems to regress.
Unfortunately this turns out to be a death sentence to the 'multiple patch touching the same file' patch.
Note that patches from a single 'git-diff-*' run are supposed to be independent, so the second one that copy-edits from frotz to rezrov copies frotz _before_ it is modified and then makes its modification.
On the other hand, we can have one 'git-diff-*' run that modifies frotz in place, and then a separate 'git-diff-*' run that copies frotz to rezrov and then modifies rezrov, and feed the concatenation of the two to git-apply. That would look very similar to a single run that edits frotz and copy-edits rezrov, but the pre-image to create rezrov would use the post-edit image of frotz from the first patch. Without an explicit "here an output from the next git-diff-*' run begins" marker, we cannot tell these two apart.
This means that applying multiple-patches concatenated in a single file does not make much sense. There is no guarantee that we can detect where the first 'git-diff-*' output ends, at which point we need to reset the status of 'frotz' to post-modification image of the first patch. And if we can detect reliably where one 'git-diff-*' output ends and the next one begins, we could just split the input there and run git-apply independently. So my conclusion is to just drop this patch, and instead tell people not to feed overlapping outputs from multiple git-diff-* runs to a single invocation of git-apply.
I personally do not mind considering --ignore-applied independently from the multiple-patch-files changes, though.
------------
echo frotz >frotz
git-apply --check <<\EOF
frotz +1 -1
@@ -1 +1 @@ -frotz +frottz
rezrov renamed from frotz +1 -1
@@ -1 +1 @@ -frotz +frotttz
EOF
Re: [PATCH 1/9] Fix git patch header processing in git-apply.
In reply to Robert FitzsimonsQuoted 3 lines · Robert Fitzsimons
I should have mentioned before, these changes are being used in a SourcePuller dump file to git import script I've been working on.
Hmm. Anything that depends on "--ignore-applied" is fundamentally broken as an import tool. There's no way something like that can be valid, since it inhently means that somebody is trying to apply a patch that simple isn't applicable.
So it seems like you're trying to work around some other serious problems in the import, and as a result the import is clearly not trustworthy.
Don't do this.
Linus
Re: [PATCH 1/9] Fix git patch header processing in git-apply.
Quoted 6 lines · Linus Torvalds
Hmm. Anything that depends on "--ignore-applied" is fundamentally broken as an import tool. There's no way something like that can be valid, since it inhently means that somebody is trying to apply a patch that simple isn't applicable.
Don't do this.
I suspect that it is exported data that is broken, if this --ignore-applied flag is needed for importing something somebody exported.
I, however, am not 100% convinced --ignore-applied is too dangerous to be useful in any conceivable use cases, at least not yet. For example, you might be cherry-picking a change from a foreign branch with 'git-diff-tree -p other~45 | git apply', where the current branch may have already aquired part of the changes that foreign branch contains since 45 generations ago.
Re: [PATCH 1/9] Fix git patch header processing in git-apply.
Quoted 7 lines · Junio C Hamano
I, however, am not 100% convinced --ignore-applied is too dangerous to be useful in any conceivable use cases, at least not yet. For example, you might be cherry-picking a change from a foreign branch with 'git-diff-tree -p other~45 | git apply', where the current branch may have already aquired part of the changes that foreign branch contains since 45 generations ago.
.. and that's why we use a three-way merge, and not diffs, to merge two branches.
--ignore-applied really is a pretty dangerous thing. I can see that it might do the right thing in many cases, but definitely not in a scripted environment, much less in an "import".
By definition, importing stuff must apply, since it clearly applied in whatever original source control tool you are importing from. So if an importer needs --ignore-applied, it is fundamentally doing something wrong.
And I just realized what "SourcePuller" is. Gaah. Trust me, that export is bad, and the original source control tool did everything very well indeed.
Linus
Re: [PATCH 1/9] Fix git patch header processing in git-apply.
Quoted 11 lines · Linus Torvalds
Quoted 7 lines · Junio C Hamano
I, however, am not 100% convinced --ignore-applied is too dangerous to be useful in any conceivable use cases, at least not yet. For example, you might be cherry-picking a change from a foreign branch with 'git-diff-tree -p other~45 | git apply', where the current branch may have already aquired part of the changes that foreign branch contains since 45 generations ago.
.. and that's why we use a three-way merge, and not diffs, to merge two branches.
.. and that's why I recently rewrote revert, cherry-pick and rebase to use three-way merge as well ;-).
The code is simple enough and I see some beauty in it, but I honor your veto, at least for now.
Re: [PATCH 1/9] Fix git patch header processing in git-apply.
Quoted 3 lines · Junio C Hamano
The code is simple enough and I see some beauty in it, but I honor your veto, at least for now.
Well, I didn't actually veto it when I saw the feature the first time, because I think it's a valid thing to do when applying patches by hand.
It was only when I realized that Robert was trying to use it to import from somewhere else that it became broken. An import should use the most anal patches possible.
Actually, an import should preferably never use patches at all, since that never really works for binary data. This is why the CVS importer actually checks out full files and imports them that way. The patch-based ones will fundamentally always have problems.
Linus
Re: [PATCH 1/9] Fix git patch header processing in git-apply.
Quoted 4 lines · Linus Torvalds
Actually, an import should preferably never use patches at all, since that never really works for binary data. This is why the CVS importer actually checks out full files and imports them that way. The patch-based ones will fundamentally always have problems.
<AOL>. Patches can be useful during the process, but you should (a) prefer more solid manifestations of the file/tree if possible, and (b) be extremely strict with them. Any hint that they don't match means that something is borked.
However, if you're importing from patch-oriented SCMs that tend to 'replay' patch sequences from one branch to the other, asking git to skip applied patches *could* be a shortcut. Not 100% correctness guarantee, but if the resulting history makes sense, it can be useful.
The other alternative is to do what I'll start to do soon on the archimport script: keep a tally of what was merged where according to the records that the source SCM has.
cheers,
martin