Re: [PATCH v3 05/19] update-ref.c: use the error string from _commit to print better message
From: Jonathan Nieder <hidden>
Date: 2016-06-15 23:00:50
Ronnie Sahlberg wrote:
Call ref_transaction_commit with QUIET_ON_ERR and use the error string that is returned to print a better log message if/after the transaction fails.
Ah, so that's how the transition to a better API happens. Makes sense. (A mention of QUIET_ON_ERR in the patch that introduces the &err parameter could help, or feel free to ignore these comments, since it's all well by the end of the series.)
Update the tests to reflect that the log message is now slightly different fatal: update_ref failed: Cannot lock the ref 'some ref' versus from the previous fatal: Cannot lock the ref 'some ref'
Makes sense as a demo of what the new code allows, but is this error
message better? The use of 'git update-ref' is an implementation
detail that the user doesn't need to know about.
I think I'd prefer the result of plain die("%s", err), even though
that's a no-op.
[...]quoted hunk ↗ jump to hunk
+++ b/builtin/update-ref.c
[...]
quoted hunk ↗ jump to hunk
@@ -359,17 +360,18 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix) die("Refusing to perform update with empty message."); if (read_stdin) { - int ret; transaction = ref_transaction_begin(); - + if (!transaction) + die("failed to update refs");
This can't happen (xcalloc is defined to die() on malloc failure).
If you want to protect against it anyway, die("BUG: ...")?
Thanks,
Jonathan