All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jack Bates <bk874k@nottheoilrig.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Jack Bates <jack@nottheoilrig.com>
Subject: [PATCH v4] diff: handle --no-abbrev in no-index case
Date: Tue,  6 Dec 2016 09:56:14 -0700	[thread overview]
Message-ID: <20161206165614.22921-1-jack@nottheoilrig.com> (raw)
In-Reply-To: <20161206010134.21856-1-jack@nottheoilrig.com>

There are two different places where the --no-abbrev option is parsed,
and two different places where SHA-1s are abbreviated. We normally parse
--no-abbrev with setup_revisions(), but in the no-index case, "git diff"
calls diff_opt_parse() directly, and diff_opt_parse() didn't handle
--no-abbrev until now. (It did handle --abbrev, however.) We normally
abbreviate SHA-1s with find_unique_abbrev(), but commit 4f03666 ("diff:
handle sha1 abbreviations outside of repository, 2016-10-20) recently
introduced a special case when you run "git diff" outside of a
repository.

setup_revisions() does also call diff_opt_parse(), but not for --abbrev
or --no-abbrev, which it handles itself. setup_revisions() sets
rev_info->abbrev, and later copies that to diff_options->abbrev. It
handles --no-abbrev by setting abbrev to zero. (This change doesn't
touch that.)

Setting abbrev to zero was broken in the outside-of-a-repository special
case, which until now resulted in a truly zero-length SHA-1, rather than
taking zero to mean do not abbreviate. The only way to trigger this bug,
however, was by running "git diff --raw" without either the --abbrev or
--no-abbrev options, because 1) without --raw it doesn't respect abbrev
(which is bizarre, but has been that way forever), 2) we silently clamp
--abbrev=0 to MINIMUM_ABBREV, and 3) --no-abbrev wasn't handled until
now.

The outside-of-a-repository case is one of three no-index cases. The
other two are when one of the files you're comparing is outside of the
repository you're in, and the --no-index option.

Signed-off-by: Jack Bates <jack@nottheoilrig.com>
---
 diff.c                                                  | 6 +++++-
 t/t4013-diff-various.sh                                 | 7 +++++++
 t/t4013/diff.diff_--no-index_--raw_--abbrev=4_dir2_dir  | 3 +++
 t/t4013/diff.diff_--no-index_--raw_--no-abbrev_dir2_dir | 3 +++
 t/t4013/diff.diff_--no-index_--raw_dir2_dir             | 3 +++
 t/t4013/diff.diff_--raw_--abbrev=4_initial              | 6 ++++++
 t/t4013/diff.diff_--raw_--no-abbrev_initial             | 6 ++++++
 t/t4013/diff.diff_--raw_initial                         | 6 ++++++
 8 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 t/t4013/diff.diff_--no-index_--raw_--abbrev=4_dir2_dir
 create mode 100644 t/t4013/diff.diff_--no-index_--raw_--no-abbrev_dir2_dir
 create mode 100644 t/t4013/diff.diff_--no-index_--raw_dir2_dir
 create mode 100644 t/t4013/diff.diff_--raw_--abbrev=4_initial
 create mode 100644 t/t4013/diff.diff_--raw_--no-abbrev_initial
 create mode 100644 t/t4013/diff.diff_--raw_initial

diff --git a/diff.c b/diff.c
index ec87283..84dba60 100644
--- a/diff.c
+++ b/diff.c
@@ -3106,7 +3106,8 @@ static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
 			abbrev = FALLBACK_DEFAULT_ABBREV;
 		if (abbrev > GIT_SHA1_HEXSZ)
 			die("BUG: oid abbreviation out of range: %d", abbrev);
-		hex[abbrev] = '\0';
+		if (abbrev)
+			hex[abbrev] = '\0';
 		return hex;
 	}
 }
@@ -3364,6 +3365,7 @@ void diff_setup(struct diff_options *options)
 
 	options->file = stdout;
 
+	options->abbrev = DEFAULT_ABBREV;
 	options->line_termination = '\n';
 	options->break_opt = -1;
 	options->rename_limit = -1;
@@ -4024,6 +4026,8 @@ int diff_opt_parse(struct diff_options *options,
 			    offending, optarg);
 		return argcount;
 	}
