[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 Signed-off-by: Christian Couder --- builtin/apply.c | 22 +++--

[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 Signed-off-by: Christian Couder --- builtin/apply.c | 2 +

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

2016-05-11 Thread Christian Couder
} - if (orig_name) { - int len = strlen(orig_name); + if (*name) { + int len = strlen(*name); char *another; if (isnull) die(_("git apply: bad git-diff - expected /dev/nul

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

2016-05-11 Thread Christian Couder
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 (isnul

[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 ++-- 1 file changed, 10 insertions(+), 10 deletions

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

2016-05-11 Thread Christian Couder
} @@ -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"), *name, linenr);

[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

[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 Signed-off-by: Christian Couder --- builtin/apply.c |

[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 Signed-off-by: Christian Couder --- builtin/apply.c | 42 -

[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 Reviewed-by: Ste

[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 --- builtin/apply.c | 126

[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 significan

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

2016-05-08 Thread Christian Couder
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 -1 -o ../../patches/test-libify-apply-use-in-am

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 majord...@vger.kernel.org More major

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

2016-05-06 Thread Christian Couder
27 >>> commit 20f1d274609f5dde2eaaa279e7ee79fd5ef9c849 >>> Author: Christian Couder >>> Date: Fri Apr 22 20:55:46 2016 +0200 >>> >>> Move libified code from builtin/apply.c to apply.{c,h} >>> >>> Signed-off-by: Christian Coud

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 knowi

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

2016-05-05 Thread Christian Couder
; + 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 it could mean many changes not

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

2016-05-05 Thread Christian Couder
n Couder >> 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 >> >> copy builtin/apply.c => apply.c (96%) >> rewrite builtin/apply.c (96%) > > Ah

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. Oh, don't get me wro

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

2016-05-04 Thread Duy Nguyen
ply.c which is about 90 lines... I haven't tested if this is >> true though. > > Right now I get: > >> git log --summary -M -C -B -1 20f1d27 > commit 20f1d274609f5dde2eaaa279e7ee79fd5ef9c849 > Author: Christian Couder > Date: Fri Apr 22 20:55:46 2016 +0200

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

2016-05-04 Thread Christian Couder
-summary -M -C -B -1 20f1d27 commit 20f1d274609f5dde2eaaa279e7ee79fd5ef9c849 Author: Christian Couder 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 copy builtin/apply.c => apply.c (96%) rewrite builtin/apply.c

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 $fallback_encoding i

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 Shift_JIS aka MS-

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 >> @@ -4145,28 +4151,32 @@ static int try_create_file(const char *path, >> unsigned int m

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

2016-05-03 Thread Christian Couder
errs = 1; >> @@ -4484,11 +4490,16 @@ static int apply_patch(struct apply_state *state, >> !state->apply_with_reject) >> return -1; >> >> - if (state->apply && write_out_results(state

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 >> @@ -4234,8 +4234,11 @@ static void add_conflicted_stages_file(struct >> apply_state *s

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 >> @@ -3913,31 +3913,34 @@ static void build_fake_ancestor(struct patch *list, >> const 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 == die_on_ws_error) >> -

[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
lback_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 the "rebase-apply/"

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 felt a bit too dense > to my taste b

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, const char *buf, > fd = open(path, O_CREAT | O_EXC

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 expects UTF-8 > e

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

2016-05-02 Thread Eric Sunshine
!state->apply_with_reject) > return -1; > > - if (state->apply && write_out_results(state, list)) { > - if (state->apply_with_reject) > + if (state->apply) { > + int res = write_out_results(state, list); > +

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, > ce->ce_namelen = namelen; >

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 *filename) > ce = make_cache_entry(patch->o

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

2016-05-02 Thread Eric Sunshine
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 the "rebase-a

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 int apply_all_patches(struct apply_state > > *state,

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); > + return error(Q_("%d line adds whitespace errors.", > + "%d lines add whitespace errors.", > + state->whitespace_error), > +

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 @@ >> +#include "cache.h" >> +#include "apply.h" >> + >> + >> + > > Too many blank lines?

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 by apply_patch() which already >> returns -1 when an

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 git_apply_config(void) > +{ > + git_config_get_

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, > fragment = xcalloc(1, sizeof(*fragment)); >

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 ++-- >> 1 file changed, 30 insertions(+), 22 deletions(-) >> >> diff

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 return -1 when > parse_chunk() re

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(&the_index, state->lock_file, >> COMMIT_LOCK)) >> - die(_("Unable to write new ind

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 insertions(+), 7 deletions(-) >> >> diff --git a/builtin/apply.c b/builtin/app

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 handling in builtin/apply.c, >>> find_header() should return -1 instead of cal

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 die(). >> >> Unfortunately find_header() already retu

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 wrote: > On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder > 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

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 build_fake_ancestor() deserve the

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

2016-04-28 Thread Junio C Hamano
Christian Couder 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 called >> multiple times to proc

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

2016-04-28 Thread Christian Couder
e 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 instances of it yet. And a lockfile, after the caller >

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(). >> >> Unfortunately find_header() already returns

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, > so let's make it return -2 in

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 do the same thing to

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 later steps, right? > builtin/apply.c | 4 ++-- > 1

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 > in case you want to split the series in here (as indicated in the > cover letter, th

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 > comment that starts with "For "di

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 unrelated to the general intent of the patch. No ne

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 apply_state { >> const char *prefix; >> int p

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 do so immediately next to 06/83, "options" thing? Ok, in the next rerol

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 --git a/builtin/apply.c b/builtin/apply.c >> index 506357c..c45e481 1006

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

2016-04-27 Thread Christian Couder
y(const struct option >> *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

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

2016-04-26 Thread Junio C Hamano
..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 check; > + > + /* --index updates the cache as well. */

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

2016-04-26 Thread Junio C Hamano
;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 caller-supplied

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 apply_state { > int whi

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 > --- a/builtin/apply.c > +++ b/builtin/apply.c > @@ -57,6 +57,8

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 b/builtin/apply.c > index 699cabf..be237d1 100644 > --- a

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 alone, it just took me a while to

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 option_parse_directory(const struct option *opt, >>

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

2016-04-26 Thread Christian Couder
gt; 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 { >> /* --stat does just a diffstat, and doesn't actually apply */ >> i

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 wrote: >> >>> /* >>> - * --check turns on checking that the working tree matches the >>> - *files that are being modified, but doesn't apply the patch >> >> Oh I see it was moved from here. N

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

2016-04-26 Thread Christian Couder
> + /* >> +* --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 at this part of the file/code we rather want to know what > `check` does, instead of what t

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; >> + char *fixed_buf, *orig,

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 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 errors instead of dying. For

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

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

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 = find_name_traditional(second, first_name, > state->p_value); > + first_name = find_name_traditional(state, first, NULL, > state->p_value); > + name = find_name_traditional(state, second, first_name, > state->p_value); > free(fir

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

2016-04-25 Thread Stefan Beller
t; --- 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 diffstat, and doesn't actually apply */ > +

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

2016-04-25 Thread Stefan Beller
t; --- a/builtin/apply.c > +++ b/builtin/apply.c > @@ -25,12 +25,15 @@ struct apply_state { > const char *prefix; > int prefix_length; > > + /* > +* --check turns on checking that the working tree matches the > +*files that are bein

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 fixe

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

2016-04-25 Thread Junio C Hamano
e 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 multiple times to proces

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
Hi Chris, On Sun, 24 Apr 2016, Christian Couder wrote: > On Sun, Apr 24, 2016 at 6:27 PM, Christian Couder > wrote: > > On Sun, Apr 24, 2016 at 5:23 PM, Ramsay Jones > > wrote: > >> > >> > >> On 24/04/16 14:33, Christian Couder wrote: > >&

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

2016-04-25 Thread Johannes Schindelin
e) { > - 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 qui

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 -- > 3 f

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 a/builtin/apply.c b/builtin/apply.c > index 6b8ba2a..26

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(&the_index, state->lock_file, > COMMIT_LOCK)) > - die(_("Unable to write new index file")); > + return error(_("Unable

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

2016-04-25 Thread Duy Nguyen
ith_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 || state->check || state->fake_ancestor)) > state-

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, > so let's make it return -2 in

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 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 applying the >> whole series, or after

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

2016-04-25 Thread Christian Couder
ole 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, the picture is not so > surprising. We run git-app

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

2016-04-25 Thread Christian Couder
, and it made it >>>> to chrisc...@tuxfamily.org. > > #78 moves 4000k lines around and ends up ~260k in size, I think it may > have hit vger limit. Yeah probably. Thanks for looking at this. I think I will have to split it into smaller patches in a v2. >>> Instead of

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

2016-04-25 Thread Duy Nguyen
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. Without the series, the picture is

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 was to move the global lock_

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

2016-04-25 Thread Eric Sunshine
apply_state *state, const char *prefix_) > +{ > + memset(state, 0, sizeof(*state)); > + state->prefix = prefix_; > + state->prefix_length = state->prefix ? strlen(state->prefix) : 0; > + state->apply = 1; > + state->line_termination

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

2016-04-24 Thread Duy Nguyen
> >>>> 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

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

2016-04-24 Thread Ramsay Jones
On 24/04/16 17:56, Christian Couder wrote: > On Sun, Apr 24, 2016 at 6:27 PM, Christian Couder > wrote: >> On Sun, Apr 24, 2016 at 5:23 PM, Ramsay Jones >> wrote: >>> >>> >>> On 24/04/16 14:33, Christian Couder wrote: >>>> This is a pa

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