@matheusbsbs / matheus-bagliano / commits / c1beba5b47

Fork 0

git-apply --whitespace=fix: fix whitespace fuzz introduced by previous run

When you have more than one patch series, an earlier one of which tries to introduce whitespace breakages and a later one of which has such a new line in its context, "git-apply --whitespace=fix" will apply and fix the whitespace breakages in the earlier one, making the resulting file not to match the context of the later patch. A short demonstration is in the new test, t4125. For example, suppose the first patch is: diff a/hello.txt b/hello.txt --- a/hello.txt +++ b/hello.txt @@ -20,3 +20,3 @@ Hello world.$ -How Are you$ -Today?$ +How are you $ +today? $ to fix broken case in the string, but it introduces unwanted trailing whitespaces to the result (pretend you are looking at "cat -e" output of the patch --- '$' signs are not in the patch but are shown to make the EOL stand out). And the second patch is to change the wording of the greeting further: diff a/hello.txt b/hello.txt --- a/hello.txt +++ b/hello.txt @@ -18,5 +18,5 @@ Greetings $ -Hello world.$ +Hello, everybody. $ How are you $ -today? $ +these days? $ If you apply the first one with --whitespace=fix, you will get this as the result: Hello world.$ How are you$ today?$ and this does not match the preimage of the second patch, which demands extra whitespace after "How are you" and "today?". This series is about teaching "git apply --whitespace=fix" to cope with this situation better. If the patch does not apply, it rewrites the second patch like this and retries: diff a/hello.txt b/hello.txt --- a/hello.txt +++ b/hello.txt @@ -18,5 +18,5 @@ Greetings$ -Hello world.$ +Hello, everybody.$ How are you$ -today?$ +these days?$ This is done by rewriting the preimage lines in the hunk (i.e. the lines that begin with ' ' or '-'), using the same whitespace fixing rules as it is using to apply the patches, so that it can notice what it did to the previous ones in the series. A careful reader may notice that the first patch in the example did not touch the "Greetings" line, so the trailing whitespace that is in the original preimage of the second patch is not from the series. Is rewriting this context line a problem? If you think about it, you will realize that the reason for the difference is because the submitter's tree was based on an earlier version of the file that had whitespaces wrong on that "Greetings" line, and the change that introduced the "Greetings" line was added independently of this two-patch series to our tree already with an earlier "git apply --whitespace=fix". So it may appear this logic is rewriting too much, it is not so. It is just rewriting what we would have rewritten in the past. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Jan 30, 2008 at 15:24 UTC c1beba5b479a39143ebef96ba10103bbd9a70089

2 files changed +231 -5

builtin-apply.c

+128 -5

@@ -1516,7 +1516,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)

1516 }

1517

1518 static int copy_wsfix(char *output, const char *patch, int plen,

1519 - unsigned ws_rule)

1519 + unsigned ws_rule, int count_error)

