git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org, gitster@pobox.com, torvalds@linux-foundation.org
Subject: Re: [PATCH] diff: correct newline in summary for renamed files
Date: Wed, 27 Sep 2017 18:32:39 -0400	[thread overview]
Message-ID: <20170927223239.t6gvfztkdjn2vltb@sigill.intra.peff.net> (raw)
In-Reply-To: <20170927220925.u622metv2fcdtcfq@sigill.intra.peff.net>

On Wed, Sep 27, 2017 at 06:09:25PM -0400, Jeff King wrote:

> > diff --git a/t/t4016-diff-quote.sh b/t/t4016-diff-quote.sh
> > index 9c48e5c2c9..514056dd10 100755
> > --- a/t/t4016-diff-quote.sh
> > +++ b/t/t4016-diff-quote.sh
> > @@ -30,6 +30,7 @@ test_expect_success setup '
> >  	git add . &&
> >  	git commit -m initial &&
> >  	git mv "$P0.0" "R$P0.0" &&
> > +	chmod a+x "R$P0.0" &&
> >  	git mv "$P0.1" "R$P1.0" &&
> >  	git mv "$P0.2" "R$P2.0" &&
> >  	git mv "$P0.3" "R$P3.0" &&
> 
> Won't this chmod be a problem for platforms without an executable bit?
> I think you'd need to use "update-index --chmod=+x" here, or require the
> FILEMODE prereq.
> 
> The whole script is marked as !MINGW, so that makes it less of a
> problem, but it's still possible have !FILEMODE on a Linux system, if
> you're on a funny filesystem. That also seems like a good reason to make
> sure this is in a script which is run more widely, since Windows folks
> would want to run this test, too.

The most appropriate place seems like t4013. I tried adding to its big
list of tested formats, but it's quite fragile. The patch below is what
I came up with, but it still needs updated to cover the cases which call
"log --all".

I think we'd do better to just do a set of new tests at the end (or even
a new test script for diffing mode changes in various formats).

-- >8 --
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index d09acfe48e..c515e3e53f 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -90,6 +90,14 @@ test_expect_success setup '
 	git commit -m "Rearranged lines in dir/sub" &&
 	git checkout master &&
 
+	GIT_AUTHOR_DATE="2006-06-26 00:06:00 +0000" &&
+	GIT_COMMITTER_DATE="2006-06-26 00:06:00 +0000" &&
+	export GIT_AUTHOR_DATE GIT_COMMITTER_DATE &&
+	git checkout -b mode initial &&
+	git update-index --chmod=+x file0 &&
+	git commit -m "update mode" &&
+	git checkout -f master &&
+
 	git config diff.renames false &&
 
 	git show-branch
@@ -192,6 +200,10 @@ diff-tree --pretty side
 diff-tree --pretty -p side
 diff-tree --pretty --patch-with-stat side
 
+diff-tree initial mode
+diff-tree --stat initial mode
+diff-tree --summary initial mode
+
 diff-tree master
 diff-tree -p master
 diff-tree -p -m master
diff --git a/t/t4013/diff.diff-tree_--stat_initial_mode b/t/t4013/diff.diff-tree_--stat_initial_mode
new file mode 100644
index 0000000000..0e5943c2c6
--- /dev/null
+++ b/t/t4013/diff.diff-tree_--stat_initial_mode
@@ -0,0 +1,4 @@
+$ git diff-tree --stat initial mode
+ file0 | 0
+ 1 file changed, 0 insertions(+), 0 deletions(-)
+$
diff --git a/t/t4013/diff.diff-tree_--summary_initial_mode b/t/t4013/diff.diff-tree_--summary_initial_mode
new file mode 100644
index 0000000000..25846b6af8
--- /dev/null
+++ b/t/t4013/diff.diff-tree_--summary_initial_mode
@@ -0,0 +1,3 @@
+$ git diff-tree --summary initial mode
+ mode change 100644 => 100755 file0
+$
diff --git a/t/t4013/diff.diff-tree_initial_mode b/t/t4013/diff.diff-tree_initial_mode
new file mode 100644
index 0000000000..c47c09423e
--- /dev/null
+++ b/t/t4013/diff.diff-tree_initial_mode
@@ -0,0 +1,3 @@
+$ git diff-tree initial mode
+:100644 100755 01e79c32a8c99c557f0757da7cb6d65b3414466d 01e79c32a8c99c557f0757da7cb6d65b3414466d M	file0
+$

  reply	other threads:[~2017-09-27 22:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-27 18:15 diffstat summary mode change bug Linus Torvalds
2017-09-27 20:40 ` Stefan Beller
2017-09-27 21:02   ` Linus Torvalds
2017-09-27 21:09     ` Stefan Beller
2017-09-27 21:58       ` [PATCH] diff: correct newline in summary for renamed files Stefan Beller
2017-09-27 22:04         ` Linus Torvalds
2017-09-27 22:09         ` Jeff King
2017-09-27 22:32           ` Jeff King [this message]
2017-09-27 22:39             ` Stefan Beller
2017-09-27 22:49               ` Jeff King
2017-09-27 22:51                 ` Stefan Beller
2017-09-27 23:15                   ` Jonathan Nieder
2017-09-27 23:49                   ` Ramsay Jones
2017-09-27 23:57                     ` Stefan Beller
2017-09-28  0:39                       ` Jeff King
2017-09-28  4:14                     ` Junio C Hamano
2017-09-28  0:35                   ` Jeff King
2017-09-27 22:34           ` Stefan Beller
2017-09-28  4:12 ` diffstat summary mode change bug Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170927223239.t6gvfztkdjn2vltb@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).