Re: Improving merge failure message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano schrieb:
> Nanako Shiraishi <nanako3@xxxxxxxxxxx> writes:
> 
>> [2]% git merge feature
>> error: Entry 'cool' not uptodate. Cannot merge.
>> fatal: merging of trees 8ec1d96451ff05451720e4e8968812c46b35e5e4 and aad8d5cef3915ab78b3227abaaac99b62db9eb54 failed
>>
>> ... the messages look unnecessarily scary, with two
>> "error" and "fatal" comments, and long sha1 commit names.
> 
>> It would be nice if the message in the latter case can be toned down.
> 
> Yeah, it would be nice.  This actually was something that bothered me as
> well while trying to explain the recovery procedure for these two cases.
> Give me half an hour or so to cook up something...

I think that this is a symptom of a much more involved issue about error
handling.

Currently, it is customary in the code to report an error at each location
where an exceptional condition is detected:

	if (frotz())
		ret = error("frotz messed up");

If frotz() is a "low-level" routine, like a C library function, then this
is the right thing to do. However, if frotz() is our own function
("high-level") which itself calls low-level functions with the same
pattern, then this is obviously the wrong thing to do, because it results
into two error messages.

We need a guideline how errors are reported. This guideline could be:

(1) Low-level functions do not print error messages, but set an error code
and leave the reporting to the caller.

(2) High-level functions must print an error message (through error() or
die()) when they detect a failure of a low-level function that was called
directly.

(3) High-level functions must not print an error message when they detect
a failure of another high-level function that was called directly.

There remains to classify our functions into high-level or low-level. But
I think we won't have a lot of low-level functions.

BTW, I applied this guideline when I worked on {start,finish,run}_command
recently, and IMHO it worked out quite nicely because exactly one error is
reported after a failure.

-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]