Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
> We have been silently tolerating errors by returning early with an
> error that the caller ignores since rerere.autoupdate was introduced
> in v1.6.0-rc0~120^2 (2008-06-22). So on error (for example if the
> index is already locked), rerere can return success silently without
> updating the index or with only some items in the index updated.
>
> Better to treat such failures as a fatal error so the operator can
> figure out what is wrong and fix it.
>
> Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
> ---
> I ran into this while auditing hold_locked_index callers. Tests
> still pass after this change. Sensible?
I think what the change wants to do is sensible, but "tests still
pass" probably does not tell us very much. You'd need a competing
writers (e.g. in real life a human user may open two terminals and
try to do things in the same repository from both terminals without
realizing that she is stomping on herself), which is expected to
break in unexpected ways ;-), which is not something the existing
tests would try to catch anyway.
> rerere.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/rerere.c b/rerere.c
> index 1b0555f..195f663 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -477,27 +477,23 @@ out:
>
> static struct lock_file index_lock;
>
> -static int update_paths(struct string_list *update)
> +static void update_paths(struct string_list *update)
> {
> int i;
> - int fd = hold_locked_index(&index_lock, 0);
> - int status = 0;
>
> - if (fd < 0)
> - return -1;
> + hold_locked_index(&index_lock, 1);
>
> for (i = 0; i < update->nr; i++) {
> struct string_list_item *item = &update->items[i];
> if (add_file_to_cache(item->string, ADD_CACHE_IGNORE_ERRORS))
> - status = -1;
> + die("staging updated %s failed", item->string);
Instead of crafting a new message, why not just stop passing IGNORE_ERRORS
and have add_file_to_cache() report the failure? That is:
if (add_file_to_cache(item->string, 0))
return -1;
That way, the user will get more useful diagnosis because there are
different reasons why an "add" may fail and we give different error
messages to them.
--
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