{"thread":{"id":"45955","subject":"[PATCH v2 00/29] Easy to review grep & pre-PCRE changes","startedAt":"2017-05-13T23:15:38Z","lastAt":"2017-05-16T00:50:49Z","messageCount":42,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":29},"messages":[{"id":"319632","messageId":"20170513231509.7834-1-avarab@gmail.com","threadId":"45955","inReplyTo":null,"subject":"[PATCH v2 00/29] Easy to review grep & pre-PCRE changes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:40Z","receivedAt":"2017-05-13T23:15:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Easy to review? 29 patches? Are you kidding me?!\n\nAs noted in v1 (<20170511091829.5634-1-avarab@gmail.com>;\nhttps://public-inbox.org/git/20170511091829.5634-1-avarab@gmail.com/)\nthese are all doc, test, refactoring etc. changes needed by the\nsubsequent \"PCRE v2, PCRE v1 JIT, log -P & fixes\" series.\n\nThanks a lot for the review everyone. This fixes all the issues\nraised. Changes noted below, with names prefixed by the person who\nraised the issue.\n\nÆvar Arnfjörð Bjarmason (29):\n Makefile & configure: reword inaccurate comment about PCRE\n grep & rev-list doc: stop promising libpcre for --perl-regexp\n test-lib: rename the LIBPCRE prerequisite to PCRE\n\nNo changes.\n\n log: add exhaustive tests for pattern style options & config\n\nJohannes: Now doesn't create a \"(1|2)\" tag, so should work on Windows\n& beyond (wasn't needed, just created as a side-effect of test_commit)\n\nJunio: Added comments for tricky basic/extended/perl\n\nJunio: Moved all the 'test_have_prereq PCRE' test / test_cmp code\ntogether, not apart as before.\n\n grep: add a test asserting that --perl-regexp dies when !PCRE\n grep: add a test for backreferences in PCRE patterns\n\nNo changes.\n\n grep: change non-ASCII -i test to stop using --debug\n\nBrandon: Removed stray leftover unused --debug\n\n grep: add tests for --threads=N and grep.threads\n\nBrandon: Amended commit message to clarify that this test doesn't need\na NO_PTHREADS prerequisite, and we actually get coverage out of\ntesting with --threads=N when not with threads, or at least it doesn't\nharm anything.\n\n grep: amend submodule recursion test for regex engine testing\n\nJunio: Now \"foo\" -> \"(1|2)\" as the commit message claims, not ->\n\"(1|2)d\".\n\n grep: add tests for grep pattern types being passed to submodules\n\nNo changes.\n\n grep: add a test helper function for less verbose -f \\0 tests\n\nJunio: \"nul_match() {\" -> \"nul_match () {\" & quote \"$status\" & don't\nquote non-variable strings.\n\n grep: prepare for testing binary regexes containing rx metacharacters\n\nNo changes.\n\n grep: add tests to fix blind spots with \\0 patterns\n\nJunio: Also fixed quoted variable strings here as above.\n\n perf: add a GIT_PERF_MAKE_COMMAND for when *_MAKE_OPTS won't do\n perf: emit progress output when unpacking & building\n\nNo changes.\n\n perf: add a performance comparison test of grep -G, -E and -P\n\nAll my multibyte performance tests were done with the string\n'm(ú|u)ult.b(æ|y)te' which didn't match anything in the kernel, now\ndone with 'm(ú|u)lt.b(æ|y)te' instead.\n\nI re-ran all the performance tests mentioned in the commit messages\nwhere applicable.\n\n perf: add a performance comparison of fixed-string grep\n\nOne test_cmp was run twice due to rebasing from the pcre1/pcre2 days\nof this series. Fixed.\n\n grep: catch a missing enum in switch statement\n\nStefan: Removed the comment about die(..BUG) & put the relevant detail\nin the commit message instead.\n\n grep: remove redundant regflags assignment under PCRE\n grep: remove redundant `regflags &= ~REG_EXTENDED` assignments\n\nNo changes.\n\n grep: factor test for \\0 in grep patterns into a function\n\nBrandon: Fix comment syntax creating the function, and move it to the\ncorrect place now instead of later in the \"move is_fixed()\" commit.\n\n grep: change the internal PCRE macro names to be PCRE1\n grep: change internal *pcre* variable & function names to be *pcre1*\n\nNo changes.\n\n grep: move is_fixed() earlier to avoid forward declaration\n\nBrandon: Now just moves is_fixed() instead of is_fixed() & has_null()\n\n test-lib: add a PTHREADS prerequisite\n pack-objects & index-pack: add test for --threads warning\n pack-objects: fix buggy warning about threads\n\nNo changes.\n\n grep: given --threads with NO_PTHREADS=YesPlease, warn\n\nUse Git standard comment syntax for TRANSLATORS comment.\n\n grep: assert that threading is enabled when calling grep_{lock,unlock}\n\n Documentation/git-grep.txt | 7 +-\n Documentation/rev-list-options.txt | 8 +-\n Makefile | 14 ++-\n builtin/grep.c | 23 +++-\n builtin/pack-objects.c | 4 +-\n configure.ac | 12 ++-\n grep.c | 108 ++++++++++---------\n grep.h | 10 +-\n t/README | 8 +-\n t/perf/README | 19 +++-\n t/perf/p7820-grep-engines.sh | 35 ++++++\n t/perf/p7821-grep-engines-fixed.sh | 26 +++++\n t/perf/run | 13 ++-\n t/t4202-log.sh | 96 ++++++++++++++++-\n t/t5300-pack-object.sh | 33 ++++++\n t/t7008-grep-binary.sh | 135 +++++++++++++++++------\n t/t7810-grep.sh | 81 +++++++++++---\n t/t7812-grep-icase-non-ascii.sh | 29 ++---\n t/t7813-grep-icase-iso.sh | 2 +-\n t/t7814-grep-recurse-submodules.sh | 215 +++++++++++++++++++++++--------------\n t/test-lib.sh | 3 +-\n 21 files changed, 646 insertions(+), 235 deletions(-)\n create mode 100755 t/perf/p7820-grep-engines.sh\n create mode 100755 t/perf/p7821-grep-engines-fixed.sh\n\n-- \n2.11.0\n\n"},{"id":"319633","messageId":"20170513231509.7834-2-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 01/29] Makefile & configure: reword inaccurate comment about PCRE","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:41Z","receivedAt":"2017-05-13T23:15:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Reword an outdated & inaccurate comment which suggests that only\ngit-grep can use PCRE.\n\nThis comment was added back when PCRE support was initially added in\ncommit 63e7e9d8b6 (\"git-grep: Learn PCRE\", 2011-05-09), and was true\nat the time.\n\nIt hasn't been telling the full truth since git-log learned to use\nPCRE with --grep in commit 727b6fc3ed (\"log --grep: accept\n--basic-regexp and --perl-regexp\", 2012-10-03), and more importantly\nis likely to get more inaccurate over time as more use is made of PCRE\nin other areas.\n\nReword it to be more future-proof, and to more clearly explain that\nthis enables user-initiated runtime behavior.\n\nCopy/pasting this so much in configure.ac is lame, these Makefile-like\nflags aren't even used by autoconf, just the corresponding\n--with[out]-* options. But copy/pasting the comments that make sense\nfor the Makefile to configure.ac where they make less sense is the\npattern everything else follows in that file. I'm not going to war\nagainst that as part of this change, just following the existing\npattern.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 6 ++++--\n configure.ac | 12 ++++++++----\n 2 files changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex e35542e631..eedadb8056 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -24,8 +24,10 @@ all::\n # Define NO_OPENSSL environment variable if you do not have OpenSSL.\n # This also implies BLK_SHA1.\n #\n-# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be\n-# able to use Perl-compatible regular expressions.\n+# Define USE_LIBPCRE if you have and want to use libpcre. Various\n+# commands such as log and grep offer runtime options to use\n+# Perl-compatible regular expressions instead of standard or extended\n+# POSIX regular expressions.\n #\n # Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in\n # /foo/bar/include and /foo/bar/lib directories.\ndiff --git a/configure.ac b/configure.ac\nindex 128165529f..deeb968daa 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -250,8 +250,10 @@ AS_HELP_STRING([--with-openssl],[use OpenSSL library (default is YES)])\n AS_HELP_STRING([], [ARG can be prefix for openssl library and headers]),\n GIT_PARSE_WITH([openssl]))\n \n-# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be\n-# able to use Perl-compatible regular expressions.\n+# Define USE_LIBPCRE if you have and want to use libpcre. Various\n+# commands such as log and grep offer runtime options to use\n+# Perl-compatible regular expressions instead of standard or extended\n+# POSIX regular expressions.\n #\n # Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in\n # /foo/bar/include and /foo/bar/lib directories.\n@@ -499,8 +501,10 @@ GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO])\n GIT_CONF_SUBST([NO_OPENSSL])\n \n #\n-# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be\n-# able to use Perl-compatible regular expressions.\n+# Define USE_LIBPCRE if you have and want to use libpcre. Various\n+# commands such as log and grep offer runtime options to use\n+# Perl-compatible regular expressions instead of standard or extended\n+# POSIX regular expressions.\n #\n \n if test -n \"$USE_LIBPCRE\"; then\n-- \n2.11.0\n\n"},{"id":"319634","messageId":"20170513231509.7834-3-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 02/29] grep & rev-list doc: stop promising libpcre for --perl-regexp","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:42Z","receivedAt":"2017-05-13T23:15:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Stop promising in our grep & rev-list options documentation that we're\nalways going to be using libpcre when given the --perl-regexp option.\n\nInstead talk about using \"Perl-compatible regular expressions\" and\nusing these types of patterns using \"a compile-time dependency\".\n\nSaying \"libpcre\" means that we're talking about libpcre.so, which is\nalways going to be v1. This change is part of an ongoing saga to add\nsupport for libpcre2, which comes with PCRE v2.\n\nIn the future we might use some completely unrelated library to\nprovide perl-compatible regular expression support. By wording the\ndocumentation differently and not promising any specific version of\nPCRE or even PCRE at all we have more wiggle room to change the\nimplementation.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-grep.txt | 7 +++++--\n Documentation/rev-list-options.txt | 8 ++++++--\n 2 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 71f32f3508..5033483db4 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -161,8 +161,11 @@ OPTIONS\n \n -P::\n --perl-regexp::\n-\tUse Perl-compatible regexp for patterns. Requires libpcre to be\n-\tcompiled in.\n+\tUse Perl-compatible regular expressions for patterns.\n++\n+Support for these types of regular expressions is an optional\n+compile-time dependency. If Git wasn't compiled with support for them\n+providing this option will cause it to die.\n \n -F::\n --fixed-strings::\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex a02f7324c0..a46f70c2b1 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -92,8 +92,12 @@ endif::git-rev-list[]\n \tpattern as a regular expression).\n \n --perl-regexp::\n-\tConsider the limiting patterns to be Perl-compatible regular expressions.\n-\tRequires libpcre to be compiled in.\n+\tConsider the limiting patterns to be Perl-compatible regular\n+\texpressions.\n++\n+Support for these types of regular expressions is an optional\n+compile-time dependency. If Git wasn't compiled with support for them\n+providing this option will cause it to die.\n \n --remove-empty::\n \tStop when a given path disappears from the tree.\n-- \n2.11.0\n\n"},{"id":"319635","messageId":"20170513231509.7834-5-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 04/29] log: add exhaustive tests for pattern style options & config","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:44Z","receivedAt":"2017-05-13T23:15:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add exhaustive tests for how the different grep.patternType options &\nthe corresponding command-line options affect git-log.\n\nBefore this change it was possible to patch revision.c so that the\n--basic-regexp option was synonymous with --extended-regexp, and\n--perl-regexp wasn't recognized at all, and still have 100% of the\ntest suite pass.\n\nThis was because the first test being modified here, added in commit\n34a4ae55b2 (\"log --grep: use the same helper to set -E/-F options as\n\"git grep\"\", 2012-10-03), didn't actually check whether we'd enabled\nextended regular expressions as distinct from re-toggling non-fixed\nstring support.\n\nFix that by changing the pattern to a pattern that'll only match if\n--extended-regexp option is provided, but won't match under the\ndefault --basic-regexp option.\n\nOther potential regressions were possible since there were no tests\nfor the rest of the combinations of grep.patternType configuration\ntoggles & corresponding git-log command-line options. Add exhaustive\ntests for those.\n\nThe patterns being passed to fixed/basic/extended/PCRE are carefully\ncrafted to return the wrong thing if the grep engine were to pick any\nother matching method than the one it's told to use.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4202-log.sh | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 93 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex f577990716..e522a2fcd5 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -262,7 +262,28 @@ test_expect_success 'log --grep -i' '\n \n test_expect_success 'log -F -E --grep= uses ere' '\n \techo second >expect &&\n-\tgit log -1 --pretty=\"tformat:%s\" -F -E --grep=s.c.nd >actual &&\n+\t# basic would need \\(s\\) to do the same\n+\tgit log -1 --pretty=\"tformat:%s\" -F -E --grep=\"(s).c.nd\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success PCRE 'log -F -E --perl-regexp --grep= uses PCRE' '\n+\ttest_when_finished \"rm -rf num_commits\" &&\n+\tgit init num_commits &&\n+\t(\n+\t\tcd num_commits &&\n+\t\ttest_commit 1d &&\n+\t\ttest_commit 2e\n+\t) &&\n+\techo 2e >expect &&\n+\t# In PCRE \\d in [\\d] is like saying \"0-9\", and matches the 2\n+\t# in 2e...\n+\tgit -C num_commits log -1 --pretty=\"tformat:%s\" -F -E --perl-regexp --grep=\"[\\d]\" >actual &&\n+\ttest_cmp expect actual &&\n+\techo 1d >expect &&\n+\t# ...in POSIX basic & extended it is the same as [d],\n+\t# i.e. \"d\", which matches 1d, but not and does not match 2e.\n+\tgit -C num_commits log -1 --pretty=\"tformat:%s\" -F -E --grep=\"[\\d]\" >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -280,6 +301,77 @@ test_expect_success 'log with grep.patternType configuration and command line' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'log with various grep.patternType configurations & command-lines' '\n+\tgit init pattern-type &&\n+\t(\n+\t\tcd pattern-type &&\n+\t\ttest_commit 1 file A &&\n+\n+\t\t# The tagname is overridden here because creating a\n+\t\t# tag called \"(1|2)\" as test_commit would otherwise\n+\t\t# implicitly do would fail on e.g. MINGW.\n+\t\ttest_commit \"(1|2)\" file B 2 &&\n+\n+\t\techo \"(1|2)\" >expect.fixed &&\n+\t\tcp expect.fixed expect.basic &&\n+\t\tcp expect.fixed expect.extended &&\n+\t\tcp expect.fixed expect.perl &&\n+\n+\t\t# A strcmp-like match with fixed.\n+\t\tgit -c grep.patternType=fixed log --pretty=tformat:%s \\\n+\t\t\t--grep=\"(1|2)\" >actual.fixed &&\n+\n+\t\t# POSIX basic matches (, | and ) literally.\n+\t\tgit -c grep.patternType=basic log --pretty=tformat:%s \\\n+\t\t\t--grep=\"(.|.)\" >actual.basic &&\n+\n+\t\t# POSIX extended needs to have | escaped to match it\n+\t\t# literally, whereas under basic this is the same as\n+\t\t# (|2), i.e. it would also match \"1\". This test checks\n+\t\t# for extended by asserting that it is not matching\n+\t\t# what basic would match.\n+\t\tgit -c grep.patternType=extended log --pretty=tformat:%s \\\n+\t\t\t--grep=\"\\|2\" >actual.extended &&\n+\t\tif test_have_prereq PCRE\n+\t\tthen\n+\t\t\t# Only PCRE would match [\\d]\\| with only\n+\t\t\t# \"(1|2)\" due to [\\d]. POSIX basic would match\n+\t\t\t# both it and \"1\", and POSIX extended would\n+\t\t\t# match neither.\n+\t\t\tgit -c grep.patternType=perl log --pretty=tformat:%s \\\n+\t\t\t\t--grep=\"[\\d]\\|\" >actual.perl &&\n+\t\t\ttest_cmp expect.perl actual.perl\n+\t\tfi &&\n+\t\ttest_cmp expect.fixed actual.fixed &&\n+\t\ttest_cmp expect.basic actual.basic &&\n+\t\ttest_cmp expect.extended actual.extended &&\n+\n+\t\tgit log --pretty=tformat:%s -F \\\n+\t\t\t--grep=\"(1|2)\" >actual.fixed.short-arg &&\n+\t\tgit log --pretty=tformat:%s -E \\\n+\t\t\t--grep=\"\\|2\" >actual.extended.short-arg &&\n+\t\ttest_cmp expect.fixed actual.fixed.short-arg &&\n+\t\ttest_cmp expect.extended actual.extended.short-arg &&\n+\n+\t\tgit log --pretty=tformat:%s --fixed-strings \\\n+\t\t\t--grep=\"(1|2)\" >actual.fixed.long-arg &&\n+\t\tgit log --pretty=tformat:%s --basic-regexp \\\n+\t\t\t--grep=\"(.|.)\" >actual.basic.long-arg &&\n+\t\tgit log --pretty=tformat:%s --extended-regexp \\\n+\t\t\t--grep=\"\\|2\" >actual.extended.long-arg &&\n+\t\tif test_have_prereq PCRE\n+\t\tthen\n+\t\t\tgit log --pretty=tformat:%s --perl-regexp \\\n+\t\t\t\t--grep=\"[\\d]\\|\" >actual.perl.long-arg &&\n+\t\t\ttest_cmp expect.perl actual.perl.long-arg\n+\n+\t\tfi &&\n+\t\ttest_cmp expect.fixed actual.fixed.long-arg &&\n+\t\ttest_cmp expect.basic actual.basic.long-arg &&\n+\t\ttest_cmp expect.extended actual.extended.long-arg\n+\t)\n+'\n+\n cat > expect <\n---\n t/t4202-log.sh | 4 +++-\n t/t7810-grep.sh | 12 ++++++++++++\n 2 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex e522a2fcd5..9680dfe400 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -364,7 +364,9 @@ test_expect_success 'log with various grep.patternType configurations & command-\n \t\t\tgit log --pretty=tformat:%s --perl-regexp \\\n \t\t\t\t--grep=\"[\\d]\\|\" >actual.perl.long-arg &&\n \t\t\ttest_cmp expect.perl actual.perl.long-arg\n-\n+\t\telse\n+\t\t\ttest_must_fail git log --perl-regexp \\\n+\t\t\t\t--grep=\"[\\d]\\|\"\n \t\tfi &&\n \t\ttest_cmp expect.fixed actual.fixed.long-arg &&\n \t\ttest_cmp expect.basic actual.basic.long-arg &&\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex c84c4d99f9..8d69113695 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -281,6 +281,10 @@ do\n \t\ttest_cmp expected actual\n \t'\n \n+\ttest_expect_success !PCRE \"grep $L with grep.patterntype=perl errors without PCRE\" '\n+\t\ttest_must_fail git -c grep.patterntype=perl grep \"foo.*bar\"\n+\t'\n+\n \ttest_expect_success \"grep $L with grep.patternType=default and grep.extendedRegexp=true\" '\n \t\techo \"${HC}ab:abc\" >expected &&\n \t\tgit \\\n@@ -1058,11 +1062,19 @@ test_expect_success PCRE 'grep --perl-regexp pattern' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success !PCRE 'grep --perl-regexp pattern errors without PCRE' '\n+\ttest_must_fail git grep --perl-regexp \"foo.*bar\"\n+'\n+\n test_expect_success PCRE 'grep -P pattern' '\n \tgit grep -P \"\\p{Ps}.*?\\p{Pe}\" hello.c >actual &&\n \ttest_cmp expected actual\n '\n \n+test_expect_success !PCRE 'grep -P pattern errors without PCRE' '\n+\ttest_must_fail git grep -P \"foo.*bar\"\n+'\n+\n test_expect_success 'grep pattern with grep.extendedRegexp=true' '\n \t>empty &&\n \ttest_must_fail git -c grep.extendedregexp=true \\\n-- \n2.11.0\n\n"},{"id":"319637","messageId":"20170513231509.7834-4-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 03/29] test-lib: rename the LIBPCRE prerequisite to PCRE","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:43Z","receivedAt":"2017-05-13T23:16:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Rename the LIBPCRE prerequisite to PCRE. This is for preparation for\nlibpcre2 support, where having just \"LIBPCRE\" would be confusing as it\nimplies v1 of the library.\n\nNone of these tests are incompatible between versions 1 & 2 of\nlibpcre, it's less confusing to give them a more general name to make\nit clear that they work on both library versions.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/README | 4 ++--\n t/t7810-grep.sh | 28 ++++++++++++++--------------\n t/t7812-grep-icase-non-ascii.sh | 4 ++--\n t/t7813-grep-icase-iso.sh | 2 +-\n t/test-lib.sh | 2 +-\n 5 files changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex ab386c3681..a90cb62583 100644\n--- a/t/README\n+++ b/t/README\n@@ -803,9 +803,9 @@ use these, and \"test_set_prereq\" for how to define your own.\n Test is not run by root user, and an attempt to write to an\n unwritable file is expected to fail correctly.\n \n- - LIBPCRE\n+ - PCRE\n \n- Git was compiled with USE_LIBPCRE=YesPlease. Wrap any tests\n+ Git was compiled with support for PCRE. Wrap any tests\n that use git-grep --perl-regexp or git-grep -P in these.\n \n - CASE_INSENSITIVE_FS\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex cee42097b0..c84c4d99f9 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -275,7 +275,7 @@ do\n \t\ttest_cmp expected actual\n \t'\n \n-\ttest_expect_success LIBPCRE \"grep $L with grep.patterntype=perl\" '\n+\ttest_expect_success PCRE \"grep $L with grep.patterntype=perl\" '\n \t\techo \"${HC}ab:a+b*c\" >expected &&\n \t\tgit -c grep.patterntype=perl grep \"a\\x{2b}b\\x{2a}c\" $H ab >actual &&\n \t\ttest_cmp expected actual\n@@ -1053,12 +1053,12 @@ hello.c:int main(int argc, const char **argv)\n hello.c:\tprintf(\"Hello world.\\n\");\n EOF\n \n-test_expect_success LIBPCRE 'grep --perl-regexp pattern' '\n+test_expect_success PCRE 'grep --perl-regexp pattern' '\n \tgit grep --perl-regexp \"\\p{Ps}.*?\\p{Pe}\" hello.c >actual &&\n \ttest_cmp expected actual\n '\n \n-test_expect_success LIBPCRE 'grep -P pattern' '\n+test_expect_success PCRE 'grep -P pattern' '\n \tgit grep -P \"\\p{Ps}.*?\\p{Pe}\" hello.c >actual &&\n \ttest_cmp expected actual\n '\n@@ -1070,13 +1070,13 @@ test_expect_success 'grep pattern with grep.extendedRegexp=true' '\n \ttest_cmp empty actual\n '\n \n-test_expect_success LIBPCRE 'grep -P pattern with grep.extendedRegexp=true' '\n+test_expect_success PCRE 'grep -P pattern with grep.extendedRegexp=true' '\n \tgit -c grep.extendedregexp=true \\\n \t\tgrep -P \"\\p{Ps}.*?\\p{Pe}\" hello.c >actual &&\n \ttest_cmp expected actual\n '\n \n-test_expect_success LIBPCRE 'grep -P -v pattern' '\n+test_expect_success PCRE 'grep -P -v pattern' '\n \t{\n \t\techo \"ab:a+b*c\"\n \t\techo \"ab:a+bc\"\n@@ -1085,7 +1085,7 @@ test_expect_success LIBPCRE 'grep -P -v pattern' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success LIBPCRE 'grep -P -i pattern' '\n+test_expect_success PCRE 'grep -P -i pattern' '\n \tcat >expected <<-EOF &&\n \thello.c:\tprintf(\"Hello world.\\n\");\n \tEOF\n@@ -1093,7 +1093,7 @@ test_expect_success LIBPCRE 'grep -P -i pattern' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success LIBPCRE 'grep -P -w pattern' '\n+test_expect_success PCRE 'grep -P -w pattern' '\n \t{\n \t\techo \"hello_world:Hello world\"\n \t\techo \"hello_world:HeLLo world\"\n@@ -1118,11 +1118,11 @@ test_expect_success 'grep invalidpattern properly dies with grep.patternType=ext\n \ttest_must_fail git -c grep.patterntype=extended grep \"a[\"\n '\n \n-test_expect_success LIBPCRE 'grep -P invalidpattern properly dies ' '\n+test_expect_success PCRE 'grep -P invalidpattern properly dies ' '\n \ttest_must_fail git grep -P \"a[\"\n '\n \n-test_expect_success LIBPCRE 'grep invalidpattern properly dies with grep.patternType=perl' '\n+test_expect_success PCRE 'grep invalidpattern properly dies with grep.patternType=perl' '\n \ttest_must_fail git -c grep.patterntype=perl grep \"a[\"\n '\n \n@@ -1191,13 +1191,13 @@ test_expect_success 'grep pattern with grep.patternType=fixed, =basic, =perl, =e\n \ttest_cmp expected actual\n '\n \n-test_expect_success LIBPCRE 'grep -G -F -E -P pattern' '\n+test_expect_success PCRE 'grep -G -F -E -P pattern' '\n \techo \"d0:0\" >expected &&\n \tgit grep -G -F -E -P \"[\\d]\" d0 >actual &&\n \ttest_cmp expected actual\n '\n \n-test_expect_success LIBPCRE 'grep pattern with grep.patternType=fixed, =basic, =extended, =perl' '\n+test_expect_success PCRE 'grep pattern with grep.patternType=fixed, =basic, =extended, =perl' '\n \techo \"d0:0\" >expected &&\n \tgit \\\n \t\t-c grep.patterntype=fixed \\\n@@ -1208,7 +1208,7 @@ test_expect_success LIBPCRE 'grep pattern with grep.patternType=fixed, =basic, =\n \ttest_cmp expected actual\n '\n \n-test_expect_success LIBPCRE 'grep -P pattern with grep.patternType=fixed' '\n+test_expect_success PCRE 'grep -P pattern with grep.patternType=fixed' '\n \techo \"ab:a+b*c\" >expected &&\n \tgit \\\n \t\t-c grep.patterntype=fixed \\\n@@ -1343,12 +1343,12 @@ space: line with leading space2\n space: line with leading space3\n EOF\n \n-test_expect_success LIBPCRE 'grep -E \"^ \"' '\n+test_expect_success PCRE 'grep -E \"^ \"' '\n \tgit grep -E \"^ \" space >actual &&\n \ttest_cmp expected actual\n '\n \n-test_expect_success LIBPCRE 'grep -P \"^ \"' '\n+test_expect_success PCRE 'grep -P \"^ \"' '\n \tgit grep -P \"^ \" space >actual &&\n \ttest_cmp expected actual\n '\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 169fd8d706..04a61cb8e0 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -20,13 +20,13 @@ test_expect_success REGEX_LOCALE 'grep literal string, no -F' '\n \tgit grep -i \"TILRAUN: HALLÓ HEIMUR!\"\n '\n \n-test_expect_success GETTEXT_LOCALE,LIBPCRE 'grep pcre utf-8 icase' '\n+test_expect_success GETTEXT_LOCALE,PCRE 'grep pcre utf-8 icase' '\n \tgit grep --perl-regexp \"TILRAUN: H.lló Heimur!\" &&\n \tgit grep --perl-regexp -i \"TILRAUN: H.lló Heimur!\" &&\n \tgit grep --perl-regexp -i \"TILRAUN: H.LLÓ HEIMUR!\"\n '\n \n-test_expect_success GETTEXT_LOCALE,LIBPCRE 'grep pcre utf-8 string with \"+\"' '\n+test_expect_success GETTEXT_LOCALE,PCRE 'grep pcre utf-8 string with \"+\"' '\n \ttest_write_lines \"TILRAUN: Hallóó Heimur!\" >file2 &&\n \tgit add file2 &&\n \tgit grep -l --perl-regexp \"TILRAUN: H.lló+ Heimur!\" >actual &&\ndiff --git a/t/t7813-grep-icase-iso.sh b/t/t7813-grep-icase-iso.sh\nindex efef7fb81f..701e08a8e5 100755\n--- a/t/t7813-grep-icase-iso.sh\n+++ b/t/t7813-grep-icase-iso.sh\n@@ -11,7 +11,7 @@ test_expect_success GETTEXT_ISO_LOCALE 'setup' '\n \texport LC_ALL\n '\n \n-test_expect_success GETTEXT_ISO_LOCALE,LIBPCRE 'grep pcre string' '\n+test_expect_success GETTEXT_ISO_LOCALE,PCRE 'grep pcre string' '\n \tgit grep --perl-regexp -i \"TILRAUN: H.ll� Heimur!\" &&\n \tgit grep --perl-regexp -i \"TILRAUN: H.LL� HEIMUR!\"\n '\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 13b5696822..6abf1d1918 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1010,7 +1010,7 @@ esac\n ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1\n test -z \"$NO_PERL\" && test_set_prereq PERL\n test -z \"$NO_PYTHON\" && test_set_prereq PYTHON\n-test -n \"$USE_LIBPCRE\" && test_set_prereq LIBPCRE\n+test -n \"$USE_LIBPCRE\" && test_set_prereq PCRE\n test -z \"$NO_GETTEXT\" && test_set_prereq GETTEXT\n \n # Can we rely on git's output in the C locale?\n-- \n2.11.0\n\n"},{"id":"319638","messageId":"20170513231509.7834-7-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 06/29] grep: add a test for backreferences in PCRE patterns","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:46Z","receivedAt":"2017-05-13T23:16:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a test for backreferences such as (.)\\1 in PCRE patterns. This\ntest ensures that the PCRE_NO_AUTO_CAPTURE option isn't turned\non. Before this change turning it on would break these sort of\npatterns, but wouldn't break any tests.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7810-grep.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 8d69113695..daa906b9b0 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -1114,6 +1114,13 @@ test_expect_success PCRE 'grep -P -w pattern' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success PCRE 'grep -P backreferences work (the PCRE NO_AUTO_CAPTURE flag is not set)' '\n+\tgit grep -P -h \"(?P.)(?P=one)\" hello_world >actual &&\n+\ttest_cmp hello_world actual &&\n+\tgit grep -P -h \"(.)\\1\" hello_world >actual &&\n+\ttest_cmp hello_world actual\n+'\n+\n test_expect_success 'grep -G invalidpattern properly dies ' '\n \ttest_must_fail git grep -G \"a[\"\n '\n-- \n2.11.0\n\n"},{"id":"319639","messageId":"20170513231509.7834-8-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 07/29] grep: change non-ASCII -i test to stop using --debug","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:47Z","receivedAt":"2017-05-13T23:16:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a non-ASCII case-insensitive test case to stop using --debug,\nand instead simply test for the expected results.\n\nThe test coverage remains the same with this change, but the test\nwon't break due to internal refactoring.\n\nThis test was added in commit 793dc676e0 (\"grep/icase: avoid kwsset\nwhen -F is specified\", 2016-06-25). It was asserting that the regex\nmust be compiled with compile_fixed_regexp(), instead test for the\nexpected results, allowing the underlying implementation to change.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7812-grep-icase-non-ascii.sh | 25 +++++--------------------\n 1 file changed, 5 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 04a61cb8e0..0059a1f837 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -36,29 +36,14 @@ test_expect_success GETTEXT_LOCALE,PCRE 'grep pcre utf-8 string with \"+\"' '\n '\n \n test_expect_success REGEX_LOCALE 'grep literal string, with -F' '\n-\tgit grep --debug -i -F \"TILRAUN: Halló Heimur!\" 2>&1 >/dev/null |\n-\t\t grep fixed >debug1 &&\n-\ttest_write_lines \"fixed TILRAUN: Halló Heimur!\" >expect1 &&\n-\ttest_cmp expect1 debug1 &&\n-\n-\tgit grep --debug -i -F \"TILRAUN: HALLÓ HEIMUR!\" 2>&1 >/dev/null |\n-\t\t grep fixed >debug2 &&\n-\ttest_write_lines \"fixed TILRAUN: HALLÓ HEIMUR!\" >expect2 &&\n-\ttest_cmp expect2 debug2\n+\tgit grep -i -F \"TILRAUN: Halló Heimur!\" &&\n+\tgit grep -i -F \"TILRAUN: HALLÓ HEIMUR!\"\n '\n \n test_expect_success REGEX_LOCALE 'grep string with regex, with -F' '\n-\ttest_write_lines \"^*TILR^AUN:.* \\\\Halló \\$He[]imur!\\$\" >file &&\n-\n-\tgit grep --debug -i -F \"^*TILR^AUN:.* \\\\Halló \\$He[]imur!\\$\" 2>&1 >/dev/null |\n-\t\t grep fixed >debug1 &&\n-\ttest_write_lines \"fixed \\\\^*TILR^AUN:\\\\.\\\\* \\\\\\\\Halló \\$He\\\\[]imur!\\\\\\$\" >expect1 &&\n-\ttest_cmp expect1 debug1 &&\n-\n-\tgit grep --debug -i -F \"^*TILR^AUN:.* \\\\HALLÓ \\$HE[]IMUR!\\$\" 2>&1 >/dev/null |\n-\t\t grep fixed >debug2 &&\n-\ttest_write_lines \"fixed \\\\^*TILR^AUN:\\\\.\\\\* \\\\\\\\HALLÓ \\$HE\\\\[]IMUR!\\\\\\$\" >expect2 &&\n-\ttest_cmp expect2 debug2\n+\ttest_write_lines \"TILRAUN: Halló Heimur [abc]!\" >file3 &&\n+\tgit add file3 &&\n+\tgit grep -i -F \"TILRAUN: Halló Heimur [abc]!\" file3\n '\n \n test_expect_success REGEX_LOCALE 'pickaxe -i on non-ascii' '\n-- \n2.11.0\n\n"},{"id":"319640","messageId":"20170513231509.7834-9-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 08/29] grep: add tests for --threads=N and grep.threads","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:48Z","receivedAt":"2017-05-13T23:16:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add tests for --threads=N being supplied on the command-line, or when\ngrep.threads=N being supplied in the configuration.\n\nWhen the threading support was made run-time configurable in commit\n89f09dd34e (\"grep: add --threads= option and grep.threads\nconfiguration\", 2015-12-15) no tests were added for it.\n\nIn developing a change to the grep code I was able to make\n'--threads=1 ` segfault, while the test suite still passed. This\nchange fixes that blind spot in the tests.\n\nIn addition to asserting that asking for N threads shouldn't segfault,\ntest that the grep output given any N is the same.\n\nThe choice to test only 1..10 as opposed to 1..8 or 1..16 or whatever\nis arbitrary. Testing 1..1024 works locally for me (but gets\nnoticeably slower as more threads are spawned). Given the structure of\nthe code there's no reason to test an arbitrary number of threads,\nonly 0, 1 and >=2 are special modes of operation.\n\nA later patch introduces a PTHREADS test prerequisite which is true\nunder NO_PTHREADS=UnfortunatelyYes, but even under NO_PTHREADS it's\nfine to test --threads=N, we'll just ignore it and not use\nthreading. So these tests also make sense under that mode to assert\nthat --threads=N without pthreads still returns expected results.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7810-grep.sh | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex daa906b9b0..561709ef6a 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -775,6 +775,22 @@ test_expect_success 'grep -W with userdiff' '\n \ttest_cmp expected actual\n '\n \n+for threads in $(test_seq 0 10)\n+do\n+\ttest_expect_success \"grep --threads=$threads & -c grep.threads=$threads\" \"\n+\t\tgit grep --threads=$threads . >actual.$threads &&\n+\t\tif test $threads -ge 1\n+\t\tthen\n+\t\t\ttest_cmp actual.\\$(($threads - 1)) actual.$threads\n+\t\tfi &&\n+\t\tgit -c grep.threads=$threads grep . >actual.$threads &&\n+\t\tif test $threads -ge 1\n+\t\tthen\n+\t\t\ttest_cmp actual.\\$(($threads - 1)) actual.$threads\n+\t\tfi\n+\t\"\n+done\n+\n test_expect_success 'grep from a subdirectory to search wider area (1)' '\n \tmkdir -p s &&\n \t(\n-- \n2.11.0\n\n"},{"id":"319641","messageId":"20170513231509.7834-10-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 09/29] grep: amend submodule recursion test for regex engine testing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:49Z","receivedAt":"2017-05-13T23:16:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend the submodule recursion test to prepare it for subsequent tests\nof whether it passes along the grep.patternType to the submodule\ngreps.\n\nThis is the result of searching & replacing:\n\n foobar -> (1|2)d(3|4)\n foo -> (1|2)\n bar -> (3|4)\n\nCurrently there's no tests for whether e.g. -P or -E is correctly\npassed along, tests for that will be added in a follow-up change, but\nfirst add content to the tests which will match differently under\ndifferent regex engines.\n\nReuse the pattern established in an earlier commit of mine in this\nseries (\"log: add exhaustive tests for pattern style options &\nconfig\", 2017-04-07). The pattern \"(.|.)[\\d]\" will match this content\ndifferently under fixed/basic/extended & perl.\n\nThis test code was originally added in commit 0281e487fd (\"grep:\noptionally recurse into submodules\", 2016-12-16).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7814-grep-recurse-submodules.sh | 166 ++++++++++++++++++-------------------\n 1 file changed, 83 insertions(+), 83 deletions(-)\n\ndiff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh\nindex 5b6eb3a65e..1472855e1d 100755\n--- a/t/t7814-grep-recurse-submodules.sh\n+++ b/t/t7814-grep-recurse-submodules.sh\n@@ -9,13 +9,13 @@ submodules.\n . ./test-lib.sh\n \n test_expect_success 'setup directory structure and submodule' '\n-\techo \"foobar\" >a &&\n+\techo \"(1|2)d(3|4)\" >a &&\n \tmkdir b &&\n-\techo \"bar\" >b/b &&\n+\techo \"(3|4)\" >b/b &&\n \tgit add a b &&\n \tgit commit -m \"add a and b\" &&\n \tgit init submodule &&\n-\techo \"foobar\" >submodule/a &&\n+\techo \"(1|2)d(3|4)\" >submodule/a &&\n \tgit -C submodule add a &&\n \tgit -C submodule commit -m \"add a\" &&\n \tgit submodule add ./submodule &&\n@@ -24,18 +24,18 @@ test_expect_success 'setup directory structure and submodule' '\n \n test_expect_success 'grep correctly finds patterns in a submodule' '\n \tcat >expect <<-\\EOF &&\n-\ta:foobar\n-\tb/b:bar\n-\tsubmodule/a:foobar\n+\ta:(1|2)d(3|4)\n+\tb/b:(3|4)\n+\tsubmodule/a:(1|2)d(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --recurse-submodules >actual &&\n+\tgit grep -e \"(3|4)\" --recurse-submodules >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep and basic pathspecs' '\n \tcat >expect <<-\\EOF &&\n-\tsubmodule/a:foobar\n+\tsubmodule/a:(1|2)d(3|4)\n \tEOF\n \n \tgit grep -e. --recurse-submodules -- submodule >actual &&\n@@ -44,7 +44,7 @@ test_expect_success 'grep and basic pathspecs' '\n \n test_expect_success 'grep and nested submodules' '\n \tgit init submodule/sub &&\n-\techo \"foobar\" >submodule/sub/a &&\n+\techo \"(1|2)d(3|4)\" >submodule/sub/a &&\n \tgit -C submodule/sub add a &&\n \tgit -C submodule/sub commit -m \"add a\" &&\n \tgit -C submodule submodule add ./sub &&\n@@ -54,117 +54,117 @@ test_expect_success 'grep and nested submodules' '\n \tgit commit -m \"updated submodule\" &&\n \n \tcat >expect <<-\\EOF &&\n-\ta:foobar\n-\tb/b:bar\n-\tsubmodule/a:foobar\n-\tsubmodule/sub/a:foobar\n+\ta:(1|2)d(3|4)\n+\tb/b:(3|4)\n+\tsubmodule/a:(1|2)d(3|4)\n+\tsubmodule/sub/a:(1|2)d(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --recurse-submodules >actual &&\n+\tgit grep -e \"(3|4)\" --recurse-submodules >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep and multiple patterns' '\n \tcat >expect <<-\\EOF &&\n-\ta:foobar\n-\tsubmodule/a:foobar\n-\tsubmodule/sub/a:foobar\n+\ta:(1|2)d(3|4)\n+\tsubmodule/a:(1|2)d(3|4)\n+\tsubmodule/sub/a:(1|2)d(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --and -e \"foo\" --recurse-submodules >actual &&\n+\tgit grep -e \"(3|4)\" --and -e \"(1|2)\" --recurse-submodules >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep and multiple patterns' '\n \tcat >expect <<-\\EOF &&\n-\tb/b:bar\n+\tb/b:(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --and --not -e \"foo\" --recurse-submodules >actual &&\n+\tgit grep -e \"(3|4)\" --and --not -e \"(1|2)\" --recurse-submodules >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'basic grep tree' '\n \tcat >expect <<-\\EOF &&\n-\tHEAD:a:foobar\n-\tHEAD:b/b:bar\n-\tHEAD:submodule/a:foobar\n-\tHEAD:submodule/sub/a:foobar\n+\tHEAD:a:(1|2)d(3|4)\n+\tHEAD:b/b:(3|4)\n+\tHEAD:submodule/a:(1|2)d(3|4)\n+\tHEAD:submodule/sub/a:(1|2)d(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --recurse-submodules HEAD >actual &&\n+\tgit grep -e \"(3|4)\" --recurse-submodules HEAD >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep tree HEAD^' '\n \tcat >expect <<-\\EOF &&\n-\tHEAD^:a:foobar\n-\tHEAD^:b/b:bar\n-\tHEAD^:submodule/a:foobar\n+\tHEAD^:a:(1|2)d(3|4)\n+\tHEAD^:b/b:(3|4)\n+\tHEAD^:submodule/a:(1|2)d(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --recurse-submodules HEAD^ >actual &&\n+\tgit grep -e \"(3|4)\" --recurse-submodules HEAD^ >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep tree HEAD^^' '\n \tcat >expect <<-\\EOF &&\n-\tHEAD^^:a:foobar\n-\tHEAD^^:b/b:bar\n+\tHEAD^^:a:(1|2)d(3|4)\n+\tHEAD^^:b/b:(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --recurse-submodules HEAD^^ >actual &&\n+\tgit grep -e \"(3|4)\" --recurse-submodules HEAD^^ >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep tree and pathspecs' '\n \tcat >expect <<-\\EOF &&\n-\tHEAD:submodule/a:foobar\n-\tHEAD:submodule/sub/a:foobar\n+\tHEAD:submodule/a:(1|2)d(3|4)\n+\tHEAD:submodule/sub/a:(1|2)d(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --recurse-submodules HEAD -- submodule >actual &&\n+\tgit grep -e \"(3|4)\" --recurse-submodules HEAD -- submodule >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep tree and pathspecs' '\n \tcat >expect <<-\\EOF &&\n-\tHEAD:submodule/a:foobar\n-\tHEAD:submodule/sub/a:foobar\n+\tHEAD:submodule/a:(1|2)d(3|4)\n+\tHEAD:submodule/sub/a:(1|2)d(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --recurse-submodules HEAD -- \"submodule*a\" >actual &&\n+\tgit grep -e \"(3|4)\" --recurse-submodules HEAD -- \"submodule*a\" >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep tree and more pathspecs' '\n \tcat >expect <<-\\EOF &&\n-\tHEAD:submodule/a:foobar\n+\tHEAD:submodule/a:(1|2)d(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --recurse-submodules HEAD -- \"submodul?/a\" >actual &&\n+\tgit grep -e \"(3|4)\" --recurse-submodules HEAD -- \"submodul?/a\" >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep tree and more pathspecs' '\n \tcat >expect <<-\\EOF &&\n-\tHEAD:submodule/sub/a:foobar\n+\tHEAD:submodule/sub/a:(1|2)d(3|4)\n \tEOF\n \n-\tgit grep -e \"bar\" --recurse-submodules HEAD -- \"submodul*/sub/a\" >actual &&\n+\tgit grep -e \"(3|4)\" --recurse-submodules HEAD -- \"submodul*/sub/a\" >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success !MINGW 'grep recurse submodule colon in name' '\n \tgit init parent &&\n \ttest_when_finished \"rm -rf parent\" &&\n-\techo \"foobar\" >\"parent/fi:le\" &&\n+\techo \"(1|2)d(3|4)\" >\"parent/fi:le\" &&\n \tgit -C parent add \"fi:le\" &&\n \tgit -C parent commit -m \"add fi:le\" &&\n \n \tgit init \"su:b\" &&\n \ttest_when_finished \"rm -rf su:b\" &&\n-\techo \"foobar\" >\"su:b/fi:le\" &&\n+\techo \"(1|2)d(3|4)\" >\"su:b/fi:le\" &&\n \tgit -C \"su:b\" add \"fi:le\" &&\n \tgit -C \"su:b\" commit -m \"add fi:le\" &&\n \n@@ -172,30 +172,30 @@ test_expect_success !MINGW 'grep recurse submodule colon in name' '\n \tgit -C parent commit -m \"add submodule\" &&\n \n \tcat >expect <<-\\EOF &&\n-\tfi:le:foobar\n-\tsu:b/fi:le:foobar\n+\tfi:le:(1|2)d(3|4)\n+\tsu:b/fi:le:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent grep -e \"foobar\" --recurse-submodules >actual &&\n+\tgit -C parent grep -e \"(1|2)d(3|4)\" --recurse-submodules >actual &&\n \ttest_cmp expect actual &&\n \n \tcat >expect <<-\\EOF &&\n-\tHEAD:fi:le:foobar\n-\tHEAD:su:b/fi:le:foobar\n+\tHEAD:fi:le:(1|2)d(3|4)\n+\tHEAD:su:b/fi:le:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent grep -e \"foobar\" --recurse-submodules HEAD >actual &&\n+\tgit -C parent grep -e \"(1|2)d(3|4)\" --recurse-submodules HEAD >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep history with moved submoules' '\n \tgit init parent &&\n \ttest_when_finished \"rm -rf parent\" &&\n-\techo \"foobar\" >parent/file &&\n+\techo \"(1|2)d(3|4)\" >parent/file &&\n \tgit -C parent add file &&\n \tgit -C parent commit -m \"add file\" &&\n \n \tgit init sub &&\n \ttest_when_finished \"rm -rf sub\" &&\n-\techo \"foobar\" >sub/file &&\n+\techo \"(1|2)d(3|4)\" >sub/file &&\n \tgit -C sub add file &&\n \tgit -C sub commit -m \"add file\" &&\n \n@@ -203,82 +203,82 @@ test_expect_success 'grep history with moved submoules' '\n \tgit -C parent commit -m \"add submodule\" &&\n \n \tcat >expect <<-\\EOF &&\n-\tdir/sub/file:foobar\n-\tfile:foobar\n+\tdir/sub/file:(1|2)d(3|4)\n+\tfile:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent grep -e \"foobar\" --recurse-submodules >actual &&\n+\tgit -C parent grep -e \"(1|2)d(3|4)\" --recurse-submodules >actual &&\n \ttest_cmp expect actual &&\n \n \tgit -C parent mv dir/sub sub-moved &&\n \tgit -C parent commit -m \"moved submodule\" &&\n \n \tcat >expect <<-\\EOF &&\n-\tfile:foobar\n-\tsub-moved/file:foobar\n+\tfile:(1|2)d(3|4)\n+\tsub-moved/file:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent grep -e \"foobar\" --recurse-submodules >actual &&\n+\tgit -C parent grep -e \"(1|2)d(3|4)\" --recurse-submodules >actual &&\n \ttest_cmp expect actual &&\n \n \tcat >expect <<-\\EOF &&\n-\tHEAD^:dir/sub/file:foobar\n-\tHEAD^:file:foobar\n+\tHEAD^:dir/sub/file:(1|2)d(3|4)\n+\tHEAD^:file:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent grep -e \"foobar\" --recurse-submodules HEAD^ >actual &&\n+\tgit -C parent grep -e \"(1|2)d(3|4)\" --recurse-submodules HEAD^ >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep using relative path' '\n \ttest_when_finished \"rm -rf parent sub\" &&\n \tgit init sub &&\n-\techo \"foobar\" >sub/file &&\n+\techo \"(1|2)d(3|4)\" >sub/file &&\n \tgit -C sub add file &&\n \tgit -C sub commit -m \"add file\" &&\n \n \tgit init parent &&\n-\techo \"foobar\" >parent/file &&\n+\techo \"(1|2)d(3|4)\" >parent/file &&\n \tgit -C parent add file &&\n \tmkdir parent/src &&\n-\techo \"foobar\" >parent/src/file2 &&\n+\techo \"(1|2)d(3|4)\" >parent/src/file2 &&\n \tgit -C parent add src/file2 &&\n \tgit -C parent submodule add ../sub &&\n \tgit -C parent commit -m \"add files and submodule\" &&\n \n \t# From top works\n \tcat >expect <<-\\EOF &&\n-\tfile:foobar\n-\tsrc/file2:foobar\n-\tsub/file:foobar\n+\tfile:(1|2)d(3|4)\n+\tsrc/file2:(1|2)d(3|4)\n+\tsub/file:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent grep --recurse-submodules -e \"foobar\" >actual &&\n+\tgit -C parent grep --recurse-submodules -e \"(1|2)d(3|4)\" >actual &&\n \ttest_cmp expect actual &&\n \n \t# Relative path to top\n \tcat >expect <<-\\EOF &&\n-\t../file:foobar\n-\tfile2:foobar\n-\t../sub/file:foobar\n+\t../file:(1|2)d(3|4)\n+\tfile2:(1|2)d(3|4)\n+\t../sub/file:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent/src grep --recurse-submodules -e \"foobar\" -- .. >actual &&\n+\tgit -C parent/src grep --recurse-submodules -e \"(1|2)d(3|4)\" -- .. >actual &&\n \ttest_cmp expect actual &&\n \n \t# Relative path to submodule\n \tcat >expect <<-\\EOF &&\n-\t../sub/file:foobar\n+\t../sub/file:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent/src grep --recurse-submodules -e \"foobar\" -- ../sub >actual &&\n+\tgit -C parent/src grep --recurse-submodules -e \"(1|2)d(3|4)\" -- ../sub >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'grep from a subdir' '\n \ttest_when_finished \"rm -rf parent sub\" &&\n \tgit init sub &&\n-\techo \"foobar\" >sub/file &&\n+\techo \"(1|2)d(3|4)\" >sub/file &&\n \tgit -C sub add file &&\n \tgit -C sub commit -m \"add file\" &&\n \n \tgit init parent &&\n \tmkdir parent/src &&\n-\techo \"foobar\" >parent/src/file &&\n+\techo \"(1|2)d(3|4)\" >parent/src/file &&\n \tgit -C parent add src/file &&\n \tgit -C parent submodule add ../sub src/sub &&\n \tgit -C parent submodule add ../sub sub &&\n@@ -286,19 +286,19 @@ test_expect_success 'grep from a subdir' '\n \n \t# Verify grep from root works\n \tcat >expect <<-\\EOF &&\n-\tsrc/file:foobar\n-\tsrc/sub/file:foobar\n-\tsub/file:foobar\n+\tsrc/file:(1|2)d(3|4)\n+\tsrc/sub/file:(1|2)d(3|4)\n+\tsub/file:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent grep --recurse-submodules -e \"foobar\" >actual &&\n+\tgit -C parent grep --recurse-submodules -e \"(1|2)d(3|4)\" >actual &&\n \ttest_cmp expect actual &&\n \n \t# Verify grep from a subdir works\n \tcat >expect <<-\\EOF &&\n-\tfile:foobar\n-\tsub/file:foobar\n+\tfile:(1|2)d(3|4)\n+\tsub/file:(1|2)d(3|4)\n \tEOF\n-\tgit -C parent/src grep --recurse-submodules -e \"foobar\" >actual &&\n+\tgit -C parent/src grep --recurse-submodules -e \"(1|2)d(3|4)\" >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.11.0\n\n"},{"id":"319642","messageId":"20170513231509.7834-11-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 10/29] grep: add tests for grep pattern types being passed to submodules","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:50Z","receivedAt":"2017-05-13T23:16:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add testing for grep pattern types being correctly passed to\nsubmodules. The pattern \"(.|.)[\\d]\" matches differently under\nfixed (not at all), and then matches different lines under\nbasic/extended & perl regular expressions, so this change asserts that\nthe pattern type is passed along correctly.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7814-grep-recurse-submodules.sh | 49 ++++++++++++++++++++++++++++++++++++++\n 1 file changed, 49 insertions(+)\n\ndiff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh\nindex 1472855e1d..3a58197f47 100755\n--- a/t/t7814-grep-recurse-submodules.sh\n+++ b/t/t7814-grep-recurse-submodules.sh\n@@ -313,4 +313,53 @@ test_incompatible_with_recurse_submodules ()\n test_incompatible_with_recurse_submodules --untracked\n test_incompatible_with_recurse_submodules --no-index\n \n+test_expect_success 'grep --recurse-submodules should pass the pattern type along' '\n+\t# Fixed\n+\ttest_must_fail git grep -F --recurse-submodules -e \"(.|.)[\\d]\" &&\n+\ttest_must_fail git -c grep.patternType=fixed grep --recurse-submodules -e \"(.|.)[\\d]\" &&\n+\n+\t# Basic\n+\tgit grep -G --recurse-submodules -e \"(.|.)[\\d]\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\ta:(1|2)d(3|4)\n+\tsubmodule/a:(1|2)d(3|4)\n+\tsubmodule/sub/a:(1|2)d(3|4)\n+\tEOF\n+\ttest_cmp expect actual &&\n+\tgit -c grep.patternType=basic grep --recurse-submodules -e \"(.|.)[\\d]\" >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# Extended\n+\tgit grep -E --recurse-submodules -e \"(.|.)[\\d]\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t.gitmodules:[submodule \"submodule\"]\n+\t.gitmodules:\tpath = submodule\n+\t.gitmodules:\turl = ./submodule\n+\ta:(1|2)d(3|4)\n+\tsubmodule/.gitmodules:[submodule \"sub\"]\n+\tsubmodule/a:(1|2)d(3|4)\n+\tsubmodule/sub/a:(1|2)d(3|4)\n+\tEOF\n+\ttest_cmp expect actual &&\n+\tgit -c grep.patternType=extended grep --recurse-submodules -e \"(.|.)[\\d]\" >actual &&\n+\ttest_cmp expect actual &&\n+\tgit -c grep.extendedRegexp=true grep --recurse-submodules -e \"(.|.)[\\d]\" >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# Perl\n+\tif test_have_prereq PCRE\n+\tthen\n+\t\tgit grep -P --recurse-submodules -e \"(.|.)[\\d]\" >actual &&\n+\t\tcat >expect <<-\\EOF &&\n+\t\ta:(1|2)d(3|4)\n+\t\tb/b:(3|4)\n+\t\tsubmodule/a:(1|2)d(3|4)\n+\t\tsubmodule/sub/a:(1|2)d(3|4)\n+\t\tEOF\n+\t\ttest_cmp expect actual &&\n+\t\tgit -c grep.patternType=perl grep --recurse-submodules -e \"(.|.)[\\d]\" >actual &&\n+\t\ttest_cmp expect actual\n+\tfi\n+'\n+\n test_done\n-- \n2.11.0\n\n"},{"id":"319643","messageId":"20170513231509.7834-12-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 11/29] grep: add a test helper function for less verbose -f \\0 tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:51Z","receivedAt":"2017-05-13T23:16:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a helper function to make the tests which check for patterns with\n\\0 in them more succinct. Right now this isn't a big win, but\nsubsequent commits will add a lot more of these tests.\n\nThe helper is based on the match() function in t3070-wildmatch.sh.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7008-grep-binary.sh | 58 +++++++++++++++++++++++++-------------------------\n 1 file changed, 29 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh\nindex 9c9c378119..70e7868829 100755\n--- a/t/t7008-grep-binary.sh\n+++ b/t/t7008-grep-binary.sh\n@@ -4,6 +4,29 @@ test_description='git grep in binary files'\n \n . ./test-lib.sh\n \n+nul_match () {\n+\tstatus=$1\n+\tflags=$2\n+\tpattern=$3\n+\tpattern_human=$(echo \"$pattern\" | sed 's/Q//g')\n+\n+\tif test \"$status\" = 1\n+\tthen\n+\t\ttest_expect_success \"git grep -f f $flags '$pattern_human' a\" \"\n+\t\t\tprintf '$pattern' | q_to_nul >f &&\n+\t\t\tgit grep -f f $flags a\n+\t\t\"\n+\telif test \"$status\" = 0\n+\tthen\n+\t\ttest_expect_success \"git grep -f f $flags '$pattern_human' a\" \"\n+\t\t\tprintf '$pattern' | q_to_nul >f &&\n+\t\t\ttest_must_fail git grep -f f $flags a\n+\t\t\"\n+\telse\n+\t\ttest_expect_success \"PANIC: Test framework error. Unknown status $status\" 'false'\n+\tfi\n+}\n+\n test_expect_success 'setup' \"\n \techo 'binaryQfile' | q_to_nul >a &&\n \tgit add a &&\n@@ -69,35 +92,12 @@ test_expect_failure 'git grep .fi a' '\n \tgit grep .fi a\n '\n \n-test_expect_success 'git grep -F yf a' \"\n-\tprintf 'yQf' | q_to_nul >f &&\n-\tgit grep -f f -F a\n-\"\n-\n-test_expect_success 'git grep -F yx a' \"\n-\tprintf 'yQx' | q_to_nul >f &&\n-\ttest_must_fail git grep -f f -F a\n-\"\n-\n-test_expect_success 'git grep -Fi Yf a' \"\n-\tprintf 'YQf' | q_to_nul >f &&\n-\tgit grep -f f -Fi a\n-\"\n-\n-test_expect_success 'git grep -Fi Yx a' \"\n-\tprintf 'YQx' | q_to_nul >f &&\n-\ttest_must_fail git grep -f f -Fi a\n-\"\n-\n-test_expect_success 'git grep yf a' \"\n-\tprintf 'yQf' | q_to_nul >f &&\n-\tgit grep -f f a\n-\"\n-\n-test_expect_success 'git grep yx a' \"\n-\tprintf 'yQx' | q_to_nul >f &&\n-\ttest_must_fail git grep -f f a\n-\"\n+nul_match 1 '-F' 'yQf'\n+nul_match 0 '-F' 'yQx'\n+nul_match 1 '-Fi' 'YQf'\n+nul_match 0 '-Fi' 'YQx'\n+nul_match 1 '' 'yQf'\n+nul_match 0 '' 'yQx'\n \n test_expect_success 'grep respects binary diff attribute' '\n \techo text >t &&\n-- \n2.11.0\n\n"},{"id":"319644","messageId":"20170513231509.7834-13-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 12/29] grep: prepare for testing binary regexes containing rx metacharacters","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:52Z","receivedAt":"2017-05-13T23:16:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add setup code needed for testing regexes that contain both binary\ndata and regex metacharacters.\n\nThe POSIX regcomp() function inherently can't support that, because it\ntakes a \\0-delimited char *, but other regex engines APIs like PCRE v2\ntake a pattern/length pair, and are thus able to handle \\0s in\npatterns as well as any other character.\n\nWhen kwset was imported in commit 9eceddeec6 (\"Use kwset in grep\",\n2011-08-21) this limitation was fixed, but at the expense of\nintroducing the undocumented limitation that any pattern containing \\0\nimplicitly becomes a fixed match (equivalent to -F having been\nprovided).\n\nThat's not something we'd like to keep in the future. The inability to\nmatch patterns containing \\0 is a leaky implementation detail.\n\nSo add tests as a first step towards changing that. In order to test\nthat \\0-patterns can properly match as regexes the test string needs\nto have some regex metacharacters in it.\n\nThere were other blind spots in the tests. The code around kwset\nspecially handles case-insensitive & non-ASCII data, but there were no\ntests for this.\n\nFix all of that by amending the text being matched to contain both\nregex metacharacters & non-ASCII data.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7008-grep-binary.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh\nindex 70e7868829..e7754c3946 100755\n--- a/t/t7008-grep-binary.sh\n+++ b/t/t7008-grep-binary.sh\n@@ -28,7 +28,7 @@ nul_match () {\n }\n \n test_expect_success 'setup' \"\n-\techo 'binaryQfile' | q_to_nul >a &&\n+\techo 'binaryQfileQm[*]cQ*æQð' | q_to_nul >a &&\n \tgit add a &&\n \tgit commit -m.\n \"\n@@ -162,7 +162,7 @@ test_expect_success 'grep does not honor textconv' '\n '\n \n test_expect_success 'grep --textconv honors textconv' '\n-\techo \"a:binaryQfile\" >expect &&\n+\techo \"a:binaryQfileQm[*]cQ*æQð\" >expect &&\n \tgit grep --textconv Qfile >actual &&\n \ttest_cmp expect actual\n '\n@@ -172,7 +172,7 @@ test_expect_success 'grep --no-textconv does not honor textconv' '\n '\n \n test_expect_success 'grep --textconv blob honors textconv' '\n-\techo \"HEAD:a:binaryQfile\" >expect &&\n+\techo \"HEAD:a:binaryQfileQm[*]cQ*æQð\" >expect &&\n \tgit grep --textconv Qfile HEAD:a >actual &&\n \ttest_cmp expect actual\n '\n-- \n2.11.0\n\n"},{"id":"319645","messageId":"20170513231509.7834-14-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 13/29] grep: add tests to fix blind spots with \\0 patterns","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:53Z","receivedAt":"2017-05-13T23:16:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Address a big blind spot in the tests for patterns containing \\0. The\nis_fixed() function considers any string that contains \\0 fixed, even\nif it contains regular expression metacharacters, those patterns are\ncurrently matched with kwset.\n\nBefore this change removing that memchr(s, 0, len) check from\nis_fixed() wouldn't change the result of any of the tests, since\nregcomp() will happily match the part before the \\0.\n\nFurthermore, the kwset path is dependent on whether the the -i flag is\non, and whether the pattern has any non-ASCII characters, but none of\nthis was tested for.\n\nSee the a previous commit in this series (\"grep: add tests to fix\nblind spots with \\0 patterns\", 2017-04-21) for further details &\nrationale.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7008-grep-binary.sh | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 71 insertions(+)\n\ndiff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh\nindex e7754c3946..079395ba54 100755\n--- a/t/t7008-grep-binary.sh\n+++ b/t/t7008-grep-binary.sh\n@@ -22,6 +22,18 @@ nul_match () {\n \t\t\tprintf '$pattern' | q_to_nul >f &&\n \t\t\ttest_must_fail git grep -f f $flags a\n \t\t\"\n+\telif test \"$status\" = T1\n+\tthen\n+\t\ttest_expect_failure \"git grep -f f $flags '$pattern_human' a\" \"\n+\t\t\tprintf '$pattern' | q_to_nul >f &&\n+\t\t\tgit grep -f f $flags a\n+\t\t\"\n+\telif test \"$status\" = T0\n+\tthen\n+\t\ttest_expect_failure \"git grep -f f $flags '$pattern_human' a\" \"\n+\t\t\tprintf '$pattern' | q_to_nul >f &&\n+\t\t\ttest_must_fail git grep -f f $flags a\n+\t\t\"\n \telse\n \t\ttest_expect_success \"PANIC: Test framework error. Unknown status $status\" 'false'\n \tfi\n@@ -98,6 +110,65 @@ nul_match 1 '-Fi' 'YQf'\n nul_match 0 '-Fi' 'YQx'\n nul_match 1 '' 'yQf'\n nul_match 0 '' 'yQx'\n+nul_match 1 '' 'æQð'\n+nul_match 1 '-F' 'eQm[*]c'\n+nul_match 1 '-Fi' 'EQM[*]C'\n+\n+# Regex patterns that would match but shouldn't with -F\n+nul_match 0 '-F' 'yQ[f]'\n+nul_match 0 '-F' '[y]Qf'\n+nul_match 0 '-Fi' 'YQ[F]'\n+nul_match 0 '-Fi' '[Y]QF'\n+nul_match 0 '-F' 'æQ[ð]'\n+nul_match 0 '-F' '[æ]Qð'\n+nul_match 0 '-Fi' 'ÆQ[Ð]'\n+nul_match 0 '-Fi' '[Æ]QÐ'\n+\n+# kwset is disabled on -i & non-ASCII. No way to match non-ASCII \\0\n+# patterns case-insensitively.\n+nul_match T1 '-i' 'ÆQÐ'\n+\n+# \\0 implicitly disables regexes. This is an undocumented internal\n+# limitation.\n+nul_match T1 '' 'yQ[f]'\n+nul_match T1 '' '[y]Qf'\n+nul_match T1 '-i' 'YQ[F]'\n+nul_match T1 '-i' '[Y]Qf'\n+nul_match T1 '' 'æQ[ð]'\n+nul_match T1 '' '[æ]Qð'\n+nul_match T1 '-i' 'ÆQ[Ð]'\n+\n+# ... because of \\0 implicitly disabling regexes regexes that\n+# should/shouldn't match don't do the right thing.\n+nul_match T1 '' 'eQm.*cQ'\n+nul_match T1 '-i' 'EQM.*cQ'\n+nul_match T0 '' 'eQm[*]c'\n+nul_match T0 '-i' 'EQM[*]C'\n+\n+# Due to the REG_STARTEND extension when kwset() is disabled on -i &\n+# non-ASCII the string will be matched in its entirety, but the\n+# pattern will be cut off at the first \\0.\n+nul_match 0 '-i' 'NOMATCHQð'\n+nul_match T0 '-i' '[Æ]QNOMATCH'\n+nul_match T0 '-i' '[æ]QNOMATCH'\n+# Matches, but for the wrong reasons, just stops at [æ]\n+nul_match 1 '-i' '[Æ]Qð'\n+nul_match 1 '-i' '[æ]Qð'\n+\n+# Ensure that the matcher doesn't regress to something that stops at\n+# \\0\n+nul_match 0 '-F' 'yQ[f]'\n+nul_match 0 '-Fi' 'YQ[F]'\n+nul_match 0 '' 'yQNOMATCH'\n+nul_match 0 '' 'QNOMATCH'\n+nul_match 0 '-i' 'YQNOMATCH'\n+nul_match 0 '-i' 'QNOMATCH'\n+nul_match 0 '-F' 'æQ[ð]'\n+nul_match 0 '-Fi' 'ÆQ[Ð]'\n+nul_match 0 '' 'yQNÓMATCH'\n+nul_match 0 '' 'QNÓMATCH'\n+nul_match 0 '-i' 'YQNÓMATCH'\n+nul_match 0 '-i' 'QNÓMATCH'\n \n test_expect_success 'grep respects binary diff attribute' '\n \techo text >t &&\n-- \n2.11.0\n\n"},{"id":"319646","messageId":"20170513231509.7834-15-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 14/29] perf: add a GIT_PERF_MAKE_COMMAND for when *_MAKE_OPTS won't do","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:54Z","receivedAt":"2017-05-13T23:16:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a git GIT_PERF_MAKE_COMMAND variable to compliment the existing\nGIT_PERF_MAKE_OPTS facility. This allows specifying an arbitrary shell\ncommand to execute instead of 'make'.\n\nThis is useful e.g. in cases where the name, semantics or defaults of\na Makefile flag have changed over time. It can even be used to change\nthe contents of the tree, useful for monkeypatching ancient versions\nof git to get them to build.\n\nThis opens Pandora's box in some ways, it's now possible to\n\"jailbreak\" the perf environment and e.g. modify the source tree via\nthis arbitrary instead of just issuing a custom \"make\" command, such a\ncommand has to be re-entrant in the sense that subsequent perf runs\nwill re-use the possibly modified tree.\n\nIt would be pointless to try to mitigate or work around that caveat in\na tool purely aimed at Git developers, so this change makes no attempt\nto do so.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 3 +++\n t/perf/README | 19 +++++++++++++++++--\n t/perf/run | 11 +++++++++--\n 3 files changed, 29 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex eedadb8056..d1587452f1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2272,6 +2272,9 @@ endif\n ifdef GIT_PERF_MAKE_OPTS\n \t@echo GIT_PERF_MAKE_OPTS=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_PERF_MAKE_OPTS)))'\\' >>$@+\n endif\n+ifdef GIT_PERF_MAKE_COMMAND\n+\t@echo GIT_PERF_MAKE_COMMAND=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_PERF_MAKE_COMMAND)))'\\' >>$@+\n+endif\n ifdef GIT_INTEROP_MAKE_OPTS\n \t@echo GIT_INTEROP_MAKE_OPTS=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_INTEROP_MAKE_OPTS)))'\\' >>$@+\n endif\ndiff --git a/t/perf/README b/t/perf/README\nindex 49ea4349be..b3d95042a8 100644\n--- a/t/perf/README\n+++ b/t/perf/README\n@@ -60,8 +60,23 @@ You can set the following variables (also in your config.mak):\n \n GIT_PERF_MAKE_OPTS\n \tOptions to use when automatically building a git tree for\n-\tperformance testing. E.g., -j6 would be useful.\n-\n+\tperformance testing. E.g., -j6 would be useful. Passed\n+\tdirectly to make as \"make $GIT_PERF_MAKE_OPTS\".\n+\n+ GIT_PERF_MAKE_COMMAND\n+\tAn arbitrary command that'll be run in place of the make\n+\tcommand, if set the GIT_PERF_MAKE_OPTS variable is\n+\tignored. Useful in cases where source tree changes might\n+\trequire issuing a different make command to different\n+\trevisions.\n+\n+\tThis can be (ab)used to monkeypatch or otherwise change the\n+\ttree about to be built. Note that the build directory can be\n+\tre-used for subsequent runs so the make command might get\n+\texecuted multiple times on the same tree, but don't count on\n+\tany of that, that's an implementation detail that might change\n+\tin the future.\n+ \n GIT_PERF_REPO\n GIT_PERF_LARGE_REPO\n \tRepositories to copy for the performance tests. The normal\ndiff --git a/t/perf/run b/t/perf/run\nindex c788d713ae..b61024a830 100755\n--- a/t/perf/run\n+++ b/t/perf/run\n@@ -37,8 +37,15 @@ build_git_rev () {\n \t\t\tcp \"../../$config\" \"build/$rev/\"\n \t\tfi\n \tdone\n-\t(cd build/$rev && make $GIT_PERF_MAKE_OPTS) ||\n-\tdie \"failed to build revision '$mydir'\"\n+\t(\n+\t\tcd build/$rev &&\n+\t\tif test -n \"$GIT_PERF_MAKE_COMMAND\"\n+\t\tthen\n+\t\t\tsh -c \"$GIT_PERF_MAKE_COMMAND\"\n+\t\telse\n+\t\t\tmake $GIT_PERF_MAKE_OPTS\n+\t\tfi\n+\t) || die \"failed to build revision '$mydir'\"\n }\n \n run_dirs_helper () {\n-- \n2.11.0\n\n"},{"id":"319647","messageId":"20170513231509.7834-16-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 15/29] perf: emit progress output when unpacking & building","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:55Z","receivedAt":"2017-05-13T23:16:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend the t/perf/run output so that in addition to the \"Running N\ntests\" heading currently being emitted, it also emits \"Unpacking $rev\"\nand \"Building $rev\" when setting up the build/$rev directory & when\nbuilding it, respectively.\n\nThis makes it easier to see what's going on and what revision is being\ntested as the output scrolls by.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/perf/run | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/perf/run b/t/perf/run\nindex b61024a830..beb4acc0e4 100755\n--- a/t/perf/run\n+++ b/t/perf/run\n@@ -24,6 +24,7 @@ run_one_dir () {\n \n unpack_git_rev () {\n \trev=$1\n+\techo \"=== Unpacking $rev in build/$rev ===\"\n \tmkdir -p build/$rev\n \t(cd \"$(git rev-parse --show-cdup)\" && git archive --format=tar $rev) |\n \t(cd build/$rev && tar x)\n@@ -37,6 +38,7 @@ build_git_rev () {\n \t\t\tcp \"../../$config\" \"build/$rev/\"\n \t\tfi\n \tdone\n+\techo \"=== Building $rev ===\"\n \t(\n \t\tcd build/$rev &&\n \t\tif test -n \"$GIT_PERF_MAKE_COMMAND\"\n-- \n2.11.0\n\n"},{"id":"319648","messageId":"20170513231509.7834-17-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 16/29] perf: add a performance comparison test of grep -G, -E and -P","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:56Z","receivedAt":"2017-05-13T23:16:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a very basic performance comparison test comparing the POSIX\nbasic, extended and perl engines.\n\nIn theory the \"basic\" and \"extended\" engines should be implemented\nusing the same underlying code with a slightly different pattern\nparser, but some implementations may not do this. Jump through some\nslight hoops to test both, which is worthwhile since \"basic\" is the\ndefault.\n\nRunning this on an i7 3.4GHz Linux 4.9.0-2 Debian testing against a\ncheckout of linux.git & latest upstream PCRE, both PCRE and git\ncompiled with -O3 using gcc 7.1.1:\n\n $ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux ./run p7820-grep-engines.sh\n [...]\n -------------------------------------------------------------\n 7820.1: basic grep how.to 0.28(1.27+0.41)\n 7820.2: extended grep how.to 0.29(1.23+0.44)\n 7820.3: perl grep how.to 0.27(1.14+0.47)\n 7820.5: basic grep ^how to 0.28(1.18+0.46)\n 7820.6: extended grep ^how to 0.28(1.25+0.40)\n 7820.7: perl grep ^how to 0.48(2.76+0.40)\n 7820.9: basic grep [how] to 0.42(2.20+0.42)\n 7820.10: extended grep [how] to 0.41(2.12+0.50)\n 7820.11: perl grep [how] to 0.48(2.59+0.42)\n 7820.13: basic grep \\(e.t[^ ]*\\|v.ry\\) rare 0.55(3.26+0.44)\n 7820.14: extended grep (e.t[^ ]*|v.ry) rare 0.54(3.34+0.38)\n 7820.15: perl grep (e.t[^ ]*|v.ry) rare 0.88(5.82+0.35)\n 7820.17: basic grep m\\(ú\\|u\\)lt.b\\(æ\\|y\\)te 0.29(1.26+0.48)\n 7820.18: extended grep m(ú|u)lt.b(æ|y)te 0.29(1.36+0.39)\n 7820.19: perl grep m(ú|u)lt.b(æ|y)te 0.32(1.61+0.43)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/perf/p7820-grep-engines.sh | 35 +++++++++++++++++++++++++++++++++++\n 1 file changed, 35 insertions(+)\n create mode 100755 t/perf/p7820-grep-engines.sh\n\ndiff --git a/t/perf/p7820-grep-engines.sh b/t/perf/p7820-grep-engines.sh\nnew file mode 100755\nindex 0000000000..eb439900f3\n--- /dev/null\n+++ b/t/perf/p7820-grep-engines.sh\n@@ -0,0 +1,35 @@\n+#!/bin/sh\n+\n+test_description=\"Comparison of git-grep's regex engines\"\n+\n+. ./perf-lib.sh\n+\n+test_perf_large_repo\n+test_checkout_worktree\n+\n+for pattern in \\\n+\t'how.to' \\\n+\t'^how to' \\\n+\t'[how] to' \\\n+\t'\\(e.t[^ ]*\\|v.ry\\) rare' \\\n+\t'm\\(ú\\|u\\)lt.b\\(æ\\|y\\)te'\n+do\n+\tfor engine in basic extended perl\n+\tdo\n+\t\tif test $engine != \"basic\"\n+\t\tthen\n+\t\t\t# Poor man's basic -> extended converter.\n+\t\t\tpattern=$(echo $pattern | sed 's/\\\\//g')\n+\t\tfi\n+\t\ttest_perf \"$engine grep $pattern\" \"\n+\t\t\tgit -c grep.patternType=$engine grep -- '$pattern' >'out.$engine' || :\n+\t\t\"\n+\tdone\n+\n+\ttest_expect_success \"assert that all engines found the same for $pattern\" \"\n+\t\ttest_cmp 'out.basic' 'out.extended' &&\n+\t\ttest_cmp 'out.basic' 'out.perl'\n+\t\"\n+done\n+\n+test_done\n-- \n2.11.0\n\n"},{"id":"319649","messageId":"20170513231509.7834-19-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 18/29] grep: catch a missing enum in switch statement","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:58Z","receivedAt":"2017-05-13T23:16:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a die(...) to a default case for the switch statement selecting\nbetween grep pattern types under --recurse-submodules.\n\nNormally this would be caught by -Wswitch, but the grep_pattern_type\ntype is converted to int by going through parse_options(). Changing\nthe argument type passed to compile_submodule_options() won't work,\nthe value will just get coerced. The -Wswitch-default warning will\nwarn about it, but that produces a lot of noise across the codebase,\nthis potential issue would be drowned in that noise.\n\nThus catching this at runtime is the least worst option. This won't\never trigger in practice, but if a new pattern type were to be added\nthis catches an otherwise silent bug during development.\n\nSee commit 0281e487fd (\"grep: optionally recurse into submodules\",\n2016-12-16) for the initial addition of this code.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/grep.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 3ffb5b4e81..a191e2976b 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -495,6 +495,8 @@ static void compile_submodule_options(const struct grep_opt *opt,\n \t\tbreak;\n \tcase GREP_PATTERN_TYPE_UNSPECIFIED:\n \t\tbreak;\n+\tdefault:\n+\t\tdie(\"BUG: Added a new grep pattern type without updating switch statement\");\n \t}\n \n \tfor (pattern = opt->pattern_list; pattern != NULL;\n-- \n2.11.0\n\n"},{"id":"319650","messageId":"20170513231509.7834-18-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 17/29] perf: add a performance comparison of fixed-string grep","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:57Z","receivedAt":"2017-05-13T23:17:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a performance comparison test which compares both case-sensitive &\ncase-insensitive fixed-string grep, as well as non-ASCII\ncase-sensitive & case-insensitive grep.\n\nCurrently only the \"-i æ\" performance test doesn't go through the same\nkwset.[ch] codepath, see the \"Even when -F...\" comment in grep.c.\n\n $ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux ./run p7821-grep-engines-fixed.sh\n ----------------------------------------------\n 7821.1: fixed grep int 1.75(1.39+0.34)\n 7821.2: basic grep int 1.68(1.38+0.28)\n 7821.3: extended grep int 1.75(1.41+0.29)\n 7821.4: perl grep int 1.73(1.40+0.30)\n 7821.6: fixed grep -i int 1.94(1.54+0.35)\n 7821.7: basic grep -i int 1.92(1.57+0.32)\n 7821.8: extended grep -i int 1.90(1.54+0.30)\n 7821.9: perl grep -i int 1.91(1.53+0.36)\n 7821.11: fixed grep æ 1.35(1.14+0.18)\n 7821.12: basic grep æ 1.34(1.16+0.16)\n 7821.13: extended grep æ 1.33(1.15+0.17)\n 7821.14: perl grep æ 1.35(1.12+0.20)\n 7821.16: fixed grep -i æ 0.72(0.49+0.22)\n 7821.17: basic grep -i æ 0.74(0.49+0.21)\n 7821.18: extended grep -i æ 0.72(0.48+0.22)\n 7821.19: perl grep -i æ 0.71(0.44+0.23)\n\nI'm planning to make that not be the case, this performance test gives\na baseline for comparing performance before & after any such change.\n\nSee commit (\"perf: add a performance comparison test of grep -G, -E\nand -P\", 2017-04-19) for details on the machine the above test run was\nexecuted on.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/perf/p7821-grep-engines-fixed.sh | 26 ++++++++++++++++++++++++++\n 1 file changed, 26 insertions(+)\n create mode 100755 t/perf/p7821-grep-engines-fixed.sh\n\ndiff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh\nnew file mode 100755\nindex 0000000000..d771cccfdf\n--- /dev/null\n+++ b/t/perf/p7821-grep-engines-fixed.sh\n@@ -0,0 +1,26 @@\n+#!/bin/sh\n+\n+test_description=\"Comparison of fixed string grep under git-grep's regex engines\"\n+\n+. ./perf-lib.sh\n+\n+test_perf_large_repo\n+test_checkout_worktree\n+\n+for args in 'int' '-i int' 'æ' '-i æ'\n+do\n+\tfor engine in fixed basic extended perl\n+\tdo\n+\t\ttest_perf \"$engine grep $args\" \"\n+\t\t\tgit -c grep.patternType=$engine grep $args >'out.$engine.$args' || :\n+\t\t\"\n+\tdone\n+\n+\ttest_expect_success \"assert that all engines found the same for $args\" \"\n+\t\ttest_cmp 'out.fixed.$args' 'out.basic.$args' &&\n+\t\ttest_cmp 'out.fixed.$args' 'out.extended.$args' &&\n+\t\ttest_cmp 'out.fixed.$args' 'out.perl.$args'\n+\t\"\n+done\n+\n+test_done\n-- \n2.11.0\n\n"},{"id":"319651","messageId":"20170513231509.7834-21-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 20/29] grep: remove redundant `regflags &= ~REG_EXTENDED` assignments","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:00Z","receivedAt":"2017-05-13T23:17:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove redundant assignments to the \"regflags\" variable. There are no\ncode paths that have previously set the regflags to anything, and\ncertainly not to `|= REG_EXTENDED`.\n\nThis code gave the impression that it had to reset its environment,\nbut it doesn't. This dates back to the initial introduction of\ngit-grep in commit 5010cb5fcc (\"built-in \"git grep\"\", 2006-04-30).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 59ae7809f2..bf6c2494fd 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -179,7 +179,6 @@ static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, st\n \tcase GREP_PATTERN_TYPE_BRE:\n \t\topt->fixed = 0;\n \t\topt->pcre = 0;\n-\t\topt->regflags &= ~REG_EXTENDED;\n \t\tbreak;\n \n \tcase GREP_PATTERN_TYPE_ERE:\n@@ -191,7 +190,6 @@ static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, st\n \tcase GREP_PATTERN_TYPE_FIXED:\n \t\topt->fixed = 1;\n \t\topt->pcre = 0;\n-\t\topt->regflags &= ~REG_EXTENDED;\n \t\tbreak;\n \n \tcase GREP_PATTERN_TYPE_PCRE:\n@@ -414,10 +412,9 @@ static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tint err;\n-\tint regflags;\n+\tint regflags = opt->regflags;\n \n \tbasic_regex_quote_buf(&sb, p->pattern);\n-\tregflags = opt->regflags & ~REG_EXTENDED;\n \tif (opt->ignore_case)\n \t\tregflags |= REG_ICASE;\n \terr = regcomp(&p->regexp, sb.buf, regflags);\n-- \n2.11.0\n\n"},{"id":"319652","messageId":"20170513231509.7834-20-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 19/29] grep: remove redundant regflags assignment under PCRE","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:14:59Z","receivedAt":"2017-05-13T23:17:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove a redundant assignment to the \"regflags\" variable. This\nvariable is only used for POSIX regular expression matching, not when\nthe PCRE library is used.\n\nThis redundant assignment was added as a result of copy/paste\nprogramming in commit 84befcd0a4 (\"grep: add a grep.patternType\nconfiguration setting\", 2012-08-03). That commit modified already\nworking code in commit cca2c172e0 (\"git-grep: do not die upon -F/-P\nwhen grep.extendedRegexp is set.\", 2011-05-09) which didn't assign to\nregflags when under PCRE.\n\nRevert back to that behavior, more to reduce \"wait this is used under\nPCRE how?\" confusion when reading the code, than to to save ourselves\ntrivial CPU cycles by removing one assignment.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/grep.c b/grep.c\nindex 47cee45067..59ae7809f2 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -197,7 +197,6 @@ static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, st\n \tcase GREP_PATTERN_TYPE_PCRE:\n \t\topt->fixed = 0;\n \t\topt->pcre = 1;\n-\t\topt->regflags &= ~REG_EXTENDED;\n \t\tbreak;\n \t}\n }\n-- \n2.11.0\n\n"},{"id":"319653","messageId":"20170513231509.7834-22-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 21/29] grep: factor test for \\0 in grep patterns into a function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:01Z","receivedAt":"2017-05-13T23:17:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Factor the test for \\0 in grep patterns into a function. Since commit\n9eceddeec6 (\"Use kwset in grep\", 2011-08-21) any pattern containing a\n\\0 is considered fixed as regcomp() can't handle it.\n\nThis limitation was never documented, and other some regular\nexpression engines are capable of compiling a pattern containing a\n\\0. Factoring this out makes a subsequent change which does that\nsmaller.\n\nSee a previous commit in this series (\"grep: add tests to fix blind\nspots with \\0 patterns\", 2017-04-21) for further details & rationale.\n\nWhile I'm at it make the comment conform to the style guide, i.e. add\nan opening \"/*\\n\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 20 +++++++++++++-------\n 1 file changed, 13 insertions(+), 7 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex bf6c2494fd..79eb681c6e 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -321,6 +321,18 @@ static NORETURN void compile_regexp_failed(const struct grep_pat *p,\n \tdie(\"%s'%s': %s\", where, p->pattern, error);\n }\n \n+static int has_null(const char *s, size_t len)\n+{\n+\t/*\n+\t * regcomp cannot accept patterns with NULs so when using it\n+\t * we consider any pattern containing a NUL fixed.\n+\t */\n+\tif (memchr(s, 0, len))\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n #ifdef USE_LIBPCRE\n static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)\n {\n@@ -394,12 +406,6 @@ static int is_fixed(const char *s, size_t len)\n {\n \tsize_t i;\n \n-\t/* regcomp cannot accept patterns with NULs so we\n-\t * consider any pattern containing a NUL fixed.\n-\t */\n-\tif (memchr(s, 0, len))\n-\t\treturn 1;\n-\n \tfor (i = 0; i < len; i++) {\n \t\tif (is_regex_special(s[i]))\n \t\t\treturn 0;\n@@ -451,7 +457,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \t * simple string match using kws. p->fixed tells us if we\n \t * want to use kws.\n \t */\n-\tif (opt->fixed || is_fixed(p->pattern, p->patternlen))\n+\tif (opt->fixed || has_null(p->pattern, p->patternlen) || is_fixed(p->pattern, p->patternlen))\n \t\tp->fixed = !icase || ascii_only;\n \telse\n \t\tp->fixed = 0;\n-- \n2.11.0\n\n"},{"id":"319654","messageId":"20170513231509.7834-23-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 22/29] grep: change the internal PCRE macro names to be PCRE1","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:02Z","receivedAt":"2017-05-13T23:17:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the internal USE_LIBPCRE define, & build options flag to use a\nnaming convention ending in PCRE1, without changing the long-standing\nUSE_LIBPCRE Makefile flag which enables this code.\n\nThis is for preparation for libpcre2 support where having things like\nUSE_LIBPCRE and USE_LIBPCRE2 in any more places than we absolutely\nneed to for backwards compatibility with old Makefile arguments would\nbe confusing.\n\nIn some ways it would be better to change everything that now uses\nUSE_LIBPCRE to use USE_LIBPCRE1, and to make specifying\nUSE_LIBPCRE (or --with-pcre) an error. This would impose a one-time\nburden on packagers of git to s/USE_LIBPCRE/USE_LIBPCRE1/ in their\nbuild scripts.\n\nHowever I'd like to leave the door open to making\nUSE_LIBPCRE=YesPlease eventually mean USE_LIBPCRE2=YesPlease,\ni.e. once PCRE v2 is ubiquitous enough that it makes sense to make it\nthe default.\n\nThis code and the USE_LIBPCRE Makefile argument was added in commit\n63e7e9d8b6 (\"git-grep: Learn PCRE\", 2011-05-09). At the time there was\nno indication that the PCRE project would release an entirely new &\nincompatible API around 3 years later.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 4 ++--\n grep.c | 6 +++---\n grep.h | 2 +-\n t/test-lib.sh | 2 +-\n 4 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex d1587452f1..374fbc7e58 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1088,7 +1088,7 @@ ifdef NO_LIBGEN_H\n endif\n \n ifdef USE_LIBPCRE\n-\tBASIC_CFLAGS += -DUSE_LIBPCRE\n+\tBASIC_CFLAGS += -DUSE_LIBPCRE1\n \tifdef LIBPCREDIR\n \t\tBASIC_CFLAGS += -I$(LIBPCREDIR)/include\n \t\tEXTLIBS += -L$(LIBPCREDIR)/$(lib) $(CC_LD_DYNPATH)$(LIBPCREDIR)/$(lib)\n@@ -2240,7 +2240,7 @@ GIT-BUILD-OPTIONS: FORCE\n \t@echo TAR=\\''$(subst ','\\'',$(subst ','\\'',$(TAR)))'\\' >>$@+\n \t@echo NO_CURL=\\''$(subst ','\\'',$(subst ','\\'',$(NO_CURL)))'\\' >>$@+\n \t@echo NO_EXPAT=\\''$(subst ','\\'',$(subst ','\\'',$(NO_EXPAT)))'\\' >>$@+\n-\t@echo USE_LIBPCRE=\\''$(subst ','\\'',$(subst ','\\'',$(USE_LIBPCRE)))'\\' >>$@+\n+\t@echo USE_LIBPCRE1=\\''$(subst ','\\'',$(subst ','\\'',$(USE_LIBPCRE)))'\\' >>$@+\n \t@echo NO_PERL=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PERL)))'\\' >>$@+\n \t@echo NO_PYTHON=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PYTHON)))'\\' >>$@+\n \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\ndiff --git a/grep.c b/grep.c\nindex 79eb681c6e..854f2404be 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -333,7 +333,7 @@ static int has_null(const char *s, size_t len)\n \treturn 0;\n }\n \n-#ifdef USE_LIBPCRE\n+#ifdef USE_LIBPCRE1\n static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)\n {\n \tconst char *error;\n@@ -385,7 +385,7 @@ static void free_pcre_regexp(struct grep_pat *p)\n \tpcre_free(p->pcre_extra_info);\n \tpcre_free((void *)p->pcre_tables);\n }\n-#else /* !USE_LIBPCRE */\n+#else /* !USE_LIBPCRE1 */\n static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)\n {\n \tdie(\"cannot use Perl-compatible regexes when not compiled with USE_LIBPCRE\");\n@@ -400,7 +400,7 @@ static int pcrematch(struct grep_pat *p, const char *line, const char *eol,\n static void free_pcre_regexp(struct grep_pat *p)\n {\n }\n-#endif /* !USE_LIBPCRE */\n+#endif /* !USE_LIBPCRE1 */\n \n static int is_fixed(const char *s, size_t len)\n {\ndiff --git a/grep.h b/grep.h\nindex 267534ca24..073b0e4c92 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -1,7 +1,7 @@\n #ifndef GREP_H\n #define GREP_H\n #include \"color.h\"\n-#ifdef USE_LIBPCRE\n+#ifdef USE_LIBPCRE1\n #include \n #else\n typedef int pcre;\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 6abf1d1918..e5cfbcc36b 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1010,7 +1010,7 @@ esac\n ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1\n test -z \"$NO_PERL\" && test_set_prereq PERL\n test -z \"$NO_PYTHON\" && test_set_prereq PYTHON\n-test -n \"$USE_LIBPCRE\" && test_set_prereq PCRE\n+test -n \"$USE_LIBPCRE1\" && test_set_prereq PCRE\n test -z \"$NO_GETTEXT\" && test_set_prereq GETTEXT\n \n # Can we rely on git's output in the C locale?\n-- \n2.11.0\n\n"},{"id":"319655","messageId":"20170513231509.7834-24-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 23/29] grep: change internal *pcre* variable & function names to be *pcre1*","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:03Z","receivedAt":"2017-05-13T23:17:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the internal PCRE variable & function names to have a \"1\"\nsuffix. This is for preparation for libpcre2 support, where having\nnon-versioned names would be confusing.\n\nAn earlier change in this series (\"grep: change the internal PCRE\nmacro names to be PCRE1\", 2017-04-07) elaborates on the motivations\nbehind this change.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 52 ++++++++++++++++++++++++++--------------------------\n grep.h | 8 ++++----\n 2 files changed, 30 insertions(+), 30 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 854f2404be..07512346b1 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -178,23 +178,23 @@ static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, st\n \n \tcase GREP_PATTERN_TYPE_BRE:\n \t\topt->fixed = 0;\n-\t\topt->pcre = 0;\n+\t\topt->pcre1 = 0;\n \t\tbreak;\n \n \tcase GREP_PATTERN_TYPE_ERE:\n \t\topt->fixed = 0;\n-\t\topt->pcre = 0;\n+\t\topt->pcre1 = 0;\n \t\topt->regflags |= REG_EXTENDED;\n \t\tbreak;\n \n \tcase GREP_PATTERN_TYPE_FIXED:\n \t\topt->fixed = 1;\n-\t\topt->pcre = 0;\n+\t\topt->pcre1 = 0;\n \t\tbreak;\n \n \tcase GREP_PATTERN_TYPE_PCRE:\n \t\topt->fixed = 0;\n-\t\topt->pcre = 1;\n+\t\topt->pcre1 = 1;\n \t\tbreak;\n \t}\n }\n@@ -334,7 +334,7 @@ static int has_null(const char *s, size_t len)\n }\n \n #ifdef USE_LIBPCRE1\n-static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)\n+static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n {\n \tconst char *error;\n \tint erroffset;\n@@ -342,23 +342,23 @@ static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)\n \n \tif (opt->ignore_case) {\n \t\tif (has_non_ascii(p->pattern))\n-\t\t\tp->pcre_tables = pcre_maketables();\n+\t\t\tp->pcre1_tables = pcre_maketables();\n \t\toptions |= PCRE_CASELESS;\n \t}\n \tif (is_utf8_locale() && has_non_ascii(p->pattern))\n \t\toptions |= PCRE_UTF8;\n \n-\tp->pcre_regexp = pcre_compile(p->pattern, options, &error, &erroffset,\n-\t\t\t\t p->pcre_tables);\n-\tif (!p->pcre_regexp)\n+\tp->pcre1_regexp = pcre_compile(p->pattern, options, &error, &erroffset,\n+\t\t\t\t p->pcre1_tables);\n+\tif (!p->pcre1_regexp)\n \t\tcompile_regexp_failed(p, error);\n \n-\tp->pcre_extra_info = pcre_study(p->pcre_regexp, 0, &error);\n-\tif (!p->pcre_extra_info && error)\n+\tp->pcre1_extra_info = pcre_study(p->pcre1_regexp, 0, &error);\n+\tif (!p->pcre1_extra_info && error)\n \t\tdie(\"%s\", error);\n }\n \n-static int pcrematch(struct grep_pat *p, const char *line, const char *eol,\n+static int pcre1match(struct grep_pat *p, const char *line, const char *eol,\n \t\tregmatch_t *match, int eflags)\n {\n \tint ovector[30], ret, flags = 0;\n@@ -366,7 +366,7 @@ static int pcrematch(struct grep_pat *p, const char *line, const char *eol,\n \tif (eflags & REG_NOTBOL)\n \t\tflags |= PCRE_NOTBOL;\n \n-\tret = pcre_exec(p->pcre_regexp, p->pcre_extra_info, line, eol - line,\n+\tret = pcre_exec(p->pcre1_regexp, p->pcre1_extra_info, line, eol - line,\n \t\t\t0, flags, ovector, ARRAY_SIZE(ovector));\n \tif (ret < 0 && ret != PCRE_ERROR_NOMATCH)\n \t\tdie(\"pcre_exec failed with error code %d\", ret);\n@@ -379,25 +379,25 @@ static int pcrematch(struct grep_pat *p, const char *line, const char *eol,\n \treturn ret;\n }\n \n-static void free_pcre_regexp(struct grep_pat *p)\n+static void free_pcre1_regexp(struct grep_pat *p)\n {\n-\tpcre_free(p->pcre_regexp);\n-\tpcre_free(p->pcre_extra_info);\n-\tpcre_free((void *)p->pcre_tables);\n+\tpcre_free(p->pcre1_regexp);\n+\tpcre_free(p->pcre1_extra_info);\n+\tpcre_free((void *)p->pcre1_tables);\n }\n #else /* !USE_LIBPCRE1 */\n-static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)\n+static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n {\n \tdie(\"cannot use Perl-compatible regexes when not compiled with USE_LIBPCRE\");\n }\n \n-static int pcrematch(struct grep_pat *p, const char *line, const char *eol,\n+static int pcre1match(struct grep_pat *p, const char *line, const char *eol,\n \t\tregmatch_t *match, int eflags)\n {\n \treturn 1;\n }\n \n-static void free_pcre_regexp(struct grep_pat *p)\n+static void free_pcre1_regexp(struct grep_pat *p)\n {\n }\n #endif /* !USE_LIBPCRE1 */\n@@ -477,8 +477,8 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \t\treturn;\n \t}\n \n-\tif (opt->pcre) {\n-\t\tcompile_pcre_regexp(p, opt);\n+\tif (opt->pcre1) {\n+\t\tcompile_pcre1_regexp(p, opt);\n \t\treturn;\n \t}\n \n@@ -834,8 +834,8 @@ void free_grep_patterns(struct grep_opt *opt)\n \t\tcase GREP_PATTERN_BODY:\n \t\t\tif (p->kws)\n \t\t\t\tkwsfree(p->kws);\n-\t\t\telse if (p->pcre_regexp)\n-\t\t\t\tfree_pcre_regexp(p);\n+\t\t\telse if (p->pcre1_regexp)\n+\t\t\t\tfree_pcre1_regexp(p);\n \t\t\telse\n \t\t\t\tregfree(&p->regexp);\n \t\t\tfree(p->pattern);\n@@ -914,8 +914,8 @@ static int patmatch(struct grep_pat *p, char *line, char *eol,\n \n \tif (p->fixed)\n \t\thit = !fixmatch(p, line, eol, match);\n-\telse if (p->pcre_regexp)\n-\t\thit = !pcrematch(p, line, eol, match, eflags);\n+\telse if (p->pcre1_regexp)\n+\t\thit = !pcre1match(p, line, eol, match, eflags);\n \telse\n \t\thit = !regexec_buf(&p->regexp, line, eol - line, 1, match,\n \t\t\t\t eflags);\ndiff --git a/grep.h b/grep.h\nindex 073b0e4c92..38ac82b638 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -46,9 +46,9 @@ struct grep_pat {\n \tsize_t patternlen;\n \tenum grep_header_field field;\n \tregex_t regexp;\n-\tpcre *pcre_regexp;\n-\tpcre_extra *pcre_extra_info;\n-\tconst unsigned char *pcre_tables;\n+\tpcre *pcre1_regexp;\n+\tpcre_extra *pcre1_extra_info;\n+\tconst unsigned char *pcre1_tables;\n \tkwset_t kws;\n \tunsigned fixed:1;\n \tunsigned ignore_case:1;\n@@ -111,7 +111,7 @@ struct grep_opt {\n \tint allow_textconv;\n \tint extended;\n \tint use_reflog_filter;\n-\tint pcre;\n+\tint pcre1;\n \tint relative;\n \tint pathname;\n \tint null_following_name;\n-- \n2.11.0\n\n"},{"id":"319656","messageId":"20170513231509.7834-26-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 25/29] test-lib: add a PTHREADS prerequisite","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:05Z","receivedAt":"2017-05-13T23:17:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a PTHREADS prerequisite which is false when git is compiled with\nNO_PTHREADS=YesPlease.\n\nThere's lots of custom code that runs when threading isn't available,\nbut before this prerequisite there was no way to test it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n t/README | 4 ++++\n t/test-lib.sh | 1 +\n 3 files changed, 6 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 374fbc7e58..a79274e5e6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2242,6 +2242,7 @@ GIT-BUILD-OPTIONS: FORCE\n \t@echo NO_EXPAT=\\''$(subst ','\\'',$(subst ','\\'',$(NO_EXPAT)))'\\' >>$@+\n \t@echo USE_LIBPCRE1=\\''$(subst ','\\'',$(subst ','\\'',$(USE_LIBPCRE)))'\\' >>$@+\n \t@echo NO_PERL=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PERL)))'\\' >>$@+\n+\t@echo NO_PTHREADS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PTHREADS)))'\\' >>$@+\n \t@echo NO_PYTHON=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PYTHON)))'\\' >>$@+\n \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n \t@echo PAGER_ENV=\\''$(subst ','\\'',$(subst ','\\'',$(PAGER_ENV)))'\\' >>$@+\ndiff --git a/t/README b/t/README\nindex a90cb62583..2f95860369 100644\n--- a/t/README\n+++ b/t/README\n@@ -817,6 +817,10 @@ use these, and \"test_set_prereq\" for how to define your own.\n Test is run on a filesystem which converts decomposed utf-8 (nfd)\n to precomposed utf-8 (nfc).\n \n+ - PTHREADS\n+\n+ Git wasn't compiled with NO_PTHREADS=YesPlease.\n+\n Tips for Writing Tests\n ----------------------\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex e5cfbcc36b..ab92c0ebaa 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1009,6 +1009,7 @@ esac\n \n ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1\n test -z \"$NO_PERL\" && test_set_prereq PERL\n+test -z \"$NO_PTHREADS\" && test_set_prereq PTHREADS\n test -z \"$NO_PYTHON\" && test_set_prereq PYTHON\n test -n \"$USE_LIBPCRE1\" && test_set_prereq PCRE\n test -z \"$NO_GETTEXT\" && test_set_prereq GETTEXT\n-- \n2.11.0\n\n"},{"id":"319657","messageId":"20170513231509.7834-27-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 26/29] pack-objects & index-pack: add test for --threads warning","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:06Z","receivedAt":"2017-05-13T23:17:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a test for the warning that's emitted when --threads or\npack.threads is provided under NO_PTHREADS=YesPlease. This uses the\nnew PTHREADS prerequisite.\n\nThe assertion for C_LOCALE_OUTPUT in the latter test is currently\nredundant, since unlike index-pack the pack-objects warnings aren't\ni18n'd. However they might be changed to be i18n'd in the future, and\nthere's no harm in future-proofing the test.\n\nThere's an existing bug in the implementation of pack-objects which\nthis test currently tests for as-is. Details about the bug & the fix\nare included in a follow-up change.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5300-pack-object.sh | 34 ++++++++++++++++++++++++++++++++++\n 1 file changed, 34 insertions(+)\n\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 43a672c345..1629fa80b0 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -421,6 +421,40 @@ test_expect_success 'index-pack works in non-repo' '\n \ttest_path_is_file foo.idx\n '\n \n+test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n+\ttest_must_fail git index-pack --threads=2 2>err &&\n+\tgrep ^warning: err >warnings &&\n+\ttest_line_count = 1 warnings &&\n+\tgrep -F \"no threads support, ignoring --threads=2\" err &&\n+\ttest_must_fail git -c pack.threads=2 index-pack 2>err &&\n+\tgrep ^warning: err >warnings &&\n+\ttest_line_count = 1 warnings &&\n+\tgrep -F \"no threads support, ignoring pack.threads\" err &&\n+\ttest_must_fail git -c pack.threads=2 index-pack --threads=4 2>err &&\n+\tgrep ^warning: err >warnings &&\n+\ttest_line_count = 2 warnings &&\n+\tgrep -F \"no threads support, ignoring --threads=4\" err &&\n+\tgrep -F \"no threads support, ignoring pack.threads\" err\n+'\n+\n+test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'pack-objects --threads=N or pack.threads=N warns when no pthreads' '\n+\tgit pack-objects --threads=2 --stdout --all /dev/null 2>err &&\n+\tgrep ^warning: err >warnings &&\n+\ttest_line_count = 1 warnings &&\n+\tgrep -F \"no threads support, ignoring --threads\" err &&\n+\tgit -c pack.threads=2 pack-objects --stdout --all /dev/null 2>err &&\n+\tcat err &&\n+\tgrep ^warning: err >warnings &&\n+\ttest_line_count = 2 warnings &&\n+\tgrep -F \"no threads support, ignoring --threads\" err &&\n+\tgrep -F \"no threads support, ignoring pack.threads\" err &&\n+\tgit -c pack.threads=2 pack-objects --threads=4 --stdout --all /dev/null 2>err &&\n+\tgrep ^warning: err >warnings &&\n+\ttest_line_count = 2 warnings &&\n+\tgrep -F \"no threads support, ignoring --threads\" err &&\n+\tgrep -F \"no threads support, ignoring pack.threads\" err\n+'\n+\n #\n # WARNING!\n #\n-- \n2.11.0\n\n"},{"id":"319658","messageId":"20170513231509.7834-25-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 24/29] grep: move is_fixed() earlier to avoid forward declaration","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:04Z","receivedAt":"2017-05-13T23:17:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the is_fixed() function which are currently only used in\ncompile_regexp() earlier so it can be used in the PCRE family of\nfunctions in a later change.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 07512346b1..1157529115 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -321,6 +321,18 @@ static NORETURN void compile_regexp_failed(const struct grep_pat *p,\n \tdie(\"%s'%s': %s\", where, p->pattern, error);\n }\n \n+static int is_fixed(const char *s, size_t len)\n+{\n+\tsize_t i;\n+\n+\tfor (i = 0; i < len; i++) {\n+\t\tif (is_regex_special(s[i]))\n+\t\t\treturn 0;\n+\t}\n+\n+\treturn 1;\n+}\n+\n static int has_null(const char *s, size_t len)\n {\n \t/*\n@@ -402,18 +414,6 @@ static void free_pcre1_regexp(struct grep_pat *p)\n }\n #endif /* !USE_LIBPCRE1 */\n \n-static int is_fixed(const char *s, size_t len)\n-{\n-\tsize_t i;\n-\n-\tfor (i = 0; i < len; i++) {\n-\t\tif (is_regex_special(s[i]))\n-\t\t\treturn 0;\n-\t}\n-\n-\treturn 1;\n-}\n-\n static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n-- \n2.11.0\n\n"},{"id":"319659","messageId":"20170513231509.7834-28-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 27/29] pack-objects: fix buggy warning about threads","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:07Z","receivedAt":"2017-05-13T23:17:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a buggy warning about threads under NO_PTHREADS=YesPlease. Due to\nre-using the delta_search_threads variable for both the state of the\n\"pack.threads\" config & the --threads option, setting \"pack.threads\"\nbut not supplying --threads would trigger the warning for both\n\"pack.threads\" & --threads.\n\nSolve this bug by resetting the delta_search_threads variable in\ngit_pack_config(), it might then be set by --threads again and be\nsubsequently warned about, as the test I'm changing here asserts.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/pack-objects.c | 4 +++-\n t/t5300-pack-object.sh | 3 +--\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 0fe35d1b5a..f1baf05dfe 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2472,8 +2472,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)\n \t\t\tdie(\"invalid number of threads specified (%d)\",\n \t\t\t delta_search_threads);\n #ifdef NO_PTHREADS\n-\t\tif (delta_search_threads != 1)\n+\t\tif (delta_search_threads != 1) {\n \t\t\twarning(\"no threads support, ignoring %s\", k);\n+\t\t\tdelta_search_threads = 0;\n+\t\t}\n #endif\n \t\treturn 0;\n \t}\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 1629fa80b0..0ec25c4966 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -445,8 +445,7 @@ test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'pack-objects --threads=N or pack.\n \tgit -c pack.threads=2 pack-objects --stdout --all /dev/null 2>err &&\n \tcat err &&\n \tgrep ^warning: err >warnings &&\n-\ttest_line_count = 2 warnings &&\n-\tgrep -F \"no threads support, ignoring --threads\" err &&\n+\ttest_line_count = 1 warnings &&\n \tgrep -F \"no threads support, ignoring pack.threads\" err &&\n \tgit -c pack.threads=2 pack-objects --threads=4 --stdout --all /dev/null 2>err &&\n \tgrep ^warning: err >warnings &&\n-- \n2.11.0\n\n"},{"id":"319660","messageId":"20170513231509.7834-29-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 28/29] grep: given --threads with NO_PTHREADS=YesPlease, warn","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:08Z","receivedAt":"2017-05-13T23:17:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a warning about missing thread support when grep.threads or\n--threads is set to a non 0 (default) or 1 (no parallelism) value\nunder NO_PTHREADS=YesPlease.\n\nThis is for consistency with the index-pack & pack-objects commands,\nwhich also take a --threads option & are configurable via\npack.threads, and have long warned about the same under\nNO_PTHREADS=YesPlease.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/grep.c | 13 +++++++++++++\n t/t7810-grep.sh | 18 ++++++++++++++++++\n 2 files changed, 31 insertions(+)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex a191e2976b..3c721b75a5 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -289,6 +289,17 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \t\tif (num_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d) for %s\"),\n \t\t\t num_threads, var);\n+#ifdef NO_PTHREADS\n+\t\telse if (num_threads && num_threads != 1) {\n+\t\t\t/*\n+\t\t\t * TRANSLATORS: %s is the configuration\n+\t\t\t * variable for tweaking threads, currently\n+\t\t\t * grep.threads\n+\t\t\t */\n+\t\t\twarning(_(\"no threads support, ignoring %s\"), var);\n+\t\t\tnum_threads = 0;\n+\t\t}\n+#endif\n \t}\n \n \treturn st;\n@@ -1229,6 +1240,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \telse if (num_threads < 0)\n \t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n #else\n+\tif (num_threads)\n+\t\twarning(_(\"no threads support, ignoring --threads\"));\n \tnum_threads = 0;\n #endif\n \ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 561709ef6a..f106387820 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -791,6 +791,24 @@ do\n \t\"\n done\n \n+test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'grep --threads=N or pack.threads=N warns when no pthreads' '\n+\tgit grep --threads=2 Hello hello_world 2>err &&\n+\tgrep ^warning: err >warnings &&\n+\ttest_line_count = 1 warnings &&\n+\tgrep -F \"no threads support, ignoring --threads\" err &&\n+\tgit -c grep.threads=2 grep Hello hello_world 2>err &&\n+\tgrep ^warning: err >warnings &&\n+\ttest_line_count = 1 warnings &&\n+\tgrep -F \"no threads support, ignoring grep.threads\" err &&\n+\tgit -c grep.threads=2 grep --threads=4 Hello hello_world 2>err &&\n+\tgrep ^warning: err >warnings &&\n+\ttest_line_count = 2 warnings &&\n+\tgrep -F \"no threads support, ignoring --threads\" err &&\n+\tgrep -F \"no threads support, ignoring grep.threads\" err &&\n+\tgit -c grep.threads=0 grep --threads=0 Hello hello_world 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n test_expect_success 'grep from a subdirectory to search wider area (1)' '\n \tmkdir -p s &&\n \t(\n-- \n2.11.0\n\n"},{"id":"319661","messageId":"20170513231509.7834-30-avarab@gmail.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"[PATCH v2 29/29] grep: assert that threading is enabled when calling grep_{lock,unlock}","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T23:15:09Z","receivedAt":"2017-05-13T23:17:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the grep_{lock,unlock} functions to assert that num_threads is\ntrue, instead of only locking & unlocking the pthread mutex lock when\nit is.\n\nThese functions are never called when num_threads isn't true, this\nlogic has gone through multiple iterations since the initial\nintroduction of grep threading in commit 5b594f457a (\"Threaded grep\",\n2010-01-25), but ever since then they'd only be called if num_threads\nwas true, so this check made the code confusing to read.\n\nReplace the check with an assertion, so that it's clear to the reader\nthat this code path is never taken unless we're spawning threads.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/grep.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 3c721b75a5..b1095362fb 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -73,14 +73,14 @@ static pthread_mutex_t grep_mutex;\n \n static inline void grep_lock(void)\n {\n-\tif (num_threads)\n-\t\tpthread_mutex_lock(&grep_mutex);\n+\tassert(num_threads);\n+\tpthread_mutex_lock(&grep_mutex);\n }\n \n static inline void grep_unlock(void)\n {\n-\tif (num_threads)\n-\t\tpthread_mutex_unlock(&grep_mutex);\n+\tassert(num_threads);\n+\tpthread_mutex_unlock(&grep_mutex);\n }\n \n /* Signalled when a new work_item is added to todo. */\n-- \n2.11.0\n\n"},{"id":"319755","messageId":"xmqq1srqwwbu.fsf@gitster.mtv.corp.google.com","threadId":"45955","inReplyTo":"20170513231509.7834-5-avarab@gmail.com","subject":"Re: [PATCH v2 04/29] log: add exhaustive tests for pattern style options & config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-15T04:57:57Z","receivedAt":"2017-05-15T04:58:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> +\techo 2e >expect &&\n> +\t# In PCRE \\d in [\\d] is like saying \"0-9\", and matches the 2\n> +\t# in 2e...\n> +\tgit -C num_commits log -1 --pretty=\"tformat:%s\" -F -E --perl-regexp --grep=\"[\\d]\" >actual &&\n> +\ttest_cmp expect actual &&\n> +\techo 1d >expect &&\n> +\t# ...in POSIX basic & extended it is the same as [d],\n> +\t# i.e. \"d\", which matches 1d, but not and does not match 2e.\n\ns/not and//; I think.\n\n> +\tgit -C num_commits log -1 --pretty=\"tformat:%s\" -F -E --grep=\"[\\d]\" >actual &&\n> \ttest_cmp expect actual\n> '\n> \n> @@ -280,6 +301,77 @@ test_expect_success 'log with grep.patternType configuration and command line' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'log with various grep.patternType configurations & command-lines' '\n> +\tgit init pattern-type &&\n> +\t(\n> +\t\tcd pattern-type &&\n> +\t\ttest_commit 1 file A &&\n> +\n> +\t\t# The tagname is overridden here because creating a\n> +\t\t# tag called \"(1|2)\" as test_commit would otherwise\n> +\t\t# implicitly do would fail on e.g. MINGW.\n\nThanks.\n\n> +\t\t# POSIX extended needs to have | escaped to match it\n> +\t\t# literally, whereas under basic this is the same as\n> +\t\t# (|2), i.e. it would also match \"1\". This test checks\n> +\t\t# for extended by asserting that it is not matching\n> +\t\t# what basic would match.\n> +\t\tgit -c grep.patternType=extended log --pretty=tformat:%s \\\n> +\t\t\t--grep=\"\\|2\" >actual.extended &&\n\nMakes sense.\n\n> +\t\tif test_have_prereq PCRE\n> +\t\tthen\n> +\t\t\t# Only PCRE would match [\\d]\\| with only\n> +\t\t\t# \"(1|2)\" due to [\\d]. POSIX basic would match\n> +\t\t\t# both it and \"1\", and POSIX extended would\n> +\t\t\t# match neither.\n\nOK. BRE would match because the other side of \"\\|\" is empty to\nmatch anything?\n\n> +\t\t\tgit -c grep.patternType=perl log --pretty=tformat:%s \\\n> +\t\t\t\t--grep=\"[\\d]\\|\" >actual.perl &&\n> +\t\t\ttest_cmp expect.perl actual.perl\n> +\t\tfi &&\n\n"},{"id":"319756","messageId":"xmqqwp9ivfbu.fsf@gitster.mtv.corp.google.com","threadId":"45955","inReplyTo":"20170513231509.7834-19-avarab@gmail.com","subject":"Re: [PATCH v2 18/29] grep: catch a missing enum in switch statement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-15T05:50:29Z","receivedAt":"2017-05-15T05:50:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Add a die(...) to a default case for the switch statement selecting\n> between grep pattern types under --recurse-submodules.\n>\n> Normally this would be caught by -Wswitch, but the grep_pattern_type\n> type is converted to int by going through parse_options(). Changing\n> the argument type passed to compile_submodule_options() won't work,\n> the value will just get coerced. The -Wswitch-default warning will\n> warn about it, but that produces a lot of noise across the codebase,\n> this potential issue would be drowned in that noise.\n>\n> Thus catching this at runtime is the least worst option. This won't\n\nleast bad, I guess?\n\n> ever trigger in practice, but if a new pattern type were to be added\n> this catches an otherwise silent bug during development.\n\nGood future-proofing. When this and Peff's \"BUG\" thing both\ngraduates, we can turn this into a BUG, I think.\n\nThanks.\n\n>\n> See commit 0281e487fd (\"grep: optionally recurse into submodules\",\n> 2016-12-16) for the initial addition of this code.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/grep.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 3ffb5b4e81..a191e2976b 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -495,6 +495,8 @@ static void compile_submodule_options(const struct grep_opt *opt,\n> \t\tbreak;\n> \tcase GREP_PATTERN_TYPE_UNSPECIFIED:\n> \t\tbreak;\n> +\tdefault:\n> +\t\tdie(\"BUG: Added a new grep pattern type without updating switch statement\");\n> \t}\n> \n> \tfor (pattern = opt->pattern_list; pattern != NULL;\n"},{"id":"319757","messageId":"xmqqshk6ve8k.fsf@gitster.mtv.corp.google.com","threadId":"45955","inReplyTo":"20170513231509.7834-21-avarab@gmail.com","subject":"Re: [PATCH v2 20/29] grep: remove redundant `regflags &= ~REG_EXTENDED` assignments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-15T06:14:03Z","receivedAt":"2017-05-15T06:14:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Remove redundant assignments to the \"regflags\" variable. There are no\n> code paths that have previously set the regflags to anything, and\n> certainly not to `|= REG_EXTENDED`.\n>\n> This code gave the impression that it had to reset its environment,\n> but it doesn't. This dates back to the initial introduction of\n> git-grep in commit 5010cb5fcc (\"built-in \"git grep\"\", 2006-04-30).\n\nBack in 5010cb5fcc, we did do \"opt.regflags &= ~REG_EXTENDED\" upon\nseeing \"-G\" on the command line and flipped the bit on upon seeing\n\"-E\", but I think that was perfectly sensible and it would have been\na bug if we didn't. They were part of the command line parsing that\ncould have seen \"-E\" on the command line earlier.\n\nIf we want to find a commit to assign blame to, I think it is more\ncorrect to say that this came from the same one as 19/29 fixes.\n\nWhen cca2c172 (\"git-grep: do not die upon -F/-P when\ngrep.extendedRegexp is set.\", 2011-05-09) switched the command line\nparsing to \"read into a 'tentatively this is what we saw the last'\nvariable and then finally commit just once\", we didn't touch\nopt.regflags for PCRE and FIXED, but we still had to flip regflags\nbetween BRE and ERE, because parsing of grep.extendedregexp\nconfiguration variable directly touched opt.regflags back then,\nwhich was done by b22520a3 (\"grep: allow -E and -n to be turned on\nby default via configuration\", 2011-03-30). When 84befcd0 (\"grep:\nadd a grep.patternType configuration setting\", 2012-08-03)\nintroduced extended_regexp_option field, we stopped flipping\nregflags while reading the configuration, and that was when we\nshould have noticed and stopped dropping REG_EXTENDED bit in the\n\"now we can commit what type to use\" helper function.\n\nIn any case, I think this change is safe to do in the current\ncodebase. I wonder if this and 19/29 should be a single patch,\nthough, as the unnecessary bit-flipping all are blamed to the same\norigin.\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> grep.c | 5 +----\n> 1 file changed, 1 insertion(+), 4 deletions(-)\n>\n> diff --git a/grep.c b/grep.c\n> index 59ae7809f2..bf6c2494fd 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -179,7 +179,6 @@ static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, st\n> \tcase GREP_PATTERN_TYPE_BRE:\n> \t\topt->fixed = 0;\n> \t\topt->pcre = 0;\n> -\t\topt->regflags &= ~REG_EXTENDED;\n> \t\tbreak;\n> \n> \tcase GREP_PATTERN_TYPE_ERE:\n> @@ -191,7 +190,6 @@ static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, st\n> \tcase GREP_PATTERN_TYPE_FIXED:\n> \t\topt->fixed = 1;\n> \t\topt->pcre = 0;\n> -\t\topt->regflags &= ~REG_EXTENDED;\n> \t\tbreak;\n> \n> \tcase GREP_PATTERN_TYPE_PCRE:\n> @@ -414,10 +412,9 @@ static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt)\n> {\n> \tstruct strbuf sb = STRBUF_INIT;\n> \tint err;\n> -\tint regflags;\n> +\tint regflags = opt->regflags;\n> \n> \tbasic_regex_quote_buf(&sb, p->pattern);\n> -\tregflags = opt->regflags & ~REG_EXTENDED;\n> \tif (opt->ignore_case)\n> \t\tregflags |= REG_ICASE;\n> \terr = regcomp(&p->regexp, sb.buf, regflags);\n"},{"id":"319758","messageId":"xmqqo9uuvdra.fsf@gitster.mtv.corp.google.com","threadId":"45955","inReplyTo":"20170513231509.7834-22-avarab@gmail.com","subject":"Re: [PATCH v2 21/29] grep: factor test for \\0 in grep patterns into a function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-15T06:24:25Z","receivedAt":"2017-05-15T06:24:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Factor the test for \\0 in grep patterns into a function. Since commit\n> 9eceddeec6 (\"Use kwset in grep\", 2011-08-21) any pattern containing a\n> \\0 is considered fixed as regcomp() can't handle it.\n>\n> This limitation was never documented, and other some regular\n> expression engines are capable of compiling a pattern containing a\n> \\0. Factoring this out makes a subsequent change which does that\n> smaller.\n\nThe latter half of the first sentence of this paragraph does not\nparse well, around \"and other some\". \"never documented\" makes\nreaders expect \"... so let's document it\", but I think you are\ndriving in a different direction, something like...\n\n Since some other regular expression engines are capable of a\n pattern to match a string with NUL in it, it would be nice\n if we can use such an engine and match against NUL; as this\n \"patterns with NUL always use --fixed match\" is not documented,\n\tlet's hope that nobody depend on that current behaviour.\n This step does not yet change the behaviour yet, but it\n makes it easier to do so in later steps.\n\nperhaps? I tend to agree that it is OK to break users who depended\non that \"patterns with NULL match with --fixed\" (partly because it\nis so unintuitive and not useful behaviour, and partly because it is\neasy for them to explicitly say -F), but let's make sure we clearly\nsay that we will be knowingly breaking them.\n\nThanks.\n"},{"id":"319765","messageId":"xmqqfug6v6l3.fsf@gitster.mtv.corp.google.com","threadId":"45955","inReplyTo":"20170513231509.7834-28-avarab@gmail.com","subject":"Re: [PATCH v2 27/29] pack-objects: fix buggy warning about threads","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-15T08:59:20Z","receivedAt":"2017-05-15T08:59:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Fix a buggy warning about threads under NO_PTHREADS=YesPlease. Due to\n> re-using the delta_search_threads variable for both the state of the\n> \"pack.threads\" config & the --threads option, setting \"pack.threads\"\n> but not supplying --threads would trigger the warning for both\n> \"pack.threads\" & --threads.\n>\n> Solve this bug by resetting the delta_search_threads variable in\n> git_pack_config(), it might then be set by --threads again and be\n> subsequently warned about, as the test I'm changing here asserts.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/pack-objects.c | 4 +++-\n> t/t5300-pack-object.sh | 3 +--\n> 2 files changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index 0fe35d1b5a..f1baf05dfe 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -2472,8 +2472,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)\n> \t\t\tdie(\"invalid number of threads specified (%d)\",\n> \t\t\t delta_search_threads);\n> #ifdef NO_PTHREADS\n> -\t\tif (delta_search_threads != 1)\n> +\t\tif (delta_search_threads != 1) {\n> \t\t\twarning(\"no threads support, ignoring %s\", k);\n> +\t\t\tdelta_search_threads = 0;\n> +\t\t}\n> #endif\n> \t\treturn 0;\n> \t}\n> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n> index 1629fa80b0..0ec25c4966 100755\n> --- a/t/t5300-pack-object.sh\n> +++ b/t/t5300-pack-object.sh\n> @@ -445,8 +445,7 @@ test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'pack-objects --threads=N or pack.\n> \tgit -c pack.threads=2 pack-objects --stdout --all /dev/null 2>err &&\n> \tcat err &&\n> \tgrep ^warning: err >warnings &&\n> -\ttest_line_count = 2 warnings &&\n> -\tgrep -F \"no threads support, ignoring --threads\" err &&\n> +\ttest_line_count = 1 warnings &&\n> \tgrep -F \"no threads support, ignoring pack.threads\" err &&\n> \tgit -c pack.threads=2 pack-objects --threads=4 --stdout --all /dev/null 2>err &&\n> \tgrep ^warning: err >warnings &&\n\nCommenting on both 26 and 27.\n\nThe usual way we document a known breakage to be fixed is to write a\ntest that checks for the desired outcome with test_expect_failure,\nand when a patch corrects the behaviour we just flip it to expect\nsuccess instead. On the other hand, when we document a behaviour\nthat is accepted/acceptable we would have a test that checks for the\nthen-accepted behaviour with test_expect_success, and a patch that\nimproves the behaviour would update the expectation.\n\nThis follows the second pattern, even though the log message for the\npatches claim this is about an existing bug and its fix.\n\nNow, I am not saying (at least not yet) that 26 & 27 violates the\nestablished practice and they must be changed to expect seeing only\none line of output in warnings with test_expect_failure in patch 26\nwhich is flipped to test_expect_success in patch 27. Yes, it does\nnot follow the usual pattern, but it gives a good food-for-thought.\n\nPerhaps our usual pattern may be suboptimal in illustrating in what\nway the current behaviour is not desirable, and the way these two\npatches document the current breakage and then documents the fixed\nbehaviour may be a better example to follow? With our usual way, it\nis not apparent until you actually run the tests with the current\ncode what the questionable current behaviour is, but with the way\npatch 26 is written, we can tell that two warnings are given,\ninstead of one.\n\nI dunno. What do others think?\n"},{"id":"319766","messageId":"xmqqbmquv64o.fsf@gitster.mtv.corp.google.com","threadId":"45955","inReplyTo":"20170513231509.7834-1-avarab@gmail.com","subject":"Re: [PATCH v2 00/29] Easy to review grep & pre-PCRE changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-15T09:09:11Z","receivedAt":"2017-05-15T09:09:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Except for a few nits and huh?s, I found this round quite a pleasant\nread in general. Thanks.\n"},{"id":"319800","messageId":"CACBZZX61W_MxJJ=YFEGLCk-ktb9xj=D-_jet02McriKJHiHO9g@mail.gmail.com","threadId":"45955","inReplyTo":"xmqqfug6v6l3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 27/29] pack-objects: fix buggy warning about threads","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-15T17:16:32Z","receivedAt":"2017-05-15T17:16:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, May 15, 2017 at 10:59 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Fix a buggy warning about threads under NO_PTHREADS=YesPlease. Due to\n>> re-using the delta_search_threads variable for both the state of the\n>> \"pack.threads\" config & the --threads option, setting \"pack.threads\"\n>> but not supplying --threads would trigger the warning for both\n>> \"pack.threads\" & --threads.\n>>\n>> Solve this bug by resetting the delta_search_threads variable in\n>> git_pack_config(), it might then be set by --threads again and be\n>> subsequently warned about, as the test I'm changing here asserts.\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>> builtin/pack-objects.c | 4 +++-\n>> t/t5300-pack-object.sh | 3 +--\n>> 2 files changed, 4 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n>> index 0fe35d1b5a..f1baf05dfe 100644\n>> --- a/builtin/pack-objects.c\n>> +++ b/builtin/pack-objects.c\n>> @@ -2472,8 +2472,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)\n>> die(\"invalid number of threads specified (%d)\",\n>> delta_search_threads);\n>> #ifdef NO_PTHREADS\n>> - if (delta_search_threads != 1)\n>> + if (delta_search_threads != 1) {\n>> warning(\"no threads support, ignoring %s\", k);\n>> + delta_search_threads = 0;\n>> + }\n>> #endif\n>> return 0;\n>> }\n>> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n>> index 1629fa80b0..0ec25c4966 100755\n>> --- a/t/t5300-pack-object.sh\n>> +++ b/t/t5300-pack-object.sh\n>> @@ -445,8 +445,7 @@ test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'pack-objects --threads=N or pack.\n>> git -c pack.threads=2 pack-objects --stdout --all /dev/null 2>err &&\n>> cat err &&\n>> grep ^warning: err >warnings &&\n>> - test_line_count = 2 warnings &&\n>> - grep -F \"no threads support, ignoring --threads\" err &&\n>> + test_line_count = 1 warnings &&\n>> grep -F \"no threads support, ignoring pack.threads\" err &&\n>> git -c pack.threads=2 pack-objects --threads=4 --stdout --all /dev/null 2>err &&\n>> grep ^warning: err >warnings &&\n>\n> Commenting on both 26 and 27.\n>\n> The usual way we document a known breakage to be fixed is to write a\n> test that checks for the desired outcome with test_expect_failure,\n> and when a patch corrects the behaviour we just flip it to expect\n> success instead. On the other hand, when we document a behaviour\n> that is accepted/acceptable we would have a test that checks for the\n> then-accepted behaviour with test_expect_success, and a patch that\n> improves the behaviour would update the expectation.\n>\n> This follows the second pattern, even though the log message for the\n> patches claim this is about an existing bug and its fix.\n>\n> Now, I am not saying (at least not yet) that 26 & 27 violates the\n> established practice and they must be changed to expect seeing only\n> one line of output in warnings with test_expect_failure in patch 26\n> which is flipped to test_expect_success in patch 27. Yes, it does\n> not follow the usual pattern, but it gives a good food-for-thought.\n>\n> Perhaps our usual pattern may be suboptimal in illustrating in what\n> way the current behaviour is not desirable, and the way these two\n> patches document the current breakage and then documents the fixed\n> behaviour may be a better example to follow? With our usual way, it\n> is not apparent until you actually run the tests with the current\n> code what the questionable current behaviour is, but with the way\n> patch 26 is written, we can tell that two warnings are given,\n> instead of one.\n>\n> I dunno. What do others think?\n\nI think it makes sense to make use of test_expect_failure here. I'll\ndo that in v3.\n"},{"id":"319803","messageId":"CACBZZX4EfWCD3rwQbRJ1TokDkH=7EfoepMd_ipHr5NSv9kWOJg@mail.gmail.com","threadId":"45955","inReplyTo":"xmqq1srqwwbu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 04/29] log: add exhaustive tests for pattern style options & config","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-15T17:38:14Z","receivedAt":"2017-05-15T17:38:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, May 15, 2017 at 6:57 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> + echo 2e >expect &&\n>> + # In PCRE \\d in [\\d] is like saying \"0-9\", and matches the 2\n>> + # in 2e...\n>> + git -C num_commits log -1 --pretty=\"tformat:%s\" -F -E --perl-regexp --grep=\"[\\d]\" >actual &&\n>> + test_cmp expect actual &&\n>> + echo 1d >expect &&\n>> + # ...in POSIX basic & extended it is the same as [d],\n>> + # i.e. \"d\", which matches 1d, but not and does not match 2e.\n>\n> s/not and//; I think.\n\nWill fix.\n\n>> + git -C num_commits log -1 --pretty=\"tformat:%s\" -F -E --grep=\"[\\d]\" >actual &&\n>> test_cmp expect actual\n>> '\n>>\n>> @@ -280,6 +301,77 @@ test_expect_success 'log with grep.patternType configuration and command line' '\n>> test_cmp expect actual\n>> '\n>>\n>> +test_expect_success 'log with various grep.patternType configurations & command-lines' '\n>> + git init pattern-type &&\n>> + (\n>> + cd pattern-type &&\n>> + test_commit 1 file A &&\n>> +\n>> + # The tagname is overridden here because creating a\n>> + # tag called \"(1|2)\" as test_commit would otherwise\n>> + # implicitly do would fail on e.g. MINGW.\n>\n> Thanks.\n>\n>> + # POSIX extended needs to have | escaped to match it\n>> + # literally, whereas under basic this is the same as\n>> + # (|2), i.e. it would also match \"1\". This test checks\n>> + # for extended by asserting that it is not matching\n>> + # what basic would match.\n>> + git -c grep.patternType=extended log --pretty=tformat:%s \\\n>> + --grep=\"\\|2\" >actual.extended &&\n>\n> Makes sense.\n>\n>> + if test_have_prereq PCRE\n>> + then\n>> + # Only PCRE would match [\\d]\\| with only\n>> + # \"(1|2)\" due to [\\d]. POSIX basic would match\n>> + # both it and \"1\", and POSIX extended would\n>> + # match neither.\n>\n> OK. BRE would match because the other side of \"\\|\" is empty to\n> match anything?\n\nYes. I'll clarify this. It's not just a POSIX basic feature. The same\nis true of extended and perl. E.g.:\n\n git grep [-E|-P] 'foo|bar'\n\nBoth match the same as:\n\n git grep [-E|-P] '(foo|bar)'\n\n>> + git -c grep.patternType=perl log --pretty=tformat:%s \\\n>> + --grep=\"[\\d]\\|\" >actual.perl &&\n>> + test_cmp expect.perl actual.perl\n>> + fi &&\n>\n"},{"id":"319804","messageId":"CACBZZX5upAY2MB7vk9wNaXUjnEZmw1vEkp3eKk+UyFvPm567Gw@mail.gmail.com","threadId":"45955","inReplyTo":"xmqqwp9ivfbu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 18/29] grep: catch a missing enum in switch statement","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-15T17:39:22Z","receivedAt":"2017-05-15T17:39:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, May 15, 2017 at 7:50 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Add a die(...) to a default case for the switch statement selecting\n>> between grep pattern types under --recurse-submodules.\n>>\n>> Normally this would be caught by -Wswitch, but the grep_pattern_type\n>> type is converted to int by going through parse_options(). Changing\n>> the argument type passed to compile_submodule_options() won't work,\n>> the value will just get coerced. The -Wswitch-default warning will\n>> warn about it, but that produces a lot of noise across the codebase,\n>> this potential issue would be drowned in that noise.\n>>\n>> Thus catching this at runtime is the least worst option. This won't\n>\n> least bad, I guess?\n\nWill fix.\n\n>> ever trigger in practice, but if a new pattern type were to be added\n>> this catches an otherwise silent bug during development.\n>\n> Good future-proofing. When this and Peff's \"BUG\" thing both\n> graduates, we can turn this into a BUG, I think.\n\nYup, this and a bunch of other stuff presumably. The BUG feature is nice.\n\n> Thanks.\n>\n>>\n>> See commit 0281e487fd (\"grep: optionally recurse into submodules\",\n>> 2016-12-16) for the initial addition of this code.\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>> builtin/grep.c | 2 ++\n>> 1 file changed, 2 insertions(+)\n>>\n>> diff --git a/builtin/grep.c b/builtin/grep.c\n>> index 3ffb5b4e81..a191e2976b 100644\n>> --- a/builtin/grep.c\n>> +++ b/builtin/grep.c\n>> @@ -495,6 +495,8 @@ static void compile_submodule_options(const struct grep_opt *opt,\n>> break;\n>> case GREP_PATTERN_TYPE_UNSPECIFIED:\n>> break;\n>> + default:\n>> + die(\"BUG: Added a new grep pattern type without updating switch statement\");\n>> }\n>>\n>> for (pattern = opt->pattern_list; pattern != NULL;\n"},{"id":"319805","messageId":"CACBZZX5jRVtJd8wj93TGS4mcE=9qaxwQjmJQqNkcxuRkJ9fXaQ@mail.gmail.com","threadId":"45955","inReplyTo":"xmqqshk6ve8k.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 20/29] grep: remove redundant `regflags &= ~REG_EXTENDED` assignments","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-15T17:41:41Z","receivedAt":"2017-05-15T17:42:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, May 15, 2017 at 8:14 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Remove redundant assignments to the \"regflags\" variable. There are no\n>> code paths that have previously set the regflags to anything, and\n>> certainly not to `|= REG_EXTENDED`.\n>>\n>> This code gave the impression that it had to reset its environment,\n>> but it doesn't. This dates back to the initial introduction of\n>> git-grep in commit 5010cb5fcc (\"built-in \"git grep\"\", 2006-04-30).\n>\n> Back in 5010cb5fcc, we did do \"opt.regflags &= ~REG_EXTENDED\" upon\n> seeing \"-G\" on the command line and flipped the bit on upon seeing\n> \"-E\", but I think that was perfectly sensible and it would have been\n> a bug if we didn't. They were part of the command line parsing that\n> could have seen \"-E\" on the command line earlier.\n>\n> If we want to find a commit to assign blame to, I think it is more\n> correct to say that this came from the same one as 19/29 fixes.\n>\n> When cca2c172 (\"git-grep: do not die upon -F/-P when\n> grep.extendedRegexp is set.\", 2011-05-09) switched the command line\n> parsing to \"read into a 'tentatively this is what we saw the last'\n> variable and then finally commit just once\", we didn't touch\n> opt.regflags for PCRE and FIXED, but we still had to flip regflags\n> between BRE and ERE, because parsing of grep.extendedregexp\n> configuration variable directly touched opt.regflags back then,\n> which was done by b22520a3 (\"grep: allow -E and -n to be turned on\n> by default via configuration\", 2011-03-30). When 84befcd0 (\"grep:\n> add a grep.patternType configuration setting\", 2012-08-03)\n> introduced extended_regexp_option field, we stopped flipping\n> regflags while reading the configuration, and that was when we\n> should have noticed and stopped dropping REG_EXTENDED bit in the\n> \"now we can commit what type to use\" helper function.\n\nThanks for the history. I'll amend the commit message to note this.\n\n> In any case, I think this change is safe to do in the current\n> codebase. I wonder if this and 19/29 should be a single patch,\n> though, as the unnecessary bit-flipping all are blamed to the same\n> origin.\n\nSquashing these two makes sense. I'll do that.\n\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>> grep.c | 5 +----\n>> 1 file changed, 1 insertion(+), 4 deletions(-)\n>>\n>> diff --git a/grep.c b/grep.c\n>> index 59ae7809f2..bf6c2494fd 100644\n>> --- a/grep.c\n>> +++ b/grep.c\n>> @@ -179,7 +179,6 @@ static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, st\n>> case GREP_PATTERN_TYPE_BRE:\n>> opt->fixed = 0;\n>> opt->pcre = 0;\n>> - opt->regflags &= ~REG_EXTENDED;\n>> break;\n>>\n>> case GREP_PATTERN_TYPE_ERE:\n>> @@ -191,7 +190,6 @@ static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, st\n>> case GREP_PATTERN_TYPE_FIXED:\n>> opt->fixed = 1;\n>> opt->pcre = 0;\n>> - opt->regflags &= ~REG_EXTENDED;\n>> break;\n>>\n>> case GREP_PATTERN_TYPE_PCRE:\n>> @@ -414,10 +412,9 @@ static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt)\n>> {\n>> struct strbuf sb = STRBUF_INIT;\n>> int err;\n>> - int regflags;\n>> + int regflags = opt->regflags;\n>>\n>> basic_regex_quote_buf(&sb, p->pattern);\n>> - regflags = opt->regflags & ~REG_EXTENDED;\n>> if (opt->ignore_case)\n>> regflags |= REG_ICASE;\n>> err = regcomp(&p->regexp, sb.buf, regflags);\n"},{"id":"319807","messageId":"CACBZZX7DbZh0jYG5SAFvFiOHvowyG=_bdbPbQj7Cr+zw-jOXJg@mail.gmail.com","threadId":"45955","inReplyTo":"xmqqo9uuvdra.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 21/29] grep: factor test for \\0 in grep patterns into a function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-15T18:07:49Z","receivedAt":"2017-05-15T18:08:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, May 15, 2017 at 8:24 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Factor the test for \\0 in grep patterns into a function. Since commit\n>> 9eceddeec6 (\"Use kwset in grep\", 2011-08-21) any pattern containing a\n>> \\0 is considered fixed as regcomp() can't handle it.\n>>\n>> This limitation was never documented, and other some regular\n>> expression engines are capable of compiling a pattern containing a\n>> \\0. Factoring this out makes a subsequent change which does that\n>> smaller.\n>\n> The latter half of the first sentence of this paragraph does not\n> parse well, around \"and other some\". \"never documented\" makes\n> readers expect \"... so let's document it\", but I think you are\n> driving in a different direction, something like...\n\nI'll try to reword it.\n\n> Since some other regular expression engines are capable of a\n> pattern to match a string with NUL in it, it would be nice\n> if we can use such an engine and match against NUL; as this\n> \"patterns with NUL always use --fixed match\" is not documented,\n> let's hope that nobody depend on that current behaviour.\n> This step does not yet change the behaviour yet, but it\n> makes it easier to do so in later steps.\n>\n> perhaps? I tend to agree that it is OK to break users who depended\n> on that \"patterns with NULL match with --fixed\" (partly because it\n> is so unintuitive and not useful behaviour, and partly because it is\n> easy for them to explicitly say -F), but let's make sure we clearly\n> say that we will be knowingly breaking them.\n\nI think there's a few different things going on here, which I'll\naddress accordingly.\n\n * Yes, if someone is relying on the undocumented behavior of a \\0\npattern implying -F if and when I change that their stuff will break.\nI'm going to actually just drop this whole discussion from this\ncommit, we can have an explanation for that when and if I actually\nchange it in a later series.\n\n* I'm making things more confusing by saying \"engines\" here, but it's\nimportant to note that git has never in its docs promised to use the C\nlibrary implementation of POSIX regexes, it's just advertised that it\nuses POSIX regular *expressions*, which are a different thing.\nImplementation != syntax.\n\nE.g. GNU grep uses POSIX regex syntax, but its engine does not have\nthe same limitation as ours:\n\n $ (command cd /tmp && printf \"æ\\0ð\\n\" >a && printf \"æ\\n\" >> a;\nprintf \"æ\\0[öð]\" >f; grep -a -f f a; echo $?)\n æð\n 0\n\nI went down this particular rabbit hole because I was genuinely\nsurprised that this didn't work with git-grep, until I realized that\nof course the implementation detail of using C strings for this under\nthe hood was bleeding through.\n"},{"id":"319857","messageId":"xmqqy3tx4obg.fsf@gitster.mtv.corp.google.com","threadId":"45955","inReplyTo":"CACBZZX4EfWCD3rwQbRJ1TokDkH=7EfoepMd_ipHr5NSv9kWOJg@mail.gmail.com","subject":"Re: [PATCH v2 04/29] log: add exhaustive tests for pattern style options & config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-16T00:50:43Z","receivedAt":"2017-05-16T00:50:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Mon, May 15, 2017 at 6:57 AM, Junio C Hamano wrote:\n>> Ævar Arnfjörð Bjarmason writes:\n>>> + if test_have_prereq PCRE\n>>> + then\n>>> + # Only PCRE would match [\\d]\\| with only\n>>> + # \"(1|2)\" due to [\\d]. POSIX basic would match\n>>> + # both it and \"1\", and POSIX extended would\n>>> + # match neither.\n>>\n>> OK. BRE would match because the other side of \"\\|\" is empty to\n>> match anything?\n>\n> Yes. I'll clarify this. It's not just a POSIX basic feature. The same\n> is true of extended and perl. E.g.:\n\nYes, but \"\\|\" won't be taken as alternative in ERE or PCRE, and that\nis why \"[\\d]\\|\" would match everything with BRE (as opposed to\nothers---PCRE matches \"(1|2)\" not because \"\\|\" is an alternative but\nbecause the pattern looks for a digit followed by a literal vert-bar,\nand ERE does not match any because there is no 'd' followed by a\nliteral vert-bar).\n\nI was mostly reacting to \"BRE would match both it and '1'\", which\nsingled out \"1\" as if \"1\" is special and gives a false impression\nthat it wouldn't have matched if it were \"7\".\n\n>\n> git grep [-E|-P] 'foo|bar'\n>\n> Both match the same as:\n>\n> git grep [-E|-P] '(foo|bar)'\n>\n>>> + git -c grep.patternType=perl log --pretty=tformat:%s \\\n>>> + --grep=\"[\\d]\\|\" >actual.perl &&\n>>> + test_cmp expect.perl actual.perl\n>>> + fi &&\n>>\n"}]}