summaryrefslogtreecommitdiff
path: root/diffcore-pickaxe.c
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'tb/log-G-binary'Junio C Hamano2019-01-141-0/+6
|\ | | | | | | | | | | | | | | | | | | | | "git log -G<regex>" looked for a hunk in the "git log -p" patch output that contained a string that matches the given pattern. Optimize this code to ignore binary files, which by default will not show any hunk that would match any pattern (unless textconv or the --text option is in effect, that is). * tb/log-G-binary: log -G: ignore binary files
| * log -G: ignore binary filesThomas Braun2018-12-261-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The -G<regex> option of log looks for the differences whose patch text contains added/removed lines that match regex. Currently -G looks also into patches of binary files (which according to [1]) is binary as well. This has a couple of issues: - It makes the pickaxe search slow. In a proprietary repository of the author with only ~5500 commits and a total .git size of ~300MB searching takes ~13 seconds $time git log -Gwave > /dev/null real 0m13,241s user 0m12,596s sys 0m0,644s whereas when we ignore binary files with this patch it takes ~4s $time ~/devel/git/git log -Gwave > /dev/null real 0m3,713s user 0m3,608s sys 0m0,105s which is a speedup of more than fourfold. - The internally used algorithm for generating patch text is based on xdiff and its states in [1] > The output format of the binary patch file is proprietary > (and binary) and it is basically a collection of copy and insert > commands [..] which means that the current format could change once the internal algorithm is changed as the format is not standardized. In addition the git binary patch format used for preparing patches for git apply is *different* from the xdiff format as can be seen by comparing git log -p -a commit 6e95bf4bafccf14650d02ab57f3affe669be10cf Author: A U Thor <author@example.com> Date: Thu Apr 7 15:14:13 2005 -0700 modify binary file diff --git a/data.bin b/data.bin index f414c84..edfeb6f 100644 --- a/data.bin +++ b/data.bin @@ -1,2 +1,4 @@ a a^@a +a +a^@a with git log --binary commit 6e95bf4bafccf14650d02ab57f3affe669be10cf Author: A U Thor <author@example.com> Date: Thu Apr 7 15:14:13 2005 -0700 modify binary file diff --git a/data.bin b/data.bin index f414c84bd3aa25fa07836bb1fb73db784635e24b..edfeb6f501[..] GIT binary patch literal 12 QcmYe~N@Pgn0zx1O01)N^ZvX%Q literal 6 NcmYe~N@Pgn0ssWg0XP5v which seems unexpected. To resolve these issues this patch makes -G<regex> ignore binary files by default. Textconv filters are supported and also -a/--text for getting the old and broken behaviour back. The -S<block of text> option of log looks for differences that changes the number of occurrences of the specified block of text (i.e. addition/deletion) in a file. As we want to keep the current behaviour, add a test to ensure it stays that way. [1]: http://www.xmailserver.org/xdiff.html Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'nd/the-index'Junio C Hamano2019-01-041-2/+2
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | More codepaths become aware of working with in-core repository instance other than the default "the_repository". * nd/the-index: (22 commits) rebase-interactive.c: remove the_repository references rerere.c: remove the_repository references pack-*.c: remove the_repository references pack-check.c: remove the_repository references notes-cache.c: remove the_repository references line-log.c: remove the_repository reference diff-lib.c: remove the_repository references delta-islands.c: remove the_repository references cache-tree.c: remove the_repository references bundle.c: remove the_repository references branch.c: remove the_repository reference bisect.c: remove the_repository reference blame.c: remove implicit dependency the_repository sequencer.c: remove implicit dependency on the_repository sequencer.c: remove implicit dependency on the_index transport.c: remove implicit dependency on the_index notes-merge.c: remove implicit dependency the_repository notes-merge.c: remove implicit dependency on the_index list-objects.c: reduce the_repository references list-objects-filter.c: remove implicit dependency on the_index ...
| * notes-cache.c: remove the_repository referencesNguyễn Thái Ngọc Duy2018-11-121-2/+2
| | | | | | | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/xdiff-interface'Junio C Hamano2018-11-131-1/+2
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The interface into "xdiff" library used to discover the offset and size of a generated patch hunk by first formatting it into the textual hunk header "@@ -n,m +k,l @@" and then parsing the numbers out. A new interface has been introduced to allow callers a more direct access to them. * jk/xdiff-interface: xdiff-interface: drop parse_hunk_header() range-diff: use a hunk callback diff: convert --check to use a hunk callback combine-diff: use an xdiff hunk callback diff: use hunk callback for word-diff diff: discard hunk headers for patch-ids earlier diff: avoid generating unused hunk header lines xdiff-interface: provide a separate consume callback for hunks xdiff: provide a separate emit callback for hunks
| * diff: avoid generating unused hunk header linesJeff King2018-11-051-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | Some callers of xdi_diff_outf() do not look at the generated hunk header lines at all. By plugging in a no-op hunk callback, this tells xdiff not to even bother formatting them. This patch introduces a stock no-op callback and uses it with a few callers whose line callbacks explicitly ignore hunk headers (because they look only for +/- lines). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * xdiff-interface: provide a separate consume callback for hunksJeff King2018-11-021-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous commit taught xdiff to optionally provide the hunk header data to a specialized callback. But most users of xdiff actually use our more convenient xdi_diff_outf() helper, which ensures that our callbacks are always fed whole lines. Let's plumb the special hunk-callback through this interface, too. It will follow the same rule as xdiff when the hunk callback is NULL (i.e., continue to pass a stringified hunk header to the line callback). Since we add NULL to each caller, there should be no behavior change yet. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | userdiff.c: remove implicit dependency on the_indexNguyễn Thái Ngọc Duy2018-09-211-2/+2
| | | | | | | | | | | | | | | | [jc: squashed in missing forward decl in userdiff.h found by Ramsay] Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | diff.c: remove the_index dependency in textconv() functionsNguyễn Thái Ngọc Duy2018-09-211-2/+2
|/ | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* regex: do not call `regfree()` if compilation failsMartin Ågren2018-05-211-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It is apparently undefined behavior to call `regfree()` on a regex where `regcomp()` failed. The language in [1] is a bit muddy, at least to me, but the clearest hint is this (`preg` is the `regex_t *`): Upon successful completion, the regcomp() function shall return 0. Otherwise, it shall return an integer value indicating an error as described in <regex.h>, and the content of preg is undefined. Funnily enough, there is also the `regerror()` function which should be given a pointer to such a "failed" `regex_t` -- the content of which would supposedly be undefined -- and which may investigate it to come up with a detailed error message. In any case, the example in that document shows how `regfree()` is not called after `regcomp()` fails. We have quite a few users of this API and most get this right. These three users do not. Several implementations can handle this just fine [2] and these code paths supposedly have not wreaked havoc or we'd have heard about it. (These are all in code paths where git got bad input and is just about to die anyway.) But let's just avoid the issue altogether. [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/regcomp.html [2] https://www.redhat.com/archives/libvir-list/2013-September/msg00262.html Researched-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-byi Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diff: properly error out when combining multiple pickaxe optionsStefan Beller2018-01-041-1/+0
| | | | | | | | | | | In f506b8e8b5 (git log/diff: add -G<regexp> that greps in the patch text, 2010-08-23) we were hesitant to check if the user requests both -S and -G at the same time. Now that the pickaxe family also offers --find-object, which looks slightly more different than the former two, let's add a check that those are not used at the same time. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diffcore: add a pickaxe option to find a specific blobStefan Beller2018-01-041-18/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Sometimes users are given a hash of an object and they want to identify it further (ex.: Use verify-pack to find the largest blobs, but what are these? or [1]) One might be tempted to extend git-describe to also work with blobs, such that `git describe <blob-id>` gives a description as '<commit-ish>:<path>'. This was implemented at [2]; as seen by the sheer number of responses (>110), it turns out this is tricky to get right. The hard part to get right is picking the correct 'commit-ish' as that could be the commit that (re-)introduced the blob or the blob that removed the blob; the blob could exist in different branches. Junio hinted at a different approach of solving this problem, which this patch implements. Teach the diff machinery another flag for restricting the information to what is shown. For example: $ ./git log --oneline --find-object=v2.0.0:Makefile b2feb64309 Revert the whole "ask curl-config" topic for now 47fbfded53 i18n: only extract comments marked with "TRANSLATORS:" we observe that the Makefile as shipped with 2.0 was appeared in v1.9.2-471-g47fbfded53 and in v2.0.0-rc1-5-gb2feb6430b. The reason why these commits both occur prior to v2.0.0 are evil merges that are not found using this new mechanism. [1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob [2] https://public-inbox.org/git/20171028004419.10139-1-sbeller@google.com/ Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diff: migrate diff_flags.pickaxe_ignore_case to a pickaxe_opts bitStefan Beller2018-01-041-3/+3
| | | | | | | | | Currently flags for pickaxing are found in different places. Unify the flags into the `pickaxe_opts` field, which will contain any pickaxe related flags. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diff: make struct diff_flags members lowercasebw/diff-opt-impl-to-bitfieldsBrandon Williams2017-11-011-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that the flags stored in struct diff_flags are being accessed directly and not through macros, change all struct members from being uppercase to lowercase. This conversion is done using the following semantic patch: @@ expression E; @@ - E.RECURSIVE + E.recursive @@ expression E; @@ - E.TREE_IN_RECURSIVE + E.tree_in_recursive @@ expression E; @@ - E.BINARY + E.binary @@ expression E; @@ - E.TEXT + E.text @@ expression E; @@ - E.FULL_INDEX + E.full_index @@ expression E; @@ - E.SILENT_ON_REMOVE + E.silent_on_remove @@ expression E; @@ - E.FIND_COPIES_HARDER + E.find_copies_harder @@ expression E; @@ - E.FOLLOW_RENAMES + E.follow_renames @@ expression E; @@ - E.RENAME_EMPTY + E.rename_empty @@ expression E; @@ - E.HAS_CHANGES + E.has_changes @@ expression E; @@ - E.QUICK + E.quick @@ expression E; @@ - E.NO_INDEX + E.no_index @@ expression E; @@ - E.ALLOW_EXTERNAL + E.allow_external @@ expression E; @@ - E.EXIT_WITH_STATUS + E.exit_with_status @@ expression E; @@ - E.REVERSE_DIFF + E.reverse_diff @@ expression E; @@ - E.CHECK_FAILED + E.check_failed @@ expression E; @@ - E.RELATIVE_NAME + E.relative_name @@ expression E; @@ - E.IGNORE_SUBMODULES + E.ignore_submodules @@ expression E; @@ - E.DIRSTAT_CUMULATIVE + E.dirstat_cumulative @@ expression E; @@ - E.DIRSTAT_BY_FILE + E.dirstat_by_file @@ expression E; @@ - E.ALLOW_TEXTCONV + E.allow_textconv @@ expression E; @@ - E.TEXTCONV_SET_VIA_CMDLINE + E.textconv_set_via_cmdline @@ expression E; @@ - E.DIFF_FROM_CONTENTS + E.diff_from_contents @@ expression E; @@ - E.DIRTY_SUBMODULES + E.dirty_submodules @@ expression E; @@ - E.IGNORE_UNTRACKED_IN_SUBMODULES + E.ignore_untracked_in_submodules @@ expression E; @@ - E.IGNORE_DIRTY_SUBMODULES + E.ignore_dirty_submodules @@ expression E; @@ - E.OVERRIDE_SUBMODULE_CONFIG + E.override_submodule_config @@ expression E; @@ - E.DIRSTAT_BY_LINE + E.dirstat_by_line @@ expression E; @@ - E.FUNCCONTEXT + E.funccontext @@ expression E; @@ - E.PICKAXE_IGNORE_CASE + E.pickaxe_ignore_case @@ expression E; @@ - E.DEFAULT_FOLLOW_RENAMES + E.default_follow_renames Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diff: remove DIFF_OPT_TST macroBrandon Williams2017-11-011-4/+4
| | | | | | | | | | | | | | | | | | | | | | | Remove the `DIFF_OPT_TST` macro and instead access the flags directly. This conversion is done using the following semantic patch: @@ expression E; identifier fld; @@ - DIFF_OPT_TST(&E, fld) + E.flags.fld @@ type T; T *ptr; identifier fld; @@ - DIFF_OPT_TST(ptr, fld) + ptr->flags.fld Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'js/regexec-buf'Junio C Hamano2017-03-241-2/+5
|\ | | | | | | | | | | | | | | Fix for potential segv introduced in v2.11.0 and later (also v2.10.2). * js/regexec-buf: pickaxe: fix segfault with '-S<...> --pickaxe-regex'
| * pickaxe: fix segfault with '-S<...> --pickaxe-regex'js/regexec-bufSZEDER Gábor2017-03-181-2/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 'git {log,diff,...} -S<...> --pickaxe-regex' can segfault as a result of out-of-bounds memory reads. diffcore-pickaxe.c:contains() looks for all matches of the given regex in a buffer in a loop, advancing the buffer pointer to the end of the last match in each iteration. When we switched to REG_STARTEND in b7d36ffca (regex: use regexec_buf(), 2016-09-21), we started passing the size of that buffer to the regexp engine, too. Unfortunately, this buffer size is never updated on subsequent iterations, and as the buffer pointer advances on each iteration, this "bufptr+bufsize" points past the end of the buffer. This results in segmentation fault, if that memory can't be accessed. In case of 'git log' it can also result in erroneously listed commits, if the memory past the end of buffer is accessible and happens to contain data matching the regex. Reduce the buffer size on each iteration as the buffer pointer is advanced, thus maintaining the correct end of buffer location. Furthermore, make sure that the buffer pointer is not dereferenced in the control flow statements when we already reached the end of the buffer. The new test is flaky, I've never seen it fail on my Linux box even without the fix, but this is expected according to db5dfa3 (regex: -G<pattern> feeds a non NUL-terminated string to regexec() and fails, 2016-09-21). However, it did fail on Travis CI with the first (and incomplete) version of the fix, and based on that commit message I would expect the new test without the fix to fail most of the time on Windows. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'js/regexec-buf'Junio C Hamano2016-09-261-10/+8
|\ \ | |/ | | | | | | | | | | | | | | | | | | | | | | Some codepaths in "git diff" used regexec(3) on a buffer that was mmap(2)ed, which may not have a terminating NUL, leading to a read beyond the end of the mapped region. This was fixed by introducing a regexec_buf() helper that takes a <ptr,len> pair with REG_STARTEND extension. * js/regexec-buf: regex: use regexec_buf() regex: add regexec_buf() that can work on a non NUL-terminated string regex: -G<pattern> feeds a non NUL-terminated string to regexec() and fails
| * regex: use regexec_buf()Johannes Schindelin2016-09-211-10/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The new regexec_buf() function operates on buffers with an explicitly specified length, rather than NUL-terminated strings. We need to use this function whenever the buffer we want to pass to regexec(3) may have been mmap(2)ed (and is hence not NUL-terminated). Note: the original motivation for this patch was to fix a bug where `git diff -G <regex>` would crash. This patch converts more callers, though, some of which allocated to construct NUL-terminated strings, or worse, modified buffers to temporarily insert NULs while calling regexec(3). By converting them to use regexec_buf(), the code has become much cleaner. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | diffcore-pickaxe: support case insensitive match on non-asciiNguyễn Thái Ngọc Duy2016-07-011-0/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | Similar to the "grep -F -i" case, we can't use kws on icase search outside ascii range, so we quote the string and pass it to regcomp as a basic regexp and let regex engine deal with case sensitivity. The new test is put in t7812 instead of t4209-log-pickaxe because lib-gettext.sh might cause problems elsewhere, probably. Noticed-by: Plamen Totev <plamen.totev@abv.bg> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | diffcore-pickaxe: Add regcomp_or_die()Nguyễn Thái Ngọc Duy2016-07-011-9/+13
|/ | | | | | | | | | There's another regcomp code block coming in this function that needs the same error handling. This function can help avoid duplicating error handling code. Helped-by: Jeff King <peff@peff.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* react to errors in xdi_diffJeff King2015-09-281-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we call into xdiff to perform a diff, we generally lose the return code completely. Typically by ignoring the return of our xdi_diff wrapper, but sometimes we even propagate that return value up and then ignore it later. This can lead to us silently producing incorrect diffs (e.g., "git log" might produce no output at all, not even a diff header, for a content-level diff). In practice this does not happen very often, because the typical reason for xdiff to report failure is that it malloc() failed (it uses straight malloc, and not our xmalloc wrapper). But it could also happen when xdiff triggers one our callbacks, which returns an error (e.g., outf() in builtin/rerere.c tries to report a write failure in this way). And the next patch also plans to add more failure modes. Let's notice an error return from xdiff and react appropriately. In most of the diff.c code, we can simply die(), which matches the surrounding code (e.g., that is what we do if we fail to load a file for diffing in the first place). This is not that elegant, but we are probably better off dying to let the user know there was a problem, rather than simply generating bogus output. We could also just die() directly in xdi_diff, but the callers typically have a bit more context, and can provide a better message (and if we do later decide to pass errors up, we're one step closer to doing so). There is one interesting case, which is in diff_grep(). Here if we cannot generate the diff, there is nothing to match, and we silently return "no hits". This is actually what the existing code does already, but we make it a little more explicit. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: simplify kwset loop in contains()rs/pickaxe-iRené Scharfe2014-03-241-5/+2
| | | | | | | | Inlining the variable "found" actually makes the code shorter and easier to read. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: call strlen only when necessary in diffcore_pickaxe_count()René Scharfe2014-03-241-2/+1
| | | | | | | | | We need to determine the search term's length only when fixed-string matching is used; regular expression compilation takes a NUL-terminated string directly. Only call strlen() in the former case. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: move pickaxe() after pickaxe_match()René Scharfe2014-03-241-41/+38
| | | | | | | | | pickaxe() calls pickaxe_match(); moving the definition of the former after the latter allows us to do without an explicit function declaration. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: merge diffcore_pickaxe_grep() and diffcore_pickaxe_count() into ↵René Scharfe2014-03-241-37/+7
| | | | | | | | | | | | | | diffcore_pickaxe() diffcore_pickaxe_count() initializes the regular expression or kwset for the search term, calls pickaxe() with the callback has_changes() and cleans up afterwards. diffcore_pickaxe_grep() does the same, only it doesn't support kwset and uses the callback diff_grep() instead. Merge the two functions to form the new diffcore_pickaxe() and thus get rid of the duplicate regex setup and cleanup code. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: honor -i when used with -S and --pickaxe-regexRené Scharfe2014-03-241-1/+4
| | | | | | | | | | accccde4 (pickaxe: allow -i to search in patch case-insensitively) allowed case-insenitive matching for -G and -S, but for the latter only if fixed string matching is used. Allow it for -S and regular expression matching as well to make the support complete. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'rs/pickaxe-simplify'Junio C Hamano2013-07-121-7/+4
|\ | | | | | | | | * rs/pickaxe-simplify: diffcore-pickaxe: simplify has_changes and contains
| * diffcore-pickaxe: simplify has_changes and containsrs/pickaxe-simplifyRené Scharfe2013-07-071-7/+4
| | | | | | | | | | | | | | | | | | | | Halve the number of callsites of contains() to two using temporary variables, simplifying the code. While at it, get rid of the diff_options parameter, which became unused with 8fa4b09f. Signed-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | diffcore-pickaxe: make error messages more consistentRamkumar Ramachandra2013-06-031-2/+2
|/ | | | | | | | | | | | | | | | | | | Currently, diffcore-pickaxe reports two distinct errors for the same user error: $ git log --pickaxe-regex -S'\1' fatal: invalid pickaxe regex: Invalid back reference $ git log -G'\1' fatal: invalid log-grep regex: Invalid back reference This "log-grep" was only an internal name for the -G feature during development, and invite confusion with "git log --grep=<pattern>". Change the error messages to say "invalid regex". Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diffcore-pickaxe: unify code for log -S/-GJeff King2013-04-051-69/+49
| | | | | | | | | | | | | | | | | | | The logic flow of has_changes() used for "log -S" and diff_grep() used for "log -G" are essentially the same. See if we have both sides that could be different in any interesting way, slurp the contents in core, possibly after applying textconv, inspect the contents, clean-up and report the result. The only difference between the two is how "inspect" step works. Unify this codeflow in a helper, pickaxe_match(), which takes a callback function that implements the specific "inspect" step. After removing the common scaffolding code from the existing has_changes() and diff_grep(), they each becomes such a callback function suitable for passing to pickaxe_match(). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diffcore-pickaxe: fix leaks in "log -S<block>" and "log -G<pattern>"Junio C Hamano2013-04-051-5/+7
| | | | | | | | | | | The diff_grep() and has_changes() functions had early return codepaths for unmerged filepairs, which simply returned 0. When we taught textconv filter to them, one was ignored and continued to return early without freeing the result filtered by textconv, and the other had a failed attempt to fix, which allowed the planned return value 0 to be overwritten by a bogus call to contains(). Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diffcore-pickaxe: port optimization from has_changes() to diff_grep()Junio C Hamano2013-04-051-1/+6
| | | | | | | | | | | | | | | | | | | These two functions are called in the same codeflow to implement "log -S<block>" and "log -G<pattern>", respectively, but the latter lacked two obvious optimizations the former implemented, namely: - When a pickaxe limit is not given at all, they should return without wasting any cycle; - When both sides of the filepair are the same, and the same textconv conversion apply to them, return early, as there will be no interesting differences between the two anyway. Also release the filespec data once the processing is done (this is not about leaking memory--it is about releasing data we finished looking at as early as possible). Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diffcore-pickaxe: respect --no-textconvSimon Ruderich2013-04-051-4/+8
| | | | | | | | | | | | | git log -S doesn't respect --no-textconv: $ echo '*.txt diff=wrong' > .gitattributes $ git -c diff.wrong.textconv='xxx' log --no-textconv -Sfoo error: cannot run xxx: No such file or directory fatal: unable to read files to diff Reported-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> Signed-off-by: Simon Ruderich <simon@ruderich.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diffcore-pickaxe: remove fill_one()Jeff King2013-04-041-20/+10
| | | | | | | | | | | | | fill_one is _almost_ identical to just calling fill_textconv; the exception is that for the !DIFF_FILE_VALID case, fill_textconv gives us an empty buffer rather than a NULL one. Since we currently use the NULL pointer as a signal that the file is not present on one side of the diff, we must now switch to using DIFF_FILE_VALID to make the same check. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Simon Ruderich <simon@ruderich.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diffcore-pickaxe: remove unnecessary call to get_textconv()Simon Ruderich2013-04-041-9/+14
| | | | | | | | | | | | | | | | | | | | | | | | | The fill_one() function is responsible for finding and filling the textconv filter as necessary, and is called by diff_grep() function that implements "git log -G<pattern>". The has_changes() function that implements "git log -S<block>" calls get_textconv() for two sides being compared, before it checks to see if it was asked to perform the pickaxe limiting. Move the code around to avoid this wastage. After has_changes() calls get_textconv() to obtain textconv for both sides, fill_one() is called to use them. By adding get_textconv() to diff_grep() and relieving fill_one() of responsibility to find the textconv filter, we can avoid calling get_textconv() twice in has_changes(). With this change it's also no longer necessary for fill_one() to modify the textconv argument, therefore pass a pointer instead of a pointer to a pointer. Signed-off-by: Simon Ruderich <simon@ruderich.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: use textconv for -S countingJeff King2012-10-281-17/+39
| | | | | | | | | | | | | | | We currently just look at raw blob data when using "-S" to pickaxe. This is mostly historical, as pickaxe predates the textconv feature. If the user has bothered to define a textconv filter, it is more likely that their search string will be on the textconv output, as that is what they will see in the diff (and we do not even provide a mechanism for them to search for binary needles that contain NUL characters). This patch teaches "-S" to use textconv, just as we already do for "-G". Signed-off-by: Jeff King <peff@peff.net>
* pickaxe: hoist empty needle checkJeff King2012-10-281-2/+3
| | | | | | | | | | | If we are given an empty pickaxe needle like "git log -S ''", it is impossible for us to find anything (because no matter what the content, the count will always be 0). We currently check this at the lowest level of contains(). Let's hoist the logic much earlier to has_changes(), so that it is simpler to return our answer before loading any blob data. Signed-off-by: Jeff King <peff@peff.net>
* diff_grep: use textconv buffers for add/deleted filesJeff King2012-10-281-2/+2
| | | | | | | | | | | | | | | | | | | | | | | If you use "-G" to grep a diff, we will apply a configured textconv filter to the data before generating the diff. However, if the diff is an addition or deletion, we do not bother running the diff at all, and just look for the token in the added (or removed) content. This works because we know that the diff must contain every line of content. However, while we used the textconv-derived buffers in the regular diff, we accidentally passed the original unmodified buffers to regexec when checking the added or removed content. This could lead to an incorrect answer. Worse, in some cases we might have a textconv buffer but no original buffer (e.g., if we pulled the textconv data from cache, or if we reused a working tree file when generating it). In that case, we could actually feed NULL to regexec and segfault. Reported-by: Peter Oberndorfer <kumbayo84@arcor.de> Signed-off-by: Jeff King <peff@peff.net>
* pickaxe: allow -i to search in patch case-insensitivelyJunio C Hamano2012-02-281-2/+7
| | | | | | | | | | | | | | | | | | | "git log -S<string>" is a useful way to find the last commit in the codebase that touched the <string>. As it was designed to be used by a porcelain script to dig the history starting from a block of text that appear in the starting commit, it never had to look for anything but an exact match. When used by an end user who wants to look for the last commit that removed a string (e.g. name of a variable) that he vaguely remembers, however, it is useful to support case insensitive match. When given the "--regexp-ignore-case" (or "-i") option, which originally was designed to affect case sensitivity of the search done in the commit log part, e.g. "log --grep", the matches made with -S/-G pickaxe search is done case insensitively now. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: factor out pickaxers/pickaxeRené Scharfe2011-10-071-67/+43
| | | | | | | | Move the duplicate diff queue loop into its own function that accepts a match function: has_changes() for -S and diff_grep() for -G. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: give diff_grep the same signature as has_changesRené Scharfe2011-10-071-3/+4
| | | | | | | | | Change diff_grep() to match the signature of has_changes() as a preparation for the next patch that will use function pointers to the two. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: pass diff_options to contains and has_changesRené Scharfe2011-10-071-14/+14
| | | | | | | | | | | | | | | | Remove the unused parameter needle from contains() and has_changes(). Also replace the parameter len with a pointer to the diff_options. We can use its member pickaxe to check if the needle is an empty string and use the kwsmatch structure to find out the length of the match instead. This change is done as a preparation to unify the signatures of has_changes() and diff_grep(), which will be used in the patch after the next one to factor out common code. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: factor out has_changesRené Scharfe2011-10-071-36/+21
| | | | | | | Move duplicate if/else construct into its own helper function. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: plug regex/kws leakRené Scharfe2011-10-071-6/+7
| | | | | | | | | With -S... --pickaxe-all, free the regex or the kws before returning even if we found a match. Also get rid of the variable has_changes, as we can simply break out of the loop. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: plug regex leakRené Scharfe2011-10-071-7/+6
| | | | | | | | | With -G... --pickaxe-all, free the regex before returning even if we found a match. Also get rid of the variable has_changes, as we can simply break out of the loop. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pickaxe: plug diff filespec leak with empty needleRené Scharfe2011-10-071-2/+2
| | | | | | | | Check first for the unlikely case of an empty needle string and only then populate the filespec, lest we leak it. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Use kwset in pickaxeFredrik Kuivinen2011-08-201-11/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Benchmarks in the hot cache case: before: $ perf stat --repeat=5 git log -Sqwerty Performance counter stats for 'git log -Sqwerty' (5 runs): 47,092,744 cache-misses # 2.825 M/sec ( +- 1.607% ) 123,368,389 cache-references # 7.400 M/sec ( +- 0.812% ) 330,040,998 branch-misses # 3.134 % ( +- 0.257% ) 10,530,896,750 branches # 631.663 M/sec ( +- 0.121% ) 62,037,201,030 instructions # 1.399 IPC ( +- 0.142% ) 44,331,294,321 cycles # 2659.073 M/sec ( +- 0.326% ) 96,794 page-faults # 0.006 M/sec ( +- 11.952% ) 25 CPU-migrations # 0.000 M/sec ( +- 25.266% ) 1,424 context-switches # 0.000 M/sec ( +- 0.540% ) 16671.708650 task-clock-msecs # 0.997 CPUs ( +- 0.343% ) 16.728692052 seconds time elapsed ( +- 0.344% ) after: $ perf stat --repeat=5 git log -Sqwerty Performance counter stats for 'git log -Sqwerty' (5 runs): 51,385,522 cache-misses # 4.619 M/sec ( +- 0.565% ) 129,177,880 cache-references # 11.611 M/sec ( +- 0.219% ) 319,222,775 branch-misses # 6.946 % ( +- 0.134% ) 4,595,913,233 branches # 413.086 M/sec ( +- 0.112% ) 31,395,042,533 instructions # 1.062 IPC ( +- 0.129% ) 29,558,348,598 cycles # 2656.740 M/sec ( +- 0.204% ) 93,224 page-faults # 0.008 M/sec ( +- 4.487% ) 19 CPU-migrations # 0.000 M/sec ( +- 10.425% ) 950 context-switches # 0.000 M/sec ( +- 0.360% ) 11125.796039 task-clock-msecs # 0.997 CPUs ( +- 0.239% ) 11.164216599 seconds time elapsed ( +- 0.240% ) So the kwset code is about 33% faster. Signed-off-by: Fredrik Kuivinen <frekui@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* diffcore-pickaxe.c: a void function shouldn't try to return somethingBrandon Casey2010-10-061-2/+2
| | | | | Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'maint'Junio C Hamano2010-10-061-2/+1
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * maint: Documentation/git-clone: describe --mirror more verbosely do not depend on signed integer overflow work around buggy S_ISxxx(m) implementations xdiff: cast arguments for ctype functions to unsigned char init: plug tiny one-time memory leak diffcore-pickaxe.c: remove unnecessary curly braces t3020 (ls-files-error-unmatch): remove stray '1' from end of file setup: make sure git dir path is in a permanent buffer environment.c: remove unused variable git-svn: fix processing of decorated commit hashes git-svn: check_cherry_pick should exclude commits already in our history Documentation/git-svn: discourage "noMetadata"
View Lorry Controller Status