Re: [RFC][PATCH] index-pack: add testcases found using AFL

2017-03-13 Thread Vegard Nossum
On 13/03/2017 18:11, Junio C Hamano wrote: Vegard Nossum <vegard.nos...@oracle.com> writes: However, I think it's more useful to think of these testcases not as "binary test that nobody knows what they are doing", but as "(sometimes invalid) packfiles which tickle i

Re: [RFC][PATCH] index-pack: add testcases found using AFL

2017-03-13 Thread Vegard Nossum
On 12/03/2017 19:14, Junio C Hamano wrote: Jeff King writes: One further devil's advocate: If people really _do_ care about coverage, arguably the AFL tests are a pollution of that concept. Because they are running the code, but doing a very perfunctory job of testing it. IOW,

Re: [RFC][PATCH] index-pack: add testcases found using AFL

2017-03-12 Thread Vegard Nossum
4446ce562eee129588f2c92c4eef2c82ed4bb4f Mon Sep 17 00:00:00 2001 From: Vegard Nossum <vegard.nos...@oracle.com> Date: Sun, 12 Mar 2017 14:35:25 +0100 Subject: [PATCH] test-lib: add --fuzzing option >From t/README: This causes additional testcases found by fuzzing to be run, for more

Re: [RFC][PATCH] index-pack: add testcases found using AFL

2017-03-10 Thread Vegard Nossum
On 10/03/2017 20:42, Jeff King wrote: That's something I guess, but I'm not enthused by the idea of just dumping a bunch of binary test cases that nobody, not even the author, understands. [...] My real concern is that this is the tip of the ice berg. So we increased coverage in one program

Re: [RFC][PATCH] index-pack: add testcases found using AFL

2017-03-10 Thread Vegard Nossum
On 10/03/2017 20:06, Jeff King wrote: On Fri, Mar 10, 2017 at 04:15:56PM +0100, Vegard Nossum wrote: I've used AFL to generate a corpus of pack files that maximises the edge coverage for 'git index-pack'. This is a supplement to (and not a replacement for) the regular test cases where we know

Re: [RFC][PATCH] index-pack: add testcases found using AFL

2017-03-10 Thread Vegard Nossum
On 10/03/2017 16:15, Vegard Nossum wrote: I've used AFL to generate a corpus of pack files that maximises the edge coverage for 'git index-pack'. This is a supplement to (and not a replacement for) the regular test cases where we know exactly what each test is checking for. These testcases

Re: [PATCH] line-log: use COPY_ARRAY to fix mis-sized memcpy

2017-03-05 Thread Vegard Nossum
nvenience. I listed you as the author, since you did the hard part. If you're OK with it, please indicate that it's OK to add your signed-off-by. If you prefer to do it differently, feel free to post your own patch. Thanks. -- >8 -- From: Vegard Nossum <vegard.nos...@oracle.com> Subje

Re: [PATCH v1] Travis: also test on 32-bit Linux

2017-03-04 Thread Vegard Nossum
On 04/03/2017 20:49, Vegard Nossum wrote: On 04/03/2017 19:08, Vegard Nossum wrote: On 04/03/2017 18:23, Lars Schneider wrote: Did Travis find our first 32bit bug? I briefly looked into it and the following new topic on pu seems to cause the issue: http://public-inbox.org/git

Re: [PATCH v1] Travis: also test on 32-bit Linux

2017-03-04 Thread Vegard Nossum
On 04/03/2017 19:08, Vegard Nossum wrote: On 04/03/2017 18:23, Lars Schneider wrote: Did Travis find our first 32bit bug? I briefly looked into it and the following new topic on pu seems to cause the issue: http://public-inbox.org/git/20170302172902.16850-1-allan.x.xav...@oracle.com/ https

Re: [PATCH v1] Travis: also test on 32-bit Linux

2017-03-04 Thread Vegard Nossum
On 04/03/2017 18:23, Lars Schneider wrote: Did Travis find our first 32bit bug? I briefly looked into it and the following new topic on pu seems to cause the issue: http://public-inbox.org/git/20170302172902.16850-1-allan.x.xav...@oracle.com/

