Re: [PATCH] archive: remove USE_THE_REPOSITORY_VARIABLE from archival modules

["René Scharfe" <l.s.r@web.de>]

On 11/12/26 7:33 AM, Srujith A Poojary via GitGitGadget wrote:
> From: Srujith A Poojary <poojarysrujith@gmail.com>
> 
> Refactor archive-tar.c, archive-zip.c, archive.c, builtin/archive.c,
> builtin/upload-archive.c, and diagnose.c to accept and pass explicit
> struct repository pointers instead of relying on global repository state.
> 
> Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
> ---
>     archive: remove USE_THE_REPOSITORY_VARIABLE from archival modules
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2308%2Fpoojarysrujith%2Farchive-refactor-clean-v1-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2308/poojarysrujith/archive-refactor-clean-v1-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2308
> 
>  archive-tar.c            | 7 +++----
>  archive-zip.c            | 3 ++-
>  archive.c                | 6 +++---
>  archive.h                | 6 +++---
>  builtin/archive.c        | 2 +-
>  builtin/upload-archive.c | 2 +-
>  diagnose.c               | 2 +-
>  7 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/archive-tar.c b/archive-tar.c
> index a1c66024d4..5243446701 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -2,7 +2,6 @@
>   * Copyright (c) 2005, 2006 Rene Scharfe
>   */
>  
> -#define USE_THE_REPOSITORY_VARIABLE
>  
>  #include "git-compat-util.h"
>  #include "config.h"
> @@ -337,7 +336,7 @@ static void write_global_extended_header(struct archiver_args *args)
>  	if (oid)
>  		strbuf_append_ext_header(&ext_header, "comment",
>  					 oid_to_hex(oid),
> -					 the_hash_algo->hexsz);
> +					 args->repo->hash_algo->hexsz);
>  	if (args->time > USTAR_MAX_MTIME) {
>  		strbuf_append_ext_header_uint(&ext_header, "mtime",
>  					      args->time);
> @@ -526,7 +525,7 @@ static struct archiver tar_archiver = {
>  	.flags = ARCHIVER_REMOTE,
>  };
>  
> -void init_tar_archiver(void)
> +void init_tar_archiver(struct repository *repo)
>  {
>  	int i;
>  	register_archiver(&tar_archiver);
> @@ -535,7 +534,7 @@ void init_tar_archiver(void)
>  	tar_filter_config("tar.tgz.remote", "true", NULL);
>  	tar_filter_config("tar.tar.gz.command", internal_gzip_command, NULL);
>  	tar_filter_config("tar.tar.gz.remote", "true", NULL);
> -	repo_config(the_repository, git_tar_config, NULL);
> +	repo_config(repo, git_tar_config, NULL);
>  	for (i = 0; i < nr_tar_filters; i++) {
>  		/* omit any filters that never had a command configured */
>  		if (tar_filters[i]->filter_command)
> diff --git a/archive-zip.c b/archive-zip.c
> index 279c3a2d72..b02fb9be38 100644
> --- a/archive-zip.c
> +++ b/archive-zip.c
> @@ -655,7 +655,8 @@ static struct archiver zip_archiver = {
>  	.flags = ARCHIVER_WANT_COMPRESSION_LEVELS|ARCHIVER_REMOTE,
>  };
>  
> -void init_zip_archiver(void)
> +void init_zip_archiver(struct repository *repo)
>  {
> +	(void)repo;
>  	register_archiver(&zip_archiver);
>  }

So repo is not actually used here.  Why pass it in?  It's not like the
init functions all need have the same signature.

Not shown here: archive-zip.c still sets USE_THE_REPOSITORY_VARIABLE.
Perhaps dealing with that is better left to a separate patch, but the
commit message should reflect that only the tar format handler is
treated here.

> diff --git a/archive.c b/archive.c
> index 59790be986..e9e2e241c3 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -43,10 +43,10 @@ void register_archiver(struct archiver *ar)
>  	archivers[nr_archivers++] = ar;
>  }
>  
> -void init_archivers(void)
> +void init_archivers(struct repository *repo)
>  {
> -	init_tar_archiver();
> -	init_zip_archiver();
> +	init_tar_archiver(repo);
> +	init_zip_archiver(repo);
>  }
>  
>  static void format_subst(const struct commit *commit,
> diff --git a/archive.h b/archive.h
> index bbe65ba0f9..8ec59b22e9 100644
> --- a/archive.h
> +++ b/archive.h
> @@ -48,9 +48,9 @@ struct archiver {
>  };
>  void register_archiver(struct archiver *);
>  
> -void init_tar_archiver(void);
> -void init_zip_archiver(void);
> -void init_archivers(void);
> +void init_tar_archiver(struct repository *repo);
> +void init_zip_archiver(struct repository *repo);
> +void init_archivers(struct repository *repo);
>  
>  typedef int (*write_archive_entry_fn_t)(struct archiver_args *args,
>  					const struct object_id *oid,
> diff --git a/builtin/archive.c b/builtin/archive.c
> index 3c1288a123..2c01d7b6e7 100644
> --- a/builtin/archive.c
> +++ b/builtin/archive.c
> @@ -97,7 +97,7 @@ int cmd_archive(int argc,
>  	argc = parse_options(argc, argv, prefix, local_opts, NULL,
>  			     PARSE_OPT_KEEP_ALL);
>  
> -	init_archivers();
> +	init_archivers(repo);
>  
>  	if (output)
>  		create_output_file(output);
> diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
> index 718e74b3ac..3cfc7ffaf3 100644
> --- a/builtin/upload-archive.c
> +++ b/builtin/upload-archive.c
> @@ -34,7 +34,7 @@ int cmd_upload_archive_writer(int argc,
>  	if (!enter_repo(the_repository, argv[1], 0))
>  		die("'%s' does not appear to be a git repository", argv[1]);
>  
> -	init_archivers();
> +	init_archivers(the_repository);
>  
>  	/* put received options in sent_argv[] */
>  	strvec_push(&sent_argv, "git-upload-archive");
> diff --git a/diagnose.c b/diagnose.c
> index 2c1bcfe431..c642dde351 100644
> --- a/diagnose.c
> +++ b/diagnose.c
> @@ -212,7 +212,7 @@ int create_diagnostics_archive(struct repository *r,
>  		goto diagnose_cleanup;
>  	}
>  
> -	init_zip_archiver();
> +	init_zip_archiver(r);
>  	strvec_pushl(&archiver_args, "git-diagnose", "--format=zip", NULL);
>  
>  	strbuf_reset(&buf);
> 
> base-commit: 1891de46f79dc444a47db1c87bd0c8c76b5463f2