Taylor Blau <me@xxxxxxxxxxxx> writes:
> I wonder if there are callers of repo_refresh_and_write_index() that
> don't want any errors reported. Not having thought about it too hard
> (much less looked through any of these callers), I would expect that
> having the choice to either error() or die() is something worth keeping.
> But I do not know if there are callers which want neither.
> ...
>> add-interactive.c | 4 ++--
>> add-patch.c | 4 ++--
>> builtin/am.c | 2 +-
>> builtin/merge.c | 4 ++--
>> builtin/stash.c | 6 +++---
>> cache.h | 4 ++--
>> read-cache.c | 3 ++-
>> 7 files changed, 14 insertions(+), 13 deletions(-)
I think most of the changes in this patch, other than the ones to
builtin/stash.c, are unwanted, and I suspect what you wondered above
may be the same thing. Take for example this hunk:
diff --git a/builtin/stash.c b/builtin/stash.c
index a0ccc8654d..977fcc4e40 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -501,7 +501,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
const struct object_id *bases[1];
read_cache_preload(NULL);
- if (refresh_and_write_cache(REFRESH_QUIET, 0, 0))
+ if (refresh_and_write_cache(REFRESH_QUIET, 0, LOCK_REPORT_ON_ERROR, 0))
return -1;
if (write_cache_as_tree(&c_tree, 0, NULL))
Telling the function to be quiet and at the same time be noisy on
only one particular kind of error sounds somewhat strange. I do not
think of any reason why we should believe that failing to lock will
be the only special kind of failure to be of interest to the users.
I would think the "fix" should look more like this:
read_cache_preload(NULL);
if (refresh_and_write_cache(REFRESH_QUIET, 0, 0))
- return -1;
+ return error(_("failed to refresh the index"));
That is, tell the function that the caller will do the error
reporting (i.e. "QUIET") and do so.
Thanks.
