On Sat, Sep 26, 2009 at 02:06:42AM +0200, Miklos Vajna wrote:
> diff --git a/refs.c b/refs.c
> index 24865cf..221d49c 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
> if (!found)
> return 0;
> fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
> - if (fd < 0)
> + if (fd < 0) {
> + unable_to_lock_index_die(git_path("packed-refs"), errno);
> return error("cannot delete '%s' from packed refs", refname);
> + }
>
> for (list = packed_ref_list; list; list = list->next) {
> char line[PATH_MAX + 100];
I have several concerns about this patch:
1. We used to return error(), but now we die. Are there any callers
which care about the difference?
2. If we did want to die, then the "return error()" just below is
unreachable, and should be removed.
3. If we did want to die, should we not just pass LOCK_DIE_ON_ERROR to
hold_lock_file_for_update?
I suspect (2) and (3) are irrelevant because the answer to (1) is that
yes, some callers do care (e.g., it looks like builtin-remote calls
delete_ref, and notes an error but continues to do useful work
afterwards). So probably you would need to first refactor
unable_to_lock_index_die() to handle just printing the error without
dying.
-Peff
--
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