+	else if (!strcmp(arg, "--no-abbrev"))
+		options->abbrev = 0;
 	else if (!strcmp(arg, "--abbrev"))
 		options->abbrev = DEFAULT_ABBREV;
 	else if (skip_prefix(arg, "--abbrev=", &arg)) {
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 566817e..d09acfe 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -311,6 +311,13 @@ diff --line-prefix=abc master master^ side
 diff --dirstat master~1 master~2
 diff --dirstat initial rearrange
 diff --dirstat-by-file initial rearrange
+# No-index --abbrev and --no-abbrev
+diff --raw initial
+diff --raw --abbrev=4 initial
+diff --raw --no-abbrev initial
+diff --no-index --raw dir2 dir
+diff --no-index --raw --abbrev=4 dir2 dir
+diff --no-index --raw --no-abbrev dir2 dir
 EOF
 
 test_expect_success 'log -S requires an argument' '
diff --git a/t/t4013/diff.diff_--no-index_--raw_--abbrev=4_dir2_dir b/t/t4013/diff.diff_--no-index_--raw_--abbrev=4_dir2_dir
new file mode 100644
index 0000000..a71b38a
--- /dev/null
+++ b/t/t4013/diff.diff_--no-index_--raw_--abbrev=4_dir2_dir
@@ -0,0 +1,3 @@
+$ git diff --no-index --raw --abbrev=4 dir2 dir
+:000000 100644 0000... 0000... A	dir/sub
+$
diff --git a/t/t4013/diff.diff_--no-index_--raw_--no-abbrev_dir2_dir b/t/t4013/diff.diff_--no-index_--raw_--no-abbrev_dir2_dir
new file mode 100644
index 0000000..e0f0097
--- /dev/null
+++ b/t/t4013/diff.diff_--no-index_--raw_--no-abbrev_dir2_dir
@@ -0,0 +1,3 @@
+$ git diff --no-index --raw --no-abbrev dir2 dir
+:000000 100644 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 A	dir/sub
+$
diff --git a/t/t4013/diff.diff_--no-index_--raw_dir2_dir b/t/t4013/diff.diff_--no-index_--raw_dir2_dir
new file mode 100644
index 0000000..3cb4ee7
--- /dev/null
+++ b/t/t4013/diff.diff_--no-index_--raw_dir2_dir
@@ -0,0 +1,3 @@
+$ git diff --no-index --raw dir2 dir
+:000000 100644 0000000... 0000000... A	dir/sub
+$
diff --git a/t/t4013/diff.diff_--raw_--abbrev=4_initial b/t/t4013/diff.diff_--raw_--abbrev=4_initial
new file mode 100644
index 0000000..c3641db
--- /dev/null
+++ b/t/t4013/diff.diff_--raw_--abbrev=4_initial
@@ -0,0 +1,6 @@
+$ git diff --raw --abbrev=4 initial
+:100644 100644 35d2... 9929... M	dir/sub
+:100644 100644 01e7... 10a8... M	file0
+:000000 100644 0000... b1e6... A	file1
+:100644 000000 01e7... 0000... D	file2
+$
diff --git a/t/t4013/diff.diff_--raw_--no-abbrev_initial b/t/t4013/diff.diff_--raw_--no-abbrev_initial
new file mode 100644
index 0000000..c87a125
--- /dev/null
+++ b/t/t4013/diff.diff_--raw_--no-abbrev_initial
@@ -0,0 +1,6 @@
+$ git diff --raw --no-abbrev initial
+:100644 100644 35d242ba79ae89ac695e26b3d4c27a8e6f028f9e 992913c5aa0a5476d10c49ed0f21fc0c6d1aedf3 M	dir/sub
+:100644 100644 01e79c32a8c99c557f0757da7cb6d65b3414466d 10a8a9f3657f91a156b9f0184ed79a20adef9f7f M	file0
+:000000 100644 0000000000000000000000000000000000000000 b1e67221afe8461efd244b487afca22d46b95eb8 A	file1
+:100644 000000 01e79c32a8c99c557f0757da7cb6d65b3414466d 0000000000000000000000000000000000000000 D	file2
+$
diff --git a/t/t4013/diff.diff_--raw_initial b/t/t4013/diff.diff_--raw_initial
new file mode 100644
index 0000000..a3e9780
--- /dev/null
+++ b/t/t4013/diff.diff_--raw_initial
@@ -0,0 +1,6 @@
+$ git diff --raw initial
+:100644 100644 35d242b... 992913c... M	dir/sub
+:100644 100644 01e79c3... 10a8a9f... M	file0
+:000000 100644 0000000... b1e6722... A	file1
+:100644 000000 01e79c3... 0000000... D	file2
+$
-- 
2.10.2

  parent reply	other threads:[~2016-12-06 16:56 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-28 18:25 [PATCH] diff: handle --no-abbrev outside of repository Jack Bates
2016-11-28 23:03 ` Junio C Hamano
2016-11-29  7:06 ` Jeff King
2016-12-02 18:48   ` [PATCH v2] " Jack Bates
2016-12-05  6:01     ` Jeff King
2016-12-05  6:15       ` Jeff King
2016-12-05  6:58         ` Jeff King
2016-12-06  1:01           ` [PATCH v3] diff: handle --no-abbrev in no-index case Jack Bates
2016-12-06 16:53             ` [PATCH v4] " Jack Bates
2016-12-06 16:56             ` Jack Bates [this message]
2016-12-06 17:00               ` Jack Bates
2016-12-08 22:53               ` Junio C Hamano
2016-12-09  0:22                 ` Jack Bates

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

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

  git send-email \
    --in-reply-to=20161206165614.22921-1-jack@nottheoilrig.com \
    --to=bk874k@nottheoilrig.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jack@nottheoilrig.com \
    --cc=peff@peff.net \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.