Re: [PATCH v2 4/4] archive: remove the_repository global variable
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 30, 2024, 20:01 UTC
- Message-ID
- <xmqq1q106hdd.fsf@gitster.g>
- In-Reply-To
- <857291d7f7dffdd1a63ce9268c8ac91a82f2bdb5.1727718031.git.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 19 quoted lines
> -#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.