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

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

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

Thanks.