`pull --rebase --autostash` fails when fast forward in dirty repo

[Tyler Brazier]

Hi,

This script explains and tests what's going on:
https://gist.github.com/tylerbrazier/4478e76fe44bf6657d4d3da6c789531d

pull is failing because it shortcuts to --ff-only then calls
run_merge(), which does not know how to autostash. Removing the
shortcut fixes the problem:

diff --git a/builtin/pull.c b/builtin/pull.c
index dd1a4a94e..225a59f5f 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -868,11 +868,6 @@ int cmd_pull(int argc, const char **argv, const
char *prefix)
  head = lookup_commit_reference(orig_head.hash);
  commit_list_insert(head, );
  merge_head = lookup_commit_reference(merge_heads.oid[0].hash);
- if (is_descendant_of(merge_head, list)) {
- /* we can fast-forward this without invoking rebase */
- opt_ff = "--ff-only";
- return run_merge();
- }
  return run_rebase(_head, merge_heads.oid, _fork_point);
  } else {
  return run_merge();

Options to avoid docs generation/installation

[Neil Cafferkey]

Hi,

The INSTALL file says that docs are not built by default, but that's not my 
experience. "make all" results in the generation of several Perl man pages, 
e.g. "Git.3pm". Is it the case that the behaviour documented is not 
propagated to the perl subdir?

Also, setting --mandir=/dev/null still results in the pages being installed 
in their usual location (at least in combination with setting --prefix to 
avoid disturbing my production git installation).

Regards,
Neil

Re: [PATCH v3 15/30] perf: add a GIT_PERF_MAKE_COMMAND for when *_MAKE_OPTS won't do

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> diff --git a/t/perf/README b/t/perf/README
> index 49ea4349be..b3d95042a8 100644
> --- a/t/perf/README
> +++ b/t/perf/README
> @@ -60,8 +60,23 @@ You can set the following variables (also in your 
> config.mak):
>  
>  GIT_PERF_MAKE_OPTS
>   Options to use when automatically building a git tree for
> - performance testing.  E.g., -j6 would be useful.
> -
> +...
> + any of that, that's an implementation detail that might change
> + in the future.
> + 

I'll remove the trailing whitespace on this otherwise blank line
while queuing (no need to resend only to fix this one).

Thanks.

>  GIT_PERF_REPO
>  GIT_PERF_LARGE_REPO
>   Repositories to copy for the performance tests.  The normal
> diff --git a/t/perf/run b/t/perf/run
> index c788d713ae..b61024a830 100755
> --- a/t/perf/run
> +++ b/t/perf/run
> @@ -37,8 +37,15 @@ build_git_rev () {
>   cp "../../$config" "build/$rev/"
>   fi
>   done
> - (cd build/$rev && make $GIT_PERF_MAKE_OPTS) ||
> - die "failed to build revision '$mydir'"
> + (
> + cd build/$rev &&
> + if test -n "$GIT_PERF_MAKE_COMMAND"
> + then
> + sh -c "$GIT_PERF_MAKE_COMMAND"
> + else
> + make $GIT_PERF_MAKE_OPTS
> + fi
> + ) || die "failed to build revision '$mydir'"
>  }
>  
>  run_dirs_helper () {

Re: [PATCH v3 05/30] log: make --regexp-ignore-case work with --perl-regexp

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> Make the --regexp-ignore-case option work with --perl-regexp. This
> never worked, and there was no test for this. Fix the bug and add a
> test.
>
> When PCRE support was added in commit 63e7e9d8b6 ("git-grep: Learn
> PCRE", 2011-05-09) compile_pcre_regexp() would only check
> opt->ignore_case, but when the --perl-regexp option was added in
> commit 727b6fc3ed ("log --grep: accept --basic-regexp and
> --perl-regexp", 2012-10-03) the code didn't set the opt->ignore_case.
>
> Change the test suite to test for -i and --invert-regexp with
> basic/extended/perl patterns in addition to fixed, which was the only
> patternType that was tested for before in combination with those
> options.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  revision.c |  1 +
>  t/t4202-log.sh | 60 
> +-
>  2 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/revision.c b/revision.c
> index 8a8c1789c7..4883cdd2d0 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1991,6 +1991,7 @@ static int handle_revision_opt(struct rev_info *revs, 
> int argc, const char **arg
>   } else if (!strcmp(arg, "--extended-regexp") || !strcmp(arg, "-E")) {
>   revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_ERE;
>   } else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
> + revs->grep_filter.ignore_case = 1;
>   revs->grep_filter.regflags |= REG_ICASE;
>   DIFF_OPT_SET(>diffopt, PICKAXE_IGNORE_CASE);
>   } else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {

Looks good.

I however wonder if it is a better approach in the longer term to
treat the .ignore_case field just like .extended_regexp_option
field, i.e. not committing immediately to .regflags but commit it
after config and command line parsing is done, just like we make the
"BRE? ERE?" decision in grep_commit_pattern_type().

Thanks.

Re: [PATCH v3 00/30] Easy to review grep & pre-PCRE changes

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> Easy to review? 29 (I mean 30) 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" series.
>
> Since Junio hasn't been picking it I'm no longer sending updates to
> that patch series & waiting for this one to cook first.

I actually do not mind a reroll that goes together with this.  The
only reason why I skipped the earlier one was because I looked at
the original one, and the discussion on the reroll of this 'easy to
review' part indicated that it will be rerolled, before I got to
look at these upper layer patches.

Overall nicely done.  I only had just a few observations.

Thanks.

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Junio C Hamano]

Manish Goregaokar  writes:

> One thing which I think hasn't been covered yet is the rebase
> ORIG_HEAD. I'll see if that's still a problem on `pu` and make a patch
> for it if so.

IIRC, ORIG_HEAD, FETCH_HEAD, MERGE_HEAD and others are be transitory
and never served as the starting points of reachability traversal,
even in the primary worktree (also when you are not using any extra
worktrees).  The commits listed in the todo list of "rebase -i" and
"git cherry-pick A..B" are the same way.

Because ORIG_HEAD is expected to be in a reflog of some ref (at
least, the reflog of HEAD), I do not see much benefit (or "more
safety") in adding it to the starting points.

Among the ones that appear in .git/ and we never considered as
starting points, the commits in FETCH_HEAD might be the ones we may
want to give extra protection over what we currently have, simply
because the ones that do not have remote-tracking branches have NO
other refs pointing at them (compared to these transitory artifacts
resulting from a local operation, i.e. ORIG_HEAD and ones in the
todo list).  But they by definition are "new" objects, and the
reason why the user does *not* use remote-tracking branches for them
is because the user does not want to keep record of them unless the
user decides to merge them somewhere in the repository's history, so
even for them the benefit is dubious...

[PATCH] diff: mark some file local symbols as static

[Ramsay Jones]

Signed-off-by: Ramsay Jones 
---

Hi Stefan,

If you need to re-roll your 'sb/diff-color-move' branch, could you
please squash this into to the relevant patches. (Each hunk would
be squashed into a separate commit, thus:
  - commit 9b68d54c11, "diff: buffer all output if asked to"
  - commit 4b71ba47ab, "submodule.c: convert show_submodule_summary to use 
emit_line_fmt"
  - commit 703d25581d, "diff.c: convert show_stats to use emit_line_*"
).

Thanks!

ATB,
Ramsay Jones

 diff.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/diff.c b/diff.c
index ba23c5862..15f389676 100644
--- a/diff.c
+++ b/diff.c
@@ -810,10 +810,10 @@ static void append_buffered_patch_line(struct 
diff_options *o,
f->line = e->line ? xmemdupz(e->line, e->len) : NULL;
 }
 
-void emit_line(struct diff_options *o,
-  const char *set, const char *reset,
-  int add_line_prefix, int markup_ws,
-  int sign, const char *line, int len)
+static void emit_line(struct diff_options *o,
+ const char *set, const char *reset,
+ int add_line_prefix, int markup_ws,
+ int sign, const char *line, int len)
 {
struct buffered_patch_line e = {set, reset, line,
len, sign, add_line_prefix,
@@ -825,10 +825,10 @@ void emit_line(struct diff_options *o,
emit_buffered_patch_line(o, );
 }
 
-void emit_line_fmt(struct diff_options *o,
-  const char *set, const char *reset,
-  int add_line_prefix,
-  const char *fmt, ...)
+static void emit_line_fmt(struct diff_options *o,
+ const char *set, const char *reset,
+ int add_line_prefix,
+ const char *fmt, ...)
 {
struct strbuf sb = STRBUF_INIT;
va_list ap;
@@ -1862,8 +1862,8 @@ static void fill_print_name(struct diffstat_file *file)
file->print_name = pname;
 }
 
-void print_stat_summary_0(struct diff_options *options, int files,
- int insertions, int deletions)
+static void print_stat_summary_0(struct diff_options *options, int files,
+int insertions, int deletions)
 {
struct strbuf sb = STRBUF_INIT;
 
-- 
2.13.0

Re: [PATCH] name-rev: use larger timestamp for is_better_name

[Junio C Hamano]

Ramsay Jones  writes:

> I test on 32-bit Linux from time to time, and tonight's 'pu'
> branch fails t4202.44, t6007.2,5-6,12-13,16, t6012.2-11,
> t6111.2-65. I bisected the t4202 failure to a merge commit
> (99d31e1378, merge branch 'jc/name-rev-lw-tag') and I spotted
> the 'unsigned long' taggerdate parameter to the is_better_name()
> function.

Thanks.  My earlier "oh, cutoff that was long also is a timestamp"
patch was what I did while making this exact "evil merge" change,
and then when I re-integrated everything, I forgot about it.

I'll teach my rerere database about this.  Thanks.

Re: [PATCHv2 12/20] submodule.c: convert show_submodule_summary to use emit_line_fmt

[Stefan Beller]

On Fri, May 19, 2017 at 9:50 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> That could be added in ws.c:ws_check_emit, as these certain words
>> are similar to coloring whitespace.
>
> I actually was envisioning of highlighting a part of a line, like
>
> -Very poor SCM
> +Very nice SCM
>
> which would be done by finding semi-matching removed and added lines
> in the same hunk (i.e. local buffering) and makes a coloring decision.
> That does not have any place in ws.c.

Yes such a feature would not want to be in ws.c

For the problem above, we're still fine with the given data structures IMO.
Though it may hint at bad naming of the struct 'buffered_patch_line'
as it is not a complete line.

Assuming the example above, running "show --word-diff" would produce
the following "buffered_patch_lines"

{show_prefix=1, sign='\0', line="-Very ", set=NULL, reset=NULL}
{show_prefix=0, sign='\0', line="{- poor}", set='red', reset='normal'}
{show_prefix=0, sign='\0', line="{+ nice}", set='green', reset='normal'}
{show_prefix=0, sign='\0', line=" SCM\n", set=NULL, reset=NULL}

so in the future, we may want to produce

{show_prefix=1, sign='-', line="Very ", set=NULL, reset=NULL}
{show_prefix=0, sign='\0', line="poor", set='red', reset='normal'}
{show_prefix=0, sign='\0', line=" SCM\n", set=NULL, reset=NULL}
{show_prefix=1, sign='+', line="Very ", set=NULL, reset=NULL}
{show_prefix=0, sign='\0', line="nice", set='gren', reset='normal'}
{show_prefix=0, sign='\0', line=" SCM\n", set=NULL, reset=NULL}

instead.

I think the data structure can be used as-is, but is just mis-named.
I'll fix that in a resend.

The algorithm to highlight what changed on a word level after a hunk
would have to be put into the current hunk finding
("mark_color_as_moved"), and then split up the actual lines into pieces
of a line and add different colors like we see above.

>
>>> Having said that, we need to start somewhere, and I think it is a
>>> reasonable first-cut attempt to work on top of the textual output
>>> like this series does (IOW, while I do agree with the NEEDSWORK and
>>> the way this series currently does things must be revamped in the
>>> longer term, I do not think we should wait until that happens to
>>> start playing with this topic).
>>
>> Ok. I share a similar reaction to submodule diffs that we discuss above
>> and word coloring, that Jonathan Tan brought up off list.
>>
>> Both of them are broken in this implementation, but the NEEDSWORK
>> would hint at how to fix them.
>
> Yes, but if NEEDSWORK has to say "the current hack is working at a
> wrong level, we need to do all of this before producing textual
> diffs that are passed to the layer that colors lines", that wouldn't
> help that much as a hint X-<.

For the word coloring, I think we'd just need better post processing.

For cros-submodule move detection, we may want to wait in Brandons
work to be able to run submodule stuff in-process and then we
have the lines buffered directly there, so we can operate on them as well.

I agree that "NEEDSWORK: the current hack is working at a wrong level"
is not useful. But I thought we're in agreement that it is not? I must have
misunderstood parts of what you were saying.

By saying it could go to ws.c in my reply I rather meant:
it could go into some post-processing function of the "buffered_patch_line"
struct. Currently we only have ws_emit_line that does it, but we can do
it either similarly or just split up one buffered_patch_line into more
than one and color up each individually.

This would also be possible for us now, too. Instead of running it through
ws_emit_line we could split up the line and color each piece differently.
However that could be a problem in the algorithm to find similar hunks
(as we do have different rules for added and deleted text).

Thanks,
Stefan

[PATCH v3 25/30] grep: move is_fixed() earlier to avoid forward declaration

[Æ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 deletions(-)

diff --git a/grep.c b/grep.c
index 07512346b1..1157529115 100644
--- a/grep.c
+++ b/grep.c
@@ -321,6 +321,18 @@ static NORETURN void compile_regexp_failed(const struct 
grep_pat *p,
die("%s'%s': %s", where, p->pattern, error);
 }
 
+static int is_fixed(const char *s, size_t len)
+{
+   size_t i;
+
+   for (i = 0; i < len; i++) {
+   if (is_regex_special(s[i]))
+   return 0;
+   }
+
+   return 1;
+}
+
 static int has_null(const char *s, size_t len)
 {
/*
@@ -402,18 +414,6 @@ static void free_pcre1_regexp(struct grep_pat *p)
 }
 #endif /* !USE_LIBPCRE1 */
 
-static int is_fixed(const char *s, size_t len)
-{
-   size_t i;
-
-   for (i = 0; i < len; i++) {
-   if (is_regex_special(s[i]))
-   return 0;
-   }
-
-   return 1;
-}
-
 static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt)
 {
struct strbuf sb = STRBUF_INIT;
-- 
2.13.0.303.g4ebf302169

[PATCH v3 27/30] pack-objects & index-pack: add test for --threads warning

[Æ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. However they might be changed to be i18n'd in the future, and
there's no harm in future-proofing the test.

There's an existing bug in the implementation of pack-objects which
this test currently tests for as-is. Details about the bug & the fix
are included in a follow-up change.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5300-pack-object.sh | 36 
 1 file changed, 36 insertions(+)

diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 43a672c345..6ed23ee1d2 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -421,6 +421,42 @@ test_expect_success 'index-pack  works in non-repo' '
test_path_is_file foo.idx
 '
 
+test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'index-pack --threads=N or 
pack.threads=N warns when no pthreads' '
+   test_must_fail git index-pack --threads=2 2>err &&
+   grep ^warning: err >warnings &&
+   test_line_count = 1 warnings &&
+   grep -F "no threads support, ignoring --threads=2" err &&
+
+   test_must_fail git -c pack.threads=2 index-pack 2>err &&
+   grep ^warning: err >warnings &&
+   test_line_count = 1 warnings &&
+   grep -F "no threads support, ignoring pack.threads" err &&
+
+   test_must_fail git -c pack.threads=2 index-pack --threads=4 2>err &&
+   grep ^warning: err >warnings &&
+   test_line_count = 2 warnings &&
+   grep -F "no threads support, ignoring --threads=4" err &&
+   grep -F "no threads support, ignoring pack.threads" err
+'
+
+test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'pack-objects --threads=N or 
pack.threads=N warns when no pthreads' '
+   git pack-objects --threads=2 --stdout --all /dev/null 2>err 
&&
+   grep ^warning: err >warnings &&
+   test_line_count = 1 warnings &&
+   grep -F "no threads support, ignoring --threads" err &&
+
+   git -c pack.threads=2 pack-objects --stdout --all /dev/null 
2>err &&
+   grep ^warning: err >warnings &&
+   test_must_fail test_line_count = 1 warnings &&
+   grep -F "no threads support, ignoring pack.threads" err &&
+
+   git -c pack.threads=2 pack-objects --threads=4 --stdout --all 
/dev/null 2>err &&
+   grep ^warning: err >warnings &&
+   test_line_count = 2 warnings &&
+   grep -F "no threads support, ignoring --threads" err &&
+   grep -F "no threads support, ignoring pack.threads" err
+'
+
 #
 # WARNING!
 #
-- 
2.13.0.303.g4ebf302169

[PATCH v3 10/30] grep: amend submodule recursion test for regex engine testing

[Æ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 e.g. -P or -E is correctly
passed along, tests for that will be added in a follow-up change, but
first add content to the tests which will match differently under
different regex engines.

Reuse the pattern established in an earlier commit of mine in this
series ("log: add exhaustive tests for pattern style options &
config", 2017-04-07). The pattern "(.|.)[\d]" will match this content
differently under fixed/basic/extended & perl.

This test code was originally added in commit 0281e487fd ("grep:
optionally recurse into submodules", 2016-12-16).

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t7814-grep-recurse-submodules.sh | 166 ++---
 1 file changed, 83 insertions(+), 83 deletions(-)

diff --git a/t/t7814-grep-recurse-submodules.sh 
b/t/t7814-grep-recurse-submodules.sh
index 5b6eb3a65e..1472855e1d 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -9,13 +9,13 @@ submodules.
 . ./test-lib.sh
 
 test_expect_success 'setup directory structure and submodule' '
-   echo "foobar" >a &&
+   echo "(1|2)d(3|4)" >a &&
mkdir b &&
-   echo "bar" >b/b &&
+   echo "(3|4)" >b/b &&
git add a b &&
git commit -m "add a and b" &&
git init submodule &&
-   echo "foobar" >submodule/a &&
+   echo "(1|2)d(3|4)" >submodule/a &&
git -C submodule add a &&
git -C submodule commit -m "add a" &&
git submodule add ./submodule &&
@@ -24,18 +24,18 @@ test_expect_success 'setup directory structure and 
submodule' '
 
 test_expect_success 'grep correctly finds patterns in a submodule' '
cat >expect <<-\EOF &&
-   a:foobar
-   b/b:bar
-   submodule/a:foobar
+   a:(1|2)d(3|4)
+   b/b:(3|4)
+   submodule/a:(1|2)d(3|4)
EOF
 
-   git grep -e "bar" --recurse-submodules >actual &&
+   git grep -e "(3|4)" --recurse-submodules >actual &&
test_cmp expect actual
 '
 
 test_expect_success 'grep and basic pathspecs' '
cat >expect <<-\EOF &&
-   submodule/a:foobar
+   submodule/a:(1|2)d(3|4)
EOF
 
git grep -e. --recurse-submodules -- submodule >actual &&
@@ -44,7 +44,7 @@ test_expect_success 'grep and basic pathspecs' '
 
 test_expect_success 'grep and nested submodules' '
git init submodule/sub &&
-   echo "foobar" >submodule/sub/a &&
+   echo "(1|2)d(3|4)" >submodule/sub/a &&
git -C submodule/sub add a &&
git -C submodule/sub commit -m "add a" &&
git -C submodule submodule add ./sub &&
@@ -54,117 +54,117 @@ test_expect_success 'grep and nested submodules' '
git commit -m "updated submodule" &&
 
cat >expect <<-\EOF &&
-   a:foobar
-   b/b:bar
-   submodule/a:foobar
-   submodule/sub/a:foobar
+   a:(1|2)d(3|4)
+   b/b:(3|4)
+   submodule/a:(1|2)d(3|4)
+   submodule/sub/a:(1|2)d(3|4)
EOF
 
-   git grep -e "bar" --recurse-submodules >actual &&
+   git grep -e "(3|4)" --recurse-submodules >actual &&
test_cmp expect actual
 '
 
 test_expect_success 'grep and multiple patterns' '
cat >expect <<-\EOF &&
-   a:foobar
-   submodule/a:foobar
-   submodule/sub/a:foobar
+   a:(1|2)d(3|4)
+   submodule/a:(1|2)d(3|4)
+   submodule/sub/a:(1|2)d(3|4)
EOF
 
-   git grep -e "bar" --and -e "foo" --recurse-submodules >actual &&
+   git grep -e "(3|4)" --and -e "(1|2)" --recurse-submodules >actual &&
test_cmp expect actual
 '
 
 test_expect_success 'grep and multiple patterns' '
cat >expect <<-\EOF &&
-   b/b:bar
+   b/b:(3|4)
EOF
 
-   git grep -e "bar" --and --not -e "foo" --recurse-submodules >actual &&
+   git grep -e "(3|4)" --and --not -e "(1|2)" --recurse-submodules >actual 
&&
test_cmp expect actual
 '
 
 test_expect_success 'basic grep tree' '
cat >expect <<-\EOF &&
-   HEAD:a:foobar
-   HEAD:b/b:bar
-   HEAD:submodule/a:foobar
-   HEAD:submodule/sub/a:foobar
+   HEAD:a:(1|2)d(3|4)
+   HEAD:b/b:(3|4)
+   HEAD:submodule/a:(1|2)d(3|4)
+   HEAD:submodule/sub/a:(1|2)d(3|4)
EOF
 
-   git grep -e "bar" --recurse-submodules HEAD >actual &&
+   git grep -e "(3|4)" --recurse-submodules HEAD >actual &&
test_cmp expect actual
 '
 
 test_expect_success 'grep tree HEAD^' '
cat >expect <<-\EOF &&
-   HEAD^:a:foobar
-   HEAD^:b/b:bar
-   HEAD^:submodule/a:foobar
+   HEAD^:a:(1|2)d(3|4)
+   HEAD^:b/b:(3|4)
+   HEAD^:submodule/a:(1|2)d(3|4)
EOF
 
-   git grep -e "bar" --recurse-submodules HEAD^ >actual &&
+   git grep 

[PATCH v3 13/30] grep: prepare for testing binary regexes containing rx metacharacters

[Æ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 \0s in
patterns as well as any other character.

When kwset was imported in commit 9eceddeec6 ("Use kwset in grep",
2011-08-21) this limitation was fixed, but at the expense of
introducing the undocumented limitation that any pattern containing \0
implicitly becomes a fixed match (equivalent to -F having been
provided).

That's not something we'd like to keep in the future. The inability to
match patterns containing \0 is a leaky implementation detail.

So add tests as a first step towards changing that. In order to test
that \0-patterns can properly match as regexes the test string needs
to have some regex metacharacters in it.

There were other blind spots in the tests. The code around kwset
specially handles case-insensitive & non-ASCII data, but there were no
tests for this.

Fix all of that by amending the text being matched to contain both
regex metacharacters & non-ASCII data.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t7008-grep-binary.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
index df93d8e44c..20370d6e0c 100755
--- a/t/t7008-grep-binary.sh
+++ b/t/t7008-grep-binary.sh
@@ -28,7 +28,7 @@ nul_match () {
 }
 
 test_expect_success 'setup' "
-   echo 'binaryQfile' | q_to_nul >a &&
+   echo 'binaryQfileQm[*]cQ*æQð' | q_to_nul >a &&
git add a &&
git commit -m.
 "
@@ -162,7 +162,7 @@ test_expect_success 'grep does not honor textconv' '
 '
 
 test_expect_success 'grep --textconv honors textconv' '
-   echo "a:binaryQfile" >expect &&
+   echo "a:binaryQfileQm[*]cQ*æQð" >expect &&
git grep --textconv Qfile >actual &&
test_cmp expect actual
 '
@@ -172,7 +172,7 @@ test_expect_success 'grep --no-textconv does not honor 
textconv' '
 '
 
 test_expect_success 'grep --textconv blob honors textconv' '
-   echo "HEAD:a:binaryQfile" >expect &&
+   echo "HEAD:a:binaryQfileQm[*]cQ*æQð" >expect &&
git grep --textconv Qfile HEAD:a >actual &&
test_cmp expect actual
 '
-- 
2.13.0.303.g4ebf302169

[PATCH v3 09/30] grep: add tests for --threads=N and grep.threads

[Æ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 for it.

In developing a change to the grep code I was able to make
'--threads=1 ` segfault, while the test suite still passed. This
change fixes that blind spot in the tests.

In addition to asserting that asking for N threads shouldn't segfault,
test that the grep output given any N is the same.

The choice to test only 1..10 as opposed to 1..8 or 1..16 or whatever
is arbitrary. Testing 1..1024 works locally for me (but gets
noticeably slower as more threads are spawned). Given the structure of
the code there's no reason to test an arbitrary number of threads,
only 0, 1 and >=2 are special modes of operation.

A later patch introduces a PTHREADS test prerequisite which is true
under NO_PTHREADS=UnfortunatelyYes, but even under NO_PTHREADS it's
fine to test --threads=N, we'll just ignore it and not use
threading. So these tests also make sense under that mode to assert
that --threads=N without pthreads still returns expected results.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t7810-grep.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index daa906b9b0..561709ef6a 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -775,6 +775,22 @@ test_expect_success 'grep -W with userdiff' '
test_cmp expected actual
 '
 
+for threads in $(test_seq 0 10)
+do
+   test_expect_success "grep --threads=$threads & -c 
grep.threads=$threads" "
+   git grep --threads=$threads . >actual.$threads &&
+   if test $threads -ge 1
+   then
+   test_cmp actual.\$(($threads - 1)) actual.$threads
+   fi &&
+   git -c grep.threads=$threads grep . >actual.$threads &&
+   if test $threads -ge 1
+   then
+   test_cmp actual.\$(($threads - 1)) actual.$threads
+   fi
+   "
+done
+
 test_expect_success 'grep from a subdirectory to search wider area (1)' '
mkdir -p s &&
(
-- 
2.13.0.303.g4ebf302169

[PATCH v3 15/30] perf: add a GIT_PERF_MAKE_COMMAND for when *_MAKE_OPTS won't do

[Æ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 even be used to change
the contents of the tree, useful for monkeypatching ancient versions
of git to get them to build.

This opens Pandora's box in some ways, it's now possible to
"jailbreak" the perf environment and e.g. modify the source tree via
this arbitrary instead of just issuing a custom "make" command, such a
command has to be re-entrant in the sense that subsequent perf runs
will re-use the possibly modified tree.

It would be pointless to try to mitigate or work around that caveat in
a tool purely aimed at Git developers, so this change makes no attempt
to do so.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Makefile  |  3 +++
 t/perf/README | 19 +--
 t/perf/run| 11 +--
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index eedadb8056..d1587452f1 100644
--- a/Makefile
+++ b/Makefile
@@ -2272,6 +2272,9 @@ endif
 ifdef GIT_PERF_MAKE_OPTS
@echo GIT_PERF_MAKE_OPTS=\''$(subst ','\'',$(subst 
','\'',$(GIT_PERF_MAKE_OPTS)))'\' >>$@+
 endif
+ifdef GIT_PERF_MAKE_COMMAND
+   @echo GIT_PERF_MAKE_COMMAND=\''$(subst ','\'',$(subst 
','\'',$(GIT_PERF_MAKE_COMMAND)))'\' >>$@+
+endif
 ifdef GIT_INTEROP_MAKE_OPTS
@echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst 
','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+
 endif
diff --git a/t/perf/README b/t/perf/README
index 49ea4349be..b3d95042a8 100644
--- a/t/perf/README
+++ b/t/perf/README
@@ -60,8 +60,23 @@ You can set the following variables (also in your 
config.mak):
 
 GIT_PERF_MAKE_OPTS
Options to use when automatically building a git tree for
-   performance testing.  E.g., -j6 would be useful.
-
+   performance testing. E.g., -j6 would be useful. Passed
+   directly to make as "make $GIT_PERF_MAKE_OPTS".
+
+GIT_PERF_MAKE_COMMAND
+   An arbitrary command that'll be run in place of the make
+   command, if set the GIT_PERF_MAKE_OPTS variable is
+   ignored. Useful in cases where source tree changes might
+   require issuing a different make command to different
+   revisions.
+
+   This can be (ab)used to monkeypatch or otherwise change the
+   tree about to be built. Note that the build directory can be
+   re-used for subsequent runs so the make command might get
+   executed multiple times on the same tree, but don't count on
+   any of that, that's an implementation detail that might change
+   in the future.
+ 
 GIT_PERF_REPO
 GIT_PERF_LARGE_REPO
Repositories to copy for the performance tests.  The normal
diff --git a/t/perf/run b/t/perf/run
index c788d713ae..b61024a830 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -37,8 +37,15 @@ build_git_rev () {
cp "../../$config" "build/$rev/"
fi
done
-   (cd build/$rev && make $GIT_PERF_MAKE_OPTS) ||
-   die "failed to build revision '$mydir'"
+   (
+   cd build/$rev &&
+   if test -n "$GIT_PERF_MAKE_COMMAND"
+   then
+   sh -c "$GIT_PERF_MAKE_COMMAND"
+   else
+   make $GIT_PERF_MAKE_OPTS
+   fi
+   ) || die "failed to build revision '$mydir'"
 }
 
 run_dirs_helper () {
-- 
2.13.0.303.g4ebf302169

[PATCH v3 18/30] perf: add a comparison test of grep regex engines with -F

[Æ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.

$ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux ./run 
p7821-grep-engines-fixed.sh
[...]
Test this tree

7821.1: fixed grep int   0.61(1.72+0.65)
7821.2: basic grep int   0.69(1.72+0.53)
7821.3: extended grep int0.60(1.72+0.54)
7821.4: perl grep int0.65(1.65+0.64)
7821.6: fixed grep uncommon  0.25(0.53+0.48)
7821.7: basic grep uncommon  0.26(0.57+0.46)
7821.8: extended grep uncommon   0.25(0.52+0.51)
7821.9: perl grep uncommon   0.26(0.56+0.48)
7821.11: fixed grep æ0.40(1.26+0.44)
7821.12: basic grep æ0.40(1.28+0.43)
7821.13: extended grep æ 0.39(1.28+0.44)
7821.14: perl grep æ 0.39(1.29+0.44)

This test needs to be run with GIT_PERF_7821_GREP_OPTS=' -i' to avoid
going through the same kwset.[ch] codepath, see the "Even when -F..."
comment in grep.c:

$ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux 
GIT_PERF_7821_GREP_OPTS=' -i' ./run p7821-grep-engines-fixed.sh
[...]
Testthis tree
---
7821.1: fixed grep -i int   1.55(1.86+0.66)
7821.2: basic grep -i int   0.66(1.97+0.54)
7821.3: extended grep -i int0.72(1.88+0.62)
7821.4: perl grep -i int0.75(1.93+0.57)
7821.6: fixed grep -i uncommon  0.27(0.52+0.54)
7821.7: basic grep -i uncommon  0.25(0.58+0.44)
7821.8: extended grep -i uncommon   0.26(0.62+0.43)
7821.9: perl grep -i uncommon   0.26(0.55+0.53)
7821.11: fixed grep -i æ0.32(0.87+0.46)
7821.12: basic grep -i æ0.30(0.90+0.41)
7821.13: extended grep -i æ 0.32(0.92+0.41)
7821.14: perl grep -i æ 0.29(0.71+0.53)

I'm planning to make that not be the case, this performance test gives
a baseline for comparing performance before & after any such change.

See commit ("perf: add a comparison test of grep regex engines",
2017-04-19) for details on the machine the above test run was executed
on.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/perf/p7821-grep-engines-fixed.sh | 32 
 1 file changed, 32 insertions(+)
 create mode 100755 t/perf/p7821-grep-engines-fixed.sh

diff --git a/t/perf/p7821-grep-engines-fixed.sh 
b/t/perf/p7821-grep-engines-fixed.sh
new file mode 100755
index 00..d935194ecf
--- /dev/null
+++ b/t/perf/p7821-grep-engines-fixed.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+test_description="Comparison of git-grep's regex engines with -F
+
+Set GIT_PERF_7821_GREP_OPTS in the environment to pass options to
+git-grep. Make sure to include a leading space,
+e.g. GIT_PERF_7821_GREP_OPTS=' -w'. See p7820-grep-engines.sh for more
+options to try.
+"
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+for args in 'int' 'uncommon' 'æ'
+do
+   for engine in fixed basic extended perl
+   do
+   test_perf "$engine grep$GIT_PERF_7821_GREP_OPTS $args" "
+   git -c grep.patternType=$engine 
grep$GIT_PERF_7821_GREP_OPTS $args >'out.$engine.$args' || :
+   "
+   done
+
+   test_expect_success "assert that all engines found the same 
for$GIT_PERF_7821_GREP_OPTS $args" "
+   test_cmp 'out.fixed.$args' 'out.basic.$args' &&
+   test_cmp 'out.fixed.$args' 'out.extended.$args' &&
+   test_cmp 'out.fixed.$args' 'out.perl.$args'
+   "
+done
+
+test_done
-- 
2.13.0.303.g4ebf302169

[PATCH v3 16/30] perf: emit progress output when unpacking & building

[Æ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 being
tested as the output scrolls by.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/perf/run | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/perf/run b/t/perf/run
index b61024a830..beb4acc0e4 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -24,6 +24,7 @@ run_one_dir () {
 
 unpack_git_rev () {
rev=$1
+   echo "=== Unpacking $rev in build/$rev ==="
mkdir -p build/$rev
(cd "$(git rev-parse --show-cdup)" && git archive --format=tar $rev) |
(cd build/$rev && tar x)
@@ -37,6 +38,7 @@ build_git_rev () {
cp "../../$config" "build/$rev/"
fi
done
+   echo "=== Building $rev ==="
(
cd build/$rev &&
if test -n "$GIT_PERF_MAKE_COMMAND"
-- 
2.13.0.303.g4ebf302169

[PATCH v3 11/30] grep: add tests for grep pattern types being passed to submodules

[Æ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.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t7814-grep-recurse-submodules.sh | 49 ++
 1 file changed, 49 insertions(+)

diff --git a/t/t7814-grep-recurse-submodules.sh 
b/t/t7814-grep-recurse-submodules.sh
index 1472855e1d..3a58197f47 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -313,4 +313,53 @@ test_incompatible_with_recurse_submodules ()
 test_incompatible_with_recurse_submodules --untracked
 test_incompatible_with_recurse_submodules --no-index
 
+test_expect_success 'grep --recurse-submodules should pass the pattern type 
along' '
+   # Fixed
+   test_must_fail git grep -F --recurse-submodules -e "(.|.)[\d]" &&
+   test_must_fail git -c grep.patternType=fixed grep --recurse-submodules 
-e "(.|.)[\d]" &&
+
+   # Basic
+   git grep -G --recurse-submodules -e "(.|.)[\d]" >actual &&
+   cat >expect <<-\EOF &&
+   a:(1|2)d(3|4)
+   submodule/a:(1|2)d(3|4)
+   submodule/sub/a:(1|2)d(3|4)
+   EOF
+   test_cmp expect actual &&
+   git -c grep.patternType=basic grep --recurse-submodules -e "(.|.)[\d]" 
>actual &&
+   test_cmp expect actual &&
+
+   # Extended
+   git grep -E --recurse-submodules -e "(.|.)[\d]" >actual &&
+   cat >expect <<-\EOF &&
+   .gitmodules:[submodule "submodule"]
+   .gitmodules:path = submodule
+   .gitmodules:url = ./submodule
+   a:(1|2)d(3|4)
+   submodule/.gitmodules:[submodule "sub"]
+   submodule/a:(1|2)d(3|4)
+   submodule/sub/a:(1|2)d(3|4)
+   EOF
+   test_cmp expect actual &&
+   git -c grep.patternType=extended grep --recurse-submodules -e 
"(.|.)[\d]" >actual &&
+   test_cmp expect actual &&
+   git -c grep.extendedRegexp=true grep --recurse-submodules -e 
"(.|.)[\d]" >actual &&
+   test_cmp expect actual &&
+
+   # Perl
+   if test_have_prereq PCRE
+   then
+   git grep -P --recurse-submodules -e "(.|.)[\d]" >actual &&
+   cat >expect <<-\EOF &&
+   a:(1|2)d(3|4)
+   b/b:(3|4)
+   submodule/a:(1|2)d(3|4)
+   submodule/sub/a:(1|2)d(3|4)
+   EOF
+   test_cmp expect actual &&
+   git -c grep.patternType=perl grep --recurse-submodules -e 
"(.|.)[\d]" >actual &&
+   test_cmp expect actual
+   fi
+'
+
 test_done
-- 
2.13.0.303.g4ebf302169

[PATCH v3 23/30] grep: change the internal PCRE macro names to be PCRE1

[Æ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 more places than we absolutely
need to for backwards compatibility with old Makefile arguments would
be confusing.

In some ways it would be better to change everything that now uses
USE_LIBPCRE to use USE_LIBPCRE1, and to make specifying
USE_LIBPCRE (or --with-pcre) an error. This would impose a one-time
burden on packagers of git to s/USE_LIBPCRE/USE_LIBPCRE1/ in their
build scripts.

However I'd like to leave the door open to making
USE_LIBPCRE=YesPlease eventually mean USE_LIBPCRE2=YesPlease,
i.e. once PCRE v2 is ubiquitous enough that it makes sense to make it
the default.

This code and the USE_LIBPCRE Makefile argument was added in commit
63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09). At the time there was
no indication that the PCRE project would release an entirely new &
incompatible API around 3 years later.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Makefile  | 4 ++--
 grep.c| 6 +++---
 grep.h| 2 +-
 t/test-lib.sh | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index d1587452f1..374fbc7e58 100644
--- a/Makefile
+++ b/Makefile
@@ -1088,7 +1088,7 @@ ifdef NO_LIBGEN_H
 endif
 
 ifdef USE_LIBPCRE
-   BASIC_CFLAGS += -DUSE_LIBPCRE
+   BASIC_CFLAGS += -DUSE_LIBPCRE1
ifdef LIBPCREDIR
BASIC_CFLAGS += -I$(LIBPCREDIR)/include
EXTLIBS += -L$(LIBPCREDIR)/$(lib) 
$(CC_LD_DYNPATH)$(LIBPCREDIR)/$(lib)
@@ -2240,7 +2240,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@+
@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@+
@echo NO_EXPAT=\''$(subst ','\'',$(subst ','\'',$(NO_EXPAT)))'\' >>$@+
-   @echo USE_LIBPCRE=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' 
>>$@+
+   @echo USE_LIBPCRE1=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' 
>>$@+
@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@+
@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst 
','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
diff --git a/grep.c b/grep.c
index 79eb681c6e..854f2404be 100644
--- a/grep.c
+++ b/grep.c
@@ -333,7 +333,7 @@ static int has_null(const char *s, size_t len)
return 0;
 }
 
-#ifdef USE_LIBPCRE
+#ifdef USE_LIBPCRE1
 static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)
 {
const char *error;
@@ -385,7 +385,7 @@ static void free_pcre_regexp(struct grep_pat *p)
pcre_free(p->pcre_extra_info);
pcre_free((void *)p->pcre_tables);
 }
-#else /* !USE_LIBPCRE */
+#else /* !USE_LIBPCRE1 */
 static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)
 {
die("cannot use Perl-compatible regexes when not compiled with 
USE_LIBPCRE");
@@ -400,7 +400,7 @@ static int pcrematch(struct grep_pat *p, const char *line, 
const char *eol,
 static void free_pcre_regexp(struct grep_pat *p)
 {
 }
-#endif /* !USE_LIBPCRE */
+#endif /* !USE_LIBPCRE1 */
 
 static int is_fixed(const char *s, size_t len)
 {
diff --git a/grep.h b/grep.h
index 267534ca24..073b0e4c92 100644
--- a/grep.h
+++ b/grep.h
@@ -1,7 +1,7 @@
 #ifndef GREP_H
 #define GREP_H
 #include "color.h"
-#ifdef USE_LIBPCRE
+#ifdef USE_LIBPCRE1
 #include 
 #else
 typedef int pcre;
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 04d857a42b..1d0f636cbd 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1014,7 +1014,7 @@ esac
 ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
 test -z "$NO_PERL" && test_set_prereq PERL
 test -z "$NO_PYTHON" && test_set_prereq PYTHON
-test -n "$USE_LIBPCRE" && test_set_prereq PCRE
+test -n "$USE_LIBPCRE1" && test_set_prereq PCRE
 test -z "$NO_GETTEXT" && test_set_prereq GETTEXT
 
 # Can we rely on git's output in the C locale?
-- 
2.13.0.303.g4ebf302169

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

[Æ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
pack.threads, and have long warned about the same under
NO_PTHREADS=YesPlease.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 builtin/grep.c  | 13 +
 t/t7810-grep.sh | 18 ++
 2 files changed, 31 insertions(+)

diff --git a/builtin/grep.c b/builtin/grep.c
index a191e2976b..3c721b75a5 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -289,6 +289,17 @@ static int grep_cmd_config(const char *var, const char 
*value, void *cb)
if (num_threads < 0)
die(_("invalid number of threads specified (%d) for 
%s"),
num_threads, var);
+#ifdef NO_PTHREADS
+   else if (num_threads && num_threads != 1) {
+   /*
+* TRANSLATORS: %s is the configuration
+* variable for tweaking threads, currently
+* grep.threads
+*/
+   warning(_("no threads support, ignoring %s"), var);
+   num_threads = 0;
+   }
+#endif
}
 
return st;
@@ -1229,6 +1240,8 @@ int cmd_grep(int argc, const char **argv, const char 
*prefix)
else if (num_threads < 0)
die(_("invalid number of threads specified (%d)"), num_threads);
 #else
+   if (num_threads)
+   warning(_("no threads support, ignoring --threads"));
num_threads = 0;
 #endif
 
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 561709ef6a..f106387820 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -791,6 +791,24 @@ do
"
 done
 
+test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'grep --threads=N or 
pack.threads=N warns when no pthreads' '
+   git grep --threads=2 Hello hello_world 2>err &&
+   grep ^warning: err >warnings &&
+   test_line_count = 1 warnings &&
+   grep -F "no threads support, ignoring --threads" err &&
+   git -c grep.threads=2 grep Hello hello_world 2>err &&
+   grep ^warning: err >warnings &&
+   test_line_count = 1 warnings &&
+   grep -F "no threads support, ignoring grep.threads" err &&
+   git -c grep.threads=2 grep --threads=4 Hello hello_world 2>err &&
+   grep ^warning: err >warnings &&
+   test_line_count = 2 warnings &&
+   grep -F "no threads support, ignoring --threads" err &&
+   grep -F "no threads support, ignoring grep.threads" err &&
+   git -c grep.threads=0 grep --threads=0 Hello hello_world 2>err &&
+   test_line_count = 0 err
+'
+
 test_expect_success 'grep from a subdirectory to search wider area (1)' '
mkdir -p s &&
(
-- 
2.13.0.303.g4ebf302169

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

[Æ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 threading in commit 5b594f457a ("Threaded grep",
2010-01-25), but ever since then they'd only be called if num_threads
was true, so this check made the code confusing to read.

Replace the check with an assertion, so that it's clear to the reader
that this code path is never taken unless we're spawning threads.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 builtin/grep.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 3c721b75a5..b1095362fb 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -73,14 +73,14 @@ static pthread_mutex_t grep_mutex;
 
 static inline void grep_lock(void)
 {
-   if (num_threads)
-   pthread_mutex_lock(_mutex);
+   assert(num_threads);
+   pthread_mutex_lock(_mutex);
 }
 
 static inline void grep_unlock(void)
 {
-   if (num_threads)
-   pthread_mutex_unlock(_mutex);
+   assert(num_threads);
+   pthread_mutex_unlock(_mutex);
 }
 
 /* Signalled when a new work_item is added to todo. */
-- 
2.13.0.303.g4ebf302169

[PATCH v3 22/30] grep: factor test for \0 in grep patterns into a function

[Æ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 change makes later changes that make use of either has_null() or
is_fixed() (but not both) smaller.

While I'm at it make the comment conform to the style guide, i.e. add
an opening "/*\n".

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 grep.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/grep.c b/grep.c
index bf6c2494fd..79eb681c6e 100644
--- a/grep.c
+++ b/grep.c
@@ -321,6 +321,18 @@ static NORETURN void compile_regexp_failed(const struct 
grep_pat *p,
die("%s'%s': %s", where, p->pattern, error);
 }
 
+static int has_null(const char *s, size_t len)
+{
+   /*
+* regcomp cannot accept patterns with NULs so when using it
+* we consider any pattern containing a NUL fixed.
+*/
+   if (memchr(s, 0, len))
+   return 1;
+
+   return 0;
+}
+
 #ifdef USE_LIBPCRE
 static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)
 {
@@ -394,12 +406,6 @@ static int is_fixed(const char *s, size_t len)
 {
size_t i;
 
-   /* regcomp cannot accept patterns with NULs so we
-* consider any pattern containing a NUL fixed.
-*/
-   if (memchr(s, 0, len))
-   return 1;
-
for (i = 0; i < len; i++) {
if (is_regex_special(s[i]))
return 0;
@@ -451,7 +457,7 @@ static void compile_regexp(struct grep_pat *p, struct 
grep_opt *opt)
 * simple string match using kws.  p->fixed tells us if we
 * want to use kws.
 */
-   if (opt->fixed || is_fixed(p->pattern, p->patternlen))
+   if (opt->fixed || has_null(p->pattern, p->patternlen) || 
is_fixed(p->pattern, p->patternlen))
p->fixed = !icase || ascii_only;
else
p->fixed = 0;
-- 
2.13.0.303.g4ebf302169

[PATCH v3 26/30] test-lib: add a PTHREADS prerequisite

[Æ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  | 1 +
 t/README  | 4 
 t/test-lib.sh | 1 +
 3 files changed, 6 insertions(+)

diff --git a/Makefile b/Makefile
index 374fbc7e58..a79274e5e6 100644
--- a/Makefile
+++ b/Makefile
@@ -2242,6 +2242,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_EXPAT=\''$(subst ','\'',$(subst ','\'',$(NO_EXPAT)))'\' >>$@+
@echo USE_LIBPCRE1=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' 
>>$@+
@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@+
+   @echo NO_PTHREADS=\''$(subst ','\'',$(subst ','\'',$(NO_PTHREADS)))'\' 
>>$@+
@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst 
','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
@echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+
diff --git a/t/README b/t/README
index a90cb62583..2f95860369 100644
--- a/t/README
+++ b/t/README
@@ -817,6 +817,10 @@ use these, and "test_set_prereq" for how to define your 
own.
Test is run on a filesystem which converts decomposed utf-8 (nfd)
to precomposed utf-8 (nfc).
 
+ - PTHREADS
+
+   Git wasn't compiled with NO_PTHREADS=YesPlease.
+
 Tips for Writing Tests
 --
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1d0f636cbd..43529451f9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1013,6 +1013,7 @@ esac
 
 ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
 test -z "$NO_PERL" && test_set_prereq PERL
+test -z "$NO_PTHREADS" && test_set_prereq PTHREADS
 test -z "$NO_PYTHON" && test_set_prereq PYTHON
 test -n "$USE_LIBPCRE1" && test_set_prereq PCRE
 test -z "$NO_GETTEXT" && test_set_prereq GETTEXT
-- 
2.13.0.303.g4ebf302169

[PATCH v3 21/30] grep: remove redundant regflags assignments

[Ævar Arnfjörð Bjarmason]

Remove redundant assignments to the "regflags" variable. This variable
is only used set under GREP_PATTERN_TYPE_ERE, so there's no need to
un-set it under GREP_PATTERN_TYPE_{FIXED,BRE,PCRE}.

Back in 5010cb5fcc[1], we did do "opt.regflags &= ~REG_EXTENDED" upon
seeing "-G" on the command line and flipped the bit on upon seeing
"-E", but I think that was perfectly sensible and it would have been a
bug if we didn't.  They were part of the command line parsing that
could have seen "-E" on the command line earlier.

When cca2c172 ("git-grep: do not die upon -F/-P when
grep.extendedRegexp is set.", 2011-05-09) switched the command line
parsing to "read into a 'tentatively this is what we saw the last'
variable and then finally commit just once", we didn't touch
opt.regflags for PCRE and FIXED, but we still had to flip regflags
between BRE and ERE, because parsing of grep.extendedregexp
configuration variable directly touched opt.regflags back then, which
was done by b22520a3 ("grep: allow -E and -n to be turned on by
default via configuration", 2011-03-30).

When 84befcd0 ("grep: add a grep.patternType configuration setting",
2012-08-03) introduced extended_regexp_option field, we stopped
flipping regflags while reading the configuration, and that was when
we should have noticed and stopped dropping REG_EXTENDED bit in the
"now we can commit what type to use" helper function.

There is no reason to do this anymore, so stop doing it, more to
reduce "wait this is used under fixed/BRE/PCRE how?" confusion when
reading the code, than to to save ourselves trivial CPU cycles by
removing one assignment.

1. "built-in "git grep"", 2006-04-30.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 grep.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/grep.c b/grep.c
index 47cee45067..bf6c2494fd 100644
--- a/grep.c
+++ b/grep.c
@@ -179,7 +179,6 @@ static void grep_set_pattern_type_option(enum 
grep_pattern_type pattern_type, st
case GREP_PATTERN_TYPE_BRE:
opt->fixed = 0;
opt->pcre = 0;
-   opt->regflags &= ~REG_EXTENDED;
break;
 
case GREP_PATTERN_TYPE_ERE:
@@ -191,13 +190,11 @@ static void grep_set_pattern_type_option(enum 
grep_pattern_type pattern_type, st
case GREP_PATTERN_TYPE_FIXED:
opt->fixed = 1;
opt->pcre = 0;
-   opt->regflags &= ~REG_EXTENDED;
break;
 
case GREP_PATTERN_TYPE_PCRE:
opt->fixed = 0;
opt->pcre = 1;
-   opt->regflags &= ~REG_EXTENDED;
break;
}
 }
@@ -415,10 +412,9 @@ static void compile_fixed_regexp(struct grep_pat *p, 
struct grep_opt *opt)
 {
struct strbuf sb = STRBUF_INIT;
int err;
-   int regflags;
+   int regflags = opt->regflags;
 
basic_regex_quote_buf(, p->pattern);
-   regflags = opt->regflags & ~REG_EXTENDED;
if (opt->ignore_case)
regflags |= REG_ICASE;
err = regcomp(>regexp, sb.buf, regflags);
-- 
2.13.0.303.g4ebf302169

[PATCH v3 20/30] grep: catch a missing enum in switch statement

[Æ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 compile_submodule_options() won't work,
the value will just get coerced. The -Wswitch-default warning will
warn about it, but that produces a lot of noise across the codebase,
this potential issue would be drowned in that noise.

Thus catching this at runtime is the least bad option. This won't ever
trigger in practice, but if a new pattern type were to be added this
catches an otherwise silent bug during development.

See commit 0281e487fd ("grep: optionally recurse into submodules",
2016-12-16) for the initial addition of this code.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 builtin/grep.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/grep.c b/builtin/grep.c
index 3ffb5b4e81..a191e2976b 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -495,6 +495,8 @@ static void compile_submodule_options(const struct grep_opt 
*opt,
break;
case GREP_PATTERN_TYPE_UNSPECIFIED:
break;
+   default:
+   die("BUG: Added a new grep pattern type without updating switch 
statement");
}
 
for (pattern = opt->pattern_list; pattern != NULL;
-- 
2.13.0.303.g4ebf302169

[PATCH v3 17/30] perf: add a comparison test of grep regex engines

[Æ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 some
slight hoops to test both, which is worthwhile since "basic" is the
default.

Running this on an i7 3.4GHz Linux 4.9.0-2 Debian testing against a
checkout of linux.git & latest upstream PCRE, both PCRE and git
compiled with -O3 using gcc 7.1.1:

$ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux ./run 
p7820-grep-engines.sh
[...]
Testthis tree
---
7820.1: basic grep 'how.to' 0.34(1.24+0.53)
7820.2: extended grep 'how.to'  0.33(1.23+0.45)
7820.3: perl grep 'how.to'  0.31(1.05+0.56)
7820.5: basic grep '^how to'0.32(1.24+0.42)
7820.6: extended grep '^how to' 0.33(1.20+0.44)
7820.7: perl grep '^how to' 0.57(2.67+0.42)
7820.9: basic grep '[how] to'   0.51(2.16+0.45)
7820.10: extended grep '[how] to'   0.49(2.20+0.43)
7820.11: perl grep '[how] to'   0.56(2.60+0.43)
7820.13: basic grep '\(e.t[^ ]*\|v.ry\) rare'   0.66(3.25+0.40)
7820.14: extended grep '(e.t[^ ]*|v.ry) rare'   0.65(3.19+0.46)
7820.15: perl grep '(e.t[^ ]*|v.ry) rare'   1.05(5.74+0.34)
7820.17: basic grep 'm\(ú\|u\)lt.b\(æ\|y\)te'   0.34(1.28+0.47)
7820.18: extended grep 'm(ú|u)lt.b(æ|y)te'  0.34(1.38+0.38)
7820.19: perl grep 'm(ú|u)lt.b(æ|y)te'  0.39(1.56+0.44)

Options can also be passed to git-grep via the GIT_PERF_7820_GREP_OPTS
environment variable. There are various modes such as "-v" that have
very different performance profiles, but handling the combinatorial
explosion of testing all those options would make this script much
more complex and harder to maintain. Instead just add the ability to
do one-shot runs with arbitrary options, e.g.:

$ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux 
GIT_PERF_7820_GREP_OPTS=" -i" ./run p7820-grep-engines.sh
[...]
Test   this tree
--
7820.1: basic grep -i 'how.to' 0.49(1.72+0.38)
7820.2: extended grep -i 'how.to'  0.46(1.64+0.42)
7820.3: perl grep -i 'how.to'  0.44(1.45+0.45)
7820.5: basic grep -i '^how to'0.47(1.76+0.38)
7820.6: extended grep -i '^how to' 0.47(1.70+0.42)
7820.7: perl grep -i '^how to' 0.65(2.72+0.37)
7820.9: basic grep -i '[how] to'   0.86(3.64+0.42)
7820.10: extended grep -i '[how] to'   0.84(3.62+0.46)
7820.11: perl grep -i '[how] to'   0.73(3.06+0.39)
7820.13: basic grep -i '\(e.t[^ ]*\|v.ry\) rare'   1.63(8.13+0.36)
7820.14: extended grep -i '(e.t[^ ]*|v.ry) rare'   1.64(8.01+0.44)
7820.15: perl grep -i '(e.t[^ ]*|v.ry) rare'   1.44(6.88+0.44)
7820.17: basic grep -i 'm\(ú\|u\)lt.b\(æ\|y\)te'   0.66(2.67+0.44)
7820.18: extended grep -i 'm(ú|u)lt.b(æ|y)te'  0.66(2.67+0.43)
7820.19: perl grep -i 'm(ú|u)lt.b(æ|y)te'  0.59(2.31+0.37)

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/perf/p7820-grep-engines.sh | 47 
 1 file changed, 47 insertions(+)
 create mode 100755 t/perf/p7820-grep-engines.sh

diff --git a/t/perf/p7820-grep-engines.sh b/t/perf/p7820-grep-engines.sh
new file mode 100755
index 00..a3a1b9fa28
--- /dev/null
+++ b/t/perf/p7820-grep-engines.sh
@@ -0,0 +1,47 @@
+#!/bin/sh
+
+test_description="Comparison of git-grep's regex engines
+
+Set GIT_PERF_7820_GREP_OPTS in the environment to pass options to
+git-grep. Make sure to include a leading space,
+e.g. GIT_PERF_7820_GREP_OPTS=' -i'. Some options to try:
+
+   -i
+   -w
+   -v
+   -vi
+   -vw
+   -viw
+"
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+for pattern in \
+   'how.to' \
+   '^how to' \
+   '[how] to' \
+   '\(e.t[^ ]*\|v.ry\) rare' \
+   'm\(ú\|u\)lt.b\(æ\|y\)te'
+do
+   for engine in basic extended perl
+   do
+   if test $engine != "basic"
+   then
+   # Poor man's basic -> extended converter.
+   pattern=$(echo "$pattern" | sed 's/\\//g')
+   fi
+   test_perf "$engine grep$GIT_PERF_7820_GREP_OPTS '$pattern'" "
+   git -c grep.patternType=$engine 
grep$GIT_PERF_7820_GREP_OPTS -- '$pattern' >'out.$engine' || :
+   "
+   done
+
+   

[PATCH v3 19/30] perf: add a comparison test of log --grep regex engines

[Ævar Arnfjörð Bjarmason]

Add a very basic performance comparison test comparing the POSIX
basic, extended and perl engines with patterns matching log messages
via --grep=.

$ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux ./run 
p4220-log-grep-engines.sh
[...]
Test  this tree
-
4220.1: basic log --grep='how.to' 6.22(6.00+0.21)
4220.2: extended log --grep='how.to'  6.23(5.98+0.23)
4220.3: perl log --grep='how.to'  6.07(5.79+0.25)
4220.5: basic log --grep='^how to'6.19(5.93+0.22)
4220.6: extended log --grep='^how to' 6.19(5.93+0.23)
4220.7: perl log --grep='^how to' 6.14(5.88+0.24)
4220.9: basic log --grep='[how] to'   6.96(6.65+0.28)
4220.10: extended log --grep='[how] to'   6.96(6.69+0.24)
4220.11: perl log --grep='[how] to'   6.95(6.58+0.33)
4220.13: basic log --grep='\(e.t[^ ]*\|v.ry\) rare'   7.10(6.80+0.27)
4220.14: extended log --grep='(e.t[^ ]*|v.ry) rare'   7.07(6.80+0.26)
4220.15: perl log --grep='(e.t[^ ]*|v.ry) rare'   7.70(7.46+0.22)
4220.17: basic log --grep='m\(ú\|u\)lt.b\(æ\|y\)te'   6.12(5.87+0.24)
4220.18: extended log --grep='m(ú|u)lt.b(æ|y)te'  6.14(5.84+0.26)
4220.19: perl log --grep='m(ú|u)lt.b(æ|y)te'  6.16(5.93+0.20)

With -i:

$ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux 
GIT_PERF_4220_LOG_OPTS=' -i' ./run p4220-log-grep-engines.sh
[...]
Test this tree

4220.1: basic log -i --grep='how.to' 6.74(6.41+0.32)
4220.2: extended log -i --grep='how.to'  6.78(6.55+0.22)
4220.3: perl log -i --grep='how.to'  6.06(5.77+0.28)
4220.5: basic log -i --grep='^how to'6.80(6.57+0.22)
4220.6: extended log -i --grep='^how to' 6.83(6.52+0.29)
4220.7: perl log -i --grep='^how to' 6.16(5.94+0.20)
4220.9: basic log -i --grep='[how] to'   7.87(7.61+0.24)
4220.10: extended log -i --grep='[how] to'   7.85(7.57+0.27)
4220.11: perl log -i --grep='[how] to'   7.03(6.75+0.25)
4220.13: basic log -i --grep='\(e.t[^ ]*\|v.ry\) rare'   8.68(8.41+0.25)
4220.14: extended log -i --grep='(e.t[^ ]*|v.ry) rare'   8.80(8.44+0.28)
4220.15: perl log -i --grep='(e.t[^ ]*|v.ry) rare'   7.85(7.56+0.26)
4220.17: basic log -i --grep='m\(ú\|u\)lt.b\(æ\|y\)te'   6.94(6.68+0.24)
4220.18: extended log -i --grep='m(ú|u)lt.b(æ|y)te'  7.04(6.76+0.24)
4220.19: perl log -i --grep='m(ú|u)lt.b(æ|y)te'  6.26(5.92+0.29)

See commit ("perf: add a comparison test of grep regex engines",
2017-04-19) for details on the machine the above test run was executed
on.

Before commit ("log: make --regexp-ignore-case work with
--perl-regexp", 2017-05-20) this test will almost definitely
fail (depending on the repo) if passed the -i option, since it wasn't
properly supported under PCRE.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/perf/p4220-log-grep-engines.sh | 44 
 1 file changed, 44 insertions(+)
 create mode 100755 t/perf/p4220-log-grep-engines.sh

diff --git a/t/perf/p4220-log-grep-engines.sh b/t/perf/p4220-log-grep-engines.sh
new file mode 100755
index 00..02793ac77b
--- /dev/null
+++ b/t/perf/p4220-log-grep-engines.sh
@@ -0,0 +1,44 @@
+#!/bin/sh
+
+test_description="Comparison of git-log's --grep regex engines
+
+Set GIT_PERF_4220_LOG_OPTS in the environment to pass options to
+git-grep. Make sure to include a leading space,
+e.g. GIT_PERF_4220_LOG_OPTS=' -i'. Some options to try:
+
+   -i
+   --invert-grep
+   -i --invert-grep
+"
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+for pattern in \
+   'how.to' \
+   '^how to' \
+   '[how] to' \
+   '\(e.t[^ ]*\|v.ry\) rare' \
+   'm\(ú\|u\)lt.b\(æ\|y\)te'
+do
+   for engine in basic extended perl
+   do
+   if test $engine != "basic"
+   then
+   # Poor man's basic -> extended converter.
+   pattern=$(echo $pattern | sed 's/\\//g')
+   fi
+   test_perf "$engine log$GIT_PERF_4220_LOG_OPTS 
--grep='$pattern'" "
+   git -c grep.patternType=$engine log 
--pretty=format:%h$GIT_PERF_4220_LOG_OPTS --grep='$pattern' >'out.$engine' || :
+   "
+   done
+
+   test_expect_success "assert that all engines found the same 
for$GIT_PERF_4220_LOG_OPTS '$pattern'" "
+   test_cmp 'out.basic' 'out.extended' &&
+   test_cmp 

[PATCH v3 28/30] pack-objects: fix buggy warning about threads

[Æ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.

Solve this bug by resetting the delta_search_threads variable in
git_pack_config(), it might then be set by --threads again and be
subsequently warned about, as the test I'm changing here asserts.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 builtin/pack-objects.c | 4 +++-
 t/t5300-pack-object.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 9b4ba8a80d..efa21a15dd 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2472,8 +2472,10 @@ static int git_pack_config(const char *k, const char *v, 
void *cb)
die("invalid number of threads specified (%d)",
delta_search_threads);
 #ifdef NO_PTHREADS
-   if (delta_search_threads != 1)
+   if (delta_search_threads != 1) {
warning("no threads support, ignoring %s", k);
+   delta_search_threads = 0;
+   }
 #endif
return 0;
}
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 6ed23ee1d2..9c68b99251 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -447,7 +447,7 @@ test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'pack-objects 
--threads=N or pack.
 
git -c pack.threads=2 pack-objects --stdout --all /dev/null 
2>err &&
grep ^warning: err >warnings &&
-   test_must_fail test_line_count = 1 warnings &&
+   test_line_count = 1 warnings &&
grep -F "no threads support, ignoring pack.threads" err &&
 
git -c pack.threads=2 pack-objects --threads=4 --stdout --all 
/dev/null 2>err &&
-- 
2.13.0.303.g4ebf302169

[PATCH v3 24/30] grep: change internal *pcre* variable & function names to be *pcre1*

[Æ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 motivations
behind this change.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 grep.c | 52 ++--
 grep.h |  8 
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/grep.c b/grep.c
index 854f2404be..07512346b1 100644
--- a/grep.c
+++ b/grep.c
@@ -178,23 +178,23 @@ static void grep_set_pattern_type_option(enum 
grep_pattern_type pattern_type, st
 
case GREP_PATTERN_TYPE_BRE:
opt->fixed = 0;
-   opt->pcre = 0;
+   opt->pcre1 = 0;
break;
 
case GREP_PATTERN_TYPE_ERE:
opt->fixed = 0;
-   opt->pcre = 0;
+   opt->pcre1 = 0;
opt->regflags |= REG_EXTENDED;
break;
 
case GREP_PATTERN_TYPE_FIXED:
opt->fixed = 1;
-   opt->pcre = 0;
+   opt->pcre1 = 0;
break;
 
case GREP_PATTERN_TYPE_PCRE:
opt->fixed = 0;
-   opt->pcre = 1;
+   opt->pcre1 = 1;
break;
}
 }
@@ -334,7 +334,7 @@ static int has_null(const char *s, size_t len)
 }
 
 #ifdef USE_LIBPCRE1
-static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)
+static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt 
*opt)
 {
const char *error;
int erroffset;
@@ -342,23 +342,23 @@ static void compile_pcre_regexp(struct grep_pat *p, const 
struct grep_opt *opt)
 
if (opt->ignore_case) {
if (has_non_ascii(p->pattern))
-   p->pcre_tables = pcre_maketables();
+   p->pcre1_tables = pcre_maketables();
options |= PCRE_CASELESS;
}
if (is_utf8_locale() && has_non_ascii(p->pattern))
options |= PCRE_UTF8;
 
-   p->pcre_regexp = pcre_compile(p->pattern, options, , ,
- p->pcre_tables);
-   if (!p->pcre_regexp)
+   p->pcre1_regexp = pcre_compile(p->pattern, options, , ,
+ p->pcre1_tables);
+   if (!p->pcre1_regexp)
compile_regexp_failed(p, error);
 
-   p->pcre_extra_info = pcre_study(p->pcre_regexp, 0, );
-   if (!p->pcre_extra_info && error)
+   p->pcre1_extra_info = pcre_study(p->pcre1_regexp, 0, );
+   if (!p->pcre1_extra_info && error)
die("%s", error);
 }
 
-static int pcrematch(struct grep_pat *p, const char *line, const char *eol,
+static int pcre1match(struct grep_pat *p, const char *line, const char *eol,
regmatch_t *match, int eflags)
 {
int ovector[30], ret, flags = 0;
@@ -366,7 +366,7 @@ static int pcrematch(struct grep_pat *p, const char *line, 
const char *eol,
if (eflags & REG_NOTBOL)
flags |= PCRE_NOTBOL;
 
-   ret = pcre_exec(p->pcre_regexp, p->pcre_extra_info, line, eol - line,
+   ret = pcre_exec(p->pcre1_regexp, p->pcre1_extra_info, line, eol - line,
0, flags, ovector, ARRAY_SIZE(ovector));
if (ret < 0 && ret != PCRE_ERROR_NOMATCH)
die("pcre_exec failed with error code %d", ret);
@@ -379,25 +379,25 @@ static int pcrematch(struct grep_pat *p, const char 
*line, const char *eol,
return ret;
 }
 
-static void free_pcre_regexp(struct grep_pat *p)
+static void free_pcre1_regexp(struct grep_pat *p)
 {
-   pcre_free(p->pcre_regexp);
-   pcre_free(p->pcre_extra_info);
-   pcre_free((void *)p->pcre_tables);
+   pcre_free(p->pcre1_regexp);
+   pcre_free(p->pcre1_extra_info);
+   pcre_free((void *)p->pcre1_tables);
 }
 #else /* !USE_LIBPCRE1 */
-static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)
+static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt 
*opt)
 {
die("cannot use Perl-compatible regexes when not compiled with 
USE_LIBPCRE");
 }
 
-static int pcrematch(struct grep_pat *p, const char *line, const char *eol,
+static int pcre1match(struct grep_pat *p, const char *line, const char *eol,
regmatch_t *match, int eflags)
 {
return 1;
 }
 
-static void free_pcre_regexp(struct grep_pat *p)
+static void free_pcre1_regexp(struct grep_pat *p)
 {
 }
 #endif /* !USE_LIBPCRE1 */
@@ -477,8 +477,8 @@ static void compile_regexp(struct grep_pat *p, struct 
grep_opt *opt)
return;
}
 
-   if (opt->pcre) {
-   compile_pcre_regexp(p, opt);
+   if (opt->pcre1) {
+   compile_pcre1_regexp(p, opt);
return;
}
 
@@ -834,8 +834,8 @@ void 

[PATCH v3 06/30] grep: add a test asserting that --perl-regexp dies when !PCRE

[Æ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 '[\d]' would match "d" instead of
digits.

The entire test suite would still pass with this serious regression,
since everything that tested for --perl-regexp would be guarded by the
PCRE prerequisite, fix that blind-spot by adding tests under !PCRE
asserting that git must die when given --perl-regexp or -P.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t4202-log.sh  |  4 +++-
 t/t7810-grep.sh | 12 
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 547f4c19a7..dbed3efeee 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -418,7 +418,9 @@ test_expect_success 'log with various grep.patternType 
configurations & command-
git log --pretty=tformat:%s --perl-regexp \
--grep="[\d]\|" >actual.perl.long-arg &&
test_cmp expect.perl actual.perl.long-arg
-
+   else
+   test_must_fail git log --perl-regexp \
+   --grep="[\d]\|"
fi &&
test_cmp expect.fixed actual.fixed.long-arg &&
test_cmp expect.basic actual.basic.long-arg &&
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index c84c4d99f9..8d69113695 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -281,6 +281,10 @@ do
test_cmp expected actual
'
 
+   test_expect_success !PCRE "grep $L with grep.patterntype=perl errors 
without PCRE" '
+   test_must_fail git -c grep.patterntype=perl grep "foo.*bar"
+   '
+
test_expect_success "grep $L with grep.patternType=default and 
grep.extendedRegexp=true" '
echo "${HC}ab:abc" >expected &&
git \
@@ -1058,11 +1062,19 @@ test_expect_success PCRE 'grep --perl-regexp pattern' '
test_cmp expected actual
 '
 
+test_expect_success !PCRE 'grep --perl-regexp pattern errors without PCRE' '
+   test_must_fail git grep --perl-regexp "foo.*bar"
+'
+
 test_expect_success PCRE 'grep -P pattern' '
git grep -P "\p{Ps}.*?\p{Pe}" hello.c >actual &&
test_cmp expected actual
 '
 
+test_expect_success !PCRE 'grep -P pattern errors without PCRE' '
+   test_must_fail git grep -P "foo.*bar"
+'
+
 test_expect_success 'grep pattern with grep.extendedRegexp=true' '
>empty &&
test_must_fail git -c grep.extendedregexp=true \
-- 
2.13.0.303.g4ebf302169

[PATCH v3 00/30] Easy to review grep & pre-PCRE changes

[Ævar Arnfjörð Bjarmason]

Easy to review? 29 (I mean 30) 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" series.

Since Junio hasn't been picking it I'm no longer sending updates to
that patch series & waiting for this one to cook first.

See <20170513231509.7834-1-ava...@gmail.com>
(https://public-inbox.org/git/20170513231509.7834-1-ava...@gmail.com/)
for v2 & notes about that version. What changed this time around? See
below:

Ævar Arnfjörð Bjarmason (30):
  Makefile & configure: reword inaccurate comment about PCRE
  grep & rev-list doc: stop promising libpcre for --perl-regexp
  test-lib: rename the LIBPCRE prerequisite to PCRE

No changes.

  log: add exhaustive tests for pattern style options & config

Test comment clarifications in t4202-log.sh as pointed out by Junio.

  log: make --regexp-ignore-case work with --perl-regexp

NEW: I noticed that the `-i` in `git log --perl-regexp -i --grep=`
never worked as intended. I.e. the flag for ignoring the case of the
pattern wasn't picked up.

Fixing this was trivial (one-line change), so I've included it in this
series since it's needed by a new t/perf patch (see below).

  grep: add a test asserting that --perl-regexp dies when !PCRE
  grep: add a test for backreferences in PCRE patterns
  grep: change non-ASCII -i test to stop using --debug
  grep: add tests for --threads=N and grep.threads
  grep: amend submodule recursion test for regex engine testing
  grep: add tests for grep pattern types being passed to submodules

No changes.

  grep: add a test helper function for less verbose -f \0 tests

Trivial style changes in nul_match() suggested by Junio. No functional
changes.

  grep: prepare for testing binary regexes containing rx metacharacters

No changes.

  grep: add tests to fix blind spots with \0 patterns

Continued trivial style changes in nul_match() (the other half of the
code in that function is added in this commit)>

  perf: add a GIT_PERF_MAKE_COMMAND for when *_MAKE_OPTS won't do
  perf: emit progress output when unpacking & building

No changes.

  perf: add a comparison test of grep regex engines
  perf: add a comparison test of grep regex engines with -F
  perf: add a comparison test of log --grep regex engines

The log --grep test is new, and all these tests learned to take an env
variable to pass arbitrary extra grep/log flags, so I can e.g. test
with -i, -v, -w etc.

Subsequent commit messages that e.g. mentioned perf tests with the
previous hardcoded -i test have been amended to mention the new test
results.

  grep: catch a missing enum in switch statement

Grammar fix in commit message.

  grep: remove redundant regflags assignments

The two commits that made changes to regflags assignments have been
squashed.

  grep: factor test for \0 in grep patterns into a function

Rewrote commit message to not go off on a tangent about what grep -f
[file-with-\0-pattern] should mean, which is not what this change is
about.

  grep: change the internal PCRE macro names to be PCRE1
  grep: change internal *pcre* variable & function names to be *pcre1*
  grep: move is_fixed() earlier to avoid forward declaration
  test-lib: add a PTHREADS prerequisite

No changes.

  pack-objects & index-pack: add test for --threads warning
  pack-objects: fix buggy warning about threads

Rewrote the tests in these two so that the first one sets up a failing
test which is subsequently fixed in the commit that fixes the bug, as
suggested by Junio.

Removed a stray `cat err` left over from debugging.

  grep: given --threads with NO_PTHREADS=YesPlease, warn
  grep: assert that threading is enabled when calling grep_{lock,unlock}

No changes.

 Documentation/git-grep.txt |   7 +-
 Documentation/rev-list-options.txt |   8 +-
 Makefile   |  14 ++-
 builtin/grep.c |  23 +++-
 builtin/pack-objects.c |   4 +-
 configure.ac   |  12 ++-
 grep.c | 108 ++-
 grep.h |  10 +-
 revision.c |   1 +
 t/README   |   8 +-
 t/perf/README  |  19 +++-
 t/perf/p4220-log-grep-engines.sh   |  44 
 t/perf/p7820-grep-engines.sh   |  47 
 t/perf/p7821-grep-engines-fixed.sh |  32 ++
 t/perf/run |  13 ++-
 t/t4202-log.sh | 160 +--
 t/t5300-pack-object.sh |  36 +++
 t/t7008-grep-binary.sh | 135 +--
 t/t7810-grep.sh|  81 +++---
 t/t7812-grep-icase-non-ascii.sh|  29 ++---
 t/t7813-grep-icase-iso.sh  |   2 +-
 t/t7814-grep-recurse-submodules.sh | 215 +++--
 

[PATCH v3 03/30] test-lib: rename the LIBPCRE prerequisite to PCRE

[Æ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 make
it clear that they work on both library versions.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/README|  4 ++--
 t/t7810-grep.sh | 28 ++--
 t/t7812-grep-icase-non-ascii.sh |  4 ++--
 t/t7813-grep-icase-iso.sh   |  2 +-
 t/test-lib.sh   |  2 +-
 5 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/t/README b/t/README
index ab386c3681..a90cb62583 100644
--- a/t/README
+++ b/t/README
@@ -803,9 +803,9 @@ use these, and "test_set_prereq" for how to define your own.
Test is not run by root user, and an attempt to write to an
unwritable file is expected to fail correctly.
 
- - LIBPCRE
+ - PCRE
 
-   Git was compiled with USE_LIBPCRE=YesPlease. Wrap any tests
+   Git was compiled with support for PCRE. Wrap any tests
that use git-grep --perl-regexp or git-grep -P in these.
 
  - CASE_INSENSITIVE_FS
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index cee42097b0..c84c4d99f9 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -275,7 +275,7 @@ do
test_cmp expected actual
'
 
-   test_expect_success LIBPCRE "grep $L with grep.patterntype=perl" '
+   test_expect_success PCRE "grep $L with grep.patterntype=perl" '
echo "${HC}ab:a+b*c" >expected &&
git -c grep.patterntype=perl grep "a\x{2b}b\x{2a}c" $H ab 
>actual &&
test_cmp expected actual
@@ -1053,12 +1053,12 @@ hello.c:int main(int argc, const char **argv)
 hello.c:   printf("Hello world.\n");
 EOF
 
-test_expect_success LIBPCRE 'grep --perl-regexp pattern' '
+test_expect_success PCRE 'grep --perl-regexp pattern' '
git grep --perl-regexp "\p{Ps}.*?\p{Pe}" hello.c >actual &&
test_cmp expected actual
 '
 
-test_expect_success LIBPCRE 'grep -P pattern' '
+test_expect_success PCRE 'grep -P pattern' '
git grep -P "\p{Ps}.*?\p{Pe}" hello.c >actual &&
test_cmp expected actual
 '
@@ -1070,13 +1070,13 @@ test_expect_success 'grep pattern with 
grep.extendedRegexp=true' '
test_cmp empty actual
 '
 
-test_expect_success LIBPCRE 'grep -P pattern with grep.extendedRegexp=true' '
+test_expect_success PCRE 'grep -P pattern with grep.extendedRegexp=true' '
git -c grep.extendedregexp=true \
grep -P "\p{Ps}.*?\p{Pe}" hello.c >actual &&
test_cmp expected actual
 '
 
-test_expect_success LIBPCRE 'grep -P -v pattern' '
+test_expect_success PCRE 'grep -P -v pattern' '
{
echo "ab:a+b*c"
echo "ab:a+bc"
@@ -1085,7 +1085,7 @@ test_expect_success LIBPCRE 'grep -P -v pattern' '
test_cmp expected actual
 '
 
-test_expect_success LIBPCRE 'grep -P -i pattern' '
+test_expect_success PCRE 'grep -P -i pattern' '
cat >expected <<-EOF &&
hello.c:printf("Hello world.\n");
EOF
@@ -1093,7 +1093,7 @@ test_expect_success LIBPCRE 'grep -P -i pattern' '
test_cmp expected actual
 '
 
-test_expect_success LIBPCRE 'grep -P -w pattern' '
+test_expect_success PCRE 'grep -P -w pattern' '
{
echo "hello_world:Hello world"
echo "hello_world:HeLLo world"
@@ -1118,11 +1118,11 @@ test_expect_success 'grep invalidpattern properly dies 
with grep.patternType=ext
test_must_fail git -c grep.patterntype=extended grep "a["
 '
 
-test_expect_success LIBPCRE 'grep -P invalidpattern properly dies ' '
+test_expect_success PCRE 'grep -P invalidpattern properly dies ' '
test_must_fail git grep -P "a["
 '
 
-test_expect_success LIBPCRE 'grep invalidpattern properly dies with 
grep.patternType=perl' '
+test_expect_success PCRE 'grep invalidpattern properly dies with 
grep.patternType=perl' '
test_must_fail git -c grep.patterntype=perl grep "a["
 '
 
@@ -1191,13 +1191,13 @@ test_expect_success 'grep pattern with 
grep.patternType=fixed, =basic, =perl, =e
test_cmp expected actual
 '
 
-test_expect_success LIBPCRE 'grep -G -F -E -P pattern' '
+test_expect_success PCRE 'grep -G -F -E -P pattern' '
echo "d0:0" >expected &&
git grep -G -F -E -P "[\d]" d0 >actual &&
test_cmp expected actual
 '
 
-test_expect_success LIBPCRE 'grep pattern with grep.patternType=fixed, =basic, 
=extended, =perl' '
+test_expect_success PCRE 'grep pattern with grep.patternType=fixed, =basic, 
=extended, =perl' '
echo "d0:0" >expected &&
git \
-c grep.patterntype=fixed \
@@ -1208,7 +1208,7 @@ test_expect_success LIBPCRE 'grep pattern with 
grep.patternType=fixed, =basic, =
test_cmp expected actual
 '
 
-test_expect_success LIBPCRE 'grep -P pattern with 

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

[Æ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, and still have 100% of the
test suite pass.

This was because the first test being modified here, added in commit
34a4ae55b2 ("log --grep: use the same helper to set -E/-F options as
"git grep"", 2012-10-03), didn't actually check whether we'd enabled
extended regular expressions as distinct from re-toggling non-fixed
string support.

Fix that by changing the pattern to a pattern that'll only match if
--extended-regexp option is provided, but won't match under the
default --basic-regexp option.

Other potential regressions were possible since there were no tests
for the rest of the combinations of grep.patternType configuration
toggles & corresponding git-log command-line options. Add exhaustive
tests for those.

The patterns being passed to fixed/basic/extended/PCRE are carefully
crafted to return the wrong thing if the grep engine were to pick any
other matching method than the one it's told to use.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t4202-log.sh | 98 +-
 1 file changed, 97 insertions(+), 1 deletion(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index f577990716..a8dce0ca2d 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -262,7 +262,30 @@ test_expect_success 'log --grep -i' '
 
 test_expect_success 'log -F -E --grep= uses ere' '
echo second >expect &&
-   git log -1 --pretty="tformat:%s" -F -E --grep=s.c.nd >actual &&
+   # basic would need \(s\) to do the same
+   git log -1 --pretty="tformat:%s" -F -E --grep="(s).c.nd" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success PCRE 'log -F -E --perl-regexp --grep= uses PCRE' '
+   test_when_finished "rm -rf num_commits" &&
+   git init num_commits &&
+   (
+   cd num_commits &&
+   test_commit 1d &&
+   test_commit 2e
+   ) &&
+
+   # In PCRE \d in [\d] is like saying "0-9", and matches the 2
+   # in 2e...
+   echo 2e >expect &&
+   git -C num_commits log -1 --pretty="tformat:%s" -F -E --perl-regexp 
--grep="[\d]" >actual &&
+   test_cmp expect actual &&
+
+   # ...in POSIX basic and extended it is the same as [d],
+   # i.e. "d", which matches 1d, but does not match 2e.
+   echo 1d >expect &&
+   git -C num_commits log -1 --pretty="tformat:%s" -F -E --grep="[\d]" 
>actual &&
test_cmp expect actual
 '
 
@@ -280,6 +303,79 @@ test_expect_success 'log with grep.patternType 
configuration and command line' '
test_cmp expect actual
 '
 
+test_expect_success 'log with various grep.patternType configurations & 
command-lines' '
+   git init pattern-type &&
+   (
+   cd pattern-type &&
+   test_commit 1 file A &&
+
+   # The tagname is overridden here because creating a
+   # tag called "(1|2)" as test_commit would otherwise
+   # implicitly do would fail on e.g. MINGW.
+   test_commit "(1|2)" file B 2 &&
+
+   echo "(1|2)" >expect.fixed &&
+   cp expect.fixed expect.basic &&
+   cp expect.fixed expect.extended &&
+   cp expect.fixed expect.perl &&
+
+   # A strcmp-like match with fixed.
+   git -c grep.patternType=fixed log --pretty=tformat:%s \
+   --grep="(1|2)" >actual.fixed &&
+
+   # POSIX basic matches (, | and ) literally.
+   git -c grep.patternType=basic log --pretty=tformat:%s \
+   --grep="(.|.)" >actual.basic &&
+
+   # POSIX extended needs to have | escaped to match it
+   # literally, whereas under basic this is the same as
+   # (|2), i.e. it would also match "1". This test checks
+   # for extended by asserting that it is not matching
+   # what basic would match.
+   git -c grep.patternType=extended log --pretty=tformat:%s \
+   --grep="\|2" >actual.extended &&
+   if test_have_prereq PCRE
+   then
+   # Only PCRE would match [\d]\| with only
+   # "(1|2)" due to [\d]. POSIX basic would match
+   # both it and "1" since similarly to the
+   # extended match above it is the same as
+   # \([\d]\|\). POSIX extended would
+   # match neither.
+   git -c grep.patternType=perl log --pretty=tformat:%s \
+   --grep="[\d]\|" >actual.perl &&
+   test_cmp expect.perl actual.perl
+   fi &&
+   test_cmp 

[PATCH v3 08/30] grep: change non-ASCII -i test to stop using --debug

[Æ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
when -F is specified", 2016-06-25). It was asserting that the regex
must be compiled with compile_fixed_regexp(), instead test for the
expected results, allowing the underlying implementation to change.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t7812-grep-icase-non-ascii.sh | 25 +
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh
index 04a61cb8e0..0059a1f837 100755
--- a/t/t7812-grep-icase-non-ascii.sh
+++ b/t/t7812-grep-icase-non-ascii.sh
@@ -36,29 +36,14 @@ test_expect_success GETTEXT_LOCALE,PCRE 'grep pcre utf-8 
string with "+"' '
 '
 
 test_expect_success REGEX_LOCALE 'grep literal string, with -F' '
-   git grep --debug -i -F "TILRAUN: Halló Heimur!"  2>&1 >/dev/null |
-grep fixed >debug1 &&
-   test_write_lines "fixed TILRAUN: Halló Heimur!" >expect1 &&
-   test_cmp expect1 debug1 &&
-
-   git grep --debug -i -F "TILRAUN: HALLÓ HEIMUR!"  2>&1 >/dev/null |
-grep fixed >debug2 &&
-   test_write_lines "fixed TILRAUN: HALLÓ HEIMUR!" >expect2 &&
-   test_cmp expect2 debug2
+   git grep -i -F "TILRAUN: Halló Heimur!" &&
+   git grep -i -F "TILRAUN: HALLÓ HEIMUR!"
 '
 
 test_expect_success REGEX_LOCALE 'grep string with regex, with -F' '
-   test_write_lines "^*TILR^AUN:.* \\Halló \$He[]imur!\$" >file &&
-
-   git grep --debug -i -F "^*TILR^AUN:.* \\Halló \$He[]imur!\$" 2>&1 
>/dev/null |
-grep fixed >debug1 &&
-   test_write_lines "fixed \\^*TILR^AUN:\\.\\* Halló 
\$He\\[]imur!\\\$" >expect1 &&
-   test_cmp expect1 debug1 &&
-
-   git grep --debug -i -F "^*TILR^AUN:.* \\HALLÓ \$HE[]IMUR!\$"  2>&1 
>/dev/null |
-grep fixed >debug2 &&
-   test_write_lines "fixed \\^*TILR^AUN:\\.\\* HALLÓ 
\$HE\\[]IMUR!\\\$" >expect2 &&
-   test_cmp expect2 debug2
+   test_write_lines "TILRAUN: Halló Heimur [abc]!" >file3 &&
+   git add file3 &&
+   git grep -i -F "TILRAUN: Halló Heimur [abc]!" file3
 '
 
 test_expect_success REGEX_LOCALE 'pickaxe -i on non-ascii' '
-- 
2.13.0.303.g4ebf302169

[PATCH v3 12/30] grep: add a test helper function for less verbose -f \0 tests

[Æ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 
---
 t/t7008-grep-binary.sh | 58 +-
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
index 9c9c378119..df93d8e44c 100755
--- a/t/t7008-grep-binary.sh
+++ b/t/t7008-grep-binary.sh
@@ -4,6 +4,29 @@ test_description='git grep in binary files'
 
 . ./test-lib.sh
 
+nul_match () {
+   matches=$1
+   flags=$2
+   pattern=$3
+   pattern_human=$(echo "$pattern" | sed 's/Q//g')
+
+   if test "$matches" = 1
+   then
+   test_expect_success "git grep -f f $flags '$pattern_human' a" "
+   printf '$pattern' | q_to_nul >f &&
+   git grep -f f $flags a
+   "
+   elif test "$matches" = 0
+   then
+   test_expect_success "git grep -f f $flags '$pattern_human' a" "
+   printf '$pattern' | q_to_nul >f &&
+   test_must_fail git grep -f f $flags a
+   "
+   else
+   test_expect_success "PANIC: Test framework error. Unknown 
matches value $matches" 'false'
+   fi
+}
+
 test_expect_success 'setup' "
echo 'binaryQfile' | q_to_nul >a &&
git add a &&
@@ -69,35 +92,12 @@ test_expect_failure 'git grep .fi a' '
git grep .fi a
 '
 
-test_expect_success 'git grep -F yf a' "
-   printf 'yQf' | q_to_nul >f &&
-   git grep -f f -F a
-"
-
-test_expect_success 'git grep -F yx a' "
-   printf 'yQx' | q_to_nul >f &&
-   test_must_fail git grep -f f -F a
-"
-
-test_expect_success 'git grep -Fi Yf a' "
-   printf 'YQf' | q_to_nul >f &&
-   git grep -f f -Fi a
-"
-
-test_expect_success 'git grep -Fi Yx a' "
-   printf 'YQx' | q_to_nul >f &&
-   test_must_fail git grep -f f -Fi a
-"
-
-test_expect_success 'git grep yf a' "
-   printf 'yQf' | q_to_nul >f &&
-   git grep -f f a
-"
-
-test_expect_success 'git grep yx a' "
-   printf 'yQx' | q_to_nul >f &&
-   test_must_fail git grep -f f a
-"
+nul_match 1 '-F' 'yQf'
+nul_match 0 '-F' 'yQx'
+nul_match 1 '-Fi' 'YQf'
+nul_match 0 '-Fi' 'YQx'
+nul_match 1 '' 'yQf'
+nul_match 0 '' 'yQx'
 
 test_expect_success 'grep respects binary diff attribute' '
echo text >t &&
-- 
2.13.0.303.g4ebf302169

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

[Æ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 that we're talking about libpcre.so, which is
always going to be v1. This change is part of an ongoing saga to add
support for libpcre2, which comes with PCRE v2.

In the future we might use some completely unrelated library to
provide perl-compatible regular expression support. By wording the
documentation differently and not promising any specific version of
PCRE or even PCRE at all we have more wiggle room to change the
implementation.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-grep.txt | 7 +--
 Documentation/rev-list-options.txt | 8 ++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 71f32f3508..5033483db4 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -161,8 +161,11 @@ OPTIONS
 
 -P::
 --perl-regexp::
-   Use Perl-compatible regexp for patterns. Requires libpcre to be
-   compiled in.
+   Use Perl-compatible regular expressions for patterns.
++
+Support for these types of regular expressions is an optional
+compile-time dependency. If Git wasn't compiled with support for them
+providing this option will cause it to die.
 
 -F::
 --fixed-strings::
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index a02f7324c0..a46f70c2b1 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -92,8 +92,12 @@ endif::git-rev-list[]
pattern as a regular expression).
 
 --perl-regexp::
-   Consider the limiting patterns to be Perl-compatible regular 
expressions.
-   Requires libpcre to be compiled in.
+   Consider the limiting patterns to be Perl-compatible regular
+   expressions.
++
+Support for these types of regular expressions is an optional
+compile-time dependency. If Git wasn't compiled with support for them
+providing this option will cause it to die.
 
 --remove-empty::
Stop when a given path disappears from the tree.
-- 
2.13.0.303.g4ebf302169

[PATCH v3 07/30] grep: add a test for backreferences in PCRE patterns

[Æ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 
---
 t/t7810-grep.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 8d69113695..daa906b9b0 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1114,6 +1114,13 @@ test_expect_success PCRE 'grep -P -w pattern' '
test_cmp expected actual
 '
 
+test_expect_success PCRE 'grep -P backreferences work (the PCRE 
NO_AUTO_CAPTURE flag is not set)' '
+   git grep -P -h "(?P.)(?P=one)" hello_world >actual &&
+   test_cmp hello_world actual &&
+   git grep -P -h "(.)\1" hello_world >actual &&
+   test_cmp hello_world actual
+'
+
 test_expect_success 'grep -G invalidpattern properly dies ' '
test_must_fail git grep -G "a["
 '
-- 
2.13.0.303.g4ebf302169

[PATCH v3 05/30] log: make --regexp-ignore-case work with --perl-regexp

[Ævar Arnfjörð Bjarmason]

Make the --regexp-ignore-case option work with --perl-regexp. This
never worked, and there was no test for this. Fix the bug and add a
test.

When PCRE support was added in commit 63e7e9d8b6 ("git-grep: Learn
PCRE", 2011-05-09) compile_pcre_regexp() would only check
opt->ignore_case, but when the --perl-regexp option was added in
commit 727b6fc3ed ("log --grep: accept --basic-regexp and
--perl-regexp", 2012-10-03) the code didn't set the opt->ignore_case.

Change the test suite to test for -i and --invert-regexp with
basic/extended/perl patterns in addition to fixed, which was the only
patternType that was tested for before in combination with those
options.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 revision.c |  1 +
 t/t4202-log.sh | 60 +-
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/revision.c b/revision.c
index 8a8c1789c7..4883cdd2d0 100644
--- a/revision.c
+++ b/revision.c
@@ -1991,6 +1991,7 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
} else if (!strcmp(arg, "--extended-regexp") || !strcmp(arg, "-E")) {
revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_ERE;
} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
+   revs->grep_filter.ignore_case = 1;
revs->grep_filter.regflags |= REG_ICASE;
DIFF_OPT_SET(>diffopt, PICKAXE_IGNORE_CASE);
} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index a8dce0ca2d..547f4c19a7 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -231,14 +231,47 @@ second
 initial
 EOF
 test_expect_success 'log --invert-grep --grep' '
-   git log --pretty="tformat:%s" --invert-grep --grep=th --grep=Sec 
>actual &&
-   test_cmp expect actual
+   # Fixed
+   git -c grep.patternType=fixed log --pretty="tformat:%s" --invert-grep 
--grep=th --grep=Sec >actual &&
+   test_cmp expect actual &&
+
+   # POSIX basic
+   git -c grep.patternType=basic log --pretty="tformat:%s" --invert-grep 
--grep=t[h] --grep=S[e]c >actual &&
+   test_cmp expect actual &&
+
+   # POSIX extended
+   git -c grep.patternType=basic log --pretty="tformat:%s" --invert-grep 
--grep=t[h] --grep=S[e]c >actual &&
+   test_cmp expect actual &&
+
+   # PCRE
+   if test_have_prereq PCRE
+   then
+   git -c grep.patternType=perl log --pretty="tformat:%s" 
--invert-grep --grep=t[h] --grep=S[e]c >actual &&
+   test_cmp expect actual
+   fi
 '
 
 test_expect_success 'log --invert-grep --grep -i' '
echo initial >expect &&
-   git log --pretty="tformat:%s" --invert-grep -i --grep=th --grep=Sec 
>actual &&
-   test_cmp expect actual
+
+   # Fixed
+   git -c grep.patternType=fixed log --pretty="tformat:%s" --invert-grep 
-i --grep=th --grep=Sec >actual &&
+   test_cmp expect actual &&
+
+   # POSIX basic
+   git -c grep.patternType=basic log --pretty="tformat:%s" --invert-grep 
-i --grep=t[h] --grep=S[e]c >actual &&
+   test_cmp expect actual &&
+
+   # POSIX extended
+   git -c grep.patternType=extended log --pretty="tformat:%s" 
--invert-grep -i --grep=t[h] --grep=S[e]c >actual &&
+   test_cmp expect actual &&
+
+   # PCRE
+   if test_have_prereq PCRE
+   then
+   git -c grep.patternType=perl log --pretty="tformat:%s" 
--invert-grep -i --grep=t[h] --grep=S[e]c >actual &&
+   test_cmp expect actual
+   fi
 '
 
 test_expect_success 'log --grep option parsing' '
@@ -256,8 +289,25 @@ test_expect_success 'log -i --grep' '
 
 test_expect_success 'log --grep -i' '
echo Second >expect &&
+
+   # Fixed
git log -1 --pretty="tformat:%s" --grep=sec -i >actual &&
-   test_cmp expect actual
+   test_cmp expect actual &&
+
+   # POSIX basic
+   git -c grep.patternType=basic log -1 --pretty="tformat:%s" --grep=s[e]c 
-i >actual &&
+   test_cmp expect actual &&
+
+   # POSIX extended
+   git -c grep.patternType=extended log -1 --pretty="tformat:%s" 
--grep=s[e]c -i >actual &&
+   test_cmp expect actual &&
+
+   # PCRE
+   if test_have_prereq PCRE
+   then
+   git -c grep.patternType=perl log -1 --pretty="tformat:%s" 
--grep=s[e]c -i >actual &&
+   test_cmp expect actual
+   fi
 '
 
 test_expect_success 'log -F -E --grep= uses ere' '
-- 
2.13.0.303.g4ebf302169

[PATCH v3 01/30] Makefile & configure: reword inaccurate comment about PCRE

[Æ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 learned to use
PCRE with --grep in commit 727b6fc3ed ("log --grep: accept
--basic-regexp and --perl-regexp", 2012-10-03), and more importantly
is likely to get more inaccurate over time as more use is made of PCRE
in other areas.

Reword it to be more future-proof, and to more clearly explain that
this enables user-initiated runtime behavior.

Copy/pasting this so much in configure.ac is lame, these Makefile-like
flags aren't even used by autoconf, just the corresponding
--with[out]-* options. But copy/pasting the comments that make sense
for the Makefile to configure.ac where they make less sense is the
pattern everything else follows in that file. I'm not going to war
against that as part of this change, just following the existing
pattern.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Makefile |  6 --
 configure.ac | 12 
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index e35542e631..eedadb8056 100644
--- a/Makefile
+++ b/Makefile
@@ -24,8 +24,10 @@ all::
 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
 # This also implies BLK_SHA1.
 #
-# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
-# able to use Perl-compatible regular expressions.
+# Define USE_LIBPCRE if you have and want to use libpcre. Various
+# commands such as log and grep offer runtime options to use
+# Perl-compatible regular expressions instead of standard or extended
+# POSIX regular expressions.
 #
 # Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.
diff --git a/configure.ac b/configure.ac
index 128165529f..deeb968daa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -250,8 +250,10 @@ AS_HELP_STRING([--with-openssl],[use OpenSSL library 
(default is YES)])
 AS_HELP_STRING([],  [ARG can be prefix for openssl library and 
headers]),
 GIT_PARSE_WITH([openssl]))
 
-# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
-# able to use Perl-compatible regular expressions.
+# Define USE_LIBPCRE if you have and want to use libpcre. Various
+# commands such as log and grep offer runtime options to use
+# Perl-compatible regular expressions instead of standard or extended
+# POSIX regular expressions.
 #
 # Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.
@@ -499,8 +501,10 @@ GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO])
 GIT_CONF_SUBST([NO_OPENSSL])
 
 #
-# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
-# able to use Perl-compatible regular expressions.
+# Define USE_LIBPCRE if you have and want to use libpcre. Various
+# commands such as log and grep offer runtime options to use
+# Perl-compatible regular expressions instead of standard or extended
+# POSIX regular expressions.
 #
 
 if test -n "$USE_LIBPCRE"; then
-- 
2.13.0.303.g4ebf302169

Re: [WIP/RFC 00/23] repository object

[Stefan Beller]

On Thu, May 18, 2017 at 4:21 PM, Brandon Williams  wrote:

> This is still very much in a WIP state, though it does pass all tests.  What
> I'm hoping for here is to get a discussion started about the feasibility of a
> change like this and hopefully to get the ball rolling.  Is this a direction 
> we
> want to move in?  Is it worth the pain?

I would be really happy to see this series land eventually.

The introduction of a repo object will deliver performance at a higher
level, such as
* (remarked by Ben): enabling of threading
* submodules do not need to spawn processes
* I would imagine that developer velocity will go up by having less global state
  in the long run.

Thanks for working on this.
Stefan

Re: [WIP/RFC 18/23] repo: add index_state to struct repo

[Stefan Beller]

>  int
> +repo_read_index(struct repo *repo, const char *index_file)
...
> +
> +int
>  repo_init(struct repo *repo, const char *gitdir, const char *worktree)

The version 2.13.0.303.g4ebf302169-goog doesn't have diff slider
heuristics on by default, and you also did not enable it?
(I am curious if the heuristics would have helped here)

Re: [WIP/RFC 17/23] repo: introduce new repository object

[Stefan Beller]

On Thu, May 18, 2017 at 4:21 PM, Brandon Williams  wrote:
> Introduce 'struct repo' an object used to represent a repository.

Is this the right place to outline what you expect from a repo object?
Are you planning to use it everywhere?
Is it lazy-init'd and it takes care of it itself, or would the caller
have to take
care of the state of the repo? ("the repo object is just a place to put the
current globals")

>
> Signed-off-by: Brandon Williams 
> ---
>  Makefile |  1 +
>  repo.c   | 42 ++
>  repo.h   | 15 +++
>  3 files changed, 58 insertions(+)
>  create mode 100644 repo.c
>  create mode 100644 repo.h
>
> diff --git a/Makefile b/Makefile
> index e35542e63..a49d2f96a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -821,6 +821,7 @@ LIB_OBJS += refs/ref-cache.o
>  LIB_OBJS += ref-filter.o
>  LIB_OBJS += remote.o
>  LIB_OBJS += replace_object.o
> +LIB_OBJS += repo.o
>  LIB_OBJS += rerere.o
>  LIB_OBJS += resolve-undo.o
>  LIB_OBJS += revision.o
> diff --git a/repo.c b/repo.c
> new file mode 100644
> index 0..d47e98d95
> --- /dev/null
> +++ b/repo.c
> @@ -0,0 +1,42 @@
> +#include "cache.h"
> +#include "repo.h"
> +
> +int
> +repo_init(struct repo *repo, const char *gitdir, const char *worktree)

style ;)


> +   /* Maybe need a check to verify that a worktree is indeed a worktree? 
> */

add NEEDSWORK/FIXME prefix to comment?

> +void
> +repo_clear(struct repo *repo)

style ;)

Re: [PATCH] name-rev: use larger timestamp for is_better_name

[Ramsay Jones]

On 20/05/17 21:36, Eric Wong wrote:
> This fixes t4202 for me at "44 - log --graph with full output"
> on 32-bit x86.
> 
> Signed-off-by: Eric Wong 
> ---
>  This is for pu, I'm still using the machine I used git with in 2005 :)
> 
>  builtin/name-rev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index f06498524..35409c03b 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -27,7 +27,7 @@ static const char *prio_names[] = {
>  
>  static int is_better_name(struct rev_name *name,
> const char *tip_name,
> -   unsigned long taggerdate,
> +   timestamp_t taggerdate,
> int generation,
> int distance,
> int from_tag)
> 

Heh, you seem to be a little ahead of me. :-D

I test on 32-bit Linux from time to time, and tonight's 'pu'
branch fails t4202.44, t6007.2,5-6,12-13,16, t6012.2-11,
t6111.2-65. I bisected the t4202 failure to a merge commit
(99d31e1378, merge branch 'jc/name-rev-lw-tag') and I spotted
the 'unsigned long' taggerdate parameter to the is_better_name()
function.

I was just about to try the patch above and retest, when I saw
your email! (so I can leave that for tonight).

Thanks!

ATB,
Ramsay Jones

[PATCH] name-rev: use larger timestamp for is_better_name

[Eric Wong]

This fixes t4202 for me at "44 - log --graph with full output"
on 32-bit x86.

Signed-off-by: Eric Wong 
---
 This is for pu, I'm still using the machine I used git with in 2005 :)

 builtin/name-rev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index f06498524..35409c03b 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -27,7 +27,7 @@ static const char *prio_names[] = {
 
 static int is_better_name(struct rev_name *name,
  const char *tip_name,
- unsigned long taggerdate,
+ timestamp_t taggerdate,
  int generation,
  int distance,
  int from_tag)
-- 
EW

Loan Offer

["Action Financial"]

Contact us for personal and business loan today, we can be of assistance 
regarding financial help. Kindly write back to us via company email: 
(actionfinancia...@gmail.com) if you are interested.

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH] mingw: simplify PATH handling

[René Scharfe]

Am 20.05.2017 um 19:00 schrieb Johannes Sixt:

Am 20.05.2017 um 17:29 schrieb René Scharfe:

-static char *path_lookup(const char *cmd, char **path, int exe_only)
+static char *path_lookup(const char *cmd, int exe_only)
  {
+const char *path;
  char *prog = NULL;
  int len = strlen(cmd);
  int isexe = len >= 4 && !strcasecmp(cmd+len-4, ".exe");
  if (strchr(cmd, '/') || strchr(cmd, '\\'))
-prog = xstrdup(cmd);
+return xstrdup(cmd);
-while (!prog && *path)
-prog = lookup_prog(*path++, cmd, isexe, exe_only);
+path = mingw_getenv("PATH");
+if (!path)
+return NULL;
+
+for (; !prog && *path; path++) {
+const char *sep = strchrnul(path, ';');
+if (sep == path)
+continue;
+prog = lookup_prog(path, sep - path, cmd, isexe, exe_only);
+path = sep;
+}


The loop termination does not work here. When the final PATH component 
is investigated, sep points to the NUL. This pointer is assigned to 
path, which is incremented and now points one past NUL. Then the loop 
condition (*path) accesses the char behind NUL.


Ugh, yes.  Thanks for catching!

Cause: Last minute/hour edit, had used strspn() before.


  return prog;
  }
@@ -1569,13 +1527,10 @@ static pid_t mingw_spawnve_fd(const char *cmd, 
const char **argv, char **deltaen

  }
  if (getenv("GIT_STRACE_COMMANDS")) {
-char **path = get_path_split();
-char *p = path_lookup("strace.exe", path, 1);
+char *p = path_lookup("strace.exe", 1);
  if (!p) {
-free_path_split(path);
  return error("strace not found!");
  }
-free_path_split(path);
  if (xutftowcs_path(wcmd, p) < 0) {
  free(p);
  return -1;


Upstream does not have this hunk.



Right, it's in next, but not yet in master.  And there are other
differences, so it's a bad time to do that cleanup. :(

René

[PATCH v2] mingw: simplify PATH handling

[René Scharfe]

On Windows the environment variable PATH contains a semicolon-separated
list of directories to search for, in order, when looking for the
location of a binary to run.  get_path_split() parses it and returns an
array of string copies, which is iterated by path_lookup(), which in
turn passes each entry to lookup_prog().

Change lookup_prog() to take the directory name as a length-limited
string instead of as a NUL-terminated one and parse PATH directly in
path_lookup().  This avoids memory allocations, simplifying the code.

Helped-by: Johannes Sixt 
Signed-off-by: Rene Scharfe 
---
Rebased against Junio's master, fixed string overrun.  Can hold and
resubmit in a few months if it gets in the way right now.

 compat/mingw.c | 91 +++---
 1 file changed, 23 insertions(+), 68 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 3fbfda5978..c6134f7c81 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -941,64 +941,14 @@ static const char *parse_interpreter(const char *cmd)
 }
 
 /*
- * Splits the PATH into parts.
- */
-static char **get_path_split(void)
-{
-   char *p, **path, *envpath = mingw_getenv("PATH");
-   int i, n = 0;
-
-   if (!envpath || !*envpath)
-   return NULL;
-
-   envpath = xstrdup(envpath);
-   p = envpath;
-   while (p) {
-   char *dir = p;
-   p = strchr(p, ';');
-   if (p) *p++ = '\0';
-   if (*dir) { /* not earlier, catches series of ; */
-   ++n;
-   }
-   }
-   if (!n)
-   return NULL;
-
-   ALLOC_ARRAY(path, n + 1);
-   p = envpath;
-   i = 0;
-   do {
-   if (*p)
-   path[i++] = xstrdup(p);
-   p = p+strlen(p)+1;
-   } while (i < n);
-   path[i] = NULL;
-
-   free(envpath);
-
-   return path;
-}
-
-static void free_path_split(char **path)
-{
-   char **p = path;
-
-   if (!path)
-   return;
-
-   while (*p)
-   free(*p++);
-   free(path);
-}
-
-/*
  * exe_only means that we only want to detect .exe files, but not scripts
  * (which do not have an extension)
  */
-static char *lookup_prog(const char *dir, const char *cmd, int isexe, int 
exe_only)
+static char *lookup_prog(const char *dir, int dirlen, const char *cmd,
+int isexe, int exe_only)
 {
char path[MAX_PATH];
-   snprintf(path, sizeof(path), "%s/%s.exe", dir, cmd);
+   snprintf(path, sizeof(path), "%.*s\\%s.exe", dirlen, dir, cmd);
 
if (!isexe && access(path, F_OK) == 0)
return xstrdup(path);
@@ -1013,17 +963,29 @@ static char *lookup_prog(const char *dir, const char 
*cmd, int isexe, int exe_on
  * Determines the absolute path of cmd using the split path in path.
  * If cmd contains a slash or backslash, no lookup is performed.
  */
-static char *path_lookup(const char *cmd, char **path, int exe_only)
+static char *path_lookup(const char *cmd, int exe_only)
 {
+   const char *path;
char *prog = NULL;
int len = strlen(cmd);
int isexe = len >= 4 && !strcasecmp(cmd+len-4, ".exe");
 
if (strchr(cmd, '/') || strchr(cmd, '\\'))
-   prog = xstrdup(cmd);
+   return xstrdup(cmd);
+
+   path = mingw_getenv("PATH");
+   if (!path)
+   return NULL;
 
-   while (!prog && *path)
-   prog = lookup_prog(*path++, cmd, isexe, exe_only);
+   while (!prog) {
+   const char *sep = strchrnul(path, ';');
+   int dirlen = sep - path;
+   if (dirlen)
+   prog = lookup_prog(path, dirlen, cmd, isexe, exe_only);
+   if (!*sep)
+   break;
+   path = sep + 1;
+   }
 
return prog;
 }
@@ -1190,8 +1152,7 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, 
char **deltaenv,
 int fhin, int fhout, int fherr)
 {
pid_t pid;
-   char **path = get_path_split();
-   char *prog = path_lookup(cmd, path, 0);
+   char *prog = path_lookup(cmd, 0);
 
if (!prog) {
errno = ENOENT;
@@ -1202,7 +1163,7 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, 
char **deltaenv,
 
if (interpr) {
const char *argv0 = argv[0];
-   char *iprog = path_lookup(interpr, path, 1);
+   char *iprog = path_lookup(interpr, 1);
argv[0] = prog;
if (!iprog) {
errno = ENOENT;
@@ -1220,21 +1181,18 @@ pid_t mingw_spawnvpe(const char *cmd, const char 
**argv, char **deltaenv,
   fhin, fhout, fherr);
free(prog);
}
-   free_path_split(path);
return pid;
 }
 
 static int 

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Manish Goregaokar]

Yes, you are right (on both counts).

One thing which I think hasn't been covered yet is the rebase
ORIG_HEAD. I'll see if that's still a problem on `pu` and make a patch
for it if so.

(I recall `git prune` during a rebase messing up repo state, though
it's really my fault for trying that in the first place. Would be nice
if it worked, though)
-Manish Goregaokar


On Sat, May 20, 2017 at 3:30 AM, Junio C Hamano  wrote:
> manishea...@gmail.com writes:
>
>> +int for_each_worktree_ref(each_ref_fn fn, void *cb_data)
>> +{
>> + int i, flag, retval = 0;
>> + struct object_id oid;
>> + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED);
>> + struct commit* commit;
>> + for (i = 0; worktrees[i]; i++) {
>> + if ((commit = 
>> lookup_commit_reference(worktrees[i]->head_sha1))) {
>> + oid = commit->object.oid;
>> + if (!read_ref_full("HEAD", RESOLVE_REF_READING, 
>> oid.hash, )) {
>> + if ((retval = fn("HEAD", , flag, cb_data)))
>> + return retval;
>> + }
>> + }
>> + }
>> + return retval;
>> +}
>
> I would have expected for-each-worktree-ref to iterate over all the
> refs in a given worktree, but that is not what this does.  This
> instead iterates over worktrees and shows only their HEAD ref, no
> other refs.  This helper is somewhat misnamed.
>
> By the way, doesn't nd/prune-in-worktree topic that has been cooking
> in 'pu' supersede this change?  It not just protects the commit at
> the tip of HEAD in each worktree, it also makes sure the ones in
> HEAD's reflog are not prematurely pruned.
>
> Thanks.
>

Re: [PATCH] mingw: simplify PATH handling

[Johannes Sixt]

Am 20.05.2017 um 17:29 schrieb René Scharfe:

-static char *path_lookup(const char *cmd, char **path, int exe_only)
+static char *path_lookup(const char *cmd, int exe_only)
  {
+   const char *path;
char *prog = NULL;
int len = strlen(cmd);
int isexe = len >= 4 && !strcasecmp(cmd+len-4, ".exe");
  
  	if (strchr(cmd, '/') || strchr(cmd, '\\'))

-   prog = xstrdup(cmd);
+   return xstrdup(cmd);
  
-	while (!prog && *path)

-   prog = lookup_prog(*path++, cmd, isexe, exe_only);
+   path = mingw_getenv("PATH");
+   if (!path)
+   return NULL;
+
+   for (; !prog && *path; path++) {
+   const char *sep = strchrnul(path, ';');
+   if (sep == path)
+   continue;
+   prog = lookup_prog(path, sep - path, cmd, isexe, exe_only);
+   path = sep;
+   }


The loop termination does not work here. When the final PATH component 
is investigated, sep points to the NUL. This pointer is assigned to 
path, which is incremented and now points one past NUL. Then the loop 
condition (*path) accesses the char behind NUL.


  
  	return prog;

  }
@@ -1569,13 +1527,10 @@ static pid_t mingw_spawnve_fd(const char *cmd, const 
char **argv, char **deltaen
}
  
  	if (getenv("GIT_STRACE_COMMANDS")) {

-   char **path = get_path_split();
-   char *p = path_lookup("strace.exe", path, 1);
+   char *p = path_lookup("strace.exe", 1);
if (!p) {
-   free_path_split(path);
return error("strace not found!");
}
-   free_path_split(path);
if (xutftowcs_path(wcmd, p) < 0) {
free(p);
return -1;


Upstream does not have this hunk.

Otherwise, good catch!

-- Hannes

Re: [PATCH v2 4/6] fsmonitor: add test cases for fsmonitor extension

[Torsten Bögershausen]

> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> new file mode 100755
> index 00..d3cffc758f
> --- /dev/null
> +++ b/t/t7519-status-fsmonitor.sh
> @@ -0,0 +1,153 @@
> +#!/bin/sh
> +
> +test_description='git status with file system watcher'
> +
> +. ./test-lib.sh
> +
> +clean_repo () {
> + git reset --hard HEAD
> + git clean -fd
> + rm marker -f

This Works under Linux, but not e.g. Mac OS X. Should be

rm -f marker

> +}

[PATCH] mingw: simplify PATH handling

[René Scharfe]

On Windows the environment variable PATH contains a semicolon-separated
list of directories to search for, in order, when looking for the
location of a binary to run.  get_path_split() parses it and returns an
array of string copies, which is iterated by path_lookup(), which in
turn passes each entry to lookup_prog().

Change lookup_prog() to take the directory name as a length-limited
string instead of as a NUL-terminated one and parse PATH directly in
path_lookup().  This avoids memory allocations, simplifying the code.

Signed-off-by: Rene Scharfe 
---
 compat/mingw.c | 96 ++
 1 file changed, 22 insertions(+), 74 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 5113071bc7..7bc61d4066 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1154,67 +1154,15 @@ static const char *parse_interpreter(const char *cmd)
 }
 
 /*
- * Splits the PATH into parts.
- */
-static char **get_path_split(void)
-{
-   char *p, **path, *envpath = mingw_getenv("PATH");
-   int i, n = 0;
-
-   if (!envpath || !*envpath)
-   return NULL;
-
-   envpath = xstrdup(envpath);
-   p = envpath;
-   while (p) {
-   char *dir = p;
-   p = strchr(p, ';');
-   if (p) *p++ = '\0';
-   if (*dir) { /* not earlier, catches series of ; */
-   ++n;
-   }
-   }
-   if (!n) {
-   free(envpath);
-   return NULL;
-   }
-
-   ALLOC_ARRAY(path, n + 1);
-   p = envpath;
-   i = 0;
-   do {
-   if (*p)
-   path[i++] = xstrdup(p);
-   p = p+strlen(p)+1;
-   } while (i < n);
-   path[i] = NULL;
-
-   free(envpath);
-
-   return path;
-}
-
-static void free_path_split(char **path)
-{
-   char **p = path;
-
-   if (!path)
-   return;
-
-   while (*p)
-   free(*p++);
-   free(path);
-}
-
-/*
  * exe_only means that we only want to detect .exe files, but not scripts
  * (which do not have an extension)
  */
-static char *lookup_prog(const char *dir, const char *cmd, int isexe, int 
exe_only)
+static char *lookup_prog(const char *dir, int dirlen, const char *cmd,
+int isexe, int exe_only)
 {
char path[MAX_PATH];
wchar_t wpath[MAX_PATH];
-   snprintf(path, sizeof(path), "%s\\%s.exe", dir, cmd);
+   snprintf(path, sizeof(path), "%.*s\\%s.exe", dirlen, dir, cmd);
 
if (xutftowcs_path(wpath, path) < 0)
return NULL;
@@ -1235,17 +1183,27 @@ static char *lookup_prog(const char *dir, const char 
*cmd, int isexe, int exe_on
  * Determines the absolute path of cmd using the split path in path.
  * If cmd contains a slash or backslash, no lookup is performed.
  */
-static char *path_lookup(const char *cmd, char **path, int exe_only)
+static char *path_lookup(const char *cmd, int exe_only)
 {
+   const char *path;
char *prog = NULL;
int len = strlen(cmd);
int isexe = len >= 4 && !strcasecmp(cmd+len-4, ".exe");
 
if (strchr(cmd, '/') || strchr(cmd, '\\'))
-   prog = xstrdup(cmd);
+   return xstrdup(cmd);
 
-   while (!prog && *path)
-   prog = lookup_prog(*path++, cmd, isexe, exe_only);
+   path = mingw_getenv("PATH");
+   if (!path)
+   return NULL;
+
+   for (; !prog && *path; path++) {
+   const char *sep = strchrnul(path, ';');
+   if (sep == path)
+   continue;
+   prog = lookup_prog(path, sep - path, cmd, isexe, exe_only);
+   path = sep;
+   }
 
return prog;
 }
@@ -1569,13 +1527,10 @@ static pid_t mingw_spawnve_fd(const char *cmd, const 
char **argv, char **deltaen
}
 
if (getenv("GIT_STRACE_COMMANDS")) {
-   char **path = get_path_split();
-   char *p = path_lookup("strace.exe", path, 1);
+   char *p = path_lookup("strace.exe", 1);
if (!p) {
-   free_path_split(path);
return error("strace not found!");
}
-   free_path_split(path);
if (xutftowcs_path(wcmd, p) < 0) {
free(p);
return -1;
@@ -1634,8 +1589,7 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, 
char **deltaenv,
 int fhin, int fhout, int fherr)
 {
pid_t pid;
-   char **path = get_path_split();
-   char *prog = path_lookup(cmd, path, 0);
+   char *prog = path_lookup(cmd, 0);
 
if (!prog) {
errno = ENOENT;
@@ -1646,7 +1600,7 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, 
char **deltaenv,
 
if (interpr) {
const char *argv0 = argv[0];
-   char *iprog = 

Re: [PATCH 01/15] handle_revision_arg: reset "dotdot" consistently

[Philip Oakley]

From: "Jeff King" 

When we are parsing a range like "a..b", we write a
temporary NUL over the first ".", so that we can access the
names "a" and "b" as C strings. But our restoration of the
original "." is done at inconsistent times, which can lead
to confusing results.

For most calls, we restore the "." after we resolve the
names, but before we call verify_non_filename().  This means
that when we later call add_pending_object(), the name for
the left-hand "a" has been re-expanded to "a..b". You can
see this with:

 git log --source a...b

where "b" will be correctly marked with "b", but "a" will be
marked with "a...b". Likewise with "a..b" (though you need
to use --boundary to even see "a" at all in that case).

To top off the confusion, when the REVARG_CANNOT_BE_FILENAME
flag is set, we skip the non-filename check, and leave the
NUL in place.

That means we do report the correct name for "a" in the
pending array. But some code paths try to show the whole
"a..b" name in error messages, and these erroneously show
only "a" instead of "a..b". E.g.:

 $ git cherry-pick HEAD:foo..HEAD:foo


shouldn't this be three dots? Also the para above uses two dot examples in 
its description but the paras before that start by describing the three dot 
case.

--
Philip

 error: object d95f3ad14dee633a758d2e331151e950dd13e4ed is a blob, not a 
commit
 error: object d95f3ad14dee633a758d2e331151e950dd13e4ed is a blob, not a 
commit

 fatal: Invalid symmetric difference expression HEAD:foo

(That last message should be "HEAD:foo...HEAD:foo"; I used
cherry-pick because it passes the CANNOT_BE_FILENAME flag).

As an interesting side note, cherry-pick actually looks at
and re-resolves the arguments from the pending->name fields.
So it would have been visibly broken by the first bug, but
the effect was canceled out by the second one.

This patch makes the whole function consistent by re-writing
the NUL immediately after calling verify_non_filename(), and
then restoring the "." as appropriate in some error-printing
and early-return code paths.

Signed-off-by: Jeff King 
---
I also considered just making a copy of the string rather than this
in-place munging (technically we get it as a pointer-to-const; it's only
the use of strstr() that lets us quietly drop the const). But it doesn't
really make the code any cleaner; now instead of restoring the dot you
have to remember to free() the string in each code path.

revision.c | 3 +++
t/t4202-log.sh | 9 +
2 files changed, 12 insertions(+)

diff --git a/revision.c b/revision.c
index 8a8c1789c..014bf52e3 100644
--- a/revision.c
+++ b/revision.c
@@ -1477,12 +1477,14 @@ int handle_revision_arg(const char *arg_, struct 
rev_info *revs, int flags, unsi

 if (!cant_be_filename) {
 *dotdot = '.';
 verify_non_filename(revs->prefix, arg);
+ *dotdot = '\0';
 }

 a_obj = parse_object(from_sha1);
 b_obj = parse_object(sha1);
 if (!a_obj || !b_obj) {
 missing:
+ *dotdot = '.';
 if (revs->ignore_missing)
 return 0;
 die(symmetric
@@ -1525,6 +1527,7 @@ int handle_revision_arg(const char *arg_, struct 
rev_info *revs, int flags, unsi

 REV_CMD_RIGHT, flags);
 add_pending_object(revs, a_obj, this);
 add_pending_object(revs, b_obj, next);
+ *dotdot = '.';
 return 0;
 }
 *dotdot = '.';
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index f57799071..6da1bbe91 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1380,4 +1380,13 @@ test_expect_success 'log --source paints tag names' 
'

 test_cmp expect actual
'

+test_expect_success 'log --source paints symmetric ranges' '
+ cat >expect <<-\EOF &&
+ 09e12a9 source-b three
+ 8e393e1 source-a two
+ EOF
+ git log --oneline --source source-a...source-b >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.13.0.219.g63f6bc368

Can You Handle This Project

[Mrdjibril Kaba]

I Am: Mr.Kaba Djibril


Attention Please,

This Message Might Meet You In Utmost Suprise. However It's Just My
Urgent Need For A Foreign Partner That Made Me Contact You For This
Transaction.

I Am A Banker By Profession From Guinea Conakry In West Africa And
Currently Holding The Post Of Director Auditing And Accounting Unit Of
The Bank.

I Have The Opportunity Of Transfering The Left Over Funds $4.7m (Four
Million Seven Hundredthousand Dollars) Of One Of My Bank Clients Who
Died Alongside With His Entire Family On 31 St, July 2000 In A Plane
Crash.

Hence, I Am Inviting You For A Mutual Business Opportunity Where This
Money Can Be Shared Between Us In The Ration Of 60/40? 40% For You And
60% For Me

If You Agree To My Business Proposal. Further Detail Of The
Transaction Will Be Forwarded
To You As Soon As I Receive Your Return Mail And Reply To Me Immediately.

Now My Questions Are:-

1 Can You Handle This Project?
2. Can I Give You This Trust?

Contact Me With My Direct E-Mail Adress djbrilka...@gmail.com

 Best Regards.
Mr.Kaba Djibril

Re: [PATCH v2 5/6] fsmonitor: add documentation for the fsmonitor extension.

[Ævar Arnfjörð Bjarmason]

On Thu, May 18, 2017 at 10:13 PM, Ben Peart  wrote:
> This includes the core.fsmonitor setting, the query-fsmonitor hook,
> and the fsmonitor index extension.
>
> Signed-off-by: Ben Peart 
> ---
>  Documentation/config.txt |  7 +++
>  Documentation/githooks.txt   | 23 +++
>  Documentation/technical/index-format.txt | 18 ++
>  3 files changed, 48 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 96e9cf8b73..4ffbf0d4c2 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -389,6 +389,13 @@ core.protectNTFS::
> 8.3 "short" names.
> Defaults to `true` on Windows, and `false` elsewhere.
>
> +core.fsmonitor::
> +   If set to true, call the query-fsmonitor hook proc which will
> +   identify all files that may have had changes since the last
> +   request. This information is used to speed up operations like
> +   'git commit' and 'git status' by limiting what git must scan to
> +   detect changes.
> +
>  core.trustctime::
> If false, the ctime differences between the index and the
> working tree are ignored; useful when the inode change time
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 706091a569..f7b4b4a844 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -448,6 +448,29 @@ The commits are guaranteed to be listed in the order 
> that they were
>  processed by rebase.
>
>
> +[[query-fsmonitor]]
> +query-fsmonitor
> +
> +
> +This hook is invoked when the configuration option core.fsmonitor is
> +set and git needs to identify changed or untracked files.  It takes
> +a single argument which is the time in elapsed seconds since midnight,
> +January 1, 1970.
> +
> +The hook should output to stdout the list of all files in the working
> +directory that may have changed since the requested time.  The logic
> +should be inclusive so that it does not miss any potential changes.
> +The paths should be relative to the root of the working directory
> +and be separated by a single NUL.
> +
> +Git will limit what files it checks for changes as well as which
> +directories are checked for untracked files based on the path names
> +given.
> +
> +The exit status determines whether git will use the data from the
> +hook to limit its search.  On error, it will fall back to verifying
> +all files and folders.
> +
>  GIT
>  ---
>  Part of the linkgit:git[1] suite
> diff --git a/Documentation/technical/index-format.txt 
> b/Documentation/technical/index-format.txt
> index ade0b0c445..b002d23c05 100644
> --- a/Documentation/technical/index-format.txt
> +++ b/Documentation/technical/index-format.txt
> @@ -295,3 +295,21 @@ The remaining data of each directory block is grouped by 
> type:
>  in the previous ewah bitmap.
>
>- One NUL.
> +
> +== File System Monitor cache
> +
> +  The file system monitor cache tracks files for which the query-fsmonitor
> +  hook has told us about changes.  The signature for this extension is
> +  { 'F', 'S', 'M', 'N' }.
> +
> +  The extension starts with
> +
> +  - 32-bit version number: the current supported version is 1.
> +
> +  - 64-bit time: the extension data reflects all changes through the given
> +   time which is stored as the seconds elapsed since midnight, January 
> 1, 1970.
> +
> +  - 32-bit bitmap size: the size of the CE_FSMONITOR_DIRTY bitmap.
> +
> +  - An ewah bitmap, the n-th bit indicates whether the n-th index entry
> +is CE_FSMONITOR_DIRTY.

We already have a uint64_t in one place in the codebase (getnanotime)
which uses a 64 bit time for nanosecond accuracy, and numerous
filesystems already support nanosecond timestamps (ext4, that new
Apple thingy...).

I don't know if any of the inotify/fsmonitor APIs support that yet,
but it seems inevitable that that'll be added if not, in some
pathological cases we can have a lot of files modified in 1 second, so
using nanosecond accuracy means there'll be a lot less data to
consider in some cases.

It does mean this'll only work until the year ~2500, but that seems
like an acceptable trade-off.

[PATCH v2 2/2] sha1dc: optionally use sha1collisiondetection as a submodule

[Ævar Arnfjörð Bjarmason]

Add an option to use the sha1collisiondetection library from the
submodule in sha1collisiondetection/ instead of in the copy in the
sha1dc/ directory.

This allows us to try out the submodule in sha1collisiondetection
without breaking the build for anyone who's not expecting them as we
work out any kinks.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 .gitmodules|  4 
 Makefile   | 12 
 hash.h |  4 
 sha1collisiondetection |  1 +
 4 files changed, 21 insertions(+)
 create mode 100644 .gitmodules
 create mode 16 sha1collisiondetection

diff --git a/.gitmodules b/.gitmodules
new file mode 100644
index 00..cbeebdab7a
--- /dev/null
+++ b/.gitmodules
@@ -0,0 +1,4 @@
+[submodule "sha1collisiondetection"]
+   path = sha1collisiondetection
+   url = https://github.com/cr-marcstevens/sha1collisiondetection.git
+   branch = master
diff --git a/Makefile b/Makefile
index ffa6da71b7..6baad1669e 100644
--- a/Makefile
+++ b/Makefile
@@ -144,6 +144,12 @@ all::
 # algorithm. This is slower, but may detect attempted collision attacks.
 # Takes priority over other *_SHA1 knobs.
 #
+# Define DC_SHA1_SUBMODULE in addition to DC_SHA1 to use the
+# sha1collisiondetection shipped as a submodule instead of the
+# non-submodule copy in sha1dc/. This is an experimental option used
+# by the git project to migrate to using sha1collisiondetection as a
+# submodule.
+#
 # Define OPENSSL_SHA1 environment variable when running make to link
 # with the SHA1 routine from openssl library.
 #
@@ -1412,8 +1418,14 @@ ifdef APPLE_COMMON_CRYPTO
BASIC_CFLAGS += -DSHA1_APPLE
 else
DC_SHA1 := YesPlease
+ifdef DC_SHA1_SUBMODULE
+   LIB_OBJS += sha1collisiondetection/lib/sha1.o
+   LIB_OBJS += sha1collisiondetection/lib/ubc_check.o
+   BASIC_CFLAGS += -DDC_SHA1_SUBMODULE
+else
LIB_OBJS += sha1dc/sha1.o
LIB_OBJS += sha1dc/ubc_check.o
+endif
BASIC_CFLAGS += \
-DSHA1_DC \
-DSHA1DC_NO_STANDARD_INCLUDES \
diff --git a/hash.h b/hash.h
index a11fc9233f..bef3e630a0 100644
--- a/hash.h
+++ b/hash.h
@@ -8,7 +8,11 @@
 #elif defined(SHA1_OPENSSL)
 #include 
 #elif defined(SHA1_DC)
+#ifdef DC_SHA1_SUBMODULE
+#include "sha1collisiondetection/lib/sha1.h"
+#else
 #include "sha1dc/sha1.h"
+#endif
 #else /* SHA1_BLK */
 #include "block-sha1/sha1.h"
 #endif
diff --git a/sha1collisiondetection b/sha1collisiondetection
new file mode 16
index 00..cc465543b3
--- /dev/null
+++ b/sha1collisiondetection
@@ -0,0 +1 @@
+Subproject commit cc465543b310e5f59a1d534381690052e8509b22
-- 
2.13.0.303.g4ebf302169

[PATCH v2 0/2] Update sha1dc from upstream & optionally make it a submodule

[Ævar Arnfjörð Bjarmason]

On Sat, May 20, 2017 at 1:13 PM, Junio C Hamano  wrote:
> Ævar Arnfjörð Bjarmason   writes:
>
>> Replace the forked sha1dc directory with a copy of the upstream code
>> imported as a submodule. This is the exact same code as now exists in
>> the sha1dc/ directory.
>>
>> The initial reason for copy/pasting the code into sha1dc and locally
>> modifying it was that it needed to be altered to work with the git
>> project.
>>
>> The upstream project has accepted my code changes to allow us to use
>> their code as-is, see the preceding commit for details. So import the
>> code as a submodule instead, this will make it easier to keep
>> up-to-date with any upstream fixes or improvements.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason 
>> ---
>>  .gitmodules| 4 
>>  Makefile   | 4 ++--
>>  hash.h | 2 +-
>>  sha1collisiondetection | 1 +
>>  4 files changed, 8 insertions(+), 3 deletions(-)
>>  create mode 100644 .gitmodules
>>  create mode 16 sha1collisiondetection
>
> I am not sure how prepared our .travis.yml is to deal with a
> submodule, I'd prefer to have this step broken down to two step
> process.
>
> That is, [PATCH 2.1/3] first adds an otherwise unused submodule, so
> that people can optionally do "git submodule init && git submodule
> update" so that they can compare the contents of sha1dc/ that has
> been updated by [PATCH 1/3] with the up-to-date upstream.  Then
> [PATCH 2.2/3] would update the Makefile and hash.h to use the code
> in the submodule.
>
> I actually would want to see us proceed even more cautiously---if
> the latter-half, i.e. [PATCH 2.2/3], is arranged so that it uses the
> new sha1collisiondetection/ only when the submodule is initialized
> and populated, and otherwise it uses sha1dc/ as before, I would feel
> a lot safer.  I wouldn't be this paranoid if this "let's start using
> submodule ourselves" were done to some optional corner (like compat/
> or contrib/ somewhere), but this is the default hash function.  I do
> want to have something like this to force us (and submodule folks)
> to get any kinks out, but I do not want to see many people not even
> be able to build while this new arrangement is eased in.  Once
> people are comfortable with the new arrangement to use code from
> submodule, we can then take [PATCH 3/3] to remove the old sha1dc/
> directory and the migration will be complete.

Makes sense to take it slow. Hopefully this addresses your comments. I
dropped the 3rd patch to remove sha1dc/ and the 2nd patch adds
sha1collisiondetection/ as submodule, but it's not used unless you
specify DC_SHA1_SUBMODULE in addition to DC_SHA1.

Both patches should be safe to include & not cause any disruption, but
now those interested in making the submodule experience in git.git
better can init/update & set DC_SHA1_SUBMODULE=Y to play with it.

Note that both patches update to a newer version of the upstream
code. I sent them another pull request with some cleanups, one of
which is to ignore .depends in their .gitignore file.

> I also am not very happy with .gitmodules pointing at a single point
> of failure.  It would be nice if you can arrange a couple of mirrors
> and have a comment in .gitmodules file to tell folks that they can
> use these alternates by insteadOf or some other mechanism.

I liked the suggestion to make the URL a relative path, but this would
require you to maintain a mirror in the same places you push git.git
to, is that something you'd be willing to do?

For now having no-mirror isn't a big issue with my new 2/2 since it'se
something you have to opt-in to with a build flag, which I suspect
only I/Brandon/Stefan & a few others will use.

Ævar Arnfjörð Bjarmason (2):
  sha1dc: update from upstream
  sha1dc: optionally use sha1collisiondetection as a submodule

 .gitmodules|  4 +++
 Makefile   | 21 +++-
 hash.h |  4 +++
 sha1collisiondetection |  1 +
 sha1dc/sha1.c  | 91 +-
 sha1dc/sha1.h  | 90 ++---
 sha1dc/ubc_check.c | 13 ++--
 sha1dc/ubc_check.h | 14 ++--
 sha1dc_git.c   | 24 +
 sha1dc_git.h   | 19 +++
 10 files changed, 193 insertions(+), 88 deletions(-)
 create mode 100644 .gitmodules
 create mode 16 sha1collisiondetection
 create mode 100644 sha1dc_git.c
 create mode 100644 sha1dc_git.h

-- 
2.13.0.303.g4ebf302169

[PATCH v2 1/2] sha1dc: update from upstream

[Ævar Arnfjörð Bjarmason]

Update sha1dc from the latest version by the upstream
maintainer[1].

This version includes a commit of mine which allows for replacing the
local modifications done to the upstream files in git.git with macro
definitions to monkeypatch it in place.

It also brings in a change[2] upstream made for the breakage 2.13.0
introduced on SPARC and other platforms that forbid unaligned
access[3].

This means that the code customizations done since the initial import
in commit 28dc98e343 ("sha1dc: add collision-detecting sha1
implementation", 2017-03-16) can be done purely via Makefile
definitions and by including the content of our own sha1dc_git.[ch] in
sha1dc/sha1.c via a macro.

1. 
https://github.com/cr-marcstevens/sha1collisiondetection/commit/cc465543b310e5f59a1d534381690052e8509b22
2. 
https://github.com/cr-marcstevens/sha1collisiondetection/commit/33a694a9ee1b79c24be45f9eab5ac0e1aeeaf271
3. "Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease
   being on by default"
   
(https://public-inbox.org/git/cacbzzx6nmkk8af0-upjckwv4r+hv-uk2xwxva5u+_uq3vxu...@mail.gmail.com/)

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Makefile   |  9 +-
 sha1dc/sha1.c  | 91 +++---
 sha1dc/sha1.h  | 90 +++--
 sha1dc/ubc_check.c | 13 ++--
 sha1dc/ubc_check.h | 14 +++--
 sha1dc_git.c   | 24 ++
 sha1dc_git.h   | 19 
 7 files changed, 172 insertions(+), 88 deletions(-)
 create mode 100644 sha1dc_git.c
 create mode 100644 sha1dc_git.h

diff --git a/Makefile b/Makefile
index e35542e631..ffa6da71b7 100644
--- a/Makefile
+++ b/Makefile
@@ -1414,7 +1414,14 @@ else
DC_SHA1 := YesPlease
LIB_OBJS += sha1dc/sha1.o
LIB_OBJS += sha1dc/ubc_check.o
-   BASIC_CFLAGS += -DSHA1_DC
+   BASIC_CFLAGS += \
+   -DSHA1_DC \
+   -DSHA1DC_NO_STANDARD_INCLUDES \
+   -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 \
+   -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" \
+   -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C="\"sha1dc_git.c\"" \
+   -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H="\"sha1dc_git.h\"" \
+   -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""
 endif
 endif
 endif
diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c
index 35e9dd5bf4..3dff80ac72 100644
--- a/sha1dc/sha1.c
+++ b/sha1dc/sha1.c
@@ -5,9 +5,23 @@
 * https://opensource.org/licenses/MIT
 ***/
 
-#include "cache.h"
-#include "sha1dc/sha1.h"
-#include "sha1dc/ubc_check.h"
+#ifndef SHA1DC_NO_STANDARD_INCLUDES
+#include 
+#include 
+#include 
+#include 
+#endif
+
+#ifdef SHA1DC_CUSTOM_INCLUDE_SHA1_C
+#include SHA1DC_CUSTOM_INCLUDE_SHA1_C
+#endif
+
+#ifndef SHA1DC_INIT_SAFE_HASH_DEFAULT
+#define SHA1DC_INIT_SAFE_HASH_DEFAULT 1
+#endif
+
+#include "sha1.h"
+#include "ubc_check.h"
 
 
 /*
@@ -18,16 +32,31 @@
If you are compiling on a big endian platform and your compiler does not 
define one of these,
you will have to add whatever macros your tool chain defines to indicate 
Big-Endianness.
  */
-#if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \
+#ifdef SHA1DC_BIGENDIAN
+#undef SHA1DC_BIGENDIAN
+#endif
+#if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \
+((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \
 (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \
-defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) ||  
defined(__AARCH64EB__) || \
-defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__)
+defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || 
defined(__THUMBEB__) ||  defined(__AARCH64EB__) || \
+defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__) || 
defined(SHA1DC_FORCE_BIGENDIAN))
+
+#define SHA1DC_BIGENDIAN
 
-#define SHA1DC_BIGENDIAN   1
-#else
-#undef SHA1DC_BIGENDIAN
 #endif /*ENDIANNESS SELECTION*/
 
+#if (defined SHA1DC_FORCE_UNALIGNED_ACCESS || \
+ defined(__amd64__) || defined(__amd64) || defined(__x86_64__) || 
defined(__x86_64) || \
+ defined(i386) || defined(__i386) || defined(__i386__) || 
defined(__i486__)  || \
+ defined(__i586__) || defined(__i686__) || defined(_M_IX86) || 
defined(__X86__) || \
+ defined(_X86_) || defined(__THW_INTEL__) || defined(__I86__) || 
defined(__INTEL__) || \
+ defined(__386) || defined(_M_X64) || defined(_M_AMD64))
+
+#define SHA1DC_ALLOW_UNALIGNED_ACCESS
+
+#endif /*UNALIGNMENT DETECTION*/
+
+
 #define rotate_right(x,n) (((x)>>(n))|((x)<<(32-(n
 #define rotate_left(x,n)  (((x)<<(n))|((x)>>(32-(n
 
@@ -36,11 +65,11 @@
 
 #define sha1_mix(W, t)  (rotate_left(W[t - 3] ^ W[t - 8] ^ W[t - 14] ^ W[t - 
16], 1))
 
-#if defined(SHA1DC_BIGENDIAN)
+#ifdef SHA1DC_BIGENDIAN
#define sha1_load(m, t, temp)  { temp = m[t]; }
 #else
#define sha1_load(m, t, temp)  { temp = m[t]; sha1_bswap32(temp); }
-#endif /* 

Re: [PATCH v2 5/6] fsmonitor: add documentation for the fsmonitor extension.

[Junio C Hamano]

Ben Peart  writes:

> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 706091a569..f7b4b4a844 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -448,6 +448,29 @@ The commits are guaranteed to be listed in the order 
> that they were
>  processed by rebase.
>  
>  
> +[[query-fsmonitor]]
> +query-fsmonitor
> +

This underline is short by 3 '~' for the string it underlines.

Re: [PATCH 2/3] sha1dc: use sha1collisiondetection as a submodule

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> Replace the forked sha1dc directory with a copy of the upstream code
> imported as a submodule. This is the exact same code as now exists in
> the sha1dc/ directory.
>
> The initial reason for copy/pasting the code into sha1dc and locally
> modifying it was that it needed to be altered to work with the git
> project.
>
> The upstream project has accepted my code changes to allow us to use
> their code as-is, see the preceding commit for details. So import the
> code as a submodule instead, this will make it easier to keep
> up-to-date with any upstream fixes or improvements.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  .gitmodules| 4 
>  Makefile   | 4 ++--
>  hash.h | 2 +-
>  sha1collisiondetection | 1 +
>  4 files changed, 8 insertions(+), 3 deletions(-)
>  create mode 100644 .gitmodules
>  create mode 16 sha1collisiondetection

I am not sure how prepared our .travis.yml is to deal with a
submodule, I'd prefer to have this step broken down to two step
process.

That is, [PATCH 2.1/3] first adds an otherwise unused submodule, so
that people can optionally do "git submodule init && git submodule
update" so that they can compare the contents of sha1dc/ that has
been updated by [PATCH 1/3] with the up-to-date upstream.  Then
[PATCH 2.2/3] would update the Makefile and hash.h to use the code
in the submodule.

I actually would want to see us proceed even more cautiously---if
the latter-half, i.e. [PATCH 2.2/3], is arranged so that it uses the
new sha1collisiondetection/ only when the submodule is initialized
and populated, and otherwise it uses sha1dc/ as before, I would feel
a lot safer.  I wouldn't be this paranoid if this "let's start using
submodule ourselves" were done to some optional corner (like compat/
or contrib/ somewhere), but this is the default hash function.  I do
want to have something like this to force us (and submodule folks)
to get any kinks out, but I do not want to see many people not even
be able to build while this new arrangement is eased in.  Once
people are comfortable with the new arrangement to use code from
submodule, we can then take [PATCH 3/3] to remove the old sha1dc/
directory and the migration will be complete.

I also am not very happy with .gitmodules pointing at a single point
of failure.  It would be nice if you can arrange a couple of mirrors
and have a comment in .gitmodules file to tell folks that they can
use these alternates by insteadOf or some other mechanism.

Thanks.

Re: [PATCH v2 3/6] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.

[Junio C Hamano]

Ben Peart  writes:

> After sending this, I noticed that using a different compiler
> generated a couple of warnings I should fix.  I'm assuming I'll need
> another re-roll but if not, here are the changes that will be squashed
> in.
>
> Ben

Thanks, in addition, I am missing the definition of TRUE and FALSE.

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Junio C Hamano]

manishea...@gmail.com writes:

> +int for_each_worktree_ref(each_ref_fn fn, void *cb_data)
> +{
> + int i, flag, retval = 0;
> + struct object_id oid;
> + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED);
> + struct commit* commit;
> + for (i = 0; worktrees[i]; i++) {
> + if ((commit = 
> lookup_commit_reference(worktrees[i]->head_sha1))) {
> + oid = commit->object.oid;
> + if (!read_ref_full("HEAD", RESOLVE_REF_READING, 
> oid.hash, )) {
> + if ((retval = fn("HEAD", , flag, cb_data)))
> + return retval;
> + }
> + }
> + }
> + return retval;
> +}

I would have expected for-each-worktree-ref to iterate over all the
refs in a given worktree, but that is not what this does.  This
instead iterates over worktrees and shows only their HEAD ref, no
other refs.  This helper is somewhat misnamed.

By the way, doesn't nd/prune-in-worktree topic that has been cooking
in 'pu' supersede this change?  It not just protects the commit at
the tip of HEAD in each worktree, it also makes sure the ones in
HEAD's reflog are not prematurely pruned.

Thanks.

[PATCH] revision.c: ignore broken tags with ignore_missing_links

[Jeff King]

When peeling a tag for prepare_revision_walk(), we do not
respect the ignore_missing_links flag. This can lead to a
bogus error when pack-objects walks the possibly-broken
unreachable-but-recent part of the object graph.

The other link-following all happens via traverse_commit_list(),
which explains why this case was missed. And our tests
covered only broken links from commits. Let's be more
comprehensive and cover broken tree entries (which do work)
and tags (which shows off this bug).

Signed-off-by: Jeff King 
---
So here's the fix I showed earlier polished up as a real patch. I still
think your patch to improve the error message is a good suggestion. I
was going to just re-send it out on top of this, but I realized it was
missing your signoff. Do you want to resend?

 revision.c |  2 +-
 t/t6501-freshen-objects.sh | 27 ++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 8a8c1789c..610638f2e 100644
--- a/revision.c
+++ b/revision.c
@@ -230,7 +230,7 @@ static struct commit *handle_commit(struct rev_info *revs,
die("bad tag");
object = parse_object(tag->tagged->oid.hash);
if (!object) {
-   if (flags & UNINTERESTING)
+   if (revs->ignore_missing_links || (flags & 
UNINTERESTING))
return NULL;
die("bad object %s", oid_to_hex(>tagged->oid));
}
diff --git a/t/t6501-freshen-objects.sh b/t/t6501-freshen-objects.sh
index cf076dcd9..394b169ea 100755
--- a/t/t6501-freshen-objects.sh
+++ b/t/t6501-freshen-objects.sh
@@ -129,7 +129,7 @@ for repack in '' true; do
'
 done
 
-test_expect_success 'do not complain about existing broken links' '
+test_expect_success 'do not complain about existing broken links (commit)' '
cat >broken-commit <<-\EOF &&
tree 0001
parent 0002
@@ -144,4 +144,29 @@ test_expect_success 'do not complain about existing broken 
links' '
test_must_be_empty stderr
 '
 
+test_expect_success 'do not complain about existing broken links (tree)' '
+   cat >broken-tree <<-\EOF &&
+   100644 blob 0003foo
+   EOF
+   tree=$(git mktree --missing stderr &&
+   git cat-file -e $tree &&
+   test_must_be_empty stderr
+'
+
+test_expect_success 'do not complain about existing broken links (tag)' '
+   cat >broken-tag <<-\EOF &&
+   object 0004
+   type commit
+   tag broken
+   tagger whatever  1234 -
+
+   this is a broken tag
+   EOF
+   tag=$(git hash-object -t tag -w broken-tag) &&
+   git gc 2>stderr &&
+   git cat-file -e $tag &&
+   test_must_be_empty stderr
+'
+
 test_done
-- 
2.13.0.234.g029c1392a

Re: die("bad object.. for duplicate tagged tag in remote

[Jeff King]

On Fri, May 19, 2017 at 06:28:56PM +0100, Chris West wrote:

> If you have an annotated tag of an annotated tag, and `remote update`
> elects not to fetch this tag (perhaps because it has a name collision
> locally), then the repo ends up corrupt: you can't gc it, but fsck
> doesn't notice.
> 
> Two repos, named "bad" and "good":
> [...]

What version of git are you using? If I run this script:

-- >8 --
#!/bin/sh

rm -rf good bad

git init bad
cd bad
git commit --allow-empty -m bad
git tag -m 'bad inner' inner
git tag -m 'bad outer' outer inner
outer=$(git -C ../bad rev-parse outer)
inner=$(git -C ../bad rev-parse inner)
git tag -d inner
cd ..

git init good
cd good
git commit --allow-empty -m good
git tag -m 'good outer' outer
git remote add bad ../bad
git fetch bad

echo "===> outer is $outer"
git cat-file tag $outer

echo "===> inner is $inner"
git cat-file tag $inner
-- >8 --

then prior to Git v2.10.1, the final cat-file fails. But after it is
fine. This is due to b773ddea2 (pack-objects: walk tag chains for
--include-tag, 2016-09-05), which dealt with this exact tag-of-tag case.

In the real world, it would depend on which version of Git the server is
running (the fix is on the pack-objects side).

There's another interesting thing going on with the fsck/gc thing,
though. The fetching repo isn't actually corrupt. The guarantee that Git
makes is that we have the complete graph of anything that's reachable
from a ref, not that we might not have stray objects (though it does try
to avoid breaking even unreachable parts of history, as I'll explain in
a moment). And that's what's happening here; the client gets "outer" but
it's not actually reachable.

So what happens in your case in more detail is:

  1. git-fetch sends the include-tag capability to the server, asking it
 to include tags that point to whatever we're fetching (master in
 this case)

  2. The server sees that "outer" eventually points to what the client
 is fetching and adds it. It doesn't do the same for "inner" because
 it no longer has a tag ref pointing at it. And because it is
 pre-v2.10.1, it doesn't walk the full chain of "outer", and so
 never considers "inner" at all.

  3. The next step would normally be for git-fetch to realize that it's
 missing an object and backfill tags with a followup request. But
 since it already has its own unrelated "outer", it knows it doesn't
 need "outer" and doesn't bother looking at it.

 So now we have "outer" in the object store (because the server
 thought we might need it), but we never actually pointed a ref at
 it.

And that explains your fsck result:

> $ git fsck
> ...
> dangling tag 07070...

We have the object, but nobody points to it.

> I actually don't get that on the real repo, but do on this testcase. Hmm.
> `git fsck` exits with success here. This is bad, as the object graph is
> incomplete?

No, that outcome is correct. The interesting thing is that your
real-world case probably _does_ have a ref pointing at it (if it's not
getting a dangling-tag). I don't know how that got there, though. The
original case that motivated the fix in v2.10.1 was cloning with a
single branch, like:

  git clone --single-branch --no-local bad broken

but that results in the clone failing, not a corrupt repo. Is it
possible you or somebody else then ran something like:

  git update-ref refs/tags/other-outer $outer_sha1

after the fetch? That would reference the broken part of the graph, and
the repository is corrupt at that point.

> $ git gc
> fatal: bad object 03030303...
> error: failed to run repack

So this is where I think there might be room for improvement, even with
current versions of git.  Traditionally, we wouldn't try to traverse or
pack that unreferenced part of the object graph. But since v2.2.0, we
traverse any objects that are "recent" (within the 2-week
prune-expiration timestamp) to try to keep whole chunks of the graph
intact (ironically, to prevent problems like the update-ref I showed
above).

We use the "ignore_missing_links" flag to tell the traversal that this
is best-effort (i.e., we try to retain unreachable history if we can,
but if it's already broken there's nothing we can do). So I wouldn't be
surprised to find that we correctly respect that flag when following
parent and tree links, but not in tags.

> diff --git a/revision.c b/revision.c
> index 8a8c178..22b6021 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -232,7 +232,8 @@ static struct commit *handle_commit(struct rev_info *revs,
>   if (!object) {
>   if (flags & UNINTERESTING)
>   return NULL;
> - die("bad object %s", oid_to_hex(>tagged->oid));
> + die("bad tagged object %s in %s", 
> oid_to_hex(>tagged->oid),
> + oid_to_hex(>object.oid));
>   }

I agree this is an improvement. And that "if" in the context 

Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary

[Ævar Arnfjörð Bjarmason]

On Fri, May 19, 2017 at 10:54 PM, Dennis Kaarsemaker
 wrote:
> Second ping. This problem is not going away, so if this solution is not
> acceptable, I'd like to know what needs to be improved.

FWIW:

Reviewed-by: Ævar Arnfjörð Bjarmason 

> On Thu, 2017-05-04 at 09:01 +0200, Dennis Kaarsemaker wrote:
>> Ping. It's a little over a month since I sent this, but I haven't seen
>> any comments. Is this commit good to go?
>>
>> On Fri, 2017-03-24 at 22:37 +0100, Dennis Kaarsemaker wrote:
>> > Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine
>> > since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28
>> > isn't that old yet, keep the old code in place and use it when
>> > necessary.
>> >
>> > While we're in the area, mark some messages for translation that were
>> > not yet marked as such.
>> >
>> > Signed-off-by: Dennis Kaarsemaker 
>> > ---
>> >  git-send-email.perl | 54 
>> > ++---
>> >  1 file changed, 35 insertions(+), 19 deletions(-)
>> >
>> > diff --git a/git-send-email.perl b/git-send-email.perl
>> > index eea0a517f7..0d90439d9a 100755
>> > --- a/git-send-email.perl
>> > +++ b/git-send-email.perl
>> > @@ -1353,10 +1353,12 @@ EOF
>> > die __("The required SMTP server is not properly 
>> > defined.")
>> > }
>> >
>> > +   require Net::SMTP;
>> > +   my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
>> > version->parse("1.28");
>> > +   $smtp_domain ||= maildomain();
>> > +
>> > if ($smtp_encryption eq 'ssl') {
>> > $smtp_server_port ||= 465; # ssmtp
>> > -   require Net::SMTP::SSL;
>> > -   $smtp_domain ||= maildomain();
>> > require IO::Socket::SSL;
>> >
>> > # Suppress "variable accessed once" warning.
>> > @@ -1368,34 +1370,48 @@ EOF
>> > # Net::SMTP::SSL->new() does not forward any SSL 
>> > options
>> > IO::Socket::SSL::set_client_defaults(
>> > ssl_verify_params());
>> > -   $smtp ||= Net::SMTP::SSL->new($smtp_server,
>> > - Hello => $smtp_domain,
>> > - Port => 
>> > $smtp_server_port,
>> > - Debug => 
>> > $debug_net_smtp);
>> > +
>> > +   if ($use_net_smtp_ssl) {
>> > +   require Net::SMTP::SSL;
>> > +   $smtp ||= Net::SMTP::SSL->new($smtp_server,
>> > + Hello => 
>> > $smtp_domain,
>> > + Port => 
>> > $smtp_server_port,
>> > + Debug => 
>> > $debug_net_smtp);
>> > +   }
>> > +   else {
>> > +   $smtp ||= Net::SMTP->new($smtp_server,
>> > +Hello => $smtp_domain,
>> > +Port => 
>> > $smtp_server_port,
>> > +Debug => 
>> > $debug_net_smtp,
>> > +SSL => 1);
>> > +   }
>> > }
>> > else {
>> > -   require Net::SMTP;
>> > -   $smtp_domain ||= maildomain();
>> > $smtp_server_port ||= 25;
>> > $smtp ||= Net::SMTP->new($smtp_server,
>> >  Hello => $smtp_domain,
>> >  Debug => $debug_net_smtp,
>> >  Port => $smtp_server_port);
>> > if ($smtp_encryption eq 'tls' && $smtp) {
>> > -   require Net::SMTP::SSL;
>> > -   $smtp->command('STARTTLS');
>> > -   $smtp->response();
>> > -   if ($smtp->code == 220) {
>> > +   if ($use_net_smtp_ssl) {
>> > +   $smtp->command('STARTTLS');
>> > +   $smtp->response();
>> > +   if ($smtp->code != 220) {
>> > +   die sprintf(__("Server does 
>> > not support STARTTLS! %s"), $smtp->message);
>> > +   }
>> > +   require Net::SMTP::SSL;
>> > $smtp = 
>> > Net::SMTP::SSL->start_SSL($smtp,
>> >   
>> > ssl_verify_params())
>> > -   or die 

Re: persistent-https, url insteadof, and `git submodule`

[Jeff King]

On Fri, May 19, 2017 at 11:55:34PM +0200, Dennis Kaarsemaker wrote:

> > > Presumably this isn't intended behaviour?
> > 
> > It actually is. git-submodule sets GIT_PROTOCOL_FROM_USER to 0, which
> > makes git not trust any urls except http(s), git, ssh and file urls
> > unless you explicitely configure git to allow it. See the
> > GIT_ALLOW_PROTOCOL section in man git and the git-config section it
> > links to.
> 
> 33cfccbbf3 (submodule: allow only certain protocols for submodule
> fetches, 2015-09-16) says:
> [...]
> But doing it this way is
> simpler, and makes it much less likely that we would miss a
> case. And since such protocols should be an exception
> (especially because nobody who clones from them will be able
> to update the submodules!), it's not likely to inconvenience
> anyone in practice.

Yeah, I think the use of "insteadOf" here is a good counter-example to
the reasoning in that commit message. The submodule itself has a vanilla
protocol, so most users wouldn't need to configure anything. But
somebody who has done a blanket insteadOf now needs to explicitly allow
the protocol, too.

So one workaround is just adding:

  [protocol "persistent-https"]
  allow = always

next to the insteadOf config. And maybe that's enough. It's a little
inconvenient, but it the user has to configure something either way. And
it does give you some flexibility in deciding whether submodules get
access to their special remote helper.

The other approach is to declare that a url rewrite resets the
protocol-from-user flag to 1. IOW, since the "persistent-https" protocol
comes from our local config, it's not dangerous and we should behave as
if the user themselves gave it to us. That makes Elliott's case work out
of the box.

-Peff

Re: [PATCH] Remove useless assignments

[Jeff King]

On Sat, May 20, 2017 at 05:52:16AM +, Дилян Палаузов wrote:

> Signed-off-by: Дилян Палаузов 

I assume this is just going through the results of clang's static
analyzer and quieting it. I think most of these are not the right fix,
though, as I discussed in

  
http://public-inbox.org/git/20170225223146.ixubnwqkfol5q...@sigill.intra.peff.net/

For instance:

> diff --git a/archive.c b/archive.c
> index 60b889198..e2534d186 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -503,7 +503,7 @@ int write_archive(int argc, const char **argv, const char 
> *prefix,
>   init_tar_archiver();
>   init_zip_archiver();
>  
> - argc = parse_archive_args(argc, argv, , , name_hint, remote);
> + parse_archive_args(argc, argv, , , name_hint, remote);
>   if (!startup_info->have_repository) {
>   /*
>* We know this will die() with an error, so we could just

It's true that nobody accesses argc after this, so the assignment is
dead. But if we _don't_ assign to argc, we're leaving a maintenance trap
for somebody who later does want to access it. Its value must match that
what is in argv, and after your patch that invariant no longer holds.

Ditto for all of the argc hunks in this patch.

> diff --git a/builtin/merge.c b/builtin/merge.c
> index 703827f00..337efef6f 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -795,7 +795,7 @@ static int merge_trivial(struct commit *head, struct 
> commit_list *remoteheads)
>   write_tree_trivial(_tree);
>   printf(_("Wonderful.\n"));
>   pptr = commit_list_append(head, pptr);
> - pptr = commit_list_append(remoteheads->item, pptr);
> + commit_list_append(remoteheads->item, pptr);
>   prepare_to_commit(remoteheads);
>   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
>   result_commit.hash, NULL, sign_commit))

This one is similar. The return value of the append function updates the
tail pointer. Nobody appends to the list, so we never look at the tail
pointer again. But it has violated the tail-pointer invariant, and is
setting a trap for somebody who tries to append to the list later.

> diff --git a/builtin/notes.c b/builtin/notes.c
> index 7b891471c..6ec753383 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -1015,7 +1015,7 @@ int cmd_notes(int argc, const char **argv, const char 
> *prefix)
>   else if (!strcmp(argv[0], "get-ref"))
>   result = get_ref(argc, argv, prefix);
>   else {
> - result = error(_("unknown subcommand: %s"), argv[0]);
> + error(_("unknown subcommand: %s"), argv[0]);
>   usage_with_options(git_notes_usage, options);
>   }
>  

This one actually seems reasonable (usage_with_options exits, so we
don't need to bother setting result).

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 0bb36d584..0fa779103 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -498,7 +498,7 @@ static const char *check_nonce(const char *buf, size_t 
> len)
>   char *nonce = find_header(buf, len, "nonce");
>   timestamp_t stamp, ostamp;
>   char *bohmac, *expect = NULL;
> - const char *retval = NONCE_BAD;
> + const char *retval;
>  
>   if (!nonce) {
>   retval = NONCE_MISSING;

I have mixed feeling on this one. It's true that the NONCE_BAD value is
never used. But it's a defensive programming measure to set the default
in case we add code paths that do not set retval. If we could trust the
compiler to tell us when the uninitialized value was used, that would
let us avoid that situation. But the maybe-uninitialized warnings have
historically been one of the least trustworthy warnings we've seen (most
of the compiler-warning workarounds we've done have been for that).

> diff --git a/fsck.c b/fsck.c
> index d589341cd..35d421c08 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -703,7 +703,6 @@ static int fsck_ident(const char **ident, struct object 
> *obj, struct fsck_option
>   !isdigit(p[4]) ||
>   (p[5] != '\n'))
>   return report(options, obj, FSCK_MSG_BAD_TIMEZONE, "invalid 
> author/committer line - bad time zone");
> - p += 6;
>   return 0;
>  }

Another "invariant" one. Anybody adding more fsck checks would be
surprised when "p" is not updated past the last check.

> diff --git a/ref-filter.c b/ref-filter.c
> index 1fc5e9970..2d06b98ce 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1634,10 +1634,6 @@ static int match_name_as_path(const struct ref_filter 
> *filter, const char *refna
>  {
>   const char **pattern = filter->name_patterns;
>   int namelen = strlen(refname);
> - unsigned flags = WM_PATHNAME;
> -
> - if (filter->ignore_case)
> - flags |= WM_CASEFOLD;
>  
>   for (; *pattern; pattern++) {
>   const char *p = *pattern;

I think this one is a real bug, but your fix is going in the wrong
direction. 

Re: [Bug] git branch -v has problems with carriage returns

[Johannes Sixt]

Am 19.05.2017 um 23:55 schrieb Atousa Duprat:

I have tried to repro this issue but git goes out of its way to store
the commit messages using unix end-of-line format.
I think that git itself cannot create a repo exhibiting this problem.


Here is a recipe to reproduce the error:

  git init
  git commit --allow-empty -m initial
  git branch crlf $(printf '%s\r\n' subject '' line3_long line4 |
   git commit-tree HEAD:)

The reason for the "bug" is obviously that a line having CR in addition 
to LF is not "an empty line". Consequently, the second line is not 
treated as a separator between subject and body, whereupon Git 
concatenates all lines into one large subject line. This strips the LFs 
but leaves the CRs in tact, which, when printed on a terminal move the 
cursor to the beginning of the line, so that text after the CRs 
overwrites what is already in the terminal.


This is just to give you a head start. I'm not going to look into this.

-- Hannes


If I do `git branch -v` with such a subject line somehow the third and
second line get combined before the hash. Example:

$ git branch -v
See merge request ! temp space 84e18d22fd Merge branch
'feature-XXX' into 'develop'
#

Before git v2.13.0 `git branch -v` worked completely normal.

[PATCH 0/2] Fix warnings on access of a remote with Windows paths

[Johannes Sixt]

This small series fixes these warnings on Windows:

C:\Temp\gittest>git fetch C:\Temp\gittest
warning: unable to access '.git/remotes/C:\Temp\gittest': Invalid argument
warning: unable to access '.git/branches/C:\Temp\gittest': Invalid argument
>From C:\Temp\gittest
 * branchHEAD   -> FETCH_HEAD

The fix is in the second patch; the first patch is a
preparation that allows to write the fix in my preferred style.

Johannes Sixt (2):
  mingw.h: permit arguments with side effects for is_dir_sep
  Windows: do not treat a path with backslashes as a remote's nick name

 compat/mingw.h | 6 +-
 remote.c   | 5 -
 2 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.13.0.55.g17b7d13330

[PATCH 2/2] Windows: do not treat a path with backslashes as a remote's nick name

[Johannes Sixt]

Fetching from a remote using a native Windows path produces these warnings:

C:\Temp\gittest>git fetch C:\Temp\gittest
warning: unable to access '.git/remotes/C:\Temp\gittest': Invalid argument
warning: unable to access '.git/branches/C:\Temp\gittest': Invalid argument
>From C:\Temp\gittest
 * branchHEAD   -> FETCH_HEAD

The functions that read the branches and remotes files are protected by
a valid_remote_nick() check. Its implementation does not take Windows
paths into account, so that the caller simply concatenates the paths,
leading to the error seen above.

Use is_dir_sep() to check for both slashes and backslashes on Windows.

Signed-off-by: Johannes Sixt 
---
 remote.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 9584af366d..9501357c06 100644
--- a/remote.c
+++ b/remote.c
@@ -649,7 +649,10 @@ static int valid_remote_nick(const char *name)
 {
if (!name[0] || is_dot_or_dotdot(name))
return 0;
-   return !strchr(name, '/'); /* no slash */
+   while (*name)
+   if (is_dir_sep(*name++))/* no slash */
+   return 0;
+   return 1;
 }
 
 const char *remote_for_branch(struct branch *branch, int *explicit)
-- 
2.13.0.55.g17b7d13330

[PATCH 1/2] mingw.h: permit arguments with side effects for is_dir_sep

[Johannes Sixt]

The implementation of is_dir_sep in git-compat-util.h uses an inline
function. Use one also for the implementation in compat/mingw.h to support
non-trivial argument expressions.

Signed-off-by: Johannes Sixt 
---
 compat/mingw.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index cdc112526a..5e8447019b 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -398,7 +398,11 @@ HANDLE winansi_get_osfhandle(int fd);
(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
 int mingw_skip_dos_drive_prefix(char **path);
 #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
-#define is_dir_sep(c) ((c) == '/' || (c) == '\\')
+static inline int mingw_is_dir_sep(int c)
+{
+   return c == '/' || c == '\\';
+}
+#define is_dir_sep mingw_is_dir_sep
 static inline char *mingw_find_last_dir_sep(const char *path)
 {
char *ret = NULL;
-- 
2.13.0.55.g17b7d13330

[PATCH] Remove useless assignments

[Дилян Палаузов]

Signed-off-by: Дилян Палаузов 

diff --git a/archive.c b/archive.c
index 60b889198..e2534d186 100644
--- a/archive.c
+++ b/archive.c
@@ -503,7 +503,7 @@ int write_archive(int argc, const char **argv, const char 
*prefix,
init_tar_archiver();
init_zip_archiver();
 
-   argc = parse_archive_args(argc, argv, , , name_hint, remote);
+   parse_archive_args(argc, argv, , , name_hint, remote);
if (!startup_info->have_repository) {
/*
 * We know this will die() with an error, so we could just
diff --git a/builtin/add.c b/builtin/add.c
index 9f53f020d..fc1481273 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -211,7 +211,7 @@ static int edit_patch(int argc, const char **argv, const 
char *prefix)
init_revisions(, prefix);
rev.diffopt.context = 7;
 
-   argc = setup_revisions(argc, argv, , NULL);
+   setup_revisions(argc, argv, , NULL);
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
rev.diffopt.use_color = 0;
DIFF_OPT_SET(, IGNORE_DIRTY_SUBMODULES);
diff --git a/builtin/help.c b/builtin/help.c
index 49f7a07f8..3d24b084e 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -453,7 +453,7 @@ int cmd_help(int argc, const char **argv, const char 
*prefix)
int nongit;
enum help_format parsed_help_format;
 
-   argc = parse_options(argc, argv, prefix, builtin_help_options,
+   parse_options(argc, argv, prefix, builtin_help_options,
builtin_help_usage, 0);
parsed_help_format = help_format;
 
diff --git a/builtin/merge.c b/builtin/merge.c
index 703827f00..337efef6f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -795,7 +795,7 @@ static int merge_trivial(struct commit *head, struct 
commit_list *remoteheads)
write_tree_trivial(_tree);
printf(_("Wonderful.\n"));
pptr = commit_list_append(head, pptr);
-   pptr = commit_list_append(remoteheads->item, pptr);
+   commit_list_append(remoteheads->item, pptr);
prepare_to_commit(remoteheads);
if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
result_commit.hash, NULL, sign_commit))
diff --git a/builtin/notes.c b/builtin/notes.c
index 7b891471c..6ec753383 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -1015,7 +1015,7 @@ int cmd_notes(int argc, const char **argv, const char 
*prefix)
else if (!strcmp(argv[0], "get-ref"))
result = get_ref(argc, argv, prefix);
else {
-   result = error(_("unknown subcommand: %s"), argv[0]);
+   error(_("unknown subcommand: %s"), argv[0]);
usage_with_options(git_notes_usage, options);
}
 
diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index c026299e7..73f424d9f 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -59,7 +59,7 @@ int cmd_prune_packed(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
-   argc = parse_options(argc, argv, prefix, prune_packed_options,
+   parse_options(argc, argv, prefix, prune_packed_options,
 prune_packed_usage, 0);
 
prune_packed_objects(opts);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0bb36d584..0fa779103 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -498,7 +498,7 @@ static const char *check_nonce(const char *buf, size_t len)
char *nonce = find_header(buf, len, "nonce");
timestamp_t stamp, ostamp;
char *bohmac, *expect = NULL;
-   const char *retval = NONCE_BAD;
+   const char *retval;
 
if (!nonce) {
retval = NONCE_MISSING;
diff --git a/builtin/reset.c b/builtin/reset.c
index fc3b906c4..f547efd8d 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -291,7 +291,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
 
git_config(git_default_config, NULL);
 
-   argc = parse_options(argc, argv, prefix, options, git_reset_usage,
+   parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
parse_args(, argv, prefix, patch_mode, );
 
diff --git a/diffcore-rename.c b/diffcore-rename.c
index f7444c86b..632e19089 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -580,7 +580,7 @@ void diffcore_rename(struct diff_options *options)
 
rename_count += find_renames(mx, dst_cnt, minimum_score, 0);
if (detect_rename == DIFF_DETECT_COPY)
-   rename_count += find_renames(mx, dst_cnt, minimum_score, 1);
+   find_renames(mx, dst_cnt, minimum_score, 1);
free(mx);
 
  cleanup:
diff --git a/fast-import.c b/fast-import.c
index cf58f875b..0369363b2 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2054,7 +2054,7 @@ static int validate_raw_date(const char *src, struct 
strbuf *result)
 
errno