git/diffcore-pickaxe.c, branch v1.8.3.4

diffcore-pickaxe: make error messages more consistent

2013-06-03T17:50:22Z

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=". Change the error messages to say "invalid regex". Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano

diffcore-pickaxe: unify code for log -S/-G

2013-04-05T17:31:09Z

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 Signed-off-by: Junio C Hamano

diffcore-pickaxe: fix leaks in "log -S" and "log -G"

2013-04-05T17:31:09Z

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

diffcore-pickaxe: port optimization from has_changes() to diff_grep()

2013-04-05T17:31:09Z

These two functions are called in the same codeflow to implement "log -S" and "log -G", 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

diffcore-pickaxe: respect --no-textconv

2013-04-05T17:30:44Z

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 Signed-off-by: Simon Ruderich Signed-off-by: Junio C Hamano

diffcore-pickaxe: remove fill_one()

2013-04-05T03:33:19Z

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 Signed-off-by: Simon Ruderich Signed-off-by: Junio C Hamano

diffcore-pickaxe: remove unnecessary call to get_textconv()

2013-04-05T03:33:19Z

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". The has_changes() function that implements "git log -S" 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 Signed-off-by: Junio C Hamano

pickaxe: use textconv for -S counting

2012-10-28T12:48:17Z

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

pickaxe: hoist empty needle check

2012-10-28T12:48:09Z

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

diff_grep: use textconv buffers for add/deleted files

2012-10-28T11:59:44Z

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 Signed-off-by: Jeff King