Re: [PATCH 18/19] diff: buffer all output if asked to

2017-05-13 Thread Stefan Beller
On Sat, May 13, 2017 at 9:06 PM, Jeff King wrote: > On Sat, May 13, 2017 at 09:01:16PM -0700, Stefan Beller wrote: > >> + for (i = 0; i < o->line_buffer_nr; i++); >> + free((void*)o->line_buffer[i].line); > > I haven't looked at the patches yet, but

Re: [PATCH 18/19] diff: buffer all output if asked to

2017-05-13 Thread Jeff King
On Sat, May 13, 2017 at 09:01:16PM -0700, Stefan Beller wrote: > + for (i = 0; i < o->line_buffer_nr; i++); > + free((void*)o->line_buffer[i].line); I haven't looked at the patches yet, but this ";" on the for line is almost certainly a typo (gcc catches it due to

[PATCH 15/19] diff.c: convert diff_flush to use emit_line_*

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 05/19] diff.c: emit_line_0 can handle no color setting

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 04/19] diff.c: factor out diff_flush_patch_all_file_pairs

2017-05-13 Thread Stefan Beller
In a later patch we want to do more things before and after all filepairs are flushed. So factor flushing out all file pairs into its own function that the new code can be plugged in easily. Signed-off-by: Stefan Beller --- diff.c | 17 - 1 file changed, 12

[PATCH 13/19] diff.c: convert show_stats to use emit_line_*

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 09/19] diff.c: convert emit_rewrite_diff to use emit_line_*

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 18/19] diff: buffer all output if asked to

2017-05-13 Thread Stefan Beller
Introduce a new option 'use_buffer' in the struct diff_options which controls whether all output is buffered up until all output is available. We'll have two new structs in diff.h, one of them 'buffered_patch_line' will be used to buffer each line, and the other 'buffered_patch_file_pair' will

[RFC PATCH 00/19] Diff machine: highlight moved lines.

2017-05-13 Thread Stefan Beller
For details on *why* see the commit message of the last commit. The first five patches are slight refactorings to get into good shape, the next patches are funneling all output through emit_line_*. The second last patch introduces an option to buffer up all output before printing, and then the

[PATCH 01/19] diff: readability fix

2017-05-13 Thread Stefan Beller
We already have dereferenced 'p->two' into a local variable 'two'. Use that. Signed-off-by: Stefan Beller --- diff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diff.c b/diff.c index 74283d9001..3f5bf8b5a4 100644 --- a/diff.c +++ b/diff.c @@

[PATCH 12/19] diff.c: convert emit_binary_diff_body to use emit_line_*

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 06/19] diff: add emit_line_fmt

2017-05-13 Thread Stefan Beller
In the following patches we'll convert all printing functions to use the emit_line_* family of functions. Many of the printing functions to be converted are formatted. So offer a formatted function in the emit_line function family as well. Signed-off-by: Stefan Beller ---

[PATCH 03/19] diff.c: drop 'nofirst' from emit_line_0

2017-05-13 Thread Stefan Beller
In 250f79930d (diff.c: split emit_line() from the first char and the rest of the line, 2009-09-14) we introduced the local variable 'nofirst' that indicates if we have no first sign character. With the given implementation we had to use an extra variable unlike reusing 'first' because the lines

[PATCH 08/19] diff.c: convert builtin_diff to use emit_line_*

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 19/19] diff.c: color moved lines differently

2017-05-13 Thread Stefan Beller
When there is a lot of code moved around such as in 11979b9 (2005-11-18, "http.c: reorder to avoid compilation failure.") for example, the review process is quite hard, as it is not mentally challenging. It is a rather tedious process, that gets boring quickly. However you still need to read

[PATCH 16/19] diff.c: convert diff_summary to use emit_line_*

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 10/19] diff.c: convert emit_rewrite_lines to use emit_line_*

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 14/19] diff.c: convert word diffing to use emit_line_*

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 11/19] submodule.c: convert show_submodule_summary to use emit_line_fmt

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 07/19] diff.c: convert fn_out_consume to use emit_line_*

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

