[GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository'

[GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository'

[Ayush Chandekar]

The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE' 
from "builtin/prune.c" by removing global variables and the global 'the_repository'.

This patch series contains two patches:

1 - Move the global variable 'repository_format_precious_objects' into 'struct repository' 
and update all affected code paths accordingly.

2 - Remove the dependency of 'the_repository' in "builtin/prunce.c", allowing the removal of 
the definition.

Ayush Chandekar (2):
  repository: move 'repository_format_precious_objects' to repo scope
  builtin/prune: stop depending on 'the_repository'

 builtin/gc.c     |  2 +-
 builtin/prune.c  | 25 ++++++++++++-------------
 builtin/repack.c |  2 +-
 environment.c    |  1 -
 environment.h    |  2 --
 repository.c     |  1 +
 repository.h     |  1 +
 setup.c          |  5 ++++-
 8 files changed, 20 insertions(+), 19 deletions(-)

-- 
2.49.0

[GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope

[Ayush Chandekar]

The 'extensions.preciousObjects' settings when set true, prevents
operations that might drop objects from the object storage.
This setting is populated in the global variable
'repository_format_precious_objects'.
Move this global variable to repo scope by adding it to 'struct
repository' and also refactor all the occurences accordingly.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/gc.c     | 2 +-
 builtin/prune.c  | 2 +-
 builtin/repack.c | 2 +-
 environment.c    | 1 -
 environment.h    | 2 --
 repository.c     | 1 +
 repository.h     | 1 +
 setup.c          | 5 ++++-
 8 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index e33ba946e4..764f123b1c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -976,7 +976,7 @@ int cmd_gc(int argc,
 
 	gc_before_repack(&opts, &cfg);
 
-	if (!repository_format_precious_objects) {
+	if (!the_repository->repository_format_precious_objects) {
 		struct child_process repack_cmd = CHILD_PROCESS_INIT;
 
 		repack_cmd.git_cmd = 1;
diff --git a/builtin/prune.c b/builtin/prune.c
index e930caa0c0..dab3c19b6f 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -177,7 +177,7 @@ int cmd_prune(int argc,
 
 	argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
 
-	if (repository_format_precious_objects)
+	if (the_repository->repository_format_precious_objects)
 		die(_("cannot prune in a precious-objects repo"));
 
 	while (argc--) {
diff --git a/builtin/repack.c b/builtin/repack.c
index 59214dbdfd..e1174a7a53 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -1235,7 +1235,7 @@ int cmd_repack(int argc,
 	po_args.depth = xstrdup_or_null(opt_depth);
 	po_args.threads = xstrdup_or_null(opt_threads);
 
-	if (delete_redundant && repository_format_precious_objects)
+	if (delete_redundant && the_repository->repository_format_precious_objects)
 		die(_("cannot delete packs in a precious-objects repo"));
 
 	die_for_incompatible_opt3(unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE), "-A",
diff --git a/environment.c b/environment.c
index c61d773e7e..f3d318f1a0 100644
--- a/environment.c
+++ b/environment.c
@@ -37,7 +37,6 @@ int ignore_case;
 int assume_unchanged;
 int is_bare_repository_cfg = -1; /* unspecified */
 int warn_on_object_refname_ambiguity = 1;
-int repository_format_precious_objects;
 char *git_commit_encoding;
 char *git_log_output_encoding;
 char *apply_default_whitespace;
diff --git a/environment.h b/environment.h
index 3d98461a06..bee1ffb91d 100644
--- a/environment.h
+++ b/environment.h
@@ -190,8 +190,6 @@ extern enum object_creation_mode object_creation_mode;
 
 extern int grafts_keep_true_parents;
 
-extern int repository_format_precious_objects;
-
 const char *get_log_output_encoding(void);
 const char *get_commit_output_encoding(void);
 
diff --git a/repository.c b/repository.c
index 9b3d6665fc..62709d1c91 100644
--- a/repository.c
+++ b/repository.c
@@ -284,6 +284,7 @@ int repo_init(struct repository *repo,
 	repo_set_ref_storage_format(repo, format.ref_storage_format);
 	repo->repository_format_worktree_config = format.worktree_config;
 	repo->repository_format_relative_worktrees = format.relative_worktrees;
+	repo->repository_format_precious_objects = format.precious_objects;
 
 	/* take ownership of format.partial_clone */
 	repo->repository_format_partial_clone = format.partial_clone;
diff --git a/repository.h b/repository.h
index c4c92b2ab9..ad23a243c6 100644
--- a/repository.h
+++ b/repository.h
@@ -151,6 +151,7 @@ struct repository {
 	/* Configurations */
 	int repository_format_worktree_config;
 	int repository_format_relative_worktrees;
+	int repository_format_precious_objects;
 
 	/* Indicate if a repository has a different 'commondir' from 'gitdir' */
 	unsigned different_commondir:1;
diff --git a/setup.c b/setup.c
index f93bd6a24a..3ea01e9331 100644
--- a/setup.c
+++ b/setup.c
@@ -753,7 +753,8 @@ static int check_repository_format_gently(const char *gitdir, struct repository_
 		die("%s", err.buf);
 	}
 
-	repository_format_precious_objects = candidate->precious_objects;
+	the_repository->repository_format_precious_objects = candidate->precious_objects;
+
 	string_list_clear(&candidate->unknown_extensions, 0);
 	string_list_clear(&candidate->v1_only_extensions, 0);
 
@@ -1864,6 +1865,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			the_repository->repository_format_partial_clone =
 				repo_fmt.partial_clone;
 			repo_fmt.partial_clone = NULL;
+			the_repository->repository_format_precious_objects =
+				repo_fmt.precious_objects;
 		}
 	}
 	/*
-- 
2.49.0

Re: [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope

[shejialuo]

On Sun, Jun 08, 2025 at 06:36:34AM +0530, Ayush Chandekar wrote:
> The 'extensions.preciousObjects' settings when set true, prevents

Should "settings" be "setting"?

> operations that might drop objects from the object storage.
> This setting is populated in the global variable
> 'repository_format_precious_objects'.
> Move this global variable to repo scope by adding it to 'struct
> repository' and also refactor all the occurences accordingly.
> 
> This change is part of an ongoing effort to eliminate global variables,
> improve modularity and help libify the codebase.
> 
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>

Maybe you should add the following things before "Signed-off-by":

    Mentored-by: ...
    Mentored-by: ...

Thanks,
Jialuo

Re: [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope

[Ayush Chandekar]

On Sat, Jun 28, 2025 at 12:56 PM shejialuo <shejialuo@gmail.com> wrote:
>
> On Sun, Jun 08, 2025 at 06:36:34AM +0530, Ayush Chandekar wrote:
> > The 'extensions.preciousObjects' settings when set true, prevents
>
> Should "settings" be "setting"?
>

Yeah, thanks for correcting!

> > operations that might drop objects from the object storage.
> > This setting is populated in the global variable
> > 'repository_format_precious_objects'.
> > Move this global variable to repo scope by adding it to 'struct
> > repository' and also refactor all the occurences accordingly.
> >
> > This change is part of an ongoing effort to eliminate global variables,
> > improve modularity and help libify the codebase.
> >
> > Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
>
> Maybe you should add the following things before "Signed-off-by":
>
>     Mentored-by: ...
>     Mentored-by: ...
>

Oh, I missed it on this patch series, apologies, I'll update it.

> Thanks,
> Jialuo

Thanks:)

[GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository'

[Ayush Chandekar]

Refactor builtin/prune.c to remove the dependency on the global
'the_repository'. Replace all the occurrences of 'the_repository' with
repo and thus remove the definition '#define
USE_THE_REPOSITORY_VARIABLE'

Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/prune.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/builtin/prune.c b/builtin/prune.c
index dab3c19b6f..2c584a6e0e 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "builtin.h"
@@ -64,7 +63,7 @@ static void perform_reachability_traversal(struct rev_info *revs)
 		return;
 
 	if (show_progress)
-		progress = start_delayed_progress(the_repository,
+		progress = start_delayed_progress(revs->repo,
 						  _("Checking connectivity"), 0);
 	mark_reachable_objects(revs, 1, expire, progress);
 	stop_progress(&progress);
@@ -78,7 +77,7 @@ static int is_object_reachable(const struct object_id *oid,
 
 	perform_reachability_traversal(revs);
 
-	obj = lookup_object(the_repository, oid);
+	obj = lookup_object(revs->repo, oid);
 	return obj && (obj->flags & SEEN);
 }
 
@@ -99,7 +98,7 @@ static int prune_object(const struct object_id *oid, const char *fullpath,
 	if (st.st_mtime > expire)
 		return 0;
 	if (show_only || verbose) {
-		enum object_type type = oid_object_info(the_repository, oid,
+		enum object_type type = oid_object_info(revs->repo, oid,
 							NULL);
 		printf("%s %s\n", oid_to_hex(oid),
 		       (type > 0) ? type_name(type) : "unknown");
@@ -154,7 +153,7 @@ static void remove_temporary_files(const char *path)
 int cmd_prune(int argc,
 	      const char **argv,
 	      const char *prefix,
-	      struct repository *repo UNUSED)
+	      struct repository *repo)
 {
 	struct rev_info revs;
 	int exclude_promisor_objects = 0;
@@ -173,19 +172,19 @@ int cmd_prune(int argc,
 	expire = TIME_MAX;
 	save_commit_buffer = 0;
 	disable_replace_refs();
-	repo_init_revisions(the_repository, &revs, prefix);
+	repo_init_revisions(repo, &revs, prefix);
 
 	argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
 
-	if (the_repository->repository_format_precious_objects)
+	if (repo->repository_format_precious_objects)
 		die(_("cannot prune in a precious-objects repo"));
 
 	while (argc--) {
 		struct object_id oid;
 		const char *name = *argv++;
 
-		if (!repo_get_oid(the_repository, name, &oid)) {
-			struct object *object = parse_object_or_die(the_repository, &oid,
+		if (!repo_get_oid(repo, name, &oid)) {
+			struct object *object = parse_object_or_die(repo, &oid,
 								    name);
 			add_pending_object(&revs, object, "");
 		}
@@ -200,16 +199,16 @@ int cmd_prune(int argc,
 		revs.exclude_promisor_objects = 1;
 	}
 
-	for_each_loose_file_in_objdir(repo_get_object_directory(the_repository),
+	for_each_loose_file_in_objdir(repo_get_object_directory(repo),
 				      prune_object, prune_cruft, prune_subdir, &revs);
 
 	prune_packed_objects(show_only ? PRUNE_PACKED_DRY_RUN : 0);
-	remove_temporary_files(repo_get_object_directory(the_repository));
-	s = mkpathdup("%s/pack", repo_get_object_directory(the_repository));
+	remove_temporary_files(repo_get_object_directory(repo));
+	s = mkpathdup("%s/pack", repo_get_object_directory(repo));
 	remove_temporary_files(s);
 	free(s);
 
-	if (is_repository_shallow(the_repository)) {
+	if (is_repository_shallow(repo)) {
 		perform_reachability_traversal(&revs);
 		prune_shallow(show_only ? PRUNE_SHOW_ONLY : 0);
 	}
-- 
2.49.0

Re: [GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository'

[shejialuo]

On Sun, Jun 08, 2025 at 06:36:35AM +0530, Ayush Chandekar wrote:

[snip]

> @@ -99,7 +98,7 @@ static int prune_object(const struct object_id *oid, const char *fullpath,
>  	if (st.st_mtime > expire)
>  		return 0;
>  	if (show_only || verbose) {
> -		enum object_type type = oid_object_info(the_repository, oid,
> +		enum object_type type = oid_object_info(revs->repo, oid,
>  							NULL);

Could we simply make `NULL` in the same line by the way? The readability
would be better.

>  		printf("%s %s\n", oid_to_hex(oid),
>  		       (type > 0) ? type_name(type) : "unknown");
> @@ -154,7 +153,7 @@ static void remove_temporary_files(const char *path)
>  int cmd_prune(int argc,
>  	      const char **argv,
>  	      const char *prefix,
> -	      struct repository *repo UNUSED)
> +	      struct repository *repo)
>  {
>  	struct rev_info revs;
>  	int exclude_promisor_objects = 0;
> @@ -173,19 +172,19 @@ int cmd_prune(int argc,
>  	expire = TIME_MAX;
>  	save_commit_buffer = 0;
>  	disable_replace_refs();
> -	repo_init_revisions(the_repository, &revs, prefix);
> +	repo_init_revisions(repo, &revs, prefix);
>  
>  	argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
>  
> -	if (the_repository->repository_format_precious_objects)
> +	if (repo->repository_format_precious_objects)
>  		die(_("cannot prune in a precious-objects repo"));
>  
>  	while (argc--) {
>  		struct object_id oid;
>  		const char *name = *argv++;
>  
> -		if (!repo_get_oid(the_repository, name, &oid)) {
> -			struct object *object = parse_object_or_die(the_repository, &oid,
> +		if (!repo_get_oid(repo, name, &oid)) {
> +			struct object *object = parse_object_or_die(repo, &oid,
>  								    name);

Same, could we just make `name` in the same line. There is no need for
the newline right now as we change "the_repository" to "repo" which
would reduce some columns.

Thanks,
Jialuo

Re: [GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository'

[Ayush Chandekar]

On Sat, Jun 28, 2025 at 1:02 PM shejialuo <shejialuo@gmail.com> wrote:
>
> On Sun, Jun 08, 2025 at 06:36:35AM +0530, Ayush Chandekar wrote:
>
> [snip]
>
> > @@ -99,7 +98,7 @@ static int prune_object(const struct object_id *oid, const char *fullpath,
> >       if (st.st_mtime > expire)
> >               return 0;
> >       if (show_only || verbose) {
> > -             enum object_type type = oid_object_info(the_repository, oid,
> > +             enum object_type type = oid_object_info(revs->repo, oid,
> >                                                       NULL);
>
> Could we simply make `NULL` in the same line by the way? The readability
> would be better.
>

Yeah, I agree, will do that.

> >               printf("%s %s\n", oid_to_hex(oid),
> >                      (type > 0) ? type_name(type) : "unknown");
> > @@ -154,7 +153,7 @@ static void remove_temporary_files(const char *path)
> >  int cmd_prune(int argc,
> >             const char **argv,
> >             const char *prefix,
> > -           struct repository *repo UNUSED)
> > +           struct repository *repo)
> >  {
> >       struct rev_info revs;
> >       int exclude_promisor_objects = 0;
> > @@ -173,19 +172,19 @@ int cmd_prune(int argc,
> >       expire = TIME_MAX;
> >       save_commit_buffer = 0;
> >       disable_replace_refs();
> > -     repo_init_revisions(the_repository, &revs, prefix);
> > +     repo_init_revisions(repo, &revs, prefix);
> >
> >       argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
> >
> > -     if (the_repository->repository_format_precious_objects)
> > +     if (repo->repository_format_precious_objects)
> >               die(_("cannot prune in a precious-objects repo"));
> >
> >       while (argc--) {
> >               struct object_id oid;
> >               const char *name = *argv++;
> >
> > -             if (!repo_get_oid(the_repository, name, &oid)) {
> > -                     struct object *object = parse_object_or_die(the_repository, &oid,
> > +             if (!repo_get_oid(repo, name, &oid)) {
> > +                     struct object *object = parse_object_or_die(repo, &oid,
> >                                                                   name);
>
> Same, could we just make `name` in the same line. There is no need for
> the newline right now as we change "the_repository" to "repo" which
> would reduce some columns.
>

Yeah, got it.

> Thanks,
> Jialuo

Thanks:)

Re: [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository'

[Ayush Chandekar]

On Sun, Jun 8, 2025 at 6:37 AM Ayush Chandekar <ayu.chandekar@gmail.com> wrote:
>
> The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE'
> from "builtin/prune.c" by removing global variables and the global 'the_repository'.
>
> This patch series contains two patches:
>
> 1 - Move the global variable 'repository_format_precious_objects' into 'struct repository'
> and update all affected code paths accordingly.
>
> 2 - Remove the dependency of 'the_repository' in "builtin/prunce.c", allowing the removal of
> the definition.
>
> Ayush Chandekar (2):
>   repository: move 'repository_format_precious_objects' to repo scope
>   builtin/prune: stop depending on 'the_repository'
>
>  builtin/gc.c     |  2 +-
>  builtin/prune.c  | 25 ++++++++++++-------------
>  builtin/repack.c |  2 +-
>  environment.c    |  1 -
>  environment.h    |  2 --
>  repository.c     |  1 +
>  repository.h     |  1 +
>  setup.c          |  5 ++++-
>  8 files changed, 20 insertions(+), 19 deletions(-)
>
> --
> 2.49.0
>

This patch appears to have gone unnoticed as it hasn't seen any
reviews yet. Resurfacing it in case it got overlooked.

[GSOC PATCH v2 0/2] builtin/prune: remove dependency on global variables and 'the_repository'

[Ayush Chandekar]

The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE' 
from "builtin/prune.c" by removing global variables and the global 'the_repository'.

This patch series contains two patches:

1 - Move the global variable 'repository_format_precious_objects' into 'struct repository' 
and update all affected code paths accordingly.

2 - Remove the dependency of 'the_repository' in "builtin/prunce.c", allowing the removal of 
the definition.

Ayush Chandekar (2):
  repository: move 'repository_format_precious_objects' to repo scope
  builtin/prune: stop depending on 'the_repository'

 builtin/gc.c     |  2 +-
 builtin/prune.c  | 27 ++++++++++++---------------
 builtin/repack.c |  2 +-
 environment.c    |  1 -
 environment.h    |  2 --
 repository.c     |  1 +
 repository.h     |  1 +
 setup.c          |  5 ++++-
 8 files changed, 20 insertions(+), 21 deletions(-)

-- 

Summary of the range-diff:
* Changed the commit message of 1/2 to use "setting" instead of "settings" since it refers to just one.
* Added "Mentored-by" tags in both 1/2 and 2/2.
* Fixed line formatting in builtin/prune.c in 2/2.

Range-diff:
1:  699f9a947d ! 1:  995389d622 repository: move 'repository_format_precious_objects' to repo scope
    @@ Metadata
      ## Commit message ##
         repository: move 'repository_format_precious_objects' to repo scope
     
    -    The 'extensions.preciousObjects' settings when set true, prevents
    +    The 'extensions.preciousObjects' setting when set true, prevents
         operations that might drop objects from the object storage.
         This setting is populated in the global variable
         'repository_format_precious_objects'.
    @@ Commit message
         This change is part of an ongoing effort to eliminate global variables,
         improve modularity and help libify the codebase.
     
    +    Mentored-by: Christian Couder <christian.couder@gmail.com>
    +    Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
         Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
     
      ## builtin/gc.c ##
2:  f22cc88e9f ! 2:  f70de9d549 builtin/prune: stop depending on 'the_repository'
    @@ Commit message
         repo and thus remove the definition '#define
         USE_THE_REPOSITORY_VARIABLE'
     
    +    Mentored-by: Christian Couder <christian.couder@gmail.com>
    +    Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
         Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
     
      ## builtin/prune.c ##
    @@ builtin/prune.c: static int prune_object(const struct object_id *oid, const char
      		return 0;
      	if (show_only || verbose) {
     -		enum object_type type = oid_object_info(the_repository, oid,
    -+		enum object_type type = oid_object_info(revs->repo, oid,
    - 							NULL);
    +-							NULL);
    ++		enum object_type type = oid_object_info(revs->repo, oid, NULL);
      		printf("%s %s\n", oid_to_hex(oid),
      		       (type > 0) ? type_name(type) : "unknown");
    + 	}
     @@ builtin/prune.c: static void remove_temporary_files(const char *path)
      int cmd_prune(int argc,
      	      const char **argv,
    @@ builtin/prune.c: int cmd_prune(int argc,
      
     -		if (!repo_get_oid(the_repository, name, &oid)) {
     -			struct object *object = parse_object_or_die(the_repository, &oid,
    +-								    name);
     +		if (!repo_get_oid(repo, name, &oid)) {
    -+			struct object *object = parse_object_or_die(repo, &oid,
    - 								    name);
    ++			struct object *object = parse_object_or_die(repo, &oid, name);
      			add_pending_object(&revs, object, "");
      		}
    + 		else
     @@ builtin/prune.c: int cmd_prune(int argc,
      		revs.exclude_promisor_objects = 1;
      	}

2.49.0

[GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope

[Ayush Chandekar]

The 'extensions.preciousObjects' setting when set true, prevents
operations that might drop objects from the object storage.
This setting is populated in the global variable
'repository_format_precious_objects'.
Move this global variable to repo scope by adding it to struct
`repository` and also refactor all the occurences accordingly.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/gc.c     | 2 +-
 builtin/prune.c  | 2 +-
 builtin/repack.c | 2 +-
 environment.c    | 1 -
 environment.h    | 2 --
 repository.c     | 1 +
 repository.h     | 1 +
 setup.c          | 5 ++++-
 8 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 845876ff02..ec10b81dcc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -998,7 +998,7 @@ int cmd_gc(int argc,
 	if (opts.detach <= 0 && !skip_foreground_tasks)
 		gc_foreground_tasks(&opts, &cfg);
 
-	if (!repository_format_precious_objects) {
+	if (!the_repository->repository_format_precious_objects) {
 		struct child_process repack_cmd = CHILD_PROCESS_INIT;
 
 		repack_cmd.git_cmd = 1;
diff --git a/builtin/prune.c b/builtin/prune.c
index e930caa0c0..dab3c19b6f 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -177,7 +177,7 @@ int cmd_prune(int argc,
 
 	argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
 
-	if (repository_format_precious_objects)
+	if (the_repository->repository_format_precious_objects)
 		die(_("cannot prune in a precious-objects repo"));
 
 	while (argc--) {
diff --git a/builtin/repack.c b/builtin/repack.c
index 5ddc6e7f95..d0e4fa6bed 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -1240,7 +1240,7 @@ int cmd_repack(int argc,
 	po_args.depth = xstrdup_or_null(opt_depth);
 	po_args.threads = xstrdup_or_null(opt_threads);
 
-	if (delete_redundant && repository_format_precious_objects)
+	if (delete_redundant && the_repository->repository_format_precious_objects)
 		die(_("cannot delete packs in a precious-objects repo"));
 
 	die_for_incompatible_opt3(unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE), "-A",
diff --git a/environment.c b/environment.c
index 7bf0390a33..7c2480b22e 100644
--- a/environment.c
+++ b/environment.c
@@ -37,7 +37,6 @@ int ignore_case;
 int assume_unchanged;
 int is_bare_repository_cfg = -1; /* unspecified */
 int warn_on_object_refname_ambiguity = 1;
-int repository_format_precious_objects;
 char *git_commit_encoding;
 char *git_log_output_encoding;
 char *apply_default_whitespace;
diff --git a/environment.h b/environment.h
index 9a3d05d414..3d806ced6e 100644
--- a/environment.h
+++ b/environment.h
@@ -189,8 +189,6 @@ extern enum object_creation_mode object_creation_mode;
 
 extern int grafts_keep_true_parents;
 
-extern int repository_format_precious_objects;
-
 const char *get_log_output_encoding(void);
 const char *get_commit_output_encoding(void);
 
diff --git a/repository.c b/repository.c
index 9b3d6665fc..62709d1c91 100644
--- a/repository.c
+++ b/repository.c
@@ -284,6 +284,7 @@ int repo_init(struct repository *repo,
 	repo_set_ref_storage_format(repo, format.ref_storage_format);
 	repo->repository_format_worktree_config = format.worktree_config;
 	repo->repository_format_relative_worktrees = format.relative_worktrees;
+	repo->repository_format_precious_objects = format.precious_objects;
 
 	/* take ownership of format.partial_clone */
 	repo->repository_format_partial_clone = format.partial_clone;
diff --git a/repository.h b/repository.h
index c4c92b2ab9..ad23a243c6 100644
--- a/repository.h
+++ b/repository.h
@@ -151,6 +151,7 @@ struct repository {
 	/* Configurations */
 	int repository_format_worktree_config;
 	int repository_format_relative_worktrees;
+	int repository_format_precious_objects;
 
 	/* Indicate if a repository has a different 'commondir' from 'gitdir' */
 	unsigned different_commondir:1;
diff --git a/setup.c b/setup.c
index f93bd6a24a..3ea01e9331 100644
--- a/setup.c
+++ b/setup.c
@@ -753,7 +753,8 @@ static int check_repository_format_gently(const char *gitdir, struct repository_
 		die("%s", err.buf);
 	}
 
-	repository_format_precious_objects = candidate->precious_objects;
+	the_repository->repository_format_precious_objects = candidate->precious_objects;
+
 	string_list_clear(&candidate->unknown_extensions, 0);
 	string_list_clear(&candidate->v1_only_extensions, 0);
 
@@ -1864,6 +1865,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			the_repository->repository_format_partial_clone =
 				repo_fmt.partial_clone;
 			repo_fmt.partial_clone = NULL;
+			the_repository->repository_format_precious_objects =
+				repo_fmt.precious_objects;
 		}
 	}
 	/*
-- 
2.49.0

Re: [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope

[Patrick Steinhardt]

On Mon, Jun 30, 2025 at 10:11:04PM +0530, Ayush Chandekar wrote:
> The 'extensions.preciousObjects' setting when set true, prevents
> operations that might drop objects from the object storage.
> This setting is populated in the global variable
> 'repository_format_precious_objects'.
> Move this global variable to repo scope by adding it to struct
> `repository` and also refactor all the occurences accordingly.

Tiny nit: the line wrapping of this paragraph is a bit weird -- it
should generally wrap at 72 characters and paragraphs are typically
separated from one another by an empty newline.

> diff --git a/repository.c b/repository.c
> index 9b3d6665fc..62709d1c91 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -284,6 +284,7 @@ int repo_init(struct repository *repo,
>  	repo_set_ref_storage_format(repo, format.ref_storage_format);
>  	repo->repository_format_worktree_config = format.worktree_config;
>  	repo->repository_format_relative_worktrees = format.relative_worktrees;
> +	repo->repository_format_precious_objects = format.precious_objects;
>  
>  	/* take ownership of format.partial_clone */
>  	repo->repository_format_partial_clone = format.partial_clone;

The list of variables that we copy from `format` grows longer and
longer. I wonder whether it would make sense to embed a `struct
repository_format` in the repository and then copy over the whole
structure?

Patrick

Re: [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope

[Ayush Chandekar]

On Tue, Jul 1, 2025 at 6:31 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Mon, Jun 30, 2025 at 10:11:04PM +0530, Ayush Chandekar wrote:
> > The 'extensions.preciousObjects' setting when set true, prevents
> > operations that might drop objects from the object storage.
> > This setting is populated in the global variable
> > 'repository_format_precious_objects'.
> > Move this global variable to repo scope by adding it to struct
> > `repository` and also refactor all the occurences accordingly.
>
> Tiny nit: the line wrapping of this paragraph is a bit weird -- it
> should generally wrap at 72 characters and paragraphs are typically
> separated from one another by an empty newline.
>

Okay, I will fix it.

> > diff --git a/repository.c b/repository.c
> > index 9b3d6665fc..62709d1c91 100644
> > --- a/repository.c
> > +++ b/repository.c
> > @@ -284,6 +284,7 @@ int repo_init(struct repository *repo,
> >       repo_set_ref_storage_format(repo, format.ref_storage_format);
> >       repo->repository_format_worktree_config = format.worktree_config;
> >       repo->repository_format_relative_worktrees = format.relative_worktrees;
> > +     repo->repository_format_precious_objects = format.precious_objects;
> >
> >       /* take ownership of format.partial_clone */
> >       repo->repository_format_partial_clone = format.partial_clone;
>
> The list of variables that we copy from `format` grows longer and
> longer. I wonder whether it would make sense to embed a `struct
> repository_format` in the repository and then copy over the whole
> structure?
>
> Patrick

Yeah, I suggested this in a discussion with my mentors and was
expecting comments regarding the same. I can create a new patch for
this change if there's consensus on this.

Re: [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope

[Patrick Steinhardt]

On Tue, Jul 01, 2025 at 11:54:02PM +0530, Ayush Chandekar wrote:
> On Tue, Jul 1, 2025 at 6:31 PM Patrick Steinhardt <ps@pks.im> wrote:
> > On Mon, Jun 30, 2025 at 10:11:04PM +0530, Ayush Chandekar wrote:
> > > diff --git a/repository.c b/repository.c
> > > index 9b3d6665fc..62709d1c91 100644
> > > --- a/repository.c
> > > +++ b/repository.c
> > > @@ -284,6 +284,7 @@ int repo_init(struct repository *repo,
> > >       repo_set_ref_storage_format(repo, format.ref_storage_format);
> > >       repo->repository_format_worktree_config = format.worktree_config;
> > >       repo->repository_format_relative_worktrees = format.relative_worktrees;
> > > +     repo->repository_format_precious_objects = format.precious_objects;
> > >
> > >       /* take ownership of format.partial_clone */
> > >       repo->repository_format_partial_clone = format.partial_clone;
> >
> > The list of variables that we copy from `format` grows longer and
> > longer. I wonder whether it would make sense to embed a `struct
> > repository_format` in the repository and then copy over the whole
> > structure?
> >
> > Patrick
> 
> Yeah, I suggested this in a discussion with my mentors and was
> expecting comments regarding the same. I can create a new patch for
> this change if there's consensus on this.

You could also do it as a follow-up change after this series has landed.
That would be perfectly fine with me.

Patrick

[GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Ayush Chandekar]

Refactor builtin/prune.c to remove the dependency on the global
'the_repository'. Replace all the occurrences of 'the_repository' with
repo and thus remove the definition '#define
USE_THE_REPOSITORY_VARIABLE'

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/prune.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/builtin/prune.c b/builtin/prune.c
index dab3c19b6f..499d2432d1 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "builtin.h"
@@ -64,7 +63,7 @@ static void perform_reachability_traversal(struct rev_info *revs)
 		return;
 
 	if (show_progress)
-		progress = start_delayed_progress(the_repository,
+		progress = start_delayed_progress(revs->repo,
 						  _("Checking connectivity"), 0);
 	mark_reachable_objects(revs, 1, expire, progress);
 	stop_progress(&progress);
@@ -78,7 +77,7 @@ static int is_object_reachable(const struct object_id *oid,
 
 	perform_reachability_traversal(revs);
 
-	obj = lookup_object(the_repository, oid);
+	obj = lookup_object(revs->repo, oid);
 	return obj && (obj->flags & SEEN);
 }
 
@@ -99,8 +98,7 @@ static int prune_object(const struct object_id *oid, const char *fullpath,
 	if (st.st_mtime > expire)
 		return 0;
 	if (show_only || verbose) {
-		enum object_type type = oid_object_info(the_repository, oid,
-							NULL);
+		enum object_type type = oid_object_info(revs->repo, oid, NULL);
 		printf("%s %s\n", oid_to_hex(oid),
 		       (type > 0) ? type_name(type) : "unknown");
 	}
@@ -154,7 +152,7 @@ static void remove_temporary_files(const char *path)
 int cmd_prune(int argc,
 	      const char **argv,
 	      const char *prefix,
-	      struct repository *repo UNUSED)
+	      struct repository *repo)
 {
 	struct rev_info revs;
 	int exclude_promisor_objects = 0;
@@ -173,20 +171,19 @@ int cmd_prune(int argc,
 	expire = TIME_MAX;
 	save_commit_buffer = 0;
 	disable_replace_refs();
-	repo_init_revisions(the_repository, &revs, prefix);
+	repo_init_revisions(repo, &revs, prefix);
 
 	argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
 
-	if (the_repository->repository_format_precious_objects)
+	if (repo->repository_format_precious_objects)
 		die(_("cannot prune in a precious-objects repo"));
 
 	while (argc--) {
 		struct object_id oid;
 		const char *name = *argv++;
 
-		if (!repo_get_oid(the_repository, name, &oid)) {
-			struct object *object = parse_object_or_die(the_repository, &oid,
-								    name);
+		if (!repo_get_oid(repo, name, &oid)) {
+			struct object *object = parse_object_or_die(repo, &oid, name);
 			add_pending_object(&revs, object, "");
 		}
 		else
@@ -200,16 +197,16 @@ int cmd_prune(int argc,
 		revs.exclude_promisor_objects = 1;
 	}
 
-	for_each_loose_file_in_objdir(repo_get_object_directory(the_repository),
+	for_each_loose_file_in_objdir(repo_get_object_directory(repo),
 				      prune_object, prune_cruft, prune_subdir, &revs);
 
 	prune_packed_objects(show_only ? PRUNE_PACKED_DRY_RUN : 0);
-	remove_temporary_files(repo_get_object_directory(the_repository));
-	s = mkpathdup("%s/pack", repo_get_object_directory(the_repository));
+	remove_temporary_files(repo_get_object_directory(repo));
+	s = mkpathdup("%s/pack", repo_get_object_directory(repo));
 	remove_temporary_files(s);
 	free(s);
 
-	if (is_repository_shallow(the_repository)) {
+	if (is_repository_shallow(repo)) {
 		perform_reachability_traversal(&revs);
 		prune_shallow(show_only ? PRUNE_SHOW_ONLY : 0);
 	}
-- 
2.49.0

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Patrick Steinhardt]

On Mon, Jun 30, 2025 at 10:11:05PM +0530, Ayush Chandekar wrote:
> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
>  	expire = TIME_MAX;
>  	save_commit_buffer = 0;
>  	disable_replace_refs();
> -	repo_init_revisions(the_repository, &revs, prefix);
> +	repo_init_revisions(repo, &revs, prefix);

Does this work correctly when running outside of a repository? In
general `cmd_prune()` is not executed and would instead die as it is
declared as `RUN_SETUP`, without the `_GENTLY` suffix. But when the user
asks for help we may still execute the function with a NULL pointer.

Patrick

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Jun 30, 2025 at 10:11:05PM +0530, Ayush Chandekar wrote:
>> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
>>  	expire = TIME_MAX;
>>  	save_commit_buffer = 0;
>>  	disable_replace_refs();
>> -	repo_init_revisions(the_repository, &revs, prefix);
>> +	repo_init_revisions(repo, &revs, prefix);
>
> Does this work correctly when running outside of a repository? In
> general `cmd_prune()` is not executed and would instead die as it is
> declared as `RUN_SETUP`, without the `_GENTLY` suffix. But when the user
> asks for help we may still execute the function with a NULL pointer.

Good eyes.  "git prune -h" would safely exit in parse_options() in
such a case, but this part happens before the parse_options() call.

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Ayush Chandekar]

On Tue, Jul 1, 2025 at 10:12 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Patrick Steinhardt <ps@pks.im> writes:
>
> > On Mon, Jun 30, 2025 at 10:11:05PM +0530, Ayush Chandekar wrote:
> >> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
> >>      expire = TIME_MAX;
> >>      save_commit_buffer = 0;
> >>      disable_replace_refs();
> >> -    repo_init_revisions(the_repository, &revs, prefix);
> >> +    repo_init_revisions(repo, &revs, prefix);
> >
> > Does this work correctly when running outside of a repository? In
> > general `cmd_prune()` is not executed and would instead die as it is
> > declared as `RUN_SETUP`, without the `_GENTLY` suffix. But when the user
> > asks for help we may still execute the function with a NULL pointer.
>
> Good eyes.  "git prune -h" would safely exit in parse_options() in
> such a case, but this part happens before the parse_options() call.
>

Thanks for pointing that out, Patrick. Right now, `parse_options()` is
called just after the `repo_init_revisions()`. I can move the call to
it before this.

Although when I tried running "git prune -h", it still gave me the
expected output.

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Usman Akinyemi]

On Tue, Jul 1, 2025 at 11:40 PM Ayush Chandekar <ayu.chandekar@gmail.com> wrote:
>
> On Tue, Jul 1, 2025 at 10:12 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > On Mon, Jun 30, 2025 at 10:11:05PM +0530, Ayush Chandekar wrote:
> > >> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
> > >>      expire = TIME_MAX;
> > >>      save_commit_buffer = 0;
> > >>      disable_replace_refs();
> > >> -    repo_init_revisions(the_repository, &revs, prefix);
> > >> +    repo_init_revisions(repo, &revs, prefix);
> > >
> > > Does this work correctly when running outside of a repository? In
> > > general `cmd_prune()` is not executed and would instead die as it is
> > > declared as `RUN_SETUP`, without the `_GENTLY` suffix. But when the user
> > > asks for help we may still execute the function with a NULL pointer.
> >
> > Good eyes.  "git prune -h" would safely exit in parse_options() in
> > such a case, but this part happens before the parse_options() call.
> >
Hello Ayush,
>
> Thanks for pointing that out, Patrick. Right now, `parse_options()` is
> called just after the `repo_init_revisions()`. I can move the call to
> it before this.
>
> Although when I tried running "git prune -h", it still gave me the
> expected output.
Do try this outside a repo, i.e outside a git repository and try to
run the all the test if it works.
>

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Ayush Chandekar]

On Wed, Jul 2, 2025 at 1:14 AM Usman Akinyemi
<usmanakinyemi202@gmail.com> wrote:
>
> On Tue, Jul 1, 2025 at 11:40 PM Ayush Chandekar <ayu.chandekar@gmail.com> wrote:
> >
> > On Tue, Jul 1, 2025 at 10:12 PM Junio C Hamano <gitster@pobox.com> wrote:
> > >
> > > Patrick Steinhardt <ps@pks.im> writes:
> > >
> > > > On Mon, Jun 30, 2025 at 10:11:05PM +0530, Ayush Chandekar wrote:
> > > >> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
> > > >>      expire = TIME_MAX;
> > > >>      save_commit_buffer = 0;
> > > >>      disable_replace_refs();
> > > >> -    repo_init_revisions(the_repository, &revs, prefix);
> > > >> +    repo_init_revisions(repo, &revs, prefix);
> > > >
> > > > Does this work correctly when running outside of a repository? In
> > > > general `cmd_prune()` is not executed and would instead die as it is
> > > > declared as `RUN_SETUP`, without the `_GENTLY` suffix. But when the user
> > > > asks for help we may still execute the function with a NULL pointer.
> > >
> > > Good eyes.  "git prune -h" would safely exit in parse_options() in
> > > such a case, but this part happens before the parse_options() call.
> > >
> Hello Ayush,

Hi Usman,

> >
> > Thanks for pointing that out, Patrick. Right now, `parse_options()` is
> > called just after the `repo_init_revisions()`. I can move the call to
> > it before this.
> >
> > Although when I tried running "git prune -h", it still gave me the
> > expected output.
> Do try this outside a repo, i.e outside a git repository and try to
> run the all the test if it works.
> >

Yes, I tried it. All my tests passed as well, you can check it here:
https://github.com/ayu-ch/git/commits/precious-objects-3

Thanks,
Ayush

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Patrick Steinhardt]

On Tue, Jul 01, 2025 at 11:39:48PM +0530, Ayush Chandekar wrote:
> On Tue, Jul 1, 2025 at 10:12 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > On Mon, Jun 30, 2025 at 10:11:05PM +0530, Ayush Chandekar wrote:
> > >> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
> > >>      expire = TIME_MAX;
> > >>      save_commit_buffer = 0;
> > >>      disable_replace_refs();
> > >> -    repo_init_revisions(the_repository, &revs, prefix);
> > >> +    repo_init_revisions(repo, &revs, prefix);
> > >
> > > Does this work correctly when running outside of a repository? In
> > > general `cmd_prune()` is not executed and would instead die as it is
> > > declared as `RUN_SETUP`, without the `_GENTLY` suffix. But when the user
> > > asks for help we may still execute the function with a NULL pointer.
> >
> > Good eyes.  "git prune -h" would safely exit in parse_options() in
> > such a case, but this part happens before the parse_options() call.
> >
> 
> Thanks for pointing that out, Patrick. Right now, `parse_options()` is
> called just after the `repo_init_revisions()`. I can move the call to
> it before this.
> 
> Although when I tried running "git prune -h", it still gave me the
> expected output.

Well, as long as it works and as long as we have a test somewhere that
ensures it keeps working I'm happy.

Patrick

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Usman Akinyemi]

On Wed, Jul 2, 2025 at 4:17 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Jul 01, 2025 at 11:39:48PM +0530, Ayush Chandekar wrote:
> > On Tue, Jul 1, 2025 at 10:12 PM Junio C Hamano <gitster@pobox.com> wrote:
> > >
> > > Patrick Steinhardt <ps@pks.im> writes:
> > >
> > > > On Mon, Jun 30, 2025 at 10:11:05PM +0530, Ayush Chandekar wrote:
> > > >> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
> > > >>      expire = TIME_MAX;
> > > >>      save_commit_buffer = 0;
> > > >>      disable_replace_refs();
> > > >> -    repo_init_revisions(the_repository, &revs, prefix);
> > > >> +    repo_init_revisions(repo, &revs, prefix);
> > > >
> > > > Does this work correctly when running outside of a repository? In
> > > > general `cmd_prune()` is not executed and would instead die as it is
> > > > declared as `RUN_SETUP`, without the `_GENTLY` suffix. But when the user
> > > > asks for help we may still execute the function with a NULL pointer.
> > >
> > > Good eyes.  "git prune -h" would safely exit in parse_options() in
> > > such a case, but this part happens before the parse_options() call.
> > >
> >
> > Thanks for pointing that out, Patrick. Right now, `parse_options()` is
> > called just after the `repo_init_revisions()`. I can move the call to
> > it before this.
> >
> > Although when I tried running "git prune -h", it still gave me the
> > expected output.
>
> Well, as long as it works and as long as we have a test somewhere that
> ensures it keeps working I'm happy.
To add to the testing part, I noticed that there is no test for
checking "git prune -h".

You(Ayush) can add that in "t/t1517-outside-repo.sh" there is a
similar test for that also in the file.
"test_expect_success 'update-server-info does not crash with -h" You
can check it out.

Usman
>
> Patrick
>

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Ben Knoble]

> Le 2 juil. 2025 à 07:18, Usman Akinyemi <usmanakinyemi202@gmail.com> a écrit :
> 
> On Wed, Jul 2, 2025 at 4:17 PM Patrick Steinhardt <ps@pks.im> wrote:
>> 
>>> On Tue, Jul 01, 2025 at 11:39:48PM +0530, Ayush Chandekar wrote:
>>> On Tue, Jul 1, 2025 at 10:12 PM Junio C Hamano <gitster@pobox.com> wrote:
>>>> 
>>>> Patrick Steinhardt <ps@pks.im> writes:
>>>> 
>>>>> On Mon, Jun 30, 2025 at 10:11:05PM +0530, Ayush Chandekar wrote:
>>>>>> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
>>>>>>     expire = TIME_MAX;
>>>>>>     save_commit_buffer = 0;
>>>>>>     disable_replace_refs();
>>>>>> -    repo_init_revisions(the_repository, &revs, prefix);
>>>>>> +    repo_init_revisions(repo, &revs, prefix);
>>>>> 
>>>>> Does this work correctly when running outside of a repository? In
>>>>> general `cmd_prune()` is not executed and would instead die as it is
>>>>> declared as `RUN_SETUP`, without the `_GENTLY` suffix. But when the user
>>>>> asks for help we may still execute the function with a NULL pointer.
>>>> 
>>>> Good eyes.  "git prune -h" would safely exit in parse_options() in
>>>> such a case, but this part happens before the parse_options() call.
>>>> 
>>> 
>>> Thanks for pointing that out, Patrick. Right now, `parse_options()` is
>>> called just after the `repo_init_revisions()`. I can move the call to
>>> it before this.
>>> 
>>> Although when I tried running "git prune -h", it still gave me the
>>> expected output.
>> 
>> Well, as long as it works and as long as we have a test somewhere that
>> ensures it keeps working I'm happy.
> To add to the testing part, I noticed that there is no test for
> checking "git prune -h".
> 
> You(Ayush) can add that in "t/t1517-outside-repo.sh" there is a
> similar test for that also in the file.
> "test_expect_success 'update-server-info does not crash with -h" You
> can check it out.

Aha, TIL! I’ve been working on an (unpublished) series to make help-all work outside repos, too, and I am going to add tests. This will be a nice place. 

> 
> Usman
>> 
>> Patrick
>> 
> 

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Junio C Hamano]

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

> To add to the testing part, I noticed that there is no test for
> checking "git prune -h".
>
> You(Ayush) can add that in "t/t1517-outside-repo.sh" there is a
> similar test for that also in the file.
> "test_expect_success 'update-server-info does not crash with -h" You
> can check it out.

Thanks for finding and suggesting a good place to add new test.

Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'

[Ayush Chandekar]

On Wed, Jul 2, 2025 at 4:48 PM Usman Akinyemi
<usmanakinyemi202@gmail.com> wrote:
>
> On Wed, Jul 2, 2025 at 4:17 PM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Tue, Jul 01, 2025 at 11:39:48PM +0530, Ayush Chandekar wrote:
> > > On Tue, Jul 1, 2025 at 10:12 PM Junio C Hamano <gitster@pobox.com> wrote:
> > > >
> > > > Patrick Steinhardt <ps@pks.im> writes:
> > > >
> > > > > On Mon, Jun 30, 2025 at 10:11:05PM +0530, Ayush Chandekar wrote:
> > > > >> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
> > > > >>      expire = TIME_MAX;
> > > > >>      save_commit_buffer = 0;
> > > > >>      disable_replace_refs();
> > > > >> -    repo_init_revisions(the_repository, &revs, prefix);
> > > > >> +    repo_init_revisions(repo, &revs, prefix);
> > > > >
> > > > > Does this work correctly when running outside of a repository? In
> > > > > general `cmd_prune()` is not executed and would instead die as it is
> > > > > declared as `RUN_SETUP`, without the `_GENTLY` suffix. But when the user
> > > > > asks for help we may still execute the function with a NULL pointer.
> > > >
> > > > Good eyes.  "git prune -h" would safely exit in parse_options() in
> > > > such a case, but this part happens before the parse_options() call.
> > > >
> > >
> > > Thanks for pointing that out, Patrick. Right now, `parse_options()` is
> > > called just after the `repo_init_revisions()`. I can move the call to
> > > it before this.
> > >
> > > Although when I tried running "git prune -h", it still gave me the
> > > expected output.
> >
> > Well, as long as it works and as long as we have a test somewhere that
> > ensures it keeps working I'm happy.
> To add to the testing part, I noticed that there is no test for
> checking "git prune -h".
>
> You(Ayush) can add that in "t/t1517-outside-repo.sh" there is a
> similar test for that also in the file.
> "test_expect_success 'update-server-info does not crash with -h" You
> can check it out.
>
> Usman
> >
> > Patrick
> >

Hey Usman,

Thanks a lot for suggesting the test file. Will include it in the
updated version of this patch series.

Ayush:)

[GSOC PATCH v3 0/2] builtin/prune: remove dependency on global variables and 'the_repository'

[Ayush Chandekar]

The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE' 
from "builtin/prune.c" by removing global variables and the global 'the_repository'.

This patch series contains two patches:

1 - Move the global variable 'repository_format_precious_objects' into 'struct repository' 
and update all affected code paths accordingly.

2 - Remove the dependency of 'the_repository' in "builtin/prunce.c", allowing the removal of 
the definition. Also, add a test to check if 'git prune -h' can be run when repository is 'NULL'.


Ayush Chandekar (2):
  repository: move 'repository_format_precious_objects' to repo scope
  builtin/prune: stop depending on 'the_repository'

 builtin/gc.c            |  2 +-
 builtin/prune.c         | 27 ++++++++++++---------------
 builtin/repack.c        |  2 +-
 environment.c           |  1 -
 environment.h           |  2 --
 repository.c            |  1 +
 repository.h            |  1 +
 setup.c                 |  5 ++++-
 t/t1517-outside-repo.sh |  7 +++++++
 9 files changed, 27 insertions(+), 21 deletions(-)

-- 

Summary of range-diff:
* Format the commit message of 1/2 correctly.
* Move the call to `repo_init_revisions()` after `parse_options()` in 2/2. 
* Add a test to check if 'git prune -h' can be run outside repository in 2/2.

Range-diff with v2:
1:  a58577a147 ! 1:  a828ade541 repository: move 'repository_format_precious_objects' to repo scope
    @@ Commit message
         repository: move 'repository_format_precious_objects' to repo scope
     
         The 'extensions.preciousObjects' setting when set true, prevents
    -    operations that might drop objects from the object storage.
    -    This setting is populated in the global variable
    +    operations that might drop objects from the object storage. This setting
    +    is populated in the global variable
         'repository_format_precious_objects'.
    -    Move this global variable to repo scope by adding it to struct
    -    `repository` and also refactor all the occurences accordingly.
    +
    +    Move this global variable to repo scope by adding it to 'struct
    +    repository and also refactor all the occurences accordingly.
     
         This change is part of an ongoing effort to eliminate global variables,
         improve modularity and help libify the codebase.
2:  c5eaebc2cc ! 2:  22fbbc8cf1 builtin/prune: stop depending on 'the_repository'
    @@ Commit message
         Refactor builtin/prune.c to remove the dependency on the global
         'the_repository'. Replace all the occurrences of 'the_repository' with
         repo and thus remove the definition '#define
    -    USE_THE_REPOSITORY_VARIABLE'
    +    USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that 'git
    +    prune -h' can be called when the repository is `NULL`.
     
         Mentored-by: Christian Couder <christian.couder@gmail.com>
         Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
    @@ builtin/prune.c: int cmd_prune(int argc,
      	save_commit_buffer = 0;
      	disable_replace_refs();
     -	repo_init_revisions(the_repository, &revs, prefix);
    -+	repo_init_revisions(repo, &revs, prefix);
      
      	argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
      
     -	if (the_repository->repository_format_precious_objects)
    ++	repo_init_revisions(repo, &revs, prefix);
     +	if (repo->repository_format_precious_objects)
      		die(_("cannot prune in a precious-objects repo"));
      
    @@ builtin/prune.c: int cmd_prune(int argc,
      		perform_reachability_traversal(&revs);
      		prune_shallow(show_only ? PRUNE_SHOW_ONLY : 0);
      	}
    +
    + ## t/t1517-outside-repo.sh ##
    +@@ t/t1517-outside-repo.sh: test_expect_success 'update-server-info does not crash with -h' '
    + 	test_grep "[Uu]sage: git update-server-info " usage
    + '
    + 
    ++test_expect_success 'prune does not crash with -h' '
    ++	test_expect_code 129 git prune -h >usage &&
    ++	test_grep "[Uu]sage: git prune " usage &&
    ++	test_expect_code 129 nongit git prune -h >usage &&
    ++	test_grep "[Uu]sage: git prune " usage
    ++'
    ++
    + test_done

2.49.0

[GSOC PATCH v3 1/2] repository: move 'repository_format_precious_objects' to repo scope

[Ayush Chandekar]

The 'extensions.preciousObjects' setting when set true, prevents
operations that might drop objects from the object storage. This setting
is populated in the global variable
'repository_format_precious_objects'.

Move this global variable to repo scope by adding it to 'struct
repository and also refactor all the occurences accordingly.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/gc.c     | 2 +-
 builtin/prune.c  | 2 +-
 builtin/repack.c | 2 +-
 environment.c    | 1 -
 environment.h    | 2 --
 repository.c     | 1 +
 repository.h     | 1 +
 setup.c          | 5 ++++-
 8 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 845876ff02..ec10b81dcc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -998,7 +998,7 @@ int cmd_gc(int argc,
 	if (opts.detach <= 0 && !skip_foreground_tasks)
 		gc_foreground_tasks(&opts, &cfg);
 
-	if (!repository_format_precious_objects) {
+	if (!the_repository->repository_format_precious_objects) {
 		struct child_process repack_cmd = CHILD_PROCESS_INIT;
 
 		repack_cmd.git_cmd = 1;
diff --git a/builtin/prune.c b/builtin/prune.c
index e930caa0c0..dab3c19b6f 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -177,7 +177,7 @@ int cmd_prune(int argc,
 
 	argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
 
-	if (repository_format_precious_objects)
+	if (the_repository->repository_format_precious_objects)
 		die(_("cannot prune in a precious-objects repo"));
 
 	while (argc--) {
diff --git a/builtin/repack.c b/builtin/repack.c
index 5ddc6e7f95..d0e4fa6bed 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -1240,7 +1240,7 @@ int cmd_repack(int argc,
 	po_args.depth = xstrdup_or_null(opt_depth);
 	po_args.threads = xstrdup_or_null(opt_threads);
 
-	if (delete_redundant && repository_format_precious_objects)
+	if (delete_redundant && the_repository->repository_format_precious_objects)
 		die(_("cannot delete packs in a precious-objects repo"));
 
 	die_for_incompatible_opt3(unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE), "-A",
diff --git a/environment.c b/environment.c
index 7bf0390a33..7c2480b22e 100644
--- a/environment.c
+++ b/environment.c
@@ -37,7 +37,6 @@ int ignore_case;
 int assume_unchanged;
 int is_bare_repository_cfg = -1; /* unspecified */
 int warn_on_object_refname_ambiguity = 1;
-int repository_format_precious_objects;
 char *git_commit_encoding;
 char *git_log_output_encoding;
 char *apply_default_whitespace;
diff --git a/environment.h b/environment.h
index 9a3d05d414..3d806ced6e 100644
--- a/environment.h
+++ b/environment.h
@@ -189,8 +189,6 @@ extern enum object_creation_mode object_creation_mode;
 
 extern int grafts_keep_true_parents;
 
-extern int repository_format_precious_objects;
-
 const char *get_log_output_encoding(void);
 const char *get_commit_output_encoding(void);
 
diff --git a/repository.c b/repository.c
index 9b3d6665fc..62709d1c91 100644
--- a/repository.c
+++ b/repository.c
@@ -284,6 +284,7 @@ int repo_init(struct repository *repo,
 	repo_set_ref_storage_format(repo, format.ref_storage_format);
 	repo->repository_format_worktree_config = format.worktree_config;
 	repo->repository_format_relative_worktrees = format.relative_worktrees;
+	repo->repository_format_precious_objects = format.precious_objects;
 
 	/* take ownership of format.partial_clone */
 	repo->repository_format_partial_clone = format.partial_clone;
diff --git a/repository.h b/repository.h
index c4c92b2ab9..ad23a243c6 100644
--- a/repository.h
+++ b/repository.h
@@ -151,6 +151,7 @@ struct repository {
 	/* Configurations */
 	int repository_format_worktree_config;
 	int repository_format_relative_worktrees;
+	int repository_format_precious_objects;
 
 	/* Indicate if a repository has a different 'commondir' from 'gitdir' */
 	unsigned different_commondir:1;
diff --git a/setup.c b/setup.c
index f93bd6a24a..3ea01e9331 100644
--- a/setup.c
+++ b/setup.c
@@ -753,7 +753,8 @@ static int check_repository_format_gently(const char *gitdir, struct repository_
 		die("%s", err.buf);
 	}
 
-	repository_format_precious_objects = candidate->precious_objects;
+	the_repository->repository_format_precious_objects = candidate->precious_objects;
+
 	string_list_clear(&candidate->unknown_extensions, 0);
 	string_list_clear(&candidate->v1_only_extensions, 0);
 
@@ -1864,6 +1865,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			the_repository->repository_format_partial_clone =
 				repo_fmt.partial_clone;
 			repo_fmt.partial_clone = NULL;
+			the_repository->repository_format_precious_objects =
+				repo_fmt.precious_objects;
 		}
 	}
 	/*
-- 
2.49.0

[GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository'

[Ayush Chandekar]

Refactor builtin/prune.c to remove the dependency on the global
'the_repository'. Replace all the occurrences of 'the_repository' with
repo and thus remove the definition '#define
USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that 'git
prune -h' can be called when the repository is `NULL`.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/prune.c         | 27 ++++++++++++---------------
 t/t1517-outside-repo.sh |  7 +++++++
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/builtin/prune.c b/builtin/prune.c
index dab3c19b6f..320e9c2341 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "builtin.h"
@@ -64,7 +63,7 @@ static void perform_reachability_traversal(struct rev_info *revs)
 		return;
 
 	if (show_progress)
-		progress = start_delayed_progress(the_repository,
+		progress = start_delayed_progress(revs->repo,
 						  _("Checking connectivity"), 0);
 	mark_reachable_objects(revs, 1, expire, progress);
 	stop_progress(&progress);
@@ -78,7 +77,7 @@ static int is_object_reachable(const struct object_id *oid,
 
 	perform_reachability_traversal(revs);
 
-	obj = lookup_object(the_repository, oid);
+	obj = lookup_object(revs->repo, oid);
 	return obj && (obj->flags & SEEN);
 }
 
@@ -99,8 +98,7 @@ static int prune_object(const struct object_id *oid, const char *fullpath,
 	if (st.st_mtime > expire)
 		return 0;
 	if (show_only || verbose) {
-		enum object_type type = oid_object_info(the_repository, oid,
-							NULL);
+		enum object_type type = oid_object_info(revs->repo, oid, NULL);
 		printf("%s %s\n", oid_to_hex(oid),
 		       (type > 0) ? type_name(type) : "unknown");
 	}
@@ -154,7 +152,7 @@ static void remove_temporary_files(const char *path)
 int cmd_prune(int argc,
 	      const char **argv,
 	      const char *prefix,
-	      struct repository *repo UNUSED)
+	      struct repository *repo)
 {
 	struct rev_info revs;
 	int exclude_promisor_objects = 0;
@@ -173,20 +171,19 @@ int cmd_prune(int argc,
 	expire = TIME_MAX;
 	save_commit_buffer = 0;
 	disable_replace_refs();
-	repo_init_revisions(the_repository, &revs, prefix);
 
 	argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
 
-	if (the_repository->repository_format_precious_objects)
+	repo_init_revisions(repo, &revs, prefix);
+	if (repo->repository_format_precious_objects)
 		die(_("cannot prune in a precious-objects repo"));
 
 	while (argc--) {
 		struct object_id oid;
 		const char *name = *argv++;
 
-		if (!repo_get_oid(the_repository, name, &oid)) {
-			struct object *object = parse_object_or_die(the_repository, &oid,
-								    name);
+		if (!repo_get_oid(repo, name, &oid)) {
+			struct object *object = parse_object_or_die(repo, &oid, name);
 			add_pending_object(&revs, object, "");
 		}
 		else
@@ -200,16 +197,16 @@ int cmd_prune(int argc,
 		revs.exclude_promisor_objects = 1;
 	}
 
-	for_each_loose_file_in_objdir(repo_get_object_directory(the_repository),
+	for_each_loose_file_in_objdir(repo_get_object_directory(repo),
 				      prune_object, prune_cruft, prune_subdir, &revs);
 
 	prune_packed_objects(show_only ? PRUNE_PACKED_DRY_RUN : 0);
-	remove_temporary_files(repo_get_object_directory(the_repository));
-	s = mkpathdup("%s/pack", repo_get_object_directory(the_repository));
+	remove_temporary_files(repo_get_object_directory(repo));
+	s = mkpathdup("%s/pack", repo_get_object_directory(repo));
 	remove_temporary_files(s);
 	free(s);
 
-	if (is_repository_shallow(the_repository)) {
+	if (is_repository_shallow(repo)) {
 		perform_reachability_traversal(&revs);
 		prune_shallow(show_only ? PRUNE_SHOW_ONLY : 0);
 	}
diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
index 6824581317..8f59b867f2 100755
--- a/t/t1517-outside-repo.sh
+++ b/t/t1517-outside-repo.sh
@@ -114,4 +114,11 @@ test_expect_success 'update-server-info does not crash with -h' '
 	test_grep "[Uu]sage: git update-server-info " usage
 '
 
+test_expect_success 'prune does not crash with -h' '
+	test_expect_code 129 git prune -h >usage &&
+	test_grep "[Uu]sage: git prune " usage &&
+	test_expect_code 129 nongit git prune -h >usage &&
+	test_grep "[Uu]sage: git prune " usage
+'
+
 test_done
-- 
2.49.0

Re: [GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository'

[Patrick Steinhardt]

On Fri, Jul 04, 2025 at 07:42:35PM +0530, Ayush Chandekar wrote:
> diff --git a/builtin/prune.c b/builtin/prune.c
> index dab3c19b6f..320e9c2341 100644
> --- a/builtin/prune.c
> +++ b/builtin/prune.c
> @@ -173,20 +171,19 @@ int cmd_prune(int argc,
>  	expire = TIME_MAX;
>  	save_commit_buffer = 0;
>  	disable_replace_refs();
> -	repo_init_revisions(the_repository, &revs, prefix);
>  
>  	argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
>  
> -	if (the_repository->repository_format_precious_objects)
> +	repo_init_revisions(repo, &revs, prefix);
> +	if (repo->repository_format_precious_objects)
>  		die(_("cannot prune in a precious-objects repo"));
>  

Okay, we now only end up using the passed-in potentially-NULL `repo`
after we have called `parse_options`. Makes sense.

> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
> index 6824581317..8f59b867f2 100755
> --- a/t/t1517-outside-repo.sh
> +++ b/t/t1517-outside-repo.sh
> @@ -114,4 +114,11 @@ test_expect_success 'update-server-info does not crash with -h' '
>  	test_grep "[Uu]sage: git update-server-info " usage
>  '
>  
> +test_expect_success 'prune does not crash with -h' '
> +	test_expect_code 129 git prune -h >usage &&
> +	test_grep "[Uu]sage: git prune " usage &&
> +	test_expect_code 129 nongit git prune -h >usage &&
> +	test_grep "[Uu]sage: git prune " usage
> +'
> +
>  test_done

And we have another test that verifies that all of this works outside of
a repository.

This addresses my review comments, so this version looks good to me.
Thanks!

Patrick

Re: [GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository'

[Ayush Chandekar]

Hi Patrick,

On Mon, Jul 7, 2025 at 11:38 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Fri, Jul 04, 2025 at 07:42:35PM +0530, Ayush Chandekar wrote:
> > diff --git a/builtin/prune.c b/builtin/prune.c
> > index dab3c19b6f..320e9c2341 100644
> > --- a/builtin/prune.c
> > +++ b/builtin/prune.c
> > @@ -173,20 +171,19 @@ int cmd_prune(int argc,
> >       expire = TIME_MAX;
> >       save_commit_buffer = 0;
> >       disable_replace_refs();
> > -     repo_init_revisions(the_repository, &revs, prefix);
> >
> >       argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
> >
> > -     if (the_repository->repository_format_precious_objects)
> > +     repo_init_revisions(repo, &revs, prefix);
> > +     if (repo->repository_format_precious_objects)
> >               die(_("cannot prune in a precious-objects repo"));
> >
>
> Okay, we now only end up using the passed-in potentially-NULL `repo`
> after we have called `parse_options`. Makes sense.
>
> > diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
> > index 6824581317..8f59b867f2 100755
> > --- a/t/t1517-outside-repo.sh
> > +++ b/t/t1517-outside-repo.sh
> > @@ -114,4 +114,11 @@ test_expect_success 'update-server-info does not crash with -h' '
> >       test_grep "[Uu]sage: git update-server-info " usage
> >  '
> >
> > +test_expect_success 'prune does not crash with -h' '
> > +     test_expect_code 129 git prune -h >usage &&
> > +     test_grep "[Uu]sage: git prune " usage &&
> > +     test_expect_code 129 nongit git prune -h >usage &&
> > +     test_grep "[Uu]sage: git prune " usage
> > +'
> > +
> >  test_done
>
> And we have another test that verifies that all of this works outside of
> a repository.
>
> This addresses my review comments, so this version looks good to me.
> Thanks!
>
> Patrick

Thanks a lot for reviewing!

Ayush