git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Derrick Stolee <stolee@gmail.com>
Subject: Re: What's cooking in git.git (Mar 2019, #04; Wed, 20)
Date: Thu, 21 Mar 2019 11:37:15 +0100	[thread overview]
Message-ID: <87sgvgcy3o.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqq7ecssgpc.fsf@gitster-ct.c.googlers.com>


On Thu, Mar 21 2019, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>     	graph_version = *(unsigned char*)(data + 4);
>>     <<<<<<< HEAD
>>     	if (!graph_version || graph_version > 2) {
>>     		error(_("unsupported graph version %X"),
>>     		      graph_version);
>>     =======
>>     	if (graph_version != GRAPH_VERSION) {
>>     		error(_("commit-graph version %X does not match version %X"),
>>     		      graph_version, GRAPH_VERSION);
>>     		return NULL;
>>     	}
>>
>>     	hash_version = *(unsigned char*)(data + 5);
>>     	if (hash_version != oid_version()) {
>>     		error(_("commit-graph hash version %X does not match version %X"),
>>     		      hash_version, oid_version());
>>     >>>>>>> commit-graph-fix-segfault-and-exit-3
>>     		return NULL;
>>     	}
>>
>> Needs to be resolved as:
>>
>> 	graph_version = *(unsigned char*)(data + 4);
>> 	if (!graph_version || graph_version > 2) {
>> 		error(_("commit-graph the graph version %X is unsupported"),
>> 		      graph_version);
>> 		return NULL;
>> 	}
>>
>> I.e. there's a test that greps out "graph version".
>
> Yikes.
>
> Given the common ancestor version's phrasing, and also the updated
> phrasing of the other message since we started supporting v2 of the
> commit-graph file, I resolved this message to
>
> 	 "unsupported commit-graph version %X"
>
> instead.  Of course, I wasn't expecting the test to be depending on
> the exact error message's phrasing X-<.
>
> Thanks.

The "pu" just pushed out as a155e16903ecb60e374d9577216bd1f548bb681a
fails the tests. The "hash_version =" part of the conflict is code that
needs to be deleted. In Derrick's v2 code it's been moved down into some
other code, and checking it there produces a failure.

This on top of "pu" fixes it. Note the changing of the error messages,
in my earlier 39cd8897bf ("commit-graph: improve & i18n error messages",
2019-03-14) I'd changed all the errors to start with "commit-graph ..."
something.

It would be really neat if you could resolve to carry that forward for
consistency, since e.g. the "hash version..." error will otherwise be
printed out by e.g. "git status" and friends without giving the user any
context about it being the commit-graph that's at fault. Maybe
"commit-graph the graph version[...]" is bad wording, but otherwise it's
the only error that doesn't start with "commit-graph...":

diff --git a/commit-graph.c b/commit-graph.c
index b2f64790aa..28b5b599ee 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -155,14 +155,8 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,

 	graph_version = *(unsigned char*)(data + 4);
 	if (!graph_version || graph_version > 2) {
-		error(_("unsupported commit-graph version %X"), graph_version);
-		return NULL;
-	}
-
-	hash_version = *(unsigned char*)(data + 5);
-	if (hash_version != oid_version()) {
-		error(_("commit-graph hash version %X does not match version %X"),
-		      hash_version, oid_version());
+		error(_("commit-graph the graph version %X is unsupported"),
+			graph_version);
 		return NULL;
 	}

@@ -172,7 +166,7 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
 	case 1:
 		hash_version = *(unsigned char*)(data + 5);
 		if (hash_version != oid_version()) {
-			error(_("hash version %X does not match version %X"),
+			error(_("commit-graph hash version %X does not match version %X"),
 			      hash_version, oid_version());
 			return NULL;
 		}

  reply	other threads:[~2019-03-21 10:37 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-20  3:13 What's cooking in git.git (Mar 2019, #04; Wed, 20) Junio C Hamano
2019-03-20  4:56 ` Junio C Hamano
2019-03-20 11:23 ` Thomas Gummerer
2019-03-20 22:05 ` Rohit Ashiwal
2019-03-20 22:56   ` Thomas Gummerer
2019-03-20 23:07     ` Rohit Ashiwal
2019-03-21  0:31     ` Junio C Hamano
2019-03-20 22:42 ` Ævar Arnfjörð Bjarmason
2019-03-21  0:46   ` Junio C Hamano
2019-03-21  5:19     ` Junio C Hamano
2019-03-21  9:13       ` Ævar Arnfjörð Bjarmason
2019-03-21  9:46         ` Junio C Hamano
2019-03-21 10:37           ` Ævar Arnfjörð Bjarmason [this message]
2019-03-21 11:14             ` Duy Nguyen

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=87sgvgcy3o.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stolee@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).