[PATCH 17/19] diff.c: factor out emit_line_ws for coloring whitespaces

2017-05-13 Thread Stefan Beller
Introduce a helper that calls ws_check_emit. We'll have it as a helper as in a later patch we'll add buffering in there. In a later patch we want to buffer up all output and to do that we'll need to keep around information for outputting a line such as the whitespace information. We choose to put

[PATCH 02/19] diff: move line ending check into emit_hunk_header

2017-05-13 Thread Stefan Beller
In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+ and -) that got moved. So to prepare the diff

Re: checkout -b remotes/origin/ should not work

2017-05-13 Thread Jeff King
On Sat, May 13, 2017 at 08:52:37PM -0700, Stefan Beller wrote: > NEEDSWORK: > > checkout -b remotes/origin/ should not work, unless force is > given (maybe?) > > (I just run into that, now I have a remote tracking branch that points > at my detached HEAD. Oh well.) To be pedantic, you have a

Re: [PATCH] interpret-trailers: obey scissors lines

2017-05-13 Thread Jeff King
On Sat, May 13, 2017 at 08:39:23PM -0700, Brian Malehorn wrote: > If a commit message is being editted as "verbose", it will contain a > scissors string ("-- >8 --") and a diff: > > my subject > > # >8 > # Do not touch the line

checkout -b remotes/origin/ should not work

2017-05-13 Thread Stefan Beller
NEEDSWORK: checkout -b remotes/origin/ should not work, unless force is given (maybe?) (I just run into that, now I have a remote tracking branch that points at my detached HEAD. Oh well.)

Re: [PATCH 0/3] interpret-trailers + commit -v bugfix

2017-05-13 Thread Brian Malehorn
> As I said, I'm a little iffy on doing this unconditionally, but it may > be the least-bad solution. I'd just worry about collateral damage to > somebody who doesn't use commit.verbose, but has something scissors-like > in their commit message. > > If you were to switch out is_scissors_line()

[PATCH] interpret-trailers: obey scissors lines

2017-05-13 Thread Brian Malehorn
If a commit message is being editted as "verbose", it will contain a scissors string ("-- >8 --") and a diff: my subject # >8 # Do not touch the line above. # Everything below will be removed. diff --git a/foo.txt b/foo.txt

[RFC PATCH v2 22/22] blame: create entry prepend function in libgit

2017-05-13 Thread Jeff Smith
Signed-off-by: Jeff Smith --- blame.c | 16 blame.h | 2 ++ builtin/blame.c | 11 +-- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/blame.c b/blame.c index f6c9cb7..00404b9 100644 --- a/blame.c +++ b/blame.c @@

[RFC PATCH v2 12/22] blame: move no_whole_file_rename flag to scoreboard

2017-05-13 Thread Jeff Smith
The no_whole_file_rename flag is used in parts of blame that are being moved to libgit, and should be accessible via the scoreboard structure. Signed-off-by: Jeff Smith --- blame.h | 1 + builtin/blame.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff

[RFC PATCH v2 16/22] blame: rework methods that determine 'final' commit

2017-05-13 Thread Jeff Smith
Either prepare_initial or prepare_final is used to determine which commit is marked as 'final'. Call the underlying methods directly to make this more clear. Signed-off-by: Jeff Smith --- builtin/blame.c | 49 +++-- 1 file changed,

[RFC PATCH v2 14/22] blame: move progess updates to a scoreboard callback

2017-05-13 Thread Jeff Smith
Allow the interface user to decide how to handle a progress update. Signed-off-by: Jeff Smith --- blame.h | 3 +++ builtin/blame.c | 24 ++-- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/blame.h b/blame.h index

[RFC PATCH v2 15/22] blame: wrap blame_sort and compare_blame_final

2017-05-13 Thread Jeff Smith
The new method's interface is marginally cleaner than blame_sort, and will avoid the need to expose the compare_blame_final method. Signed-off-by: Jeff Smith --- builtin/blame.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git

