On Thu, Mar 21 2019, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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;
}
