Re: [RFC PATCH v2] builtin/worktree: enhance worktree removal

> Eric Sunshine <sunshine@sunshineco.com> 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?
>