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

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

>> - Only update sites that do not introduce any functional changes.
>
> How did you make sure that the places touched by this patch do not?
>
> How do you deal with repo==NULL case in these code paths?

I guess this was a bit too short, so let me explain in a bit more
detail.

>> diff --git a/builtin/blame.c b/builtin/blame.c
>> index 48d5251c6d..dbf4b4ffc7 100644
>> --- a/builtin/blame.c
>> +++ b/builtin/blame.c
>> @@ -957,7 +957,7 @@ static void build_ignorelist(struct blame_scoreboard *sb,
>>  int cmd_blame(int argc,
>>  	      const char **argv,
>>  	      const char *prefix,
>> -	      struct repository *repo UNUSED)
>> +	      struct repository *repo)
>>  {
>>  	struct rev_info revs;
>>  	char *path = NULL;
>> @@ -1187,7 +1187,7 @@ int cmd_blame(int argc,
>>  
>>  	revs.disable_stdin = 1;
>>  	setup_revisions(argc, argv, &revs, NULL);
>> -	if (!revs.pending.nr && is_bare_repository(the_repository)) {
>> +	if (!revs.pending.nr && is_bare_repository(repo)) {
>>  		struct commit *head_commit;
>>  		struct object_id head_oid;

There are a handful of uses of the_repository before the execution
reaches here.  But you left them unmodified.

The original code used to consistently used the_repository.  Here
you changed it to use "repo".  In practice, they are most likely the
same when "repo" is not NULL, so in that sense, this may not be
breaking anything, but you must ask yourself what the point is,
unless you convert all uses of the_repository with "repo".  It does
not help libification effort at all.

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").

They are quite different from other parts of the system, things
outside builtin/, many of which are general utility/helper routines,
many of which should be designed to work with given repository.