[RFC PATCH v2 17/22] blame: move origin-related methods to libgit

2017-05-13 Thread Jeff Smith
Signed-off-by: Jeff Smith --- Makefile| 1 + blame.c | 62 + blame.h | 15 +++ builtin/blame.c | 120 4 files changed, 102 insertions(+), 96 deletions(-)

[RFC PATCH v2 20/22] blame: create scoreboard init function in libgit

2017-05-13 Thread Jeff Smith
Signed-off-by: Jeff Smith --- blame.c | 7 +++ blame.h | 2 ++ builtin/blame.c | 4 +--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/blame.c b/blame.c index 798e61b..17ebf64 100644 --- a/blame.c +++ b/blame.c @@ -1574,3 +1574,10 @@ void

[RFC PATCH v2 13/22] blame: make sanity_check use a callback in scoreboard

2017-05-13 Thread Jeff Smith
Allow the interface user to decide how to handle a failed sanity check, whether that be to output with the current state or to do nothing. Signed-off-by: Jeff Smith --- blame.h | 4 builtin/blame.c | 23 +++ 2 files changed, 19

[RFC PATCH v2 18/22] blame: move fake-commit-related methods to libgit

2017-05-13 Thread Jeff Smith
Signed-off-by: Jeff Smith --- blame.c | 203 +++- blame.h | 4 +- builtin/blame.c | 197 -- 3 files changed, 205 insertions(+), 199 deletions(-) diff

[RFC PATCH v2 04/22] blame: move origin and entry structures to header

2017-05-13 Thread Jeff Smith
The origin and blame_entry structures are core to the blame interface and reference each other. Since origin will be more exposed, rename it to blame_origin to clarify what it is a part of. Signed-off-by: Jeff Smith --- blame.h | 86 ++

[RFC PATCH v2 08/22] blame: move contents_from to scoreboard

2017-05-13 Thread Jeff Smith
The argument from --contents is used in parts of blame that are being moved to libgit, and should be accessible via the scoreboard structure. Signed-off-by: Jeff Smith --- blame.h | 3 +++ builtin/blame.c | 1 + 2 files changed, 4 insertions(+) diff --git a/blame.h

[RFC PATCH v2 07/22] blame: move copy/move thresholds to scoreboard

2017-05-13 Thread Jeff Smith
Copy and move score thresholds are used in parts of blame that are being moved to libgit, and should be accessible via the scoreboard structure. Signed-off-by: Jeff Smith --- blame.h | 10 ++ builtin/blame.c | 36 2 files

[RFC PATCH v2 11/22] blame: move xdl_opts flags to scoreboard

2017-05-13 Thread Jeff Smith
The xdl_opts flags are used in parts of blame that are being moved to libgit, and should be accessible via the scoreboard structure. Signed-off-by: Jeff Smith --- blame.h | 1 + builtin/blame.c | 7 --- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git

[RFC PATCH v2 05/22] blame: move scoreboard structure to header

2017-05-13 Thread Jeff Smith
The scoreboard structure is core to the blame interface. Since scoreboard will be more exposed, rename it to blame_scoreboard to clarify what it is a part of. Signed-off-by: Jeff Smith --- blame.h | 29 builtin/blame.c | 83

[RFC PATCH v2 10/22] blame: move show_root flag to scoreboard

2017-05-13 Thread Jeff Smith
The show_root flag is used in parts of blame that are being moved to libgit, and should be accessible via the scoreboard structure. Signed-off-by: Jeff Smith --- blame.h | 1 + builtin/blame.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git

[RFC PATCH v2 19/22] blame: move scoreboard-related methods to libgit

2017-05-13 Thread Jeff Smith
Signed-off-by: Jeff Smith --- blame.c | 1313 ++ blame.h | 11 + builtin/blame.c | 1330 +-- 3 files changed, 1330 insertions(+), 1324 deletions(-) diff

[RFC PATCH v2 02/22] blame: move textconv_object with related functions

