Re: [PATCHv10 02/10] receive-pack.c: die instead of error in assure_connectivity_checked
From: Jonathan Nieder <hidden>
Date: 2016-06-15 23:03:23
Stefan Beller wrote:
quoted hunk ↗ jump to hunk
--- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c@@ -1055,15 +1055,15 @@ static void assure_connectivity_checked(struct command *commands, for (cmd = commands; cmd; cmd = cmd->next) { if (should_process_cmd(cmd) && si->shallow_ref[cmd->index]) { - error("BUG: connectivity check has not been run on ref %s", - cmd->ref_name); + die("BUG: connectivity check has not been run on ref %s", + cmd->ref_name);
This could stay as error() so that the caller gets to see the list of refs that didn't experience a connectivity check. Or if that list isn't important, this error/die call could be dropped and the 'checked_connectivity = 0' setting would be enough.
checked_connectivity = 0;
}
}
if (!checked_connectivity)
- error("BUG: run 'git fsck' for safety.\n"
- "If there are errors, try to remove "
- "the reported refs above");
+ die("BUG: run 'git fsck' for safety.\n"
+ "If there are errors, try to remove "
+ "the reported refs above");I find this error message misleading and confusing. It makes it seem like this is an expected error that we are trying to help the user recover from. Instead, something impossible has happened. "Try to remove the reported refs" is actively harmful advice --- that would be a way for the user to work around a serious bug instead of figuring out what went wrong and getting git fixed. Jonathan