Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly
From: Stefan Beller <hidden>
Date: 2016-12-07 20:59:00
Possibly related (same subject, not in this thread)
- 2016-12-08 · Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly · Junio C Hamano <hidden>
- 2016-12-07 · Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly · Stefan Beller <hidden>
- 2016-12-07 · Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly · Junio C Hamano <hidden>
On Wed, Dec 7, 2016 at 11:41 AM, Junio C Hamano [off-list ref] wrote:
The require_clean_work_tree() function calls hold_locked_index() with die_on_error=0 to signal that it is OK if it fails to obtain the lock, but unconditionally calls update_index_if_able(), which will try to write into fd=-1. Signed-off-by: Junio C Hamano <redacted> ---
So my first question I had to answer was if we do the right thing here,
i.e. if we could just fail instead. But we want to continue and just
not write back the index, which is fine.
So we do not have to guard refresh_cache, but just call
update_index_if_able conditionally.
However I think the promise of that function is
to take care of the fd == -1?
/*
* Opportunistically update the index but do not complain if we can't
*/
void update_index_if_able(struct index_state *istate, struct
lock_file *lockfile)
{
if ((istate->cache_changed || has_racy_timestamp(istate)) &&
verify_index(istate) &&
write_locked_index(istate, lockfile, COMMIT_LOCK))
rollback_lock_file(lockfile);
}
So I would expect that we'd rather fix the update_index_if_able instead by
checking for the lockfile to be in the correct state?
quoted hunk ↗ jump to hunk
wt-status.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)diff --git a/wt-status.c b/wt-status.c index a2e9d332d8..a715e71906 100644 --- a/wt-status.c +++ b/wt-status.c@@ -2258,11 +2258,12 @@ int has_uncommitted_changes(int ignore_submodules) int require_clean_work_tree(const char *action, const char *hint, int ignore_submodules, int gently) { struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file)); - int err = 0; + int err = 0, fd; - hold_locked_index(lock_file, 0); + fd = hold_locked_index(lock_file, 0); refresh_cache(REFRESH_QUIET); - update_index_if_able(&the_index, lock_file); + if (0 <= fd) + update_index_if_able(&the_index, lock_file); rollback_lock_file(lock_file); if (has_unstaged_changes(ignore_submodules)) { --2.11.0-274-g0631465056