Re: [PATCH 3/3] lockfile: LOCK_REPORT_ON_ERROR

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Apologies from me, too. Once a topic is merged, the credit still remains with the contributor, but the blame is shared by the project as a whole, with those who missed breakages during their reviews, and those who didn't review or test and let breakages pass.

Quite honestly, I do not think either of us cared about preserving the exact error message the end-user was getting from each failure sites that the series changed a call with die-on-error=1 to a call with die-on-error=0 that is followed by a negative return while reviewing this series. As I wrote in the proposed log message for 3/3, this one was noticed as a end-user breaking change because it was the only one that has become totally silent. For example, this bit from sequencer.c::write_message() we can see in the output from "git show --first-parent 2a4062a4a8" does not preserve the message at all:

    diff --git a/sequencer.c b/sequencer.c
    index 3804fa931d..eec8a60d6b 100644
    --- a/sequencer.c
    +++ b/sequencer.c
    @@ -180,17 +180,20 @@
    ...
    -static void write_message(struct strbuf *msgbuf, const char *filename)
    +static int write_message(struct strbuf *msgbuf, const char *filename)
     {
            static struct lock_file msg_file;

    -	int msg_fd = hold_lock_file_for_update(&msg_file, filename,
    -					       LOCK_DIE_ON_ERROR);
    +	int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0);
    +	if (msg_fd < 0)
    +		return error_errno(_("Could not lock '%s'"), filename);

And I do not think it is necessarily bad that the error message changed with this conversion. In other words, I do not think it should have been the goal to preserve the exact error message. hold_lock*() can afford to give a detailed message that strongly sounds as being the final decision when called with die-on-error=1 because it knows it is dying. However, the message from the updated write_message(), "could not lock", cannot be final---the caller may want to add something else after it to describe what failed in a larger picture.