Re: [PATCH] diff --follow: do call diffcore_std() as necessary

Ævar Arnfjörð Bjarmason [off-list ref] writes:

On Fri, Aug 13, 2010 at 19:46, Junio C Hamano [off-list ref] wrote:

quoted

This hopefully fixes the breakage.

Hopefully. It'd also be nice if we had a regression test for this, but
I don't know how hard that would be to arrange. If it's hard to
reproduce we might get away with a filter-branch + subdir filter from
the idea repository.

As I wrote, the test added by 0cdca13 breaks with just a reversion of
1da6175 but with this patch it passes.  I didn't run any other test,
though ;-).

Add a regression test for the git log -M --follow --name-only bug
introduced in v1.7.2-rc0~103^2~2

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 t/t4202-log.sh |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 95ac3f8..ff624f4 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh

@@ -441,5 +441,14 @@ test_expect_success 'log.decorate configuration' '
 
 '
 
+test_expect_success 'Regression test for v1.7.2-rc0~103^2~2' '
+	# Needs an unrelated root commit
+	test_commit README &&
+	>Foo.bar &&
+	git add Foo.bar &&
+	git commit --allow-empty-message </dev/null &&
+	git log -M --follow --name-only Foo.bar
+'
+
 test_done
 

-- 
1.7.2.1.338.ge1a5e

On Sat, Aug 14, 2010 at 01:10, Ævar Arnfjörð Bjarmason [off-list ref] wrote:

Add a regression test for the git log -M --follow --name-only bug
introduced in v1.7.2-rc0~103^2~2

AKA "we didn't have any tests for log's --name-only *at all*".

Ævar Arnfjörð Bjarmason [off-list ref] writes:

On Sat, Aug 14, 2010 at 01:10, Ævar Arnfjörð Bjarmason [off-list ref] wrote:

quoted

Add a regression test for the git log -M --follow --name-only bug
introduced in v1.7.2-rc0~103^2~2

AKA "we didn't have any tests for log's --name-only *at all*".

But this is not related to --name-only at all; anything that is "diff"
related, e.g. -p, --stat, --name-status, will share the same issue.

This is uninformative and ugly at the same time.

 - Can't we describe the nature of the situation where the old bug
   triggers concisely?  Perhaps 'show added path under "--follow -M"?'

 - All others begin with lowercase.

+	# Needs an unrelated root commit
+	test_commit README &&

This is not a "root" commit, is it?

+	>Foo.bar &&
+	git add Foo.bar &&
+	git commit --allow-empty-message </dev/null &&

Does emptiness of the message matter?

+	git log -M --follow --name-only Foo.bar
+'
+
 test_done

Add a regression test for the git log -M --follow $diff_option bug
introduced in v1.7.2-rc0~103^2~2, $diff_option being diff related
options like -p, --stat, --name-only etc.

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---

Version two of this test case, simpler, and takes into account
commentary from Junio.

 t/t4202-log.sh |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 95ac3f8..a0be122 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh

@@ -441,5 +441,18 @@ test_expect_success 'log.decorate configuration' '
 
 '
 
+test_expect_success 'show added path under "--follow -M"' '
+	# This tests for a regression introduced in v1.7.2-rc0~103^2~2
+	test_create_repo regression &&
+	(
+		cd regression &&
+		test_commit needs-another-commit &&
+		test_commit Foo.bar &&
+		git log -M --follow -p Foo.bar.t &&
+		git log -M --follow --stat Foo.bar.t &&
+		git log -M --follow --name-only Foo.bar.t
+	)
+'
+
 test_done
 

-- 
1.7.2.1.339.gfad93

On Sun, Aug 15, 2010 at 09:08, Junio C Hamano [off-list ref] wrote:

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

On Sat, Aug 14, 2010 at 01:10, Ævar Arnfjörð Bjarmason [off-list ref] wrote:

quoted

Add a regression test for the git log -M --follow --name-only bug
introduced in v1.7.2-rc0~103^2~2

AKA "we didn't have any tests for log's --name-only *at all*".

But this is not related to --name-only at all; anything that is "diff"
related, e.g. -p, --stat, --name-status, will share the same issue.

I meant that as an extra benefit this is the first test for log +
--name-only.

quoted

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 95ac3f8..ff624f4 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh

@@ -441,5 +441,14 @@ test_expect_success 'log.decorate configuration' '

 '

+test_expect_success 'Regression test for v1.7.2-rc0~103^2~2' '

This is uninformative and ugly at the same time.

 - Can't we describe the nature of the situation where the old bug
  triggers concisely?  Perhaps 'show added path under "--follow -M"?'

I didn't grok why this was happening, but yeah, that description is
better.

quoted

+     # Needs an unrelated root commit
+     test_commit README &&

This is not a "root" commit, is it?

s/root/first/

quoted

+     >Foo.bar &&
+     git add Foo.bar &&
+     git commit --allow-empty-message </dev/null &&

Does emptiness of the message matter?

No, I was just going for a minimal test case, no commit message is
more minimal than having one.