git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: Duy Nguyen <pclouds@gmail.com>, Jeff King <peff@peff.net>,
	Junio C Hamano <gitster@pobox.com>, Paul Tan <pyokagan@gmail.com>
Subject: [PATCH] apply: tell user location of corrupted patch file
Date: Fri, 4 Oct 2019 14:59:03 -0700	[thread overview]
Message-ID: <ec38908d05f0d40190173158ef3f0753fa9f1184.1570226253.git.liu.denton@gmail.com> (raw)
In-Reply-To: <20191002184546.GA22174@generichostname>

When `git am` runs into a corrupt patch, it'll error out and give a
message such as,

	error: corrupt patch at line 87

Casual users of am may assume that this line number refers to the <mbox>
file that they provided on the command-line. This assumption, however,
is incorrect. The line count really refers to the
.git/rebase-apply/patch file genrated by am.

Teach am to print the location of corrupted patch files so that users of
the tool will know where to look when fixing their corrupted patch. Thus
the error message will look like this:

	error: corrupt patch at .git/rebase-apply/patch:87

An alternate design was considered which involved printing the line
numbers relative to the output of `git am --show-current-patch` (in
other words, the actual mail file that's provided to am). This design
was not chosen because am does not store the whole mail and instead,
splits the mail into several files. As a result of this, this would
break existing users' workflow if they piped their mail directly to am
from their mail client, the whole mail would not exist in any file and
they would have to manually recreate the mail to see the line number.

Let's avoid breaking Junio's workflow since he's probably one of the
most frequent user of `git am` in the world. ;)

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 apply.c                | 2 +-
 t/t4012-diff-binary.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/apply.c b/apply.c
index 57a61f2881..b0ba2e7b1a 100644
--- a/apply.c
+++ b/apply.c
@@ -1785,7 +1785,7 @@ static int parse_single_patch(struct apply_state *state,
 		len = parse_fragment(state, line, size, patch, fragment);
 		if (len <= 0) {
 			free(fragment);
-			return error(_("corrupt patch at line %d"), state->linenr);
+			return error(_("corrupt patch at %s:%d"), state->patch_input_file, state->linenr);
 		}
 		fragment->patch = line;
 		fragment->size = len;
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 6579c81216..42cb2dd404 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -68,7 +68,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' '
 	sed -e "s/-CIT/xCIT/" <output >broken &&
 	test_must_fail git apply --stat --summary broken 2>detected &&
 	detected=$(cat detected) &&
-	detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
+	detected=$(expr "$detected" : "error.*at broken:\\([0-9]*\\)\$") &&
 	detected=$(sed -ne "${detected}p" broken) &&
 	test "$detected" = xCIT
 '
@@ -77,7 +77,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' '
 	git diff --binary | sed -e "s/-CIT/xCIT/" >broken &&
 	test_must_fail git apply --stat --summary broken 2>detected &&
 	detected=$(cat detected) &&
-	detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
+	detected=$(expr "$detected" : "error.*at broken:\\([0-9]*\\)\$") &&
 	detected=$(sed -ne "${detected}p" broken) &&
 	test "$detected" = xCIT
 '
-- 
2.23.0.687.g391267506c


  parent reply	other threads:[~2019-10-04 21:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 18:45 [BUG] incorrect line numbers reported in git am Denton Liu
2019-10-02 19:44 ` Junio C Hamano
2019-10-02 20:08   ` Denton Liu
2019-10-02 20:03 ` Junio C Hamano
2019-10-02 20:16   ` Denton Liu
2019-10-03  0:52     ` Junio C Hamano
2019-10-03  6:17       ` Duy Nguyen
2019-10-03 22:56         ` Junio C Hamano
2019-10-04 21:59 ` Denton Liu [this message]
2019-10-05  8:33   ` [PATCH] apply: tell user location of corrupted patch file Junio C Hamano
2019-10-05 22:44     ` Junio C Hamano
2019-10-05 22:51     ` 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=ec38908d05f0d40190173158ef3f0753fa9f1184.1570226253.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=pyokagan@gmail.com \
    /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).