On Tuesday 28 November 2017 09:34 AM, Junio C Hamano wrote:
Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
With this approach, validate_worktree() will print an error message
saying that the worktree directory is missing before the control info
is actually removed. Kaartic's original patch silenced the message
(and _all_ error messages from validate_worktree()) by passing 1 as
the second argument. That seemed heavy-handed, so I suggested keeping
the validate_worktree() call as-is but doing the directory-missing
check first as a special case.
But perhaps that special case isn't necessary.
I do not think the user minds to see "there is no such directory
there"; actually that would be beneficial, even.
But you are right; validate_worktree() would need to become aware of
the possibility that it can be passed such a "corrupt" worktree to
handle if that approach is followed.
The case we are discussing, i.e. the user removed the directory
without telling Git to give it a chance to remove control
information, may be common enough that it becomes a worthwhile
addition to make the "quiet" boolean that occupies a whole int to an
unsigned that is a collection of bits, and pass "missing_ok" bit in
that flag word to the validate_worktree() to tell that the caller
can tolerate the "user removed it while we were looking the other
way" case and can handle it gracefully. But that would be a lot
larger change, and "the caller checks before calling validate" as
done with this [RFC v2] may be a good way to add the feature with
the least impact to the codebase.
I had envisioned a simple 'goto remove_control_info' rather than extra
nesting or refactoring, but that's a minor point.
Yes, use of goto is also a good way to avoid having to worry about
the future evolution of the codeflow in this function.
So perhaps
if (the directory is no longer there)
goto cleanup;
if (the worktree does not validate)
return -1;
... the original code to (carefully) remove the
... worktree itself
cleanup:
... remove control info
... free resources held in variables
... return from the function
is what we want?
Probably but I'm not interested in going for a v3 that does that as I
just wanted to show that worktree remove could be enhanced in this
aspect and show how it could be done. So, I'll leave this in the
#leftoverbits for the person who would be re-rolling nd/worktree-move.
---
Kaartic
