Subject: Re: [RFC/PATCH v11 04/13] bisect--helper: `bisect_clean_state` shell function in C

From: Junio C Hamano <[email-masked]>
Date: Thu, 04 Aug 2016 08:45:20 -0700

Pranit Bauva <[email-masked]> writes:

>> Also you do not seem to check the error from the function to smudge
>> the "result" you are returning from this function.
>
> Yes I should combine the results from every removal.
>
>> Isn't unlink_or_warn() more correct helper to use here?
>
> The shell code uses rm -f which is silent and it removes only if
> present.

Isn't that what unlink_or_warn() do?  Call unlink() and happily
return if unlink() succeeds or errors with ENOENT (i.e. path didn't
exist in the first place), but otherwise reports an error (imagine:
EPERM).

> So it makes me wonder which would be more appropriate
> unlink_or_warn() or remove_or_warn() or remove_path(). Is
> remove_path() different from its shell equivalent "rm -f"?

Read it again.

>>> +     remove_path(git_path_bisect_start());
>>
>> I can see that refs/files-backend.c misuses it already, but
>> remove_path() helper is about removing a path in the working tree,
>> together with any parent directory that becomes empty due to the
>> removal.  You do not expect $GIT_DIR/ to become an empty directory
>> after removing $GIT_DIR/BISECT_LOG nor want to rmdir $GIT_DIR even
>> if it becomes empty.  It is a wrong helper function to use here.