Re: [RFC PATCH v3 0/2] small fixes for git.c and setup.c

> 
> Lidong Yan <yldhome2d2@gmail.com> writes:
> 
>> I've been reading through the git code from the beginning. This
>> patch series fixes some NEEDSWORKs and cleans up some unnecessary
>> uses of the_repository that I came across.
> 
> FYI, when we have "NEEDSWORK: do X", the intention is often "we
> haven't spent enough brain cycles when we wrote this comment, so the
> first step is to evaluate if doing X is a sensible thing in the
> first place, and only if that is the case, do X".

> 
>> The first commit replace the use of the_repository to run_builtin()'s
>> argument repo. Since each caller pass the_repository to run_builtin(),
>> this replacement is safe.
> 
> That change is safe.  I'd rather see the comment left intact or
> reverted to the original shape to clarify what code the comment
> applies to (see the other message).

> 
>> The second commit takes care of a NEEDSWORK in setup_git_directory_gently()
>> we now properly error out if we hit a .git that is not a file or directory
>> when looking for the .git.
> 
> We used to just ignore and keep going to check the parent directory,
> right?  Now we would error out when .git is a FIFO or device or any
> other random things.  Is a bit of behaviour change, but I am not
> sure if it is worth doing.  As finding these weird non-file things
> in your working tree and naming them ".git" is extremely rare and
> useless (from Git's point of view), I suspect that the user is
> deliberately doing so for whatever reason they have, so it smells
> like this change has very little chance to detect a real problem
> with a larger chance to break a set-up that was deliberately done by
> the end-user.  I dunno.
> 
> Thanks.