Re: [PATCH v2 4/4] archive: remove the_repository global variable

> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> -#define USE_THE_REPOSITORY_VARIABLE
>>  #include "builtin.h"
>>  #include "archive.h"
>>  #include "gettext.h"
>> @@ -79,7 +78,7 @@ static int run_remote_archiver(int argc, const char **argv,
>>  int cmd_archive(int argc,
>>  		const char **argv,
>>  		const char *prefix,
>> -		struct repository *repo UNUSED)
>> +		struct repository *repo)
>>  {
>>  	const char *exec = "git-upload-archive";
>>  	char *output = NULL;
>> @@ -110,7 +109,7 @@ int cmd_archive(int argc,
>>
>>  	setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
>>
>> -	ret = write_archive(argc, argv, prefix, the_repository, output, 0);
>> +	ret = write_archive(argc, argv, prefix, repo, output, 0);
>
> This looks OK, but unlike the original, write_archive() now needs to
> be prepared to see NULL in the repo parameter.  Is that reasonable?
>
> Perdon me to think aloud for a bit.
>
> The context before this hunk handles "git archive --remote" which
> can be run outside a repository (and that is the whole reason why we
> ask SETUP_GENTLY), but this part has to happen in a repository,
> doesn't it?  Or is there some mode of operation of "git archive" I
> am forgetting that can be done without a repository?
>
>     ... goes and looks ...
>
> OK, write_archive() has its own setup_git_directory() call when
> startup_info->have_repository is false, so this happens to be OK,
> until the beginning part of archive.c:write_archive() will not
> changed to start dereferencing "repo" pointer.
>
> That sounds brittle, but probalby outside the scope of what this
> patch series wants to address.  It also makes git_config() calls
> even before it realizes there is no repository and dies, which
> smells fishy without actually doing any harm.
>
> So, after all, I think this step is probably OK.