1520 {

1521 /*

1522 * plen is number of bytes to be copied from patch, starting

@@ -1604,11 +1604,74 @@ static int copy_wsfix(char *output, const char *patch, int plen,

1604 memcpy(output, patch, plen);

1605 if (add_nl_to_tail)

1606 output[plen++] = '\n';

1607 - if (fixed)

1607 + if (fixed && count_error)

1608 applied_after_fixing_ws++;

1609 return output + plen - buf;

1610 }

1611

1612 +static void update_pre_post_images(struct image *preimage,

1613 + struct image *postimage,

1614 + char *buf,

1615 + size_t len)

1616 +{

1617 + int i, ctx;

1618 + char *new, *old, *fixed;

1619 + struct image fixed_preimage;

1620 +

1621 + /*

1622 + * Update the preimage with whitespace fixes. Note that we

1623 + * are not losing preimage->buf -- apply_one_fragment() will

1624 + * free "oldlines".

1625 + */

1626 + prepare_image(&fixed_preimage, buf, len, 1);

1627 + assert(fixed_preimage.nr == preimage->nr);

1628 + for (i = 0; i < preimage->nr; i++)

1629 + fixed_preimage.line[i].flag = preimage->line[i].flag;

1630 + free(preimage->line_allocated);

1631 + *preimage = fixed_preimage;

1632 +

1633 + /*

1634 + * Adjust the common context lines in postimage, in place.

1635 + * This is possible because whitespace fixing does not make

1636 + * the string grow.

1637 + */

1638 + new = old = postimage->buf;

1639 + fixed = preimage->buf;

1640 + for (i = ctx = 0; i < postimage->nr; i++) {

1641 + size_t len = postimage->line[i].len;

1642 + if (!(postimage->line[i].flag & LINE_COMMON)) {

1643 + /* an added line -- no counterparts in preimage */

1644 + memmove(new, old, len);

1645 + old += len;

1646 + new += len;

1647 + continue;

1648 + }

1649 +

1650 + /* a common context -- skip it in the original postimage */

1651 + old += len;

1652 +

1653 + /* and find the corresponding one in the fixed preimage */

1654 + while (ctx < preimage->nr &&

1655 + !(preimage->line[ctx].flag & LINE_COMMON)) {

1656 + fixed += preimage->line[ctx].len;

1657 + ctx++;

1658 + }

1659 + if (preimage->nr <= ctx)

1660 + die("oops");

1661 +

1662 + /* and copy it in, while fixing the line length */

1663 + len = preimage->line[ctx].len;

1664 + memcpy(new, fixed, len);

1665 + new += len;

1666 + fixed += len;

1667 + postimage->line[i].len = len;

1668 + ctx++;

1669 + }

1670 +

1671 + /* Fix the length of the whole thing */

1672 + postimage->len = new - postimage->buf;

1673 +}

1674 +

1675 static int match_fragment(struct image *img,

1676 struct image *preimage,

1677 struct image *postimage,

@@ -1618,6 +1681,7 @@ static int match_fragment(struct image *img,

1681 int match_beginning, int match_end)

1682 {

1683 int i;

1684 + char *fixed_buf, *buf, *orig, *target;

1685

1686 if (preimage->nr + try_lno > img->nr)

1687 return 0;

@@ -1646,10 +1710,68 @@ static int match_fragment(struct image *img,

1710 !memcmp(img->buf + try, preimage->buf, preimage->len))

1711 return 1;

1712

1713 + if (ws_error_action != correct_ws_error)

1714 + return 0;

1715 +

1716 + /*

1717 + * The hunk does not apply byte-by-byte, but the hash says

1718 + * it might with whitespace fuzz.

1719 + */

1720 + fixed_buf = xmalloc(preimage->len + 1);

1721 + buf = fixed_buf;

1722 + orig = preimage->buf;

1723 + target = img->buf + try;

1724 + for (i = 0; i < preimage->nr; i++) {

1725 + size_t fixlen; /* length after fixing the preimage */

1726 + size_t oldlen = preimage->line[i].len;

1727 + size_t tgtlen = img->line[try_lno + i].len;

1728 + size_t tgtfixlen; /* length after fixing the target line */

1729 + char tgtfixbuf[1024], *tgtfix;

1730 + int match;

1731 +

1732 + /* Try fixing the line in the preimage */

1733 + fixlen = copy_wsfix(buf, orig, oldlen, ws_rule, 0);

1734 +

1735 + /* Try fixing the line in the target */

1736 + if (sizeof(tgtfixbuf) < tgtlen)

1737 + tgtfix = tgtfixbuf;

1738 + else

1739 + tgtfix = xmalloc(tgtlen);

1740 + tgtfixlen = copy_wsfix(tgtfix, target, tgtlen, ws_rule, 0);

1741 +

1742 + /*

1743 + * If they match, either the preimage was based on

1744 + * a version before our tree fixed whitespace breakage,

1745 + * or we are lacking a whitespace-fix patch the tree

1746 + * the preimage was based on already had (i.e. target

1747 + * has whitespace breakage, the preimage doesn't).

1748 + * In either case, we are fixing the whitespace breakages

1749 + * so we might as well take the fix together with their

1750 + * real change.

1751 + */

1752 + match = (tgtfixlen == fixlen && !memcmp(tgtfix, buf, fixlen));

1753 +

1754 + if (tgtfix != tgtfixbuf)

1755 + free(tgtfix);

1756 + if (!match)

1757 + goto unmatch_exit;

1758 +

1759 + orig += oldlen;

1760 + buf += fixlen;

1761 + target += tgtlen;

1762 + }

1763 +

1764 /*

1650 - * NEEDSWORK: We can optionally match fuzzily here, but

1651 - * that is for a later round.

1765 + * Yes, the preimage is based on an older version that still

1766 + * has whitespace breakages unfixed, and fixing them makes the

1767 + * hunk match. Update the context lines in the postimage.

1768 */

1769 + update_pre_post_images(preimage, postimage,

1770 + fixed_buf, buf - fixed_buf);

1771 + return 1;

1772 +

1773 + unmatch_exit:

1774 + free(fixed_buf);

1775 return 0;

1776 }

1777

@@ -1871,7 +1993,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,

1993 added = plen;

1994 }

