Taylor Blau [off-list ref] 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.
...
quoted
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.