[PATCH v2 37/94] builtin/apply: remove whitespace_option arg from set_default_whitespace_mode()

2016-05-11 Thread Christian Couder
if (!whitespace_option && !apply_default_whitespace) + if (!state->whitespace_option && !apply_default_whitespace) ws_error_action = (state->apply ? warn_on_ws_error : nowarn_ws_error); } @@ -4785,11 +4784,11 @@ int cmd_apply(int argc, const char **argv,

[PATCH v2 12/94] builtin/apply: move 'check_index' global into 'struct apply_state'

2016-05-11 Thread Christian Couder
To libify the apply functionality the 'check_index' variable should not be static and global to the file. Let's move it into 'struct apply_state'. Reviewed-by: Stefan Beller <sbel...@google.com> Signed-off-by: Christian Couder <chrisc...@tuxfamily.org> --- builtin/

[PATCH v2 38/94] builtin/apply: move 'squelch_whitespace_errors' into 'struct apply_state'

2016-05-11 Thread Christian Couder
To libify the apply functionality the 'squelch_whitespace_errors' variable should not be static and global to the file. Let's move it into 'struct apply_state'. Reviewed-by: Stefan Beller <sbel...@google.com> Signed-off-by: Christian Couder <chrisc...@tuxfamily.org> --- builtin/

[PATCH v2 03/94] builtin/apply: avoid parameter shadowing 'linenr' global

2016-05-11 Thread Christian Couder
e to find filename in patch at line %d"), state_linenr); } static int gitdiff_hdrend(const char *line, struct patch *patch) @@ -937,17 +937,17 @@ static void gitdiff_verify_name(const char *line, int isnull, char **name, int s char *another; if (isnull)

[PATCH v2 04/94] builtin/apply: avoid local variable shadowing 'len' parameter

2016-05-11 Thread Christian Couder
This is just a cleanup to avoid errors when compiling with -Wshadow and to make it safer to later move global variables into a "state" struct. Reviewed-by: Stefan Beller Signed-off-by: Christian Couder --- builtin/apply.c | 20 ++--

[PATCH v2 01/94] builtin/apply: make gitdiff_verify_name() return void

2016-05-11 Thread Christian Couder
ine, NULL, p_value, TERM_TAB); + return; + } - if (orig_name) { - int len = strlen(orig_name); + if (*name) { + int len = strlen(*name); char *another; if (isnull) die(_("git apply:

[PATCH v2 06/94] builtin/apply: move 'options' variable into cmd_apply()

2016-05-11 Thread Christian Couder
The 'options' variable doesn't need to be static and global to the file. It can be local to cmd_apply(), so let's move it there. This will make it easier to libify the apply functionality. Reviewed-by: Stefan Beller <sbel...@google.com> Signed-off-by: Christian Couder <chrisc...@tuxf

[PATCH v2 02/94] builtin/apply: avoid parameter shadowing 'p_value' global

2016-05-11 Thread Christian Couder
L, state_p_value, TERM_TAB); return; } @@ -938,7 +938,7 @@ static void gitdiff_verify_name(const char *line, int isnull, char **name, int s if (isnull) die(_("git apply: bad git-diff - expected /dev/null, got %s on line %d&quo

[PATCH v2 08/94] builtin/apply: introduce 'struct apply_state' to start libifying

2016-05-11 Thread Christian Couder
Currently commands that want to use the apply functionality have to launch a "git apply" process which can be bad for performance. Let's start libifying the apply functionality and to do that we first need to get rid of the global variables in "builtin/apply.c". This patc

[PATCH v2 07/94] builtin/apply: move 'read_stdin' global into cmd_apply()

2016-05-11 Thread Christian Couder
The 'read_stdin' variable doesn't need to be static and global to the file. It can be local to cmd_apply(), so let's move it there. This will make it easier to libify the apply functionality. Reviewed-by: Stefan Beller <sbel...@google.com> Signed-off-by: Christian Couder <chrisc...@tuxf

[PATCH v2 09/94] builtin/apply: move 'state' init into init_apply_state()

2016-05-11 Thread Christian Couder
When the apply functionality will be libified, the 'struct apply_state' will be used by different pieces of code. To properly initialize a 'struct apply_state', let's provide a nice and easy to use init_apply_state() function. Helped-by: Eric Sunshine <sunsh...@sunshineco.com> Re

[PATCH v2 10/94] builtin/apply: move 'unidiff_zero' global into 'struct apply_state'

2016-05-11 Thread Christian Couder
To libify the apply functionality the 'unidiff_zero' variable should not be static and global to the file. Let's move it into 'struct apply_state'. Reviewed-by: Stefan Beller <sbel...@google.com> Signed-off-by: Christian Couder <chrisc...@tuxfamily.org> --- builtin/

[PATCH v2 05/94] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()

2016-05-11 Thread Christian Couder
The match_fragment() function is very big and contains a big special case algorithm that does line by line fuzzy matching. So let's extract this algorithm in a separate line_by_line_fuzzy_match() function. Reviewed-by: Stefan Beller Signed-off-by: Christian Couder

[PATCH v2 00/94] libify apply and use lib in am

2016-05-11 Thread Christian Couder
Goal This is a patch series about libifying `git apply` functionality, and using this libified functionality in `git am`, so that no 'git apply' process is spawn anymore. This makes `git am` significantly faster, so `git rebase`, when it uses the am backend, is also significantly faster

Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

2016-05-08 Thread Christian Couder
-B -M' > gives a broken result, do not use it". > I do not have time to dig the mail archive right now. > > Perhaps this one? > > http://thread.gmane.org/gmane.linux.kernel/1879635 I get something strange with only -B: $ git reset --keep 20f1d27~ $ git format-patch -B

Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

2016-05-06 Thread Junio C Hamano
Christian Couder writes: > By the way does someone know how such a patch can be applied? Perhaps this one? http://thread.gmane.org/gmane.linux.kernel/1879635 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to

Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

2016-05-06 Thread Christian Couder
> >>> Right now I get: >>> >>>> git log --summary -M -C -B -1 20f1d27 >>> commit 20f1d274609f5dde2eaaa279e7ee79fd5ef9c849 >>> Author: Christian Couder <chrisc...@tuxfamily.org> >>> Date: Fri Apr 22 20:55:46 2016 +0200 >>>

Re: [PATCH v2] gitweb: apply fallback encoding before highlight

2016-05-05 Thread Shin Kojima
> Oh, don't get me wrong. I do think what the patch does is very > sensible and have no intention of rejecting it. I'm sorry for making you worry, my poor English had caused some misunderstanding. I raised this Shift_JIS related problem (a.k.a "ダメ文字" in Japanese) might attract your interest

Re: [PATCH 83/83] builtin/am: use apply api in run_apply()

2016-05-05 Thread Christian Couder
up_devnull(1); >> + save_stderr_fd = dup(2); >> + dup_devnull(2); > > I wonder. It should be possible to teach the apply function to be quiet by > default, yes? That would be more elegant than dup()ing back and forth. Yes, it could be possible, but i

Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

2016-05-05 Thread Christian Couder
9f5dde2eaaa279e7ee79fd5ef9c849 >> Author: Christian Couder <chrisc...@tuxfamily.org> >> Date: Fri Apr 22 20:55:46 2016 +0200 >> >> Move libified code from builtin/apply.c to apply.{c,h} >> >> Signed-off-by: Christian Couder <chrisc...@tuxfamil

Re: [PATCH v2] gitweb: apply fallback encoding before highlight

2016-05-04 Thread Junio C Hamano
Shin Kojima writes: > I can say this patch, to consider $fallback_encoding while > highlighting, is fairly rational. But I also feel this is too much > just for specific outdated character encodings, it is completely > useless for the most part of gitweb users in the world.

Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

2016-05-04 Thread Duy Nguyen
1d274609f5dde2eaaa279e7ee79fd5ef9c849 > Author: Christian Couder <chrisc...@tuxfamily.org> > Date: Fri Apr 22 20:55:46 2016 +0200 > > Move libified code from builtin/apply.c to apply.{c,h} > > Signed-off-by: Christian Couder <chrisc...@tuxfamily.org> &g

Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

2016-05-04 Thread Christian Couder
aven't tested if this is > true though. Right now I get: > git log --summary -M -C -B -1 20f1d27 commit 20f1d274609f5dde2eaaa279e7ee79fd5ef9c849 Author: Christian Couder <chrisc...@tuxfamily.org> Date: Fri Apr 22 20:55:46 2016 +0200 Move libified code from builtin/apply.

Re: [PATCH v2] gitweb: apply fallback encoding before highlight

2016-05-04 Thread Shin Kojima
On Tue, May 03, 2016 at 11:33:42AM -0700, Junio C Hamano wrote: > Shin Kojima writes: > > > Some multi-byte character encodings (such as Shift_JIS and GBK) have > > characters whose final bytes is an ASCII '\' (0x5c), and they > > will be displayed as funny-characters even if

Re: [PATCH v2] gitweb: apply fallback encoding before highlight

2016-05-03 Thread Junio C Hamano
Shin Kojima writes: > Some multi-byte character encodings (such as Shift_JIS and GBK) have > characters whose final bytes is an ASCII '\' (0x5c), and they > will be displayed as funny-characters even if $fallback_encoding is > correct. Just out of curiosity, do people still use

Re: [PATCH 74/83] builtin/apply: make try_create_file() return -1 on error

2016-05-03 Thread Christian Couder
On Mon, May 2, 2016 at 8:01 PM, Eric Sunshine wrote: > On Sun, Apr 24, 2016 at 9:34 AM, Christian Couder > wrote: >> Signed-off-by: Christian Couder >> --- >> diff --git a/builtin/apply.c b/builtin/apply.c >> @@

Re: [PATCH 73/83] builtin/apply: make write_out_results() return -1 on error

2016-05-03 Thread Christian Couder
if (phase == 1) { >> if (write_out_one_reject(state, l)) >> errs = 1; >> @@ -4484,11 +4490,16 @@ static int apply_patch(struct apply_state *state, >> !state->apply_with_reject) >>

Re: [PATCH 69/83] builtin/apply: make add_conflicted_stages_file() return -1 on error

2016-05-03 Thread Christian Couder
On Mon, May 2, 2016 at 9:36 AM, Eric Sunshine wrote: > On Sun, Apr 24, 2016 at 9:34 AM, Christian Couder > wrote: >> Signed-off-by: Christian Couder >> --- >> diff --git a/builtin/apply.c b/builtin/apply.c >> @@

Re: [PATCH 67/83] builtin/apply: make build_fake_ancestor() return -1 on error

2016-05-03 Thread Christian Couder
On Mon, May 2, 2016 at 9:32 AM, Eric Sunshine wrote: > On Sun, Apr 24, 2016 at 9:34 AM, Christian Couder > wrote: >> Signed-off-by: Christian Couder >> --- >> diff --git a/builtin/apply.c b/builtin/apply.c >> @@

Re: [PATCH 63/83] builtin/apply: make apply_all_patches() return -1 on error

2016-05-03 Thread Christian Couder
On Sun, May 1, 2016 at 11:03 PM, Eric Sunshine wrote: >> @@ -4590,10 +4590,10 @@ static int apply_all_patches(struct apply_state >> *state, >> squelched); >> } >> if (state->ws_error_action ==

[PATCH v2] gitweb: apply fallback encoding before highlight

2016-05-03 Thread Shin Kojima
Some multi-byte character encodings (such as Shift_JIS and GBK) have characters whose final bytes is an ASCII '\' (0x5c), and they will be displayed as funny-characters even if $fallback_encoding is correct. This is because `highlight` command always expects UTF-8 encoded strings from STDIN.

Re: [PATCH 81/83] apply: roll back index in case of error

2016-05-03 Thread Christian Couder
rollback_lock_file(state->lock_file); >> return -1; >> + } >> errs |= res; >> close(fd); > > In case of error, this leaves fd open, which in the end will prevent the > "patch" file, and

Re: [PATCH] gitweb: apply fallback encoding before highlight

2016-05-02 Thread Jakub Narębski
On Mon, May 2, 2016 at 7:49 PM, Junio C Hamano wrote: > Shin Kojima writes: > >> This patch prepare git blob objects to be encoded into UTF-8 before >> highlighting in the manner of `to_utf8` subroutine. >> --- > > The single liner Perl invoked from the script

Re: [PATCH 74/83] builtin/apply: make try_create_file() return -1 on error

2016-05-02 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:34 AM, Christian Couder wrote: > Signed-off-by: Christian Couder > --- > diff --git a/builtin/apply.c b/builtin/apply.c > @@ -4145,28 +4151,32 @@ static int try_create_file(const char *path, unsigned > int mode,

Re: [PATCH] gitweb: apply fallback encoding before highlight

2016-05-02 Thread Junio C Hamano
Shin Kojima writes: > Some multi-byte character encodings (such as Shift_JIS and GBK) have > characters whose final bytes is an ASCII '\' (0x5c), and they > will be displayed as funny-characters even if $fallback_encoding is > correct. This is because `highlight` command always

Re: [PATCH 73/83] builtin/apply: make write_out_results() return -1 on error

2016-05-02 Thread Eric Sunshine
_patch(struct apply_state *state, > !state->apply_with_reject) > return -1; > > - if (state->apply && write_out_results(state, list)) { > - if (state->apply_with_reject) > + if (state->apply)

Re: [PATCH 69/83] builtin/apply: make add_conflicted_stages_file() return -1 on error

2016-05-02 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:34 AM, Christian Couder wrote: > Signed-off-by: Christian Couder > --- > diff --git a/builtin/apply.c b/builtin/apply.c > @@ -4234,8 +4234,11 @@ static void add_conflicted_stages_file(struct > apply_state *state, >

Re: [PATCH 67/83] builtin/apply: make build_fake_ancestor() return -1 on error

2016-05-02 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:34 AM, Christian Couder wrote: > Signed-off-by: Christian Couder > --- > diff --git a/builtin/apply.c b/builtin/apply.c > @@ -3913,31 +3913,34 @@ static void build_fake_ancestor(struct patch *list, > const char

Re: [PATCH 81/83] apply: roll back index in case of error

2016-05-02 Thread Eric Sunshine
ollback_lock_file(state->lock_file); >> > return -1; >> > + } >> > errs |= res; >> > close(fd); >> >> In case of error, this leaves fd open, which in the end will prevent the >> "patc

Re: [PATCH 63/83] builtin/apply: make apply_all_patches() return -1 on error

2016-05-02 Thread Johannes Schindelin
Hi Eric, On Sun, 1 May 2016, Eric Sunshine wrote: > On Sun, Apr 24, 2016 at 9:34 AM, Christian Couder > wrote: > > Signed-off-by: Christian Couder > > --- > > diff --git a/builtin/apply.c b/builtin/apply.c > > @@ -4562,12 +4562,12 @@ static

Re: [PATCH 81/83] apply: roll back index in case of error

2016-05-02 Thread Johannes Schindelin
rollback_lock_file(state->lock_file); > > return -1; > > + } > > errs |= res; > > close(fd); > > In case of error, this leaves fd open, which in the end will prevent the > "patch" file, and hence

Re: [PATCH 63/83] builtin/apply: make apply_all_patches() return -1 on error

2016-05-01 Thread Eric Sunshine
state->whitespace_error); How does this new 'return' relate to the logic below which updates the index? Does the index need to be updated here now too? > if (state->applied_after_fixing_ws && state->apply) > w

Re: [PATCH 59/83] builtin/apply: move init_apply_state() to apply.c

2016-05-01 Thread Christian Couder
On Sun, May 1, 2016 at 9:37 PM, Eric Sunshine wrote: > On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder > wrote: >> Signed-off-by: Christian Couder >> --- >> diff --git a/apply.c b/apply.c >> @@ -0,0 +1,80 @@ >>

Re: [PATCH 54/83] builtin/apply: make parse_chunk() return a negative integer on error

2016-05-01 Thread Christian Couder
On Sun, May 1, 2016 at 9:04 PM, Eric Sunshine wrote: > On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder > wrote: >> This negative number can be -2 if no patch header has been found, >> otherwise it is -1. >> >> As parse_chunk() is called only

Re: [PATCH 59/83] builtin/apply: move init_apply_state() to apply.c

2016-05-01 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder wrote: > Signed-off-by: Christian Couder > --- > diff --git a/apply.c b/apply.c > @@ -0,0 +1,80 @@ > +#include "cache.h" > +#include "apply.h" > + > + > + Too many blank lines? > +static void

Re: [PATCH 55/83] builtin/apply: make parse_single_patch() return -1 on error

2016-05-01 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder wrote: > Signed-off-by: Christian Couder > --- > diff --git a/builtin/apply.c b/builtin/apply.c > @@ -1802,8 +1806,10 @@ static int parse_single_patch(struct apply_state > *state, >

Re: [PATCH 65/83] builtin/apply: make gitdiff_verify_name() return -1 on error

2016-05-01 Thread Christian Couder
On Mon, Apr 25, 2016 at 3:36 PM, Duy Nguyen wrote: > On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder > wrote: >> Signed-off-by: Christian Couder >> --- >> builtin/apply.c | 52

Re: [PATCH 54/83] builtin/apply: make parse_chunk() return a negative integer on error

2016-05-01 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder wrote: > This negative number can be -2 if no patch header has been found, > otherwise it is -1. > > As parse_chunk() is called only by apply_patch() which already > returns -1 when an error happened, let's make it

Re: [PATCH 63/83] builtin/apply: make apply_all_patches() return -1 on error

2016-05-01 Thread Christian Couder
On Mon, Apr 25, 2016 at 3:30 PM, Duy Nguyen wrote: > On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder > wrote: >> if (state->update_index) { >> if (write_locked_index(_index, state->lock_file, >> COMMIT_LOCK)) >> -

Re: [PATCH 61/83] builtin/apply: libify check_apply_state()

2016-05-01 Thread Christian Couder
On Mon, Apr 25, 2016 at 3:26 PM, Duy Nguyen wrote: > On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder > wrote: >> Signed-off-by: Christian Couder >> --- >> builtin/apply.c | 16 +--- >> 1 file changed, 9

Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing

2016-05-01 Thread Christian Couder
On Wed, Apr 27, 2016 at 8:10 PM, Eric Sunshine wrote: > On Mon, Apr 25, 2016 at 9:18 AM, Duy Nguyen wrote: >> On Sun, Apr 24, 2016 at 8:33 PM, Christian Couder >> wrote: >>> To be compatible with the rest of the error

Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing

2016-05-01 Thread Christian Couder
On Wed, Apr 27, 2016 at 8:08 PM, Eric Sunshine wrote: > On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder > wrote: >> To be compatible with the rest of the error handling in builtin/apply.c, >> find_header() should return -1 instead of calling

Re: [PATCH 51/83] builtin/apply: make apply_patch() return -1 instead of die()ing

2016-04-30 Thread Christian Couder
On Tue, Apr 26, 2016 at 3:20 AM, Eric Sunshine <sunsh...@sunshineco.com> wrote: > On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder > <christian.cou...@gmail.com> wrote: >> To libify `git apply` functionality we have to signal errors >> to the caller instead of d

Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'

2016-04-30 Thread Christian Couder
On Mon, Apr 25, 2016 at 9:50 AM, Eric Sunshine wrote: >> @@ -4515,8 +4521,6 @@ static int write_out_results(struct apply_state >> *state, struct patch *list) >> return errs; >> } >> >> -static struct lock_file lock_file; > > Does the static lock_file in

Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'

2016-04-28 Thread Junio C Hamano
Christian Couder <christian.cou...@gmail.com> writes: >> I do not think you need to think about "free"ing. > > Yeah, lockfile.h says: > ... > and: > ... Yup, we are now on the same page. >> Even if the libified version of the apply internal can be c

Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'

2016-04-28 Thread Christian Couder
ust remain valid * throughout the life of the program (i.e. you cannot use an * on-stack variable to hold this structure). > Even if the libified version of the apply internal can be called > multiple times to process multiple patch inputs, there is no need to > run multiple instan

Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing

2016-04-27 Thread Eric Sunshine
On Mon, Apr 25, 2016 at 9:18 AM, Duy Nguyen wrote: > On Sun, Apr 24, 2016 at 8:33 PM, Christian Couder > wrote: >> To be compatible with the rest of the error handling in builtin/apply.c, >> find_header() should return -1 instead of calling die().

Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing

2016-04-27 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder wrote: > To be compatible with the rest of the error handling in builtin/apply.c, > find_header() should return -1 instead of calling die(). > > Unfortunately find_header() already returns -1 when no header is found, >

Re: [PATCH 03/83] builtin/apply: avoid parameter shadowing 'linenr' global

2016-04-27 Thread Christian Couder
On Wed, Apr 27, 2016 at 6:27 PM, Junio C Hamano wrote: > > I think 02/83 that renamed the global-to-be-moved-to-state to > state_p_value was brilliant, and this should follow suit; you would > be moving linenr into the state eventually in later steps, right? Yeah, ok, I will

Re: [PATCH 03/83] builtin/apply: avoid parameter shadowing 'linenr' global

2016-04-27 Thread Junio C Hamano
Christian Couder writes: > Signed-off-by: Christian Couder > --- I think 02/83 that renamed the global-to-be-moved-to-state to state_p_value was brilliant, and this should follow suit; you would be moving linenr into the state eventually in

Re: [PATCH 47/83] builtin/apply: move applying patches into apply_all_patches()

2016-04-27 Thread Christian Couder
On Tue, Apr 26, 2016 at 12:00 AM, Stefan Beller wrote: > On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder > wrote: >> Signed-off-by: Christian Couder > > Up to this patch, have a > Reviewed-by: Stefan Beller

Re: [PATCH 22/83] builtin/apply: move 'unsafe_paths' global into 'struct apply_state'

2016-04-27 Thread Junio C Hamano
Christian Couder writes: > The reason I added some blank lines is that I moved comments that were > all in one block at the top. > I moved each comment near the related variables and sometimes a > comment is related to 2 variables, like in the extract below the >

Re: [PATCH 05/83] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()

2016-04-27 Thread Christian Couder
On Tue, Apr 26, 2016 at 10:20 PM, Junio C Hamano wrote: > Christian Couder writes: > >>> It's not clear why we need to declare buf here? Oh wait it is. It's just >>> moved from the start of the function. But why do it in this patch? >>> It seems

Re: [PATCH 39/83] builtin/apply: move 'ws_error_action' into 'struct apply_state'

2016-04-27 Thread Christian Couder
On Tue, Apr 26, 2016 at 10:36 PM, Junio C Hamano wrote: > Christian Couder writes: > >> +enum ws_error_action { >> + nowarn_ws_error, >> + warn_on_ws_error, >> + die_on_ws_error, >> + correct_ws_error >> +}; >> + >> struct

Re: [PATCH 27/83] builtin/apply: move 'read_stdin' global into cmd_apply()

2016-04-27 Thread Christian Couder
On Tue, Apr 26, 2016 at 10:28 PM, Junio C Hamano wrote: > Christian Couder writes: > >> Signed-off-by: Christian Couder >> --- >> builtin/apply.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Makes sense but

Re: [PATCH 22/83] builtin/apply: move 'unsafe_paths' global into 'struct apply_state'

2016-04-27 Thread Christian Couder
On Tue, Apr 26, 2016 at 10:27 PM, Junio C Hamano wrote: > Christian Couder writes: > >> Signed-off-by: Christian Couder >> --- >> builtin/apply.c | 9 + >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff

Re: [PATCH 45/83] builtin/apply: move 'state' init into init_apply_state()

2016-04-27 Thread Christian Couder
state->prefix = prefix_; >> + state->prefix_length = state->prefix ? strlen(state->prefix) : 0; >> + state->apply = 1; >> + state->line_termination = '\n'; >> + state->p_value = 1; >> + state->p_context

Re: [PATCH 10/83] builtin/apply: move 'check_index' global into 'struct apply_state'

2016-04-26 Thread Junio C Hamano
t; diff --git a/builtin/apply.c b/builtin/apply.c > index 6c628f6..3f8671c 100644 > --- a/builtin/apply.c > +++ b/builtin/apply.c > @@ -30,6 +30,10 @@ struct apply_state { >*files that are being modified, but doesn't apply the patch >*/ > int c

Re: [PATCH 39/83] builtin/apply: move 'ws_error_action' into 'struct apply_state'

2016-04-26 Thread Junio C Hamano
s->prefix = prefix; > s->prefix_length = s->prefix ? strlen(s->prefix) : 0; > } > > in [PATCH 7/xx]. Just to avoid misunderstanding, I do not mean to say that the init-apply-state helper that should have been introduced in 07/xx would gain a new

Re: [PATCH 39/83] builtin/apply: move 'ws_error_action' into 'struct apply_state'

2016-04-26 Thread Junio C Hamano
Christian Couder writes: > +enum ws_error_action { > + nowarn_ws_error, > + warn_on_ws_error, > + die_on_ws_error, > + correct_ws_error > +}; > + > struct apply_state { > const char *prefix; > int prefix_length; > @@ -80,6 +87,8 @@ struct

Re: [PATCH 22/83] builtin/apply: move 'unsafe_paths' global into 'struct apply_state'

2016-04-26 Thread Junio C Hamano
Christian Couder writes: > Signed-off-by: Christian Couder > --- > builtin/apply.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/builtin/apply.c b/builtin/apply.c > index 506357c..c45e481 100644 > ---

Re: [PATCH 27/83] builtin/apply: move 'read_stdin' global into cmd_apply()

2016-04-26 Thread Junio C Hamano
Christian Couder writes: > Signed-off-by: Christian Couder > --- > builtin/apply.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Makes sense but do so immediately next to 06/83, "options" thing? > diff --git a/builtin/apply.c

Re: [PATCH 05/83] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()

2016-04-26 Thread Junio C Hamano
Christian Couder writes: >> It's not clear why we need to declare buf here? Oh wait it is. It's just >> moved from the start of the function. But why do it in this patch? >> It seems unrelated to the general intent of the patch. No need to reroll >> for this nit

Re: [PATCH 33/83] builtin/apply: move 'root' global into 'struct apply_state'

2016-04-26 Thread Christian Couder
On Mon, Apr 25, 2016 at 11:50 PM, Stefan Beller wrote: > On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder > wrote: > >> @@ -4630,9 +4644,10 @@ static int option_parse_whitespace(const struct >> option *opt, >> static int

Re: [PATCH 18/83] builtin/apply: move 'numstat' global into 'struct apply_state'

2016-04-26 Thread Christian Couder
y.c | 11 ++- >> 1 file changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/builtin/apply.c b/builtin/apply.c >> index d90948a..16d78f9 100644 >> --- a/builtin/apply.c >> +++ b/builtin/apply.c >> @@ -36,6 +36,9 @@ struct apply_state { >>

Re: [PATCH 09/83] builtin/apply: move 'check' global into 'struct apply_state'

2016-04-26 Thread Stefan Beller
On Tue, Apr 26, 2016 at 9:26 AM, Christian Couder <christian.cou...@gmail.com> wrote: >> >>> /* >>> - * --check turns on checking that the working tree matches the >>> - *files that are being modified, but doesn't apply the patch >> >> O

Re: [PATCH 09/83] builtin/apply: move 'check' global into 'struct apply_state'

2016-04-26 Thread Christian Couder
@@ struct apply_state { >> const char *prefix; >> int prefix_length; >> >> + /* >> +* --check turns on checking that the working tree matches the >> +*files that are being modified, but doesn't apply the patch > > This is true, but

Re: [PATCH 05/83] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()

2016-04-26 Thread Christian Couder
On Mon, Apr 25, 2016 at 8:50 PM, Stefan Beller wrote: >> @@ -2251,7 +2319,7 @@ static int match_fragment(struct image *img, >> int match_beginning, int match_end) >> { >> int i; >> - char *fixed_buf, *buf, *orig, *target; >> +

Re: [PATCH 51/83] builtin/apply: make apply_patch() return -1 instead of die()ing

2016-04-25 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder <christian.cou...@gmail.com> wrote: > To libify `git apply` functionality we have to signal errors > to the caller instead of die()ing. > > As a first step in this direction, let's make apply_patch() return > -1 in case of er

Re: [PATCH 47/83] builtin/apply: move applying patches into apply_all_patches()

2016-04-25 Thread Stefan Beller
"squelched %d whitespace errors", > + squelched), > + squelched); > + } > + if (state->ws_error_action == die_on_ws_error) > + die(Q_("%d line adds whit

Re: [PATCH 33/83] builtin/apply: move 'root' global into 'struct apply_state'

2016-04-25 Thread Stefan Beller
> > #define SLIDING_VIEW_INIT(..., STRBUF_INIT } Nevermind, just read patch "builtin/apply: move 'state' init into init_apply_state()" -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 33/83] builtin/apply: move 'root' global into 'struct apply_state'

2016-04-25 Thread Stefan Beller
e); > if (has_epoch_timestamp(first)) { > patch->is_new = 1; > @@ -957,7 +971,7 @@ static void gitdiff_verify_name(struct apply_state *state, > int side) > { > if (!*name &&am

Re: [PATCH 18/83] builtin/apply: move 'numstat' global into 'struct apply_state'

2016-04-25 Thread Stefan Beller
ly.c b/builtin/apply.c > index d90948a..16d78f9 100644 > --- a/builtin/apply.c > +++ b/builtin/apply.c > @@ -36,6 +36,9 @@ struct apply_state { > /* --stat does just a diffstat, and doesn't actually apply */ > int diffstat; > > + /* --numstat does numeric

Re: [PATCH 09/83] builtin/apply: move 'check' global into 'struct apply_state'

2016-04-25 Thread Stefan Beller
g that the working tree matches the > + *files that are being modified, but doesn't apply the patch This is true, but at this part of the file/code we rather want to know what `check` does, instead of what the command line option --check does. (They are 2 different things, though one lea

Re: [PATCH 05/83] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()

2016-04-25 Thread Stefan Beller
> @@ -2251,7 +2319,7 @@ static int match_fragment(struct image *img, > int match_beginning, int match_end) > { > int i; > - char *fixed_buf, *buf, *orig, *target; > + char *fixed_buf, *orig, *target; > struct strbuf fixed; > size_t

Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'

2016-04-25 Thread Junio C Hamano
; > Is there ever a time when lock_file is removed from the list (such as > upon successful write of the index), in which case it would be safe to > free() this? I do not think you need to think about "free"ing. Even if the libified version of the apply internal can be called multipl

Re: [PATCH 81/83] apply: roll back index in case of error

2016-04-25 Thread Johannes Schindelin
close(fd); In case of error, this leaves fd open, which in the end will prevent the "patch" file, and hence the "rebase-apply/" directory from being removed on Windows. This triggered a failure of t4014 here (and possibly more, but it took me quite a while to track this down,

Re: [PATCH 00/83] libify apply and use lib in am

2016-04-25 Thread Johannes Schindelin
; > >> On 24/04/16 14:33, Christian Couder wrote: > >>> This is a patch series about libifying `git apply` functionality, and > >>> using this libified functionality in `git am`, so that no 'git apply' > >>> process is spawn anymore. This makes `git am` si

Re: [PATCH 83/83] builtin/am: use apply api in run_apply()

2016-04-25 Thread Johannes Schindelin
> - cp.no_stdout = 1; > - cp.no_stderr = 1; > + save_stdout_fd = dup(1); > + dup_devnull(1); > + save_stderr_fd = dup(2); > + dup_devnull(2); I wonder. It should be possible to teach the apply function to be quiet by

Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

2016-04-25 Thread Duy Nguyen
On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder wrote: > Signed-off-by: Christian Couder > --- > apply.c | 4678 > ++- > apply.h | 19 + > builtin/apply.c | 4677

Re: [PATCH 65/83] builtin/apply: make gitdiff_verify_name() return -1 on error

2016-04-25 Thread Duy Nguyen
On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder wrote: > Signed-off-by: Christian Couder > --- > builtin/apply.c | 52 ++-- > 1 file changed, 30 insertions(+), 22 deletions(-) > > diff --git

Re: [PATCH 63/83] builtin/apply: make apply_all_patches() return -1 on error

2016-04-25 Thread Duy Nguyen
On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder wrote: > if (state->update_index) { > if (write_locked_index(_index, state->lock_file, > COMMIT_LOCK)) > - die(_("Unable to write new index file")); > +

Re: [PATCH 61/83] builtin/apply: libify check_apply_state()

2016-04-25 Thread Duy Nguyen
gt; } > if (state->apply_with_reject) > @@ -4552,14 +4552,15 @@ static void check_apply_state(struct apply_state > *state, int force_apply) > if (!force_apply && (state->diffstat || state->numstat || > state->summary ||

Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing

2016-04-25 Thread Duy Nguyen
On Sun, Apr 24, 2016 at 8:33 PM, Christian Couder wrote: > To be compatible with the rest of the error handling in builtin/apply.c, > find_header() should return -1 instead of calling die(). > > Unfortunately find_header() already returns -1 when no header is found, >

Re: [PATCH 00/83] libify apply and use lib in am

2016-04-25 Thread Duy Nguyen
On Mon, Apr 25, 2016 at 4:57 PM, Christian Couder <christian.cou...@gmail.com> wrote: >> But why write so many times when nobody reads it? We only need to >> write before git-apply exits, > > You mean `git am` here I think. > >> either after successfully apply

Re: [PATCH 00/83] libify apply and use lib in am

2016-04-25 Thread Christian Couder
commit long rebase is reduced from 58% to 19% of the whole rebase time. So further improvements in speeding up `git am` could only make such a rebase at most 19% faster. > I ran my measurement patch [1] with and without your series used this > series as rebase material. Without the series, th

Re: [PATCH 00/83] libify apply and use lib in am

2016-04-25 Thread Christian Couder
eah probably. Thanks for looking at this. I think I will have to split it into smaller patches in a v2. >>> Instead of waiting for the patch to appear on the list, you might want >>> to use branch libify-apply-use-in-am25 in my GitHub repo: >>> >>> https://github.c

Re: [PATCH 00/83] libify apply and use lib in am

2016-04-25 Thread Duy Nguyen
on unless you paired it with index-helper, to cut down both read and write time. So I ran some tests. Long story short, I think we can achieve this gain (and a little more) even without split-index. I ran my measurement patch [1] with and without your series used this series as rebase material. W

Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'

2016-04-25 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder wrote: > We cannot have a 'struct lock_file' allocated on the stack, as lockfile.c > keeps a linked list of all created lock_file structures. By talking about "the stack" here, I suppose you mean that your initial idea

Re: [PATCH 45/83] builtin/apply: move 'state' init into init_apply_state()

2016-04-25 Thread Eric Sunshine
ption > *opt, > +static void init_apply_state(struct apply_state *state, const char *prefix_) > +{ > + memset(state, 0, sizeof(*state)); > + state->prefix = prefix_; > + state->prefix_length = state->prefix ? strlen(state->prefix) : 0; > +

<    4   5   6   7   8   9   10   11   12   13   >