2017-05-13 Thread Jeff Smith
textconv_object is used in places other than blame.c and should be moved to a more appropriate location. Other textconv related functions are located in diff.c so that seems as good a place as any. Signed-off-by: Jeff Smith --- builtin.h | 2 -- builtin/blame.c

[RFC PATCH v2 06/22] blame: move stat counters to scoreboard

2017-05-13 Thread Jeff Smith
Statistic counters are used in parts of blame that are being moved to libgit, and should be accessible via the scoreboard structure. Signed-off-by: Jeff Smith --- blame.h | 5 + builtin/blame.c | 29 - 2 files changed, 17

[RFC PATCH v2 09/22] blame: move reverse flag to scoreboard

2017-05-13 Thread Jeff Smith
The reverse flag is used in parts of blame that are being moved to libgit, and should be accessible via the scoreboard structure. Signed-off-by: Jeff Smith --- blame.h | 3 +++ builtin/blame.c | 20 +++- 2 files changed, 14 insertions(+), 9

[RFC PATCH v2 00/22] Add blame to libgit

2017-05-13 Thread Jeff Smith
Rather than duplicate large portions of builtin/blame.c in cgit, it would be better to shift its core functionality into libgit.a. The functionality left in builtin/blame.c mostly relates to terminal presentation. Since initial patchset: Made commit titles consistent Broke some commits into

[RFC PATCH v2 03/22] blame: remove unused parameters

2017-05-13 Thread Jeff Smith
Clean up blame code before moving it into libgit Signed-off-by: Jeff Smith --- builtin/blame.c | 25 ++--- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index c419981..e30b3ef 100644 ---

Re: [PATCH] usage.c: drop set_error_handle()

2017-05-13 Thread Brandon Williams
On 05/12, Jeff King wrote: > The set_error_handle() function was introduced by 3b331e926 > (vreportf: report to arbitrary filehandles, 2015-08-11) so > that run-command could send post-fork, pre-exec errors to > the parent's original stderr. > > That use went away in 79319b194 (run-command:

[PATCH v2 7/7] grep: add support for PCRE v2

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add support for v2 of the PCRE API. This is a new major version of PCRE that came out in early 2015[1]. The regular expression syntax is the same, but while the API is similar, pretty much every function is either renamed or takes different arguments. Thus using it via entirely new functions

[PATCH v2 2/7] grep: skip pthreads overhead when using one thread

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Skip the administrative overhead of using pthreads when only using one thread. Instead take the non-threaded path which would be taken under NO_PTHREADS. The threading support was initially added in commit 5b594f457a ("Threaded grep", 2010-01-25) with a hardcoded compile-time number of 8 threads.

[PATCH v2 5/7] grep: un-break building with PCRE < 8.32

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Amend my change earlier in this series ("grep: add support for the PCRE v1 JIT API", 2017-04-11) to un-break the build on PCRE v1 versions earlier than 8.32. The JIT support was added in version 8.20 released on 2011-10-21, but it wasn't until 8.32 released on 2012-11-30 that the fast code path

[PATCH v2 6/7] grep: un-break building with PCRE < 8.20

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Amend my change earlier in this series ("grep: add support for the PCRE v1 JIT API", 2017-04-11) to un-break the build on PCRE v1 versions earlier than 8.20. The 8.20 release was the first release to have JIT & pcre_jit_stack in the headers, so a mock type needs to be provided for it on those

[PATCH v2 4/7] grep: add support for the PCRE v1 JIT API

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Change the grep PCRE v1 code to use JIT when available. When PCRE support was initially added in commit 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09) PCRE had no JIT support, it was integrated into 8.20 released on 2011-10-21. Enabling JIT support usually improves performance by more than 40%.

[PATCH v2 3/7] log: add -P as a synonym for --perl-regexp

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a short -P option as a synonym for the longer --perl-regexp, for consistency with the options the corresponding grep invocations accept. This was intentionally omitted in commit 727b6fc3ed ("log --grep: accept --basic-regexp and --perl-regexp", 2012-10-03) for unspecified future use. Make it