Re: [PATCH 1/2] apply: guard against renames of non-existant empty files

2017-02-25 Thread Vegard Nossum
On 25/02/2017 12:59, Philip Oakley wrote: From: "Vegard Nossum" <vegard.nos...@oracle.com> If we have a patch like the one in the new test-case, then we will "the one in the new test-case" needs a clearer reference to the particular case so that future reader

[PATCH 1/2] apply: guard against renames of non-existant empty files

2017-02-25 Thread Vegard Nossum
nce). The patch file is completely bogus as it tries to rename something that is known not to exist, so we can throw an error for this. Found using AFL. Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- apply.c | 3 ++- t/t4154-apply-git-header.sh | 15 +++

[PATCH 2/2] apply: handle assertion failure gracefully

2017-02-25 Thread Vegard Nossum
For the patches in the added testcases, we were crashing with: git-apply: apply.c:3665: check_preimage: Assertion `patch->is_new <= 0' failed. As it turns out, check_preimage() is prepared to handle these conditions, so we can remove the assertion. Found using AFL. Signed-off-by:

Re: What's cooking in git.git (Feb 2017, #03; Fri, 10)

2017-02-12 Thread Vegard Nossum
On 11/02/2017 04:12, Junio C Hamano wrote: René Scharfe writes: Am 10.02.2017 um 23:24 schrieb Junio C Hamano: * vn/xdiff-func-context (2017-01-15) 1 commit - xdiff -W: relax end-of-file function detection "git diff -W" has been taught to handle the case where a new

Re: [PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context

2017-01-15 Thread Vegard Nossum
On 15/01/2017 03:39, Junio C Hamano wrote: René Scharfe writes: I am also more focused on keeping the codebase maintainable in good health by making sure that we made an effort to find a solution that is general-enough before solving a single specific problem you have today. We

Re: [PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context

2017-01-13 Thread Vegard Nossum
On 13/01/2017 20:51, Junio C Hamano wrote: René Scharfe writes: That's true, but I'm not sure "non-empty line before function line" is good enough a definition for desirable lines. It wouldn't work for people who don't believe in empty lines. Or for those that put a blank line

[PATCH 3/3] t/t4051-diff-function-context: improve tests for new diff -W behaviour

2017-01-13 Thread Vegard Nossum
er to show that the tests do not break even when changing the behaviour of 'diff -W' in the previous commits. Cc: René Scharfe <l@web.de> Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- t/t4051-diff-function-context.sh | 2 +- t/t4051/appended1.c |

[PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context

2017-01-13 Thread Vegard Nossum
of the detected function start until we hit the first non-blank line. Cc: René Scharfe <l@web.de> Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- xdiff/xemit.c | 4 1 file changed, 4 insertions(+) diff --git a/xdiff/xemit.c b/xdiff/xemit.c index 8c88dbd..3a

[PATCH 1/3] xdiff: -W: relax end-of-file function detection

2017-01-13 Thread Vegard Nossum
. This fixes the case where we add e.g. // Begin of dummy static int dummy(void) { } to the end of the file. Cc: René Scharfe <l@web.de> Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- xdiff/xemit.c | 14 ++ 1 file changed,

[PATCH v2] diff: add interhunk context config option

2017-01-12 Thread Vegard Nossum
The --inter-hunk-context= option was added in commit 6d0e674a5754 ("diff: add option to show context between close hunks"). This patch allows configuring a default for this option. Cc: René Scharfe <l@web.de> Cc: Pranit Bauva <pranit.ba...@gmail.com> Signed-off-by: Veg

[PATCH] diff: add interhunk context config option

2017-01-02 Thread Vegard Nossum
The --inter-hunk-context= option was added in commit 6d0e674a5754 ("diff: add option to show context between close hunks"). This patch allows configuring a default for this option. Cc: René Scharfe <l@web.de> Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> -

Re: Huge performance bottleneck reading packs

2016-10-14 Thread Vegard Nossum
On 10/13/2016 10:43 PM, Jeff King wrote: No problem. I do think you'll benefit a lot from packing everything into a single pack, but it's also clear that Git was doing more work than it needed to be. This was a known issue when we added the racy-check to has_sha1_file(), and knew that we might

Re: Huge performance bottleneck reading packs

2016-10-13 Thread Vegard Nossum
On 10/13/2016 05:26 PM, Jeff King wrote: On Thu, Oct 13, 2016 at 09:20:07AM +0200, Vegard Nossum wrote: Does the patch below help? Yes, ~2m10s -> ~1m25s when I test a git fetch this morning (the other variation in time may be due to different CPU usage by other programs, but I

Re: Huge performance bottleneck reading packs

2016-10-13 Thread Vegard Nossum
On 10/13/2016 12:45 AM, Junio C Hamano wrote: > Vegard Nossum <vegard.nos...@oracle.com> writes: > >> A closer inspection reveals the problem to really be that this is an >> extremely hot path with more than -- holy cow -- 4,106,756,451 >> iterations on the 'pack

Re: Huge performance bottleneck reading packs

2016-10-13 Thread Vegard Nossum
On 10/13/2016 01:47 AM, Jeff King wrote: On Wed, Oct 12, 2016 at 07:18:07PM -0400, Jeff King wrote: Also, is it possible to make the repository in question available? I might be able to reproduce based on your description, but it would save time if I could directly run gdb on your example. I

Re: Huge performance bottleneck reading packs

2016-10-13 Thread Vegard Nossum
On 10/13/2016 01:01 AM, Jeff King wrote: On Thu, Oct 13, 2016 at 12:30:52AM +0200, Vegard Nossum wrote: However, the commit found by 'git blame' above appears just fine to me, I haven't been able to spot a bug in it. A closer inspection reveals the problem to really

Huge performance bottleneck reading packs

2016-10-12 Thread Vegard Nossum
Hi all, I've bisected a performance regression (noticed by Quentin and myself) which caused a 'git fetch' to go from ~1m30s to ~2m40s: commit 47bf4b0fc52f3ad5823185a85f5f82325787c84b Author: Jeff King Date: Mon Jun 30 13:04:03 2014 -0400 prepare_packed_git_one: refactor

[PATCH v5] revision: new rev^-n shorthand for rev^n..rev

2016-09-27 Thread Vegard Nossum
rev". For example, for a two-parent merge, you can use rev^-2 to get the set of commits which were made to the main branch while the topic branch was prepared. Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- [v2: Use ^- instead of % as suggested by Junio Hamano and use some c

[RFC PATCH v4] revision: new rev^-n shorthand for rev^n..rev

2016-09-26 Thread Vegard Nossum
the set of commits which were made to the main branch while the topic branch was prepared. Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- [v2: Use ^- instead of % as suggested by Junio Hamano and use some common helper functions for parsing.] [v3: Use 'struct object_id' i

[RFC PATCH v3] revision: new rev^-n shorthand for rev^n..rev

2016-09-25 Thread Vegard Nossum
the set of commits which were made to the main branch while the topic branch was prepared. Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- [v2: Use ^- instead of % as suggested by Junio Hamano and use some common helper functions for parsing.] [v3: Use 'struct object_id' i

[RFC PATCH v2] revision: new rev^-n shorthand for rev^n..rev

2016-09-25 Thread Vegard Nossum
.] Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- Documentation/revisions.txt | 14 +++ builtin/rev-parse.c | 28 ++ revision.c | 91 + revision.h | 1 + 4 files changed, 134 inse

[RFC PATCH] revision: new rev%n shorthand for rev^n..rev

2016-09-23 Thread Vegard Nossum
parent, but this might be questionable/unintuitive in case any of the parents that share commits (as you would get fewer commits than expected). Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- Documentation/revisions.txt | 14 + builtin/rev-parse.c