From 55d5d5bab7c3f9ab6310b9cf436a7935d7d32165 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 22 Jul 2009 14:48:28 -0700 Subject: [PATCH 1/3] combine-diff.c: fix performance problem when folding common deleted lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For a deleted line in a patch with the parent we are looking at, the append_lost() function finds the same line among a run of lines that were deleted from the same location by patches from parents we previously checked. This is so that patches with two parents @@ -1,4 +1,3 @@ @@ -1,4 +1,3 @@ one one -two -two three three -quatro -fyra +four +four can be coalesced into this sequence, reusing one line that describes the removal of "two" for both parents. @@@ -1,4 -1,4 +1,3 @@@ one --two three - quatro -frya ++four While reading the second patch (that removes "two" and then "fyra"), after finding where removal of the "two" matches, we need to find existing removal of "fyra" (if exists) in the removal list, but the match has to happen after all the existing matches (in this case "two"). The code used a naïve O(n^2) algorithm to compute this by scanning the whole removal list over and over again. This patch remembers where the next scan should be started in the existing removal list to avoid this. Noticed by Linus Torvalds. Signed-off-by: Junio C Hamano --- combine-diff.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index 60d03676bb..b82f46cc60 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -80,6 +80,7 @@ struct lline { /* Lines surviving in the merge result */ struct sline { struct lline *lost_head, **lost_tail; + struct lline *next_lost; char *bol; int len; /* bit 0 up to (N-1) are on if the parent has this line (i.e. @@ -121,18 +122,12 @@ static void append_lost(struct sline *sline, int n, const char *line, int len) /* Check to see if we can squash things */ if (sline->lost_head) { - struct lline *last_one = NULL; - /* We cannot squash it with earlier one */ - for (lline = sline->lost_head; - lline; - lline = lline->next) - if (lline->parent_map & this_mask) - last_one = lline; - lline = last_one ? last_one->next : sline->lost_head; + lline = sline->next_lost; while (lline) { if (lline->len == len && !memcmp(lline->line, line, len)) { lline->parent_map |= this_mask; + sline->next_lost = lline->next; return; } lline = lline->next; @@ -147,6 +142,7 @@ static void append_lost(struct sline *sline, int n, const char *line, int len) lline->line[len] = 0; *sline->lost_tail = lline; sline->lost_tail = &lline->next; + sline->next_lost = NULL; } struct combine_diff_state { @@ -187,6 +183,7 @@ static void consume_line(void *state_, char *line, unsigned long len) xcalloc(state->num_parent, sizeof(unsigned long)); state->sline[state->nb-1].p_lno[state->n] = state->ob; + state->lost_bucket->next_lost = state->lost_bucket->lost_head; return; } if (!state->lost_bucket) From b810cbbde9232cbe9a3841edccc5b606bbd3a82e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 22 Jul 2009 14:48:29 -0700 Subject: [PATCH 2/3] diff --cc: a lost line at the beginning of the file is shown incorrectly When combine-diff inspected the diff from one parent to the merge result, it misinterpreted a header in the form @@ -l,k +0,0 @@. This hunk header means that K lines were removed from the beginning of the file, so the lost lines must be queued to the sline that represents the first line of the merge result, but we incremented our pointer incorrectly and ended up queuing it to the second line, which in turn made the lossage appear _after_ the first line. Signed-off-by: Junio C Hamano --- combine-diff.c | 16 ++++---- t/t4038-diff-combined.sh | 84 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 7 deletions(-) create mode 100755 t/t4038-diff-combined.sh diff --git a/combine-diff.c b/combine-diff.c index b82f46cc60..1a6f9d1df5 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -164,20 +164,22 @@ static void consume_line(void *state_, char *line, unsigned long len) &state->nb, &state->nn)) return; state->lno = state->nb; - if (!state->nb) - /* @@ -1,2 +0,0 @@ to remove the - * first two lines... - */ - state->nb = 1; - if (state->nn == 0) + if (state->nn == 0) { /* @@ -X,Y +N,0 @@ removed Y lines * that would have come *after* line N * in the result. Our lost buckets hang * to the line after the removed lines, + * + * Note that this is correct even when N == 0, + * in which case the hunk removes the first + * line in the file. */ state->lost_bucket = &state->sline[state->nb]; - else + if (!state->nb) + state->nb = 1; + } else { state->lost_bucket = &state->sline[state->nb-1]; + } if (!state->sline[state->nb-1].p_lno) state->sline[state->nb-1].p_lno = xcalloc(state->num_parent, diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh new file mode 100755 index 0000000000..2cf7e01ac2 --- /dev/null +++ b/t/t4038-diff-combined.sh @@ -0,0 +1,84 @@ +#!/bin/sh + +test_description='combined diff' + +. ./test-lib.sh + +setup_helper () { + one=$1 branch=$2 side=$3 && + + git branch $side $branch && + for l in $one two three fyra + do + echo $l + done >file && + git add file && + test_tick && + git commit -m $branch && + git checkout $side && + for l in $one two three quatro + do + echo $l + done >file && + git add file && + test_tick && + git commit -m $side && + test_must_fail git merge $branch && + for l in $one three four + do + echo $l + done >file && + git add file && + test_tick && + git commit -m "merge $branch into $side" +} + +verify_helper () { + it=$1 && + + # Ignore lines that were removed only from the other parent + sed -e ' + 1,/^@@@/d + /^ -/d + s/^\(.\)./\1/ + ' "$it" >"$it.actual.1" && + sed -e ' + 1,/^@@@/d + /^- /d + s/^.\(.\)/\1/ + ' "$it" >"$it.actual.2" && + + git diff "$it^" "$it" -- | sed -e '1,/^@@/d' >"$it.expect.1" && + test_cmp "$it.expect.1" "$it.actual.1" && + + git diff "$it^2" "$it" -- | sed -e '1,/^@@/d' >"$it.expect.2" && + test_cmp "$it.expect.2" "$it.actual.2" +} + +test_expect_success setup ' + >file && + git add file && + test_tick && + git commit -m initial && + + git branch withone && + git branch sansone && + + git checkout withone && + setup_helper one withone sidewithone && + + git checkout sansone && + setup_helper "" sansone sidesansone +' + +test_expect_success 'check combined output (1)' ' + git show sidewithone -- >sidewithone && + verify_helper sidewithone +' + +test_expect_failure 'check combined output (2)' ' + git show sidesansone -- >sidesansone && + verify_helper sidesansone +' + +test_done From 735c674416b87505400fcf738fd3a38b52f0eccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Wed, 22 Jul 2009 19:24:38 -0500 Subject: [PATCH 3/3] Trailing whitespace and no newline fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a patch adds a new line to the end of a file and this line ends with one trailing whitespace character and has no newline, then '--whitespace=fix' currently does not remove that trailing whitespace. This patch fixes this by removing the check for trailing whitespace at the end of the line at a hardcoded offset which does not take the eventual absence of newline into account. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- t/t4124-apply-ws-rule.sh | 18 ++++++++++++++++++ ws.c | 5 ++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh index f83322e513..5698a9a736 100755 --- a/t/t4124-apply-ws-rule.sh +++ b/t/t4124-apply-ws-rule.sh @@ -148,4 +148,22 @@ do done done +create_patch () { + sed -e "s/_/ /" <<-\EOF + diff --git a/target b/target + index e69de29..8bd6648 100644 + --- a/target + +++ b/target + @@ -0,0 +1 @@ + +A line with trailing whitespace and no newline_ + \ No newline at end of file + EOF +} + +test_expect_success 'trailing whitespace & no newline at the end of file' ' + >target && + create_patch | git apply --whitespace=fix - && + grep "newline$" target +' + test_done diff --git a/ws.c b/ws.c index 819c797cf6..8d855b7fd5 100644 --- a/ws.c +++ b/ws.c @@ -261,9 +261,8 @@ int ws_fix_copy(char *dst, const char *src, int len, unsigned ws_rule, int *erro /* * Strip trailing whitespace */ - if ((ws_rule & WS_TRAILING_SPACE) && - (2 <= len && isspace(src[len-2]))) { - if (src[len - 1] == '\n') { + if (ws_rule & WS_TRAILING_SPACE) { + if (1 < len && src[len - 1] == '\n') { add_nl_to_tail = 1; len--; if (1 < len && src[len - 1] == '\r') {