summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* Makefile & configure: make PCRE v2 the default PCRE implementationab/grep-pcre-v2Ævar Arnfjörð Bjarmason2017-05-022-20/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the USE_LIBPCRE=YesPlease & --with-libpcre flags to the Makefile & configure script, respectively, to mean use PCRE v2, not PCRE v1. The legacy library previously available via those options is still available on request via USE_LIBPCRE1=YesPlease or --with-libpcre1. The existing USE_LIBPCRE2=YesPlease & --with-libpcre2 still explicitly ask for v2. The v2 PCRE is stable & end-user compatible, all this change does is change the default. Someone building a new git is likely to also have packaged PCRE v2 sometime in the last 2 years since it was released. If not they can choose to use the legacy v2 library by making the trivial s/USE_LIBPCRE/USE_LIBPCRE1/ change, or package up PCRE v2. New releases of PCRE v2 are already faster than PCRE v1, and not only is all significant development is happening on v2, but bugs in reported against v1 have started getting WONTFIX'd asking users to just upgrade to v2. So it makes sense to give our downstream distributors a nudge to switch over to it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: remove support for concurrent use of both PCRE v1 & v2Ævar Arnfjörð Bjarmason2017-05-0211-139/+65
| | | | | | | | | | | | | | | | | | | | Remove the support for concurrently using PCRE v1 & v2 by compiling Git with support for both at the same time. Having access to both at runtime via grep.patternType=[pcre1|pcre2] makes it easier for the developer hacking on the PCRE implementations to test them concurrently, but adds confusion for everyone else, particularly Git users who have no reason to concurrently use both libraries. Now either USE_LIBPCRE1=YesPlease (or its alias USE_LIBPCRE) or USE_LIBPCRE2=YesPlease can be supplied when building git, but providing both will yield an error, similarly providing both --with-libpcre1 & --with-libpcre2 to the configure script will produce an error. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: add support for PCRE v2Ævar Arnfjörð Bjarmason2017-05-0213-27/+309
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add support for v2 of the PCRE API. This is a new major version of PCRE that came out in early 2015[1]. The regular expression syntax is the same, but while the API is similar-ish, pretty much every function is either renamed or takes different arguments. Thus using it via entirely new functions makes sense, as opposed to trying to e.g. have one compile_pcre_pattern() that would call either PCRE v1 or v2 functions. Git can now be compiled with any combination of USE_LIBPCRE=[YesPlease|] & USE_LIBPCRE2=[YesPlease|]. If both are provided the version of the PCRE library can be selected at runtime with grep.PatternType, but the default (for now) is v1. This table shows what the various combinations do, depending on what libraries Git is compiled against: |------------------+-----+-----+----------| | grep.PatternType | v1 | v2 | v1 && v2 | |------------------+-----+-----+----------| | perl | v1 | v2 | v1 | | pcre | v1 | v2 | v1 | | pcre1 | v1 | ERR | v1 | | pcre2 | ERR | v2 | v2 | |------------------+-----+-----+----------| When Git is only compiled with v2 grep.PatternType=perl, --perl-regexp & -P will use v2. All tests pass with this new PCRE version. When Git is compiled with both v1 & v2 most of the tests will only test v1, but there are some v2-specific tests that will be run. With earlier patches to enable JIT for PCRE v1 the performance of the release versions of both libraries have almost exactly the same performance, with PCRE v2 being around 1% slower. However after I reported this to the pcre-dev mailing list[2] I got a lot of help with the API use from Zoltán Herczeg, he subsequently optimized some of the JIT functionality in v2 of the library. Running the p7820-grep-engines.sh performance test against the latest Subversion trunk of both, with both them and git compiled as -O3, and the test run against linux.git, gives the following results: 7820.1: extended with how.to 0.27(1.22+0.34) 7820.2: extended with ^how to 0.27(1.18+0.36) 7820.3: extended with \w+our\w* 6.09(38.64+0.32) 7820.4: extended with -?-?-?-?-?-?-?-?-?-?-?-----------$ 0.38(1.69+0.28) 7820.5: pcre1 with how.to 0.19(0.42+0.53) 7820.6: pcre1 with ^how to 0.23(0.58+0.50) 7820.7: pcre1 with \w+our\w* 0.50(2.93+0.34) 7820.8: pcre1 with -?-?-?-?-?-?-?-?-?-?-?-----------$ 5.12(19.32+0.38) 7820.9: pcre2 with how.to 0.19(0.34+0.57) 7820.10: pcre2 with ^how to 0.19(0.29+0.60) 7820.11: pcre2 with \w+our\w* 0.51(2.85+0.41) 7820.12: pcre2 with -?-?-?-?-?-?-?-?-?-?-?-----------$ 5.04(19.27+0.31) I.e. the two are neck-to-neck, but PCRE v2 usually pulls ahead, when it does it's up to 20% faster. A brief note on thread safety: As noted in pcre2api(3) & pcre2jit(3) the compiled pattern can be shared between threads, but not some of the JIT context, however the grep threading support does all pattern & JIT compilation in separate threads, so this code doesn't need to concern itself with thread safety. See commit 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09) for the initial addition of PCRE v1. This change follows some of the same patterns it did (and which were discussed on list at the time), e.g. mocking up types with typedef instead of ifdef-ing them out when USE_LIBPCRE2 isn't defined. This adds some trivial memory use to the program, but makes the code look nicer. 1. https://lists.exim.org/lurker/message/20150105.162835.0666407a.en.html 2. https://lists.exim.org/lurker/thread/20170419.172322.833ee099.en.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: add support for the PCRE v1 JIT APIÆvar Arnfjörð Bjarmason2017-05-022-1/+31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the grep PCRE v1 code to use JIT when available. When PCRE support was initially added in commit 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09) PCRE had no JIT support, it was integrated into 8.20 released on 2011-10-21. When JIT support is enabled the PCRE performance usually improves by more than 50%. The pattern compilation times are relatively slower, but those relative numbers are tiny, and are easily made back in all but the most trivial cases of grep. Detailed benchhmarks are available at: http://sljit.sourceforge.net/pcre.html With this change the difference in a t/perf/p7820-grep-engines.sh run is, shown with git --word-diff: 7820.1: extended with how.to [-0.28(1.23+0.44)-]{+0.28(1.18+0.39)+} 7820.2: extended with ^how to [-0.26(1.15+0.38)-]{+0.27(1.13+0.40)+} 7820.3: extended with \w+our\w* [-6.06(38.44+0.35)-]{+6.11(38.66+0.32)+} 7820.4: extended with -?-?-?-?-?-?-?-?-?-?-?-----------$ [-0.37(1.57+0.38)-]{+0.37(1.56+0.42)+} 7820.5: pcre1 with how.to [-0.26(1.15+0.37)-]{+0.19(0.39+0.55)+} 7820.6: pcre1 with ^how to [-0.46(2.66+0.31)-]{+0.22(0.67+0.44)+} 7820.7: pcre1 with \w+our\w* [-16.42(99.42+0.48)-]{+0.51(3.05+0.24)+} 7820.8: pcre1 with -?-?-?-?-?-?-?-?-?-?-?-----------$ [-81.52(275.37+0.41)-]{+5.16(19.31+0.33)+} The conditional support for JIT is implemented as suggested in the pcrejit(3) man page. E.g. defining PCRE_STUDY_JIT_COMPILE to 0 if it's not present. There's no graceful fallback if pcre_jit_stack_alloc() fails under PCRE_CONFIG_JIT, instead the program will abort. I don't think this is worth handling, it'll only fail in cases where malloc() doesn't work, in which case we're screwed anyway. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* perf: add a performance comparison test of grep -E and -PÆvar Arnfjörð Bjarmason2017-05-021-0/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a very basic performance comparison test comparing the POSIX extended & pcre1 engines. I'm skipping the "basic" POSIX engine because supporting its alternate regex syntax is hard, although it would be interesting to test it, at least under glibc it seems to be an entirely different engine, since it can have very different performance even for patterns that mean the same thing under extended and non-extended POSIX regular expression syntax. Running this on an i7 3.4GHz Linux 3.16.0-4 Debian testing against a checkout of linux.git & latest upstream PCRE, both PCRE and git compiled with -O3: $ GIT_PERF_LARGE_REPO=~/g/linux ./run p7820-grep-engines.sh [...] Test this tree ----------------------------------------------------------------------------- 7820.1: extended with how.to 0.28(1.23+0.44) 7820.2: extended with ^how to 0.26(1.15+0.38) 7820.3: extended with \w+our\w* 6.06(38.44+0.35) 7820.4: extended with -?-?-?-?-?-?-?-?-?-?-?-----------$ 0.37(1.57+0.38) 7820.5: pcre1 with how.to 0.26(1.15+0.37) 7820.6: pcre1 with ^how to 0.46(2.66+0.31) 7820.7: pcre1 with \w+our\w* 16.42(99.42+0.48) 7820.8: pcre1 with -?-?-?-?-?-?-?-?-?-?-?-----------$ 81.52(275.37+0.41) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: change the internal PCRE code & header names to be PCRE1Ævar Arnfjörð Bjarmason2017-05-024-36/+36
| | | | | | | | | | | | Change the internal PCRE variable & function names to have a "1" suffix. This is for preparation for libpcre2 support, where having non-versioned names would be confusing. The earlier "grep: change the internal PCRE macro names to be PCRE1" change elaborates on the motivations behind this commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: change the internal PCRE macro names to be PCRE1Ævar Arnfjörð Bjarmason2017-05-024-7/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the internal USE_LIBPCRE define, & build options flag to use a naming convention ending in PCRE1, without changing the long-standing USE_LIBPCRE Makefile flag which enables this code. This is for preparation for libpcre2 support where having things like USE_LIBPCRE and USE_LIBPCRE2 in any more places than we absolutely need to for backwards compatibility with old Makefile arguments would be confusing. In some ways it would be better to change everything that now uses USE_LIBPCRE to use USE_LIBPCRE1, and to make specifying USE_LIBPCRE (or --with-pcre) an error. This would impose a one-time burden on packagers of git to s/USE_LIBPCRE/USE_LIBPCRE1/ in their build scripts. However I'd like to leave the door open to making USE_LIBPCRE=YesPlease eventually mean USE_LIBPCRE2=YesPlease, i.e. once PCRE v2 is ubiquitous enough that it makes sense to make it the default. This code and the USE_LIBPCRE Makefile argument was added in commit 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09). At the time there was no indication that the PCRE project would release an entirely new & incompatible API around 3 years later. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* test-lib: rename the LIBPCRE prerequisite to PCREÆvar Arnfjörð Bjarmason2017-05-027-28/+28
| | | | | | | | | | | | | Rename the LIBPCRE prerequisite to PCRE. This is for preparation for libpcre2 support, where having just "LIBPCRE" would be confusing as it implies v1 of the library. None of these tests are incompatible between versions 1 & 2 of libpcre, it's less confusing to give them a more general name to make it clear that they work on both library versions. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"Ævar Arnfjörð Bjarmason2017-05-024-1/+26
| | | | | | | | | | | | | | | | | | | | Make the pattern types "pcre" & "pcre1" synonyms for the long-standing "perl" grep.patternType. This change is part of a longer patch series to add pcre2 support to Git. It's nice to be able to performance test PCRE v1 v.s. v2 without having to recompile git, and doing that via grep.patternType makes sense. However, just adding "pcre2" when we only have "perl" would be confusing, so start by adding a "pcre" & "pcre1" synonym. In the future "perl" and "pcre" might be changed to default to "pcre2" instead of "pcre1", and depending on how Git is compiled the more specific "pcre1" or "pcre2" pattern types might produce an error. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep & rev-list doc: stop promising libpcre for --perl-regexpÆvar Arnfjörð Bjarmason2017-05-022-4/+11
| | | | | | | | | | | | | | | | | | | | | Stop promising in our grep & rev-list options documentation that we're always going to be using libpcre when given the --perl-regexp option. Instead talk about using "Perl-compatible regular expressions" and using these types of patterns using "a compile-time dependency". Saying "libpcre" strongly suggests that we might be talking about libpcre.so, which is always going to be v1. This change is part of a series to add support for libpcre2 which comes with v2 of PCRE. In the future we might use some completely unrelated library to provide perl-compatible regular expression support. By wording the documentation differently and not promising any specific version of PCRE or ever PCRE at all we have more wiggle room to change the implementation. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* log: add -P as a synonym for --perl-regexpÆvar Arnfjörð Bjarmason2017-05-023-1/+11
| | | | | | | | | | | | | | | | | | | | | | Add a short -P option as a synonym for the longer --perl-regexp, for consistency with the options the corresponding grep invocations accept. This was intentionally omitted in commit 727b6fc3ed ("log --grep: accept --basic-regexp and --perl-regexp", 2012-10-03) for unspecified future use. Since nothing has come along in over 4 1/2 years that's wanted to use it, it's more valuable to make it consistent with "grep" than to keep it open for future use, and to avoid the confusion of -P meaning different things for grep & log, as is the case with the -G option. As noted in the aforementioned commit the --basic-regexp option can't have a corresponding -G argument, as the log command already uses that for -G<regex>. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* log: add exhaustive tests for pattern style options & configÆvar Arnfjörð Bjarmason2017-05-021-1/+76
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add exhaustive tests for how the different grep.patternType options & the corresponding command-line options affect git-log. Before this change it was possible to patch revision.c so that the --basic-regexp option was synonymous with --extended-regexp, and --perl-regexp wasn't recognized at all, and still have 100% of the test suite pass. This was because the first test being modified here, added in commit 34a4ae55b2 ("log --grep: use the same helper to set -E/-F options as "git grep"", 2012-10-03), didn't actually check whether we'd enabled extended regular expressions as distinct from re-toggling non-fixed string support. Fix that by changing the pattern to a pattern that'll only match if --extended-regexp option is provided, but won't match under the default --basic-regexp option. Other potential regressions were possible since there were no tests for the rest of the combinations of grep.patternType configuration toggles & corresponding git-log command-line options. Add exhaustive tests for those. The patterns being passed to fixed/basic/extended/PCRE are carefully crafted to return the wrong thing if the grep engine were to pick any other matching method than the one it's told to use. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: add a test for backreferences in PCRE patternsÆvar Arnfjörð Bjarmason2017-05-021-0/+7
| | | | | | | | | | Add a test for backreferences such as (.)\1 in PCRE patterns. This test ensures that the PCRE_NO_AUTO_CAPTURE option isn't turned on. Before this change turning it on would break these sort of patterns, but wouldn't break any tests. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Makefile & configure: reword outdated comment about PCREÆvar Arnfjörð Bjarmason2017-05-022-6/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reword an outdated comment which suggests that only git-grep can use PCRE. This comment was added back when PCRE support was initially added in commit 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09), and was true at the time. It hasn't been telling the full truth since git-log learned to use PCRE with --grep in commit 727b6fc3ed ("log --grep: accept --basic-regexp and --perl-regexp", 2012-10-03), and more importantly is likely to get more inaccurate over time as more use is made of PCRE in other areas. Reword it to be more future-proof, and to more clearly explain that this enables user-initiated runtime behavior. Copy/pasting this so much in configure.ac is lame, these Makefile-like flags aren't even used by autoconf, just the corresponding --with[out]-* options. But copy/pasting the comments that make sense for the Makefile to configure.ac where they make less sence is the pattern everything else follows in that file. I'm not going to war against that as part of this change, just following the existing pattern. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: remove redundant `regflags &= ~REG_EXTENDED` assignmentsÆvar Arnfjörð Bjarmason2017-05-021-4/+1
| | | | | | | | | | | | | Remove redundant assignments to the "regflags" variable. There are no code paths that have previously set the regflags to anything, and certainly not to `|= REG_EXTENDED`. This code gave the impression that it had to reset its environment, but it doesn't. This dates back to the initial introduction of git-grep in commit 5010cb5fcc ("built-in "git grep"", 2006-04-30). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: remove redundant regflags assignment under PCREÆvar Arnfjörð Bjarmason2017-04-251-1/+0
| | | | | | | | | | | | | | | | | | | | Remove a redundant assignment to the "regflags" variable. This variable is only used for POSIX regular expression matching, not when the PCRE library is used. This redundant assignment was added as a result of copy/paste programming in commit 84befcd0a4 ("grep: add a grep.patternType configuration setting", 2012-08-03). That commit modified already working code in commit cca2c172e0 ("git-grep: do not die upon -F/-P when grep.extendedRegexp is set.", 2011-05-09) which didn't assign to regflags when under PCRE. Revert back to that behavior, more to reduce "wait this is used under PCRE how?" confusion when reading the code, than to to save ourselves trivial CPU cycles by removing one assignment. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: submodule-related case statements should die if new fields are addedÆvar Arnfjörð Bjarmason2017-04-251-0/+4
| | | | | | | | | | | | | | | | | | | | Change two case statements added in commit 0281e487fd ("grep: optionally recurse into submodules", 2016-12-16) so that they die if new GREP_PATTERN_* enum fields are added without updating them. These case statements currently check for an exhaustive list of fields, but if a new field is added it's easy to introduce a bug here where the code will start subtly doing the wrong thing, e.g. if a new pattern type is added we'll fall through to GREP_PATTERN_TYPE_UNSPECIFIED, i.e. the "basic" POSIX regular expressions. This should arguably be done for the switch(opt->binary) case-statement as well, but isn't trivial to add since that code isn't currently working with an exhaustive list. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: add tests for grep pattern types being passed to submodulesÆvar Arnfjörð Bjarmason2017-04-251-0/+49
| | | | | | | | | | | Add testing for grep pattern types being correctly passed to submodules. The pattern "(.|.)[\d]" matches differently under fixed (not at all), and then matches different lines under basic/extended & perl regular expressions, so this change asserts that the pattern type is passed along correctly. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: amend submodule recursion test in preparation for rx engine testingÆvar Arnfjörð Bjarmason2017-04-251-83/+83
| | | | | | | | | | | | | | | | | | | | | | | | | | Amend the submodule recursion test added in commit 0281e487fd ("grep: optionally recurse into submodules", 2016-12-16) to prepare it for subsequent tests of whether it passes along the grep.patternType to the submodule greps. This is just the result of searching & replacing: foobar -> (1|2)d(3|4) foo -> (1|2) bar -> (3|4) Currently there's no tests for whether e.g. -P or -E is correctly passed along, tests for that will be added in a follow-up change, but first add content to the tests which will match differently under different regex engines. Reuse the pattern established in an earlier commit of mine in this series ("log: add exhaustive tests for pattern style options & config", 2017-04-07). The pattern "(.|.)[\d]" will match this content differently under fixed/basic/extended & perl. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Git 2.13-rc0v2.13.0-rc0Junio C Hamano2017-04-192-1/+36
| | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'jh/memihash-opt'Junio C Hamano2017-04-196-1/+40
|\ | | | | | | | | | | | | | | | | | | Hotfix for a topic that is already in 'master'. * jh/memihash-opt: p0004: make perf test executable t3008: skip lazy-init test on a single-core box test-online-cpus: helper to return cpu count name-hash: fix buffer overrun
| * p0004: make perf test executableChristian Couder2017-04-181-0/+0
| | | | | | | | | | | | | | | | | | | | It looks like in 89c3b0ad43 (name-hash: add perf test for lazy_init_name_hash, 2017-03-23) p0004 was not created with the execute unix rights. Let's fix that. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Acked-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t3008: skip lazy-init test on a single-core boxKevin Willford2017-04-121-3/+11
| | | | | | | | | | | | | | | | | | | | | | | | The lazy-init codepath will not be exercised uniless threaded. Skip the entire test on a single-core box. Also replace a hard-coded constant of 2000 (number of cache entries to manifacture for tests) with a variable with a human readable name. Signed-off-by: Kevin Willford <kewillf@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * test-online-cpus: helper to return cpu countJeff Hostetler2017-04-123-0/+10
| | | | | | | | | | | | | | | | Created helper executable to print the value of online_cpus() allowing multi-threaded tests to be skipped when appropriate. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * name-hash: fix buffer overrunKevin Willford2017-03-312-1/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add check for the end of the entries for the thread partition. Add test for lazy init name hash with specific directory structure The lazy init hash name was causing a buffer overflow when the last entry in the index was multiple folder deep with parent folders that did not have any files in them. This adds a test for the boundary condition of the thread partitions with the folder structure that was triggering the buffer overflow. The fix was to check if it is the last entry for the thread partition in the handle_range_dir and not try to use the next entry in the cache. Signed-off-by: Kevin Willford <kewillf@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'vn/revision-shorthand-for-side-branch-log'Junio C Hamano2017-04-191-2/+2
|\ \ | | | | | | | | | | | | | | | | | | Doc cleanup. * vn/revision-shorthand-for-side-branch-log: doc/revisions: remove brackets from rev^-n shorthand
| * | doc/revisions: remove brackets from rev^-n shorthandvn/revision-shorthand-for-side-branch-logKyle Meyer2017-04-161-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Given that other instances of "{...}" in the revision documentation represent literal characters of revision specifications, describing the rev^-n shorthand as "<rev>^-{<n>}" incorrectly suggests that something like "master^-{1}" is an acceptable form. Signed-off-by: Kyle Meyer <kyle@kyleam.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'sf/putty-w-args'Junio C Hamano2017-04-192-1/+7
|\ \ \ | | | | | | | | | | | | | | | | * sf/putty-w-args: connect.c: handle errors from split_cmdline
| * | | connect.c: handle errors from split_cmdlineJeff King2017-04-162-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit e9d9a8a4d (connect: handle putty/plink also in GIT_SSH_COMMAND, 2017-01-02) added a call to split_cmdline(), but checks only for a non-zero return to see if we got any output. Since the function returns negative values (and a NULL argv) on error, we end up dereferencing NULL and segfaulting. Arguably we could report on the parsing error here, but it's probably not worth it. This is a best-effort attempt to see if we are using plink. So we can simply return here with "no, it wasn't plink" and let the shell actually complain about the bogus quoting. Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'ld/p4-current-branch-fix'Junio C Hamano2017-04-192-9/+45
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git p4" used "name-rev HEAD" when it wants to learn what branch is checked out; it should use "symbolic-ref HEAD". * ld/p4-current-branch-fix: git-p4: don't use name-rev to get current branch git-p4: add read_pipe_text() internal function git-p4: add failing test for name-rev rather than symbolic-ref
| * | | | git-p4: don't use name-rev to get current branchld/p4-current-branch-fixLuke Diamand2017-04-162-7/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git-p4 was using "git name-rev" to find out the current branch. That is not safe, since if multiple branches or tags point at the same revision, the result obtained might not be what is expected. Instead use "git symbolic-ref". Signed-off-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | git-p4: add read_pipe_text() internal functionLuke Diamand2017-04-161-3/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The existing read_pipe() function returns an empty string on error, but also returns an empty string if the command returns an empty string. This leads to ugly constructions trying to detect error cases. Add read_pipe_text() which just returns None on error. Signed-off-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | git-p4: add failing test for name-rev rather than symbolic-refLuke Diamand2017-04-161-0/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Using name-rev to find the current git branch means that git-p4 does not correctly get the current branch name if there are multiple branches pointing at HEAD, or a tag. This change adds a test case which demonstrates the problem. Configuring which branches are allowed to be submitted from goes wrong, as git-p4 gets confused about which branch is in use. This appears to be the only place that git-p4 actually cares about the current branch. Signed-off-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'dt/gc-ignore-old-gc-logs'Junio C Hamano2017-04-191-1/+17
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | * dt/gc-ignore-old-gc-logs: t6500: wait for detached auto gc at the end of the test script
| * | | | | t6500: wait for detached auto gc at the end of the test scriptSZEDER Gábor2017-04-161-1/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The last test in 't6500-gc', 'background auto gc does not run if gc.log is present and recent but does if it is old', added in a831c06a2 (gc: ignore old gc.log files, 2017-02-10), may sporadically trigger an error message from the test harness: rm: cannot remove 'trash directory.t6500-gc/.git/objects': Directory not empty The test in question ends with executing an auto gc in the backround, which occasionally takes so long that it's still running when 'test_done' is about to remove the trash directory. This 'rm -rf $trash' in the foreground might race with the detached auto gc to create and delete files and directories, and gc might (re-)create a path that 'rm' already visited and removed, triggering the above error message when 'rm' attempts to remove its parent directory. Commit bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) fixed the same problem in a different test script by simply disallowing background gc. Unfortunately, what worked there is not applicable here, because the purpose of this test is to check the behavior of a detached auto gc. Make sure that the test doesn't continue before the gc is finished in the background with a clever bit of shell trickery: - Open fd 9 in the shell, to be inherited by the background gc process, because our daemonize() only closes the standard fds 0, 1 and 2. - Duplicate this fd 9 to stdout. - Read 'git gc's stdout, and thus fd 9, through a command substitution. We don't actually care about gc's output, but this construct has two useful properties: - This read blocks until stdout or fd 9 are open. While stdout is closed after the main gc process creates the background process and exits, fd 9 remains open until the backround process exits. - The variable assignment from the command substitution gets its exit status from the command executed within the command substitution, i.e. a failing main gc process will cause the test to fail. Note, that this fd trickery doesn't work on Windows, because due to MSYS limitations the git process only inherits the standard fds 0, 1 and 2 from the shell. Luckily, it doesn't matter in this case, because on Windows daemonize() is basically a noop, thus 'git gc --auto' always runs in the foreground. And since we can now continue the test reliably after the detached gc finished, check that there is only a single packfile left at the end, i.e. that the detached gc actually did what it was supposed to do. Also add a comment at the end of the test script to warn developers of future tests about this issue of long running detached gc processes. Helped-by: Jeff King <peff@peff.net> Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'bw/attr-pathspec'Junio C Hamano2017-04-191-1/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | * bw/attr-pathspec: pathspec: fix segfault in clear_pathspec
| * | | | | | pathspec: fix segfault in clear_pathspecbw/attr-pathspecBrandon Williams2017-04-161-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 'clear_pathspec()' the incorrect index parameter is used to bound an inner-loop which is used to free a 'struct attr_match' value field. Using the incorrect index parameter (in addition to being incorrect) occasionally causes segmentation faults when attempting to free an invalid pointer. Fix this by using the correct index parameter 'i'. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'ab/grep-plug-pathspec-leak'Junio C Hamano2017-04-191-0/+1
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Call clear_pathspec() to release resources immediately before the cmd_grep() function returns. * ab/grep-plug-pathspec-leak: grep: plug a trivial memory leak
| * | | | | | | grep: plug a trivial memory leakab/grep-plug-pathspec-leakÆvar Arnfjörð Bjarmason2017-04-161-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the cleanup phase for the grep command to free the pathspec struct that's allocated earlier in the same block, and used just a few lines earlier. With "grep hi README.md" valgrind reports a loss of 239 bytes now, down from 351. The relevant --num-callers=40 --leak-check=full --show-leak-kinds=all backtrace is: [...] 187 (112 direct, 75 indirect) bytes in 1 blocks are definitely lost in loss record 70 of 110 [...] at 0x4C2BBAF: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) [...] by 0x60B339: do_xmalloc (wrapper.c:59) [...] by 0x60B2F6: xmalloc (wrapper.c:86) [...] by 0x576B37: parse_pathspec (pathspec.c:652) [...] by 0x4519F0: cmd_grep (grep.c:1215) [...] by 0x4062EF: run_builtin (git.c:371) [...] by 0x40544D: handle_builtin (git.c:572) [...] by 0x4060A2: run_argv (git.c:624) [...] by 0x4051C6: cmd_main (git.c:701) [...] by 0x4C5901: main (common-main.c:43) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Acked-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'jk/no-looking-at-dotgit-outside-repo'Junio C Hamano2017-04-192-0/+3
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Clean up fallouts from recent tightening of the set-up sequence, where Git barfs when repository information is accessed without first ensuring that it was started in a repository. * jk/no-looking-at-dotgit-outside-repo: test-read-cache: setup git dir has_sha1_file: don't bother if we are not in a repository
| * | | | | | | | test-read-cache: setup git dirRené Scharfe2017-04-161-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | b1ef400e (setup_git_env: avoid blind fall-back to ".git") made programs that tried to access a repository without initializing properly die with a diagnostic message. One offender is test-read-cache, which is used in p0002. Fix it by calling setup_git_directory() before accessing the index. Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | has_sha1_file: don't bother if we are not in a repositoryJonathan Nieder2017-04-131-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Most callers to this function already require that they are in a git repository, but there is an exception: "git apply" uses has_sha1_file to avoid work if the result of applying a binary patch is already present in the repository. When run outside any repository, this produces an error: fatal: BUG: setup_git_env called without repository Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | Merge branch 'nd/files-backend-git-dir'Junio C Hamano2017-04-1913-403/+1319
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "submodule" specific field in the ref_store structure is replaced with a more generic "gitdir" that can later be used also when dealing with ref_store that represents the set of refs visible from the other worktrees. * nd/files-backend-git-dir: (28 commits) refs.h: add a note about sorting order of for_each_ref_* t1406: new tests for submodule ref store t1405: some basic tests on main ref store t/helper: add test-ref-store to test ref-store functions refs: delete pack_refs() in favor of refs_pack_refs() files-backend: avoid ref api targeting main ref store refs: new transaction related ref-store api refs: add new ref-store api refs: rename get_ref_store() to get_submodule_ref_store() and make it public files-backend: replace submodule_allowed check in files_downcast() refs: move submodule code out of files-backend.c path.c: move some code out of strbuf_git_path_submodule() refs.c: make get_main_ref_store() public and use it refs.c: kill register_ref_store(), add register_submodule_ref_store() refs.c: flatten get_ref_store() a bit refs: rename lookup_ref_store() to lookup_submodule_ref_store() refs.c: introduce get_main_ref_store() files-backend: remove the use of git_path() files-backend: add and use files_ref_path() files-backend: add and use files_reflog_path() ...
| * | | | | | | | | refs.h: add a note about sorting order of for_each_ref_*nd/files-backend-git-dirNguyễn Thái Ngọc Duy2017-04-143-2/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | t1406: new tests for submodule ref storeNguyễn Thái Ngọc Duy2017-04-141-0/+95
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | t1405: some basic tests on main ref storeNguyễn Thái Ngọc Duy2017-04-141-0/+123
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | t/helper: add test-ref-store to test ref-store functionsNguyễn Thái Ngọc Duy2017-04-143-0/+279
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | refs: delete pack_refs() in favor of refs_pack_refs()Nguyễn Thái Ngọc Duy2017-04-143-7/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It only has one caller, not worth keeping just for convenience. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | files-backend: avoid ref api targeting main ref storeNguyễn Thái Ngọc Duy2017-04-141-35/+49
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A small step towards making files-backend work as a non-main ref store using the newly added store-aware API. For the record, `join` and `nm` on refs.o and files-backend.o tell me that files-backend no longer uses functions that default to get_main_ref_store(). I'm not yet comfortable at the idea of removing files_assert_main_repository() (or converting REF_STORE_MAIN to REF_STORE_WRITE). More staring and testing is required before that can happen. Well, except peel_ref(). I'm pretty sure that function is safe. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | refs: new transaction related ref-store apiNguyễn Thái Ngọc Duy2017-04-143-12/+53
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The transaction struct now takes a ref store at creation and will operate on that ref store alone. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>