Re: [PATCH] builtin: replace the_repository parameter in is_bare_repository()

> "Hardik Kumar" <hardikxk@gmail.com> writes:
>
>>> In general, builtin/foo.c::cmd_foo() are concrete programs that work
>>> on specific repository (i.e., the_repository), and there is not much
>>> reason to rewrite the use of the_repository to use "repo" given by
>>> the caller which is git potty.  You'd also need to deal with the
>>> case where "repo" is NULL (hint: "cd / && git foo -h").
>>
>> Right, but would safety check be required for single instance or better
>> to find and work on only the specific ones which could lead to an
>> exception.
>
> I do not quite get what you mean.  In a single function you use
> the_repository and repo interchangeably, relying on repo, when it is
> not NULL), being the same as the_repository for correctness.  If
> they ever refer to different things, then your updated code is less
> consistent than the original, which would not be an improvement.
>
> I actually think a good medium size project is to fix the last
> parameter given to cmd_foo() built-in command implementations that
> is a pointer to "struct repository".  It was a misguided design.
>
> This pointer is either NULL or the_repository; it never takes any
> other value.  It is misleading and invites confusion that these
> functions can take arbitrary repository instance.  It also invites
> people to replace all references in cmd_foo() to "the_repository"
> with "repo", which may lead to an error.  Depending on where
> parse_options() call appears in the cmd_foo(), "cd / && git foo -h"
> will cause segfaults with such a change.
>
> If we change the function signature of cmd_foo() to receive a "bool"
> that says "true" if the command was run inside a repository and
> "false" otherwise, cmd_foo() can still tell if the command was run
> outside a repository, and programmers will not be misled to use
> anything other than "the_repository" as the repository to work on.
>
> The utility functions builtin/foo.c borrows from outside builtin/
> directory are being "libified" to reduce the hardcoded dependence on
> the_repository, and cmd_foo() can call these functions with
> the_repository as a parameter.  But we have no reason to waste our
> time updating (and also reviewing patches that make such updates)
> the built-in implementations themselves to take a pointer to an
> arbitrary repository.