[PATCH v2 1/7] grep: don't redundantly compile throwaway patterns under threading

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Change the pattern compilation logic under threading so that grep doesn't compile a pattern it never ends up using on the non-threaded code path, only to compile it again N times for N threads which will each use their own copy, ignoring the initially compiled pattern. This redundant compilation

[PATCH v2 0/7] PCRE v2, PCRE v1 JIT, log -P & fixes

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Trivial changes since v1, but I wanted to send a new one for completeness since I re-sent the "Easy to review grep & pre-PCRE changes" series. For v1 see <20170511170142.15934-1-ava...@gmail.com> (https://public-inbox.org/git/20170511170142.15934-1-ava...@gmail.com/). Changes noted below & reply

[PATCH v2 27/29] pack-objects: fix buggy warning about threads

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Fix a buggy warning about threads under NO_PTHREADS=YesPlease. Due to re-using the delta_search_threads variable for both the state of the "pack.threads" config & the --threads option, setting "pack.threads" but not supplying --threads would trigger the warning for both "pack.threads" & --threads.

[PATCH v2 20/29] grep: remove redundant `regflags &= ~REG_EXTENDED` assignments

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Remove redundant assignments to the "regflags" variable. There are no code paths that have previously set the regflags to anything, and certainly not to `|= REG_EXTENDED`. This code gave the impression that it had to reset its environment, but it doesn't. This dates back to the initial

[PATCH v2 25/29] test-lib: add a PTHREADS prerequisite

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a PTHREADS prerequisite which is false when git is compiled with NO_PTHREADS=YesPlease. There's lots of custom code that runs when threading isn't available, but before this prerequisite there was no way to test it. Signed-off-by: Ævar Arnfjörð Bjarmason --- Makefile

[PATCH v2 23/29] grep: change internal *pcre* variable & function names to be *pcre1*

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Change the internal PCRE variable & function names to have a "1" suffix. This is for preparation for libpcre2 support, where having non-versioned names would be confusing. An earlier change in this series ("grep: change the internal PCRE macro names to be PCRE1", 2017-04-07) elaborates on the

[PATCH v2 26/29] pack-objects & index-pack: add test for --threads warning

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a test for the warning that's emitted when --threads or pack.threads is provided under NO_PTHREADS=YesPlease. This uses the new PTHREADS prerequisite. The assertion for C_LOCALE_OUTPUT in the latter test is currently redundant, since unlike index-pack the pack-objects warnings aren't i18n'd.

[PATCH v2 24/29] grep: move is_fixed() earlier to avoid forward declaration

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Move the is_fixed() function which are currently only used in compile_regexp() earlier so it can be used in the PCRE family of functions in a later change. Signed-off-by: Ævar Arnfjörð Bjarmason --- grep.c | 24 1 file changed, 12 insertions(+), 12

[PATCH v2 29/29] grep: assert that threading is enabled when calling grep_{lock,unlock}

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Change the grep_{lock,unlock} functions to assert that num_threads is true, instead of only locking & unlocking the pthread mutex lock when it is. These functions are never called when num_threads isn't true, this logic has gone through multiple iterations since the initial introduction of grep

[PATCH v2 28/29] grep: given --threads with NO_PTHREADS=YesPlease, warn

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a warning about missing thread support when grep.threads or --threads is set to a non 0 (default) or 1 (no parallelism) value under NO_PTHREADS=YesPlease. This is for consistency with the index-pack & pack-objects commands, which also take a --threads option & are configurable via

[PATCH v2 22/29] grep: change the internal PCRE macro names to be PCRE1

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Change the internal USE_LIBPCRE define, & build options flag to use a naming convention ending in PCRE1, without changing the long-standing USE_LIBPCRE Makefile flag which enables this code. This is for preparation for libpcre2 support where having things like USE_LIBPCRE and USE_LIBPCRE2 in any

[PATCH v2 21/29] grep: factor test for \0 in grep patterns into a function

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Factor the test for \0 in grep patterns into a function. Since commit 9eceddeec6 ("Use kwset in grep", 2011-08-21) any pattern containing a \0 is considered fixed as regcomp() can't handle it. This limitation was never documented, and other some regular expression engines are capable of compiling

[PATCH v2 17/29] perf: add a performance comparison of fixed-string grep

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a performance comparison test which compares both case-sensitive & case-insensitive fixed-string grep, as well as non-ASCII case-sensitive & case-insensitive grep. Currently only the "-i æ" performance test doesn't go through the same kwset.[ch] codepath, see the "Even when -F..." comment in

[PATCH v2 19/29] grep: remove redundant regflags assignment under PCRE

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Remove a redundant assignment to the "regflags" variable. This variable is only used for POSIX regular expression matching, not when the PCRE library is used. This redundant assignment was added as a result of copy/paste programming in commit 84befcd0a4 ("grep: add a grep.patternType

[PATCH v2 13/29] grep: add tests to fix blind spots with \0 patterns

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Address a big blind spot in the tests for patterns containing \0. The is_fixed() function considers any string that contains \0 fixed, even if it contains regular expression metacharacters, those patterns are currently matched with kwset. Before this change removing that memchr(s, 0, len) check

[PATCH v2 14/29] perf: add a GIT_PERF_MAKE_COMMAND for when *_MAKE_OPTS won't do

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a git GIT_PERF_MAKE_COMMAND variable to compliment the existing GIT_PERF_MAKE_OPTS facility. This allows specifying an arbitrary shell command to execute instead of 'make'. This is useful e.g. in cases where the name, semantics or defaults of a Makefile flag have changed over time. It can

[PATCH v2 16/29] perf: add a performance comparison test of grep -G, -E and -P

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a very basic performance comparison test comparing the POSIX basic, extended and perl engines. In theory the "basic" and "extended" engines should be implemented using the same underlying code with a slightly different pattern parser, but some implementations may not do this. Jump through

[PATCH v2 15/29] perf: emit progress output when unpacking & building

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Amend the t/perf/run output so that in addition to the "Running N tests" heading currently being emitted, it also emits "Unpacking $rev" and "Building $rev" when setting up the build/$rev directory & when building it, respectively. This makes it easier to see what's going on and what revision is

[PATCH v2 18/29] grep: catch a missing enum in switch statement

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a die(...) to a default case for the switch statement selecting between grep pattern types under --recurse-submodules. Normally this would be caught by -Wswitch, but the grep_pattern_type type is converted to int by going through parse_options(). Changing the argument type passed to

[PATCH v2 10/29] grep: add tests for grep pattern types being passed to submodules

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add testing for grep pattern types being correctly passed to submodules. The pattern "(.|.)[\d]" matches differently under fixed (not at all), and then matches different lines under basic/extended & perl regular expressions, so this change asserts that the pattern type is passed along correctly.

[PATCH v2 12/29] grep: prepare for testing binary regexes containing rx metacharacters

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add setup code needed for testing regexes that contain both binary data and regex metacharacters. The POSIX regcomp() function inherently can't support that, because it takes a \0-delimited char *, but other regex engines APIs like PCRE v2 take a pattern/length pair, and are thus able to handle

[PATCH v2 11/29] grep: add a test helper function for less verbose -f \0 tests

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a helper function to make the tests which check for patterns with \0 in them more succinct. Right now this isn't a big win, but subsequent commits will add a lot more of these tests. The helper is based on the match() function in t3070-wildmatch.sh. Signed-off-by: Ævar Arnfjörð Bjarmason

[PATCH v2 03/29] test-lib: rename the LIBPCRE prerequisite to PCRE

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Rename the LIBPCRE prerequisite to PCRE. This is for preparation for libpcre2 support, where having just "LIBPCRE" would be confusing as it implies v1 of the library. None of these tests are incompatible between versions 1 & 2 of libpcre, it's less confusing to give them a more general name to

[PATCH v2 05/29] grep: add a test asserting that --perl-regexp dies when !PCRE

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a test asserting that when --perl-regexp (and -P for grep) is given to git-grep & git-log that we die with an error. In developing the PCRE v2 series I introduced a regression where -P would (through control-flow fall-through) become synonymous with basic POSIX matching. I.e. 'git grep -P

[PATCH v2 09/29] grep: amend submodule recursion test for regex engine testing

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Amend the submodule recursion test to prepare it for subsequent tests of whether it passes along the grep.patternType to the submodule greps. This is the result of searching & replacing: foobar -> (1|2)d(3|4) foo-> (1|2) bar-> (3|4) Currently there's no tests for whether

[PATCH v2 02/29] grep & rev-list doc: stop promising libpcre for --perl-regexp

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Stop promising in our grep & rev-list options documentation that we're always going to be using libpcre when given the --perl-regexp option. Instead talk about using "Perl-compatible regular expressions" and using these types of patterns using "a compile-time dependency". Saying "libpcre" means

[PATCH v2 07/29] grep: change non-ASCII -i test to stop using --debug

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Change a non-ASCII case-insensitive test case to stop using --debug, and instead simply test for the expected results. The test coverage remains the same with this change, but the test won't break due to internal refactoring. This test was added in commit 793dc676e0 ("grep/icase: avoid kwsset

[PATCH v2 06/29] grep: add a test for backreferences in PCRE patterns

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add a test for backreferences such as (.)\1 in PCRE patterns. This test ensures that the PCRE_NO_AUTO_CAPTURE option isn't turned on. Before this change turning it on would break these sort of patterns, but wouldn't break any tests. Signed-off-by: Ævar Arnfjörð Bjarmason ---

[PATCH v2 08/29] grep: add tests for --threads=N and grep.threads

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add tests for --threads=N being supplied on the command-line, or when grep.threads=N being supplied in the configuration. When the threading support was made run-time configurable in commit 89f09dd34e ("grep: add --threads= option and grep.threads configuration", 2015-12-15) no tests were added

[PATCH v2 01/29] Makefile & configure: reword inaccurate comment about PCRE

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Reword an outdated & inaccurate comment which suggests that only git-grep can use PCRE. This comment was added back when PCRE support was initially added in commit 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09), and was true at the time. It hasn't been telling the full truth since git-log

[PATCH v2 04/29] log: add exhaustive tests for pattern style options & config

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Add exhaustive tests for how the different grep.patternType options & the corresponding command-line options affect git-log. Before this change it was possible to patch revision.c so that the --basic-regexp option was synonymous with --extended-regexp, and --perl-regexp wasn't recognized at all,

[PATCH v2 00/29] Easy to review grep & pre-PCRE changes

2017-05-13 Thread Ævar Arnfjörð Bjarmason
Easy to review? 29 patches? Are you kidding me?! As noted in v1 (<20170511091829.5634-1-ava...@gmail.com>; https://public-inbox.org/git/20170511091829.5634-1-ava...@gmail.com/) these are all doc, test, refactoring etc. changes needed by the subsequent "PCRE v2, PCRE v1 JIT, log -P & fixes"

Re: [PATCH] Use https links to Wikipedia to avoid http redirects

2017-05-13 Thread Ævar Arnfjörð Bjarmason
On Sat, May 13, 2017 at 11:54 AM, Sven Strickroth wrote: > Signed-off-by: Sven Strickroth Thanks! FWIW: Reviewed-by: Ævar Arnfjörð Bjarmason > --- > Documentation/gitweb.txt | 2 +- > bisect.c | 2 +- > gitweb/gitweb.perl

Re: [PATCH] compat/regex: fix compilation on Windows

2017-05-13 Thread Ævar Arnfjörð Bjarmason
On Sat, May 13, 2017 at 8:30 PM, Johannes Schindelin wrote: > Hi Ævar, > > I originally replied in a very verbose manner, going step by step through > the "one-liner", but decided to rephrase everything. > > So here goes. > > On Sat, 13 May 2017, Ævar Arnfjörð

Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config

2017-05-13 Thread Johannes Schindelin
Hi Jonathan, On Fri, 12 May 2017, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > > On Windows, `(1|2)` is not a valid file name, and therefore the tag > > cannot be created as expected by the new test. > > > > So simply skip this test on Windows. > > > > Signed-off-by: Johannes

Re: [PATCH] compat/regex: fix compilation on Windows

2017-05-13 Thread Johannes Schindelin
Hi Ævar, I originally replied in a very verbose manner, going step by step through the "one-liner", but decided to rephrase everything. So here goes. On Sat, 13 May 2017, Ævar Arnfjörð Bjarmason wrote: > Let's drop this current gawk import series. Well, the reason why you imported the current

Re: [PATCH 04/29] log: add exhaustive tests for pattern style options & config

2017-05-13 Thread Ævar Arnfjörð Bjarmason
On Fri, May 12, 2017 at 6:48 AM, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >> Add exhaustive tests for how the different grep.patternType options & >> the corresponding command-line options affect git-log. >> ... >> The patterns being passed

Re: [PATCH v4 03/12] t/test-lib-functions.sh: allow to specify the tag name to test_commit

2017-05-13 Thread Ævar Arnfjörð Bjarmason
On Tue, Feb 12, 2013 at 11:17 AM, Brandon Casey wrote: > The part of test_commit() may not be appropriate for a tag name. > So let's allow test_commit to accept a fourth argument to specify the tag > name. [Kind of late to notice, I know] I see nobody spotted in four rounds

Re: [PATCH 09/29] grep: amend submodule recursion test for regex engine testing

2017-05-13 Thread Ævar Arnfjörð Bjarmason
On Fri, May 12, 2017 at 6:59 AM, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >> Amend the submodule recursion test to prepare it for subsequent tests >> of whether it passes along the grep.patternType to the submodule >> greps. >> >> This is

[PATCH 5/5] p0004: don't error out if test repo is too small

2017-05-13 Thread René Scharfe
Repositories with less than 4000 entries are always handled using a single thread, causing test-lazy-init-name-hash --multi to error out. Don't abort the whole test script in that case, but simply skip the multi-threaded performance check. We can still use it to compare the single-threaded speed

[PATCH 4/5] p0004: don't abort if multi-threaded is too slow

2017-05-13 Thread René Scharfe
If the single-threaded variant beats the multi-threaded one then we may have a performance bug, but that doesn't justify aborting the test. Drop that check; we can compare the results for --single and --multi using the actual performance tests. Signed-off-by: Rene Scharfe ---

[PATCH 3/5] p0004: use test_perf

2017-05-13 Thread René Scharfe
The perf test suite (more specifically: t/perf/aggregate.perl) requires each test script to write test results into a file, otherwise it aborts when aggregating. Add actual performance tests with test_perf to allow p0004 to be run together with other perf scripts. Calibrate the value for the

[PATCH 2/5] p0004: avoid using pipes

2017-05-13 Thread René Scharfe
The return code of commands on the producing end of a pipe is ignored. Evaluate the outcome of test-lazy-init-name-hash by calling sort separately. Signed-off-by: Rene Scharfe --- t/perf/p0004-lazy-init-name-hash.sh | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-)

[PATCH 1/5] p0004: simplify calls of test-lazy-init-name-hash

2017-05-13 Thread René Scharfe
The test library puts helpers into $PATH, so we can simply call them without specifying their location. The suffix $X is also not necessary because .exe files on Windows can be started without specifying their extension, and on other platforms it's empty anyway. Signed-off-by: Rene Scharfe

[PATCH 0/5] p0004: support being called by t/perf/run

2017-05-13 Thread René Scharfe
p0004-lazy-init-name-hash.sh errors out if the test repo is too small, and doesn't generate any perf test results even if it finishes successfully. That prevents t/perf/run from running the whole test suite. This series tries to address these issues. p0004: simplify calls of

  1   2   >