1995 else {

1874 - added = copy_wsfix(new, patch + 1, plen, ws_rule);

1996 + added = copy_wsfix(new, patch + 1, plen,

1997 + ws_rule, 1);

1998 }

1999 add_line_info(&postimage, new, added,

2000 (first == '+' ? 0 : LINE_COMMON));

t/t4125-apply-ws-fuzz.sh new

+103

@@ -0,0 +1,103 @@

1 +#!/bin/sh

2 +

3 +test_description='applying patch that has broken whitespaces in context'

4 +

5 +. ./test-lib.sh

6 +

7 +test_expect_success setup '

8 +

9 + >file &&

10 + git add file &&

11 +

12 + # file-0 is full of whitespace breakages

13 + for l in a bb c d eeee f ggg h

14 + do

15 + echo "$l "

16 + done >file-0 &&

17 +

18 + # patch-0 creates a whitespace broken file

19 + cat file-0 >file &&

20 + git diff >patch-0 &&

21 + git add file &&

22 +

23 + # file-1 is still full of whitespace breakages,

24 + # but has one line updated, without fixing any

25 + # whitespaces.

26 + # patch-1 records that change.

27 + sed -e "s/d/D/" file-0 >file-1 &&

28 + cat file-1 >file &&

29 + git diff >patch-1 &&

30 +

31 + # patch-all is the effect of both patch-0 and patch-1

32 + >file &&

33 + git add file &&

34 + cat file-1 >file &&

35 + git diff >patch-all &&

36 +

37 + # patch-2 is the same as patch-1 but is based

38 + # on a version that already has whitespace fixed,

39 + # and does not introduce whitespace breakages.

40 + sed -e "s/ $//" patch-1 >patch-2 &&

41 +

42 + # If all whitespace breakages are fixed the contents

43 + # should look like file-fixed

44 + sed -e "s/ $//" file-1 >file-fixed

45 +

46 +'

47 +

48 +test_expect_success nofix '

49 +

50 + >file &&

51 + git add file &&

52 +

53 + # Baseline. Applying without fixing any whitespace

54 + # breakages.

55 + git apply --whitespace=nowarn patch-0 &&

56 + git apply --whitespace=nowarn patch-1 &&

57 +

58 + # The result should obviously match.

59 + diff -u file-1 file

60 +'

61 +

62 +test_expect_success 'withfix (forward)' '

63 +

64 + >file &&

65 + git add file &&

66 +

67 + # The first application will munge the context lines

68 + # the second patch depends on. We should be able to

69 + # adjust and still apply.

70 + git apply --whitespace=fix patch-0 &&

71 + git apply --whitespace=fix patch-1 &&

72 +

73 + diff -u file-fixed file

74 +'

75 +

76 +test_expect_success 'withfix (backward)' '

77 +

78 + >file &&

79 + git add file &&

80 +

81 + # Now we have a whitespace breakages on our side.

82 + git apply --whitespace=nowarn patch-0 &&

83 +

84 + # And somebody sends in a patch based on image

85 + # with whitespace already fixed.

86 + git apply --whitespace=fix patch-2 &&

87 +

88 + # The result should accept the whitespace fixed

89 + # postimage. But the line with "h" is beyond context

90 + # horizon and left unfixed.

91 +

92 + sed -e /h/d file-fixed >fixed-head &&

93 + sed -e /h/d file >file-head &&

94 + diff -u fixed-head file-head &&

95 +

96 + sed -n -e /h/p file-fixed >fixed-tail &&

97 + sed -n -e /h/p file >file-tail &&

98 +

99 + ! diff -u fixed-tail file-tail

100 +

101 +'

102 +

103 +test_done