From 04427ac8483f61dcb01a48c78a821f5042c88195 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 26 Oct 2008 00:44:53 -0400 Subject: refactor userdiff textconv code The original implementation of textconv put the conversion into fill_mmfile. This was a bad idea for a number of reasons: - it made the semantics of fill_mmfile unclear. In some cases, it was allocating data (if a text conversion occurred), and in some cases not (if we could use the data directly from the filespec). But the caller had no idea which had happened, and so didn't know whether the memory should be freed - similarly, the caller had no idea if a text conversion had occurred, and so didn't know whether the contents should be treated as binary or not. This meant that we incorrectly guessed that text-converted content was binary and didn't actually show it (unless the user overrode us with "diff.foo.binary = false", which then created problems in plumbing where the text conversion did _not_ occur) - not all callers of fill_mmfile want the text contents. In particular, we don't really want diffstat, whitespace checks, patch id generation, etc, to look at the converted contents. This patch pulls the conversion code directly into builtin_diff, so that we only see the conversion when generating an actual patch. We also then know whether we are doing a conversion, so we can check the binary-ness and free the data from the mmfile appropriately (the previous version leaked quite badly when text conversion was used) Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t4030-diff-textconv.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 't/t4030-diff-textconv.sh') diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index 1b0964843e..090a21d0b5 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -52,7 +52,7 @@ test_expect_success 'setup textconv filters' ' git config diff.fail.textconv false ' -test_expect_failure 'diff produces text' ' +test_expect_success 'diff produces text' ' git diff HEAD^ HEAD >diff && find_diff actual && test_cmp expect.text actual @@ -64,7 +64,7 @@ test_expect_success 'diff-tree produces binary' ' test_cmp expect.binary actual ' -test_expect_failure 'log produces text' ' +test_expect_success 'log produces text' ' git log -1 -p >log && find_diff actual && test_cmp expect.text actual @@ -80,7 +80,7 @@ cat >expect.stat <<'EOF' file | Bin 2 -> 4 bytes 1 files changed, 0 insertions(+), 0 deletions(-) EOF -test_expect_failure 'diffstat does not run textconv' ' +test_expect_success 'diffstat does not run textconv' ' echo file diff=fail >.gitattributes && git diff --stat HEAD^ HEAD >actual && test_cmp expect.stat actual -- cgit v1.2.3 From c7534ef4a12bb44806d522fc8e3961e390f9169b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 26 Oct 2008 00:45:55 -0400 Subject: userdiff: require explicitly allowing textconv Diffs that have been produced with textconv almost certainly cannot be applied, so we want to be careful not to generate them in things like format-patch. This introduces a new diff options, ALLOW_TEXTCONV, which controls this behavior. It is off by default, but is explicitly turned on for the "log" family of commands, as well as the "diff" porcelain (but not diff-* plumbing). Because both text conversion and external diffing are controlled by these diff options, we can get rid of the "plumbing versus porcelain" distinction when reading the config. This was an attempt to control the same thing, but suffered from being too coarse-grained. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t4030-diff-textconv.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/t4030-diff-textconv.sh') diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index 090a21d0b5..1df48ae12a 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -70,7 +70,7 @@ test_expect_success 'log produces text' ' test_cmp expect.text actual ' -test_expect_failure 'format-patch produces binary' ' +test_expect_success 'format-patch produces binary' ' git format-patch --no-binary --stdout HEAD^ >patch && find_diff actual && test_cmp expect.binary actual -- cgit v1.2.3 From 2675773af893ae81f9b09f18c1f2ec86ca2158e7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 26 Oct 2008 00:46:21 -0400 Subject: only textconv regular files We treat symlinks as text containing the results of the symlink, so it doesn't make much sense to text-convert them. Similarly gitlink components just end up as the text "Subproject commit $sha1", which we should leave intact. Note that a typechange may be broken into two parts: the removal of the old part and the addition of the new. In that case, we _do_ show the textconv for any part which is the addition or removal of a file we would ordinarily textconv, since it is purely acting on the file contents. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t4030-diff-textconv.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/t4030-diff-textconv.sh') diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index 1df48ae12a..3945731e9a 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -104,7 +104,7 @@ index ad8b3d2..67be421 \ No newline at end of file EOF # make a symlink the hard way that works on symlink-challenged file systems -test_expect_failure 'textconv does not act on symlinks' ' +test_expect_success 'textconv does not act on symlinks' ' echo -n frotz > file && git add file && git ls-files -s | sed -e s/100644/120000/ | -- cgit v1.2.3 From df5e91fc2c95e051744ec9b9de66869d2b323037 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 26 Oct 2008 00:42:25 -0400 Subject: add userdiff textconv tests These tests provide a basic sanity check that textconv'd files work. The tests try to describe how this configuration _should_ work; thus some of the tests are marked to expect failure. In particular, we fail to actually textconv anything because the 'diff.foo.binary' config option is not set, which will be fixed in the next patch. This also means that some "expect_failure" tests actually seem to be fixed; in reality, this is just because textconv is broken and its failure mode happens to make these tests work. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t4030-diff-textconv.sh | 118 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100755 t/t4030-diff-textconv.sh (limited to 't/t4030-diff-textconv.sh') diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh new file mode 100755 index 0000000000..1b0964843e --- /dev/null +++ b/t/t4030-diff-textconv.sh @@ -0,0 +1,118 @@ +#!/bin/sh + +test_description='diff.*.textconv tests' +. ./test-lib.sh + +find_diff() { + sed '1,/^index /d' | sed '/^-- $/,$d' +} + +cat >expect.binary <<'EOF' +Binary files a/file and b/file differ +EOF + +cat >expect.text <<'EOF' +--- a/file ++++ b/file +@@ -1 +1,2 @@ + 0 ++1 +EOF + +cat >hexdump <<'EOF' +#!/bin/sh +perl -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' "$1" +EOF +chmod +x hexdump + +test_expect_success 'setup binary file with history' ' + printf "\\0\\n" >file && + git add file && + git commit -m one && + printf "\\1\\n" >>file && + git add file && + git commit -m two +' + +test_expect_success 'file is considered binary by porcelain' ' + git diff HEAD^ HEAD >diff && + find_diff actual && + test_cmp expect.binary actual +' + +test_expect_success 'file is considered binary by plumbing' ' + git diff-tree -p HEAD^ HEAD >diff && + find_diff actual && + test_cmp expect.binary actual +' + +test_expect_success 'setup textconv filters' ' + echo file diff=foo >.gitattributes && + git config diff.foo.textconv "$PWD"/hexdump && + git config diff.fail.textconv false +' + +test_expect_failure 'diff produces text' ' + git diff HEAD^ HEAD >diff && + find_diff actual && + test_cmp expect.text actual +' + +test_expect_success 'diff-tree produces binary' ' + git diff-tree -p HEAD^ HEAD >diff && + find_diff actual && + test_cmp expect.binary actual +' + +test_expect_failure 'log produces text' ' + git log -1 -p >log && + find_diff actual && + test_cmp expect.text actual +' + +test_expect_failure 'format-patch produces binary' ' + git format-patch --no-binary --stdout HEAD^ >patch && + find_diff actual && + test_cmp expect.binary actual +' + +cat >expect.stat <<'EOF' + file | Bin 2 -> 4 bytes + 1 files changed, 0 insertions(+), 0 deletions(-) +EOF +test_expect_failure 'diffstat does not run textconv' ' + echo file diff=fail >.gitattributes && + git diff --stat HEAD^ HEAD >actual && + test_cmp expect.stat actual +' +# restore working setup +echo file diff=foo >.gitattributes + +cat >expect.typechange <<'EOF' +--- a/file ++++ /dev/null +@@ -1,2 +0,0 @@ +-0 +-1 +diff --git a/file b/file +new file mode 120000 +index ad8b3d2..67be421 +--- /dev/null ++++ b/file +@@ -0,0 +1 @@ ++frotz +\ No newline at end of file +EOF +# make a symlink the hard way that works on symlink-challenged file systems +test_expect_failure 'textconv does not act on symlinks' ' + echo -n frotz > file && + git add file && + git ls-files -s | sed -e s/100644/120000/ | + git update-index --index-info && + git commit -m typechange && + git show >diff && + find_diff actual && + test_cmp expect.typechange actual +' + +test_done -- cgit v1.2.3 From a79b8b6623288f7d5409ad749cc6553976a4f0e8 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 26 Oct 2008 00:50:02 -0400 Subject: enable textconv for diff in verbose status/commit This diff is meant for human consumption, so it makes sense to apply text conversion here, as we would for the regular diff porcelain. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t4030-diff-textconv.sh | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 't/t4030-diff-textconv.sh') diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index 3945731e9a..09967f5663 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -76,6 +76,14 @@ test_expect_success 'format-patch produces binary' ' test_cmp expect.binary actual ' +test_expect_success 'status -v produces text' ' + git reset --soft HEAD^ && + git status -v >diff && + find_diff actual && + test_cmp expect.text actual && + git reset --soft HEAD@{1} +' + cat >expect.stat <<'EOF' file | Bin 2 -> 4 bytes 1 files changed, 0 insertions(+), 0 deletions(-) -- cgit v1.2.3 From 6ecfd91df5ec462aeded967c9ad21912c249f96e Mon Sep 17 00:00:00 2001 From: Brian Gernhardt Date: Fri, 31 Oct 2008 01:09:13 -0400 Subject: Avoid using non-portable `echo -n` in tests. Expecting echo to recognise -n is a BSDism. Using printf is far more portable. Discovered on OS X 10.5.5 in t4030-diff-textconv.sh and changed in all the test scripts. Signed-off-by: Brian Gernhardt Signed-off-by: Junio C Hamano --- t/t4030-diff-textconv.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/t4030-diff-textconv.sh') diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index 1b0964843e..3aed1bbdfe 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -105,7 +105,7 @@ index ad8b3d2..67be421 EOF # make a symlink the hard way that works on symlink-challenged file systems test_expect_failure 'textconv does not act on symlinks' ' - echo -n frotz > file && + printf frotz > file && git add file && git ls-files -s | sed -e s/100644/120000/ | git update-index --index-info && -- cgit v1.2.3 From 3b91c30b763d972c5e133507ccc4b749dc2c6df5 Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Wed, 19 Nov 2008 12:14:50 +0100 Subject: Fix t4030-diff-textconv.sh Avoid passing cygwin pathnames to Perl. Some Perls have problems using them Signed-off-by: Alex Riesen Signed-off-by: Junio C Hamano --- t/t4030-diff-textconv.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/t4030-diff-textconv.sh') diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index 03ba26a0de..0b76e7c97a 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -21,7 +21,7 @@ EOF cat >hexdump <<'EOF' #!/bin/sh -perl -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' "$1" +perl -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' < "$1" EOF chmod +x hexdump -- cgit v1.2.3 From deb13872becbc5fb49e3ab22dd9feaf4e429a853 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Tue, 2 Dec 2008 09:31:01 +0100 Subject: t4030-diff-textconv: Make octal escape sequence more portable There are printfs around that do not grok '\1', but need '\01'. Discovered on AIX 4.3.x. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4030-diff-textconv.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/t4030-diff-textconv.sh') diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index 0b76e7c97a..2f27a0ba9e 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -29,7 +29,7 @@ test_expect_success 'setup binary file with history' ' printf "\\0\\n" >file && git add file && git commit -m one && - printf "\\1\\n" >>file && + printf "\\01\\n" >>file && git add file && git commit -m two ' -- cgit v1.2.3