* [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository'
@ 2025-06-08 1:06 Ayush Chandekar
2025-06-08 1:06 ` [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
` (3 more replies)
0 siblings, 4 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-06-08 1:06 UTC (permalink / raw)
To: git; +Cc: christian.couder, shyamthakkar001, 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
^ permalink raw reply [flat|nested] 29+ messages in thread
* [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope
2025-06-08 1:06 [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
@ 2025-06-08 1:06 ` Ayush Chandekar
2025-06-28 7:26 ` shejialuo
2025-06-08 1:06 ` [GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
` (2 subsequent siblings)
3 siblings, 1 reply; 29+ messages in thread
From: Ayush Chandekar @ 2025-06-08 1:06 UTC (permalink / raw)
To: git; +Cc: christian.couder, shyamthakkar001, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository'
2025-06-08 1:06 [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
2025-06-08 1:06 ` [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
@ 2025-06-08 1:06 ` Ayush Chandekar
2025-06-28 7:33 ` shejialuo
2025-06-25 15:59 ` [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
2025-06-30 16:41 ` [GSOC PATCH v2 " Ayush Chandekar
3 siblings, 1 reply; 29+ messages in thread
From: Ayush Chandekar @ 2025-06-08 1:06 UTC (permalink / raw)
To: git; +Cc: christian.couder, shyamthakkar001, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository'
2025-06-08 1:06 [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
2025-06-08 1:06 ` [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
2025-06-08 1:06 ` [GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
@ 2025-06-25 15:59 ` Ayush Chandekar
2025-06-30 16:41 ` [GSOC PATCH v2 " Ayush Chandekar
3 siblings, 0 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-06-25 15:59 UTC (permalink / raw)
To: git
Cc: christian.couder, shyamthakkar001, peff, karthik nayak,
Patrick Steinhardt
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope
2025-06-08 1:06 ` [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
@ 2025-06-28 7:26 ` shejialuo
2025-06-28 13:14 ` Ayush Chandekar
0 siblings, 1 reply; 29+ messages in thread
From: shejialuo @ 2025-06-28 7:26 UTC (permalink / raw)
To: Ayush Chandekar; +Cc: git, christian.couder, shyamthakkar001
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository'
2025-06-08 1:06 ` [GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
@ 2025-06-28 7:33 ` shejialuo
2025-06-28 13:21 ` Ayush Chandekar
0 siblings, 1 reply; 29+ messages in thread
From: shejialuo @ 2025-06-28 7:33 UTC (permalink / raw)
To: Ayush Chandekar; +Cc: git, christian.couder, shyamthakkar001
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope
2025-06-28 7:26 ` shejialuo
@ 2025-06-28 13:14 ` Ayush Chandekar
0 siblings, 0 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-06-28 13:14 UTC (permalink / raw)
To: shejialuo; +Cc: git, christian.couder, shyamthakkar001
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:)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository'
2025-06-28 7:33 ` shejialuo
@ 2025-06-28 13:21 ` Ayush Chandekar
0 siblings, 0 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-06-28 13:21 UTC (permalink / raw)
To: shejialuo; +Cc: git, christian.couder, shyamthakkar001
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:)
^ permalink raw reply [flat|nested] 29+ messages in thread
* [GSOC PATCH v2 0/2] builtin/prune: remove dependency on global variables and 'the_repository'
2025-06-08 1:06 [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
` (2 preceding siblings ...)
2025-06-25 15:59 ` [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
@ 2025-06-30 16:41 ` Ayush Chandekar
2025-06-30 16:41 ` [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
` (2 more replies)
3 siblings, 3 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-06-30 16:41 UTC (permalink / raw)
To: ayu.chandekar; +Cc: christian.couder, git, shyamthakkar001, shejialuo
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope
2025-06-30 16:41 ` [GSOC PATCH v2 " Ayush Chandekar
@ 2025-06-30 16:41 ` Ayush Chandekar
2025-07-01 13:01 ` Patrick Steinhardt
2025-06-30 16:41 ` [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
2025-07-04 14:12 ` [GSOC PATCH v3 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
2 siblings, 1 reply; 29+ messages in thread
From: Ayush Chandekar @ 2025-06-30 16:41 UTC (permalink / raw)
To: ayu.chandekar; +Cc: christian.couder, git, shyamthakkar001, shejialuo
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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-06-30 16:41 ` [GSOC PATCH v2 " Ayush Chandekar
2025-06-30 16:41 ` [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
@ 2025-06-30 16:41 ` Ayush Chandekar
2025-07-01 13:01 ` Patrick Steinhardt
2025-07-04 14:12 ` [GSOC PATCH v3 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
2 siblings, 1 reply; 29+ messages in thread
From: Ayush Chandekar @ 2025-06-30 16:41 UTC (permalink / raw)
To: ayu.chandekar; +Cc: christian.couder, git, shyamthakkar001, shejialuo
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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope
2025-06-30 16:41 ` [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
@ 2025-07-01 13:01 ` Patrick Steinhardt
2025-07-01 18:24 ` Ayush Chandekar
0 siblings, 1 reply; 29+ messages in thread
From: Patrick Steinhardt @ 2025-07-01 13:01 UTC (permalink / raw)
To: Ayush Chandekar; +Cc: christian.couder, git, shyamthakkar001, shejialuo
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-06-30 16:41 ` [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
@ 2025-07-01 13:01 ` Patrick Steinhardt
2025-07-01 16:42 ` Junio C Hamano
0 siblings, 1 reply; 29+ messages in thread
From: Patrick Steinhardt @ 2025-07-01 13:01 UTC (permalink / raw)
To: Ayush Chandekar; +Cc: christian.couder, git, shyamthakkar001, shejialuo
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-01 13:01 ` Patrick Steinhardt
@ 2025-07-01 16:42 ` Junio C Hamano
2025-07-01 18:09 ` Ayush Chandekar
0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2025-07-01 16:42 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Ayush Chandekar, christian.couder, git, shyamthakkar001,
shejialuo
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-01 16:42 ` Junio C Hamano
@ 2025-07-01 18:09 ` Ayush Chandekar
2025-07-01 19:44 ` Usman Akinyemi
2025-07-02 2:23 ` Patrick Steinhardt
0 siblings, 2 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-07-01 18:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Patrick Steinhardt, christian.couder, git, shyamthakkar001,
shejialuo
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope
2025-07-01 13:01 ` Patrick Steinhardt
@ 2025-07-01 18:24 ` Ayush Chandekar
2025-07-02 2:23 ` Patrick Steinhardt
0 siblings, 1 reply; 29+ messages in thread
From: Ayush Chandekar @ 2025-07-01 18:24 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: christian.couder, git, shyamthakkar001, shejialuo
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-01 18:09 ` Ayush Chandekar
@ 2025-07-01 19:44 ` Usman Akinyemi
2025-07-01 22:04 ` Ayush Chandekar
2025-07-02 2:23 ` Patrick Steinhardt
1 sibling, 1 reply; 29+ messages in thread
From: Usman Akinyemi @ 2025-07-01 19:44 UTC (permalink / raw)
To: Ayush Chandekar
Cc: Junio C Hamano, Patrick Steinhardt, christian.couder, git,
shyamthakkar001, shejialuo
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.
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-01 19:44 ` Usman Akinyemi
@ 2025-07-01 22:04 ` Ayush Chandekar
0 siblings, 0 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-07-01 22:04 UTC (permalink / raw)
To: Usman Akinyemi
Cc: Junio C Hamano, Patrick Steinhardt, christian.couder, git,
shyamthakkar001, shejialuo
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-01 18:09 ` Ayush Chandekar
2025-07-01 19:44 ` Usman Akinyemi
@ 2025-07-02 2:23 ` Patrick Steinhardt
2025-07-02 11:18 ` Usman Akinyemi
1 sibling, 1 reply; 29+ messages in thread
From: Patrick Steinhardt @ 2025-07-02 2:23 UTC (permalink / raw)
To: Ayush Chandekar
Cc: Junio C Hamano, christian.couder, git, shyamthakkar001, shejialuo
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope
2025-07-01 18:24 ` Ayush Chandekar
@ 2025-07-02 2:23 ` Patrick Steinhardt
0 siblings, 0 replies; 29+ messages in thread
From: Patrick Steinhardt @ 2025-07-02 2:23 UTC (permalink / raw)
To: Ayush Chandekar; +Cc: christian.couder, git, shyamthakkar001, shejialuo
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-02 2:23 ` Patrick Steinhardt
@ 2025-07-02 11:18 ` Usman Akinyemi
2025-07-02 16:53 ` Ben Knoble
` (2 more replies)
0 siblings, 3 replies; 29+ messages in thread
From: Usman Akinyemi @ 2025-07-02 11:18 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Ayush Chandekar, Junio C Hamano, christian.couder, git,
shyamthakkar001, shejialuo
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
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-02 11:18 ` Usman Akinyemi
@ 2025-07-02 16:53 ` Ben Knoble
2025-07-02 17:06 ` Junio C Hamano
2025-07-02 23:51 ` Ayush Chandekar
2 siblings, 0 replies; 29+ messages in thread
From: Ben Knoble @ 2025-07-02 16:53 UTC (permalink / raw)
To: Usman Akinyemi
Cc: Patrick Steinhardt, Ayush Chandekar, Junio C Hamano,
christian.couder, git, shyamthakkar001, shejialuo
> 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
>>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-02 11:18 ` Usman Akinyemi
2025-07-02 16:53 ` Ben Knoble
@ 2025-07-02 17:06 ` Junio C Hamano
2025-07-02 23:51 ` Ayush Chandekar
2 siblings, 0 replies; 29+ messages in thread
From: Junio C Hamano @ 2025-07-02 17:06 UTC (permalink / raw)
To: Usman Akinyemi
Cc: Patrick Steinhardt, Ayush Chandekar, christian.couder, git,
shyamthakkar001, shejialuo
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-02 11:18 ` Usman Akinyemi
2025-07-02 16:53 ` Ben Knoble
2025-07-02 17:06 ` Junio C Hamano
@ 2025-07-02 23:51 ` Ayush Chandekar
2 siblings, 0 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-07-02 23:51 UTC (permalink / raw)
To: Usman Akinyemi
Cc: Patrick Steinhardt, Junio C Hamano, christian.couder, git,
shyamthakkar001, shejialuo
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:)
^ permalink raw reply [flat|nested] 29+ messages in thread
* [GSOC PATCH v3 0/2] builtin/prune: remove dependency on global variables and 'the_repository'
2025-06-30 16:41 ` [GSOC PATCH v2 " Ayush Chandekar
2025-06-30 16:41 ` [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
2025-06-30 16:41 ` [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
@ 2025-07-04 14:12 ` Ayush Chandekar
2025-07-04 14:12 ` [GSOC PATCH v3 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
2025-07-04 14:12 ` [GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
2 siblings, 2 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-07-04 14:12 UTC (permalink / raw)
To: ayu.chandekar
Cc: christian.couder, git, shejialuo, shyamthakkar001, ps, gitster,
usmanakinyemi202
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* [GSOC PATCH v3 1/2] repository: move 'repository_format_precious_objects' to repo scope
2025-07-04 14:12 ` [GSOC PATCH v3 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
@ 2025-07-04 14:12 ` Ayush Chandekar
2025-07-04 14:12 ` [GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
1 sibling, 0 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-07-04 14:12 UTC (permalink / raw)
To: ayu.chandekar
Cc: christian.couder, git, shejialuo, shyamthakkar001, ps, gitster,
usmanakinyemi202
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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-04 14:12 ` [GSOC PATCH v3 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
2025-07-04 14:12 ` [GSOC PATCH v3 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
@ 2025-07-04 14:12 ` Ayush Chandekar
2025-07-07 6:08 ` Patrick Steinhardt
1 sibling, 1 reply; 29+ messages in thread
From: Ayush Chandekar @ 2025-07-04 14:12 UTC (permalink / raw)
To: ayu.chandekar
Cc: christian.couder, git, shejialuo, shyamthakkar001, ps, gitster,
usmanakinyemi202
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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-04 14:12 ` [GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
@ 2025-07-07 6:08 ` Patrick Steinhardt
2025-07-08 13:52 ` Ayush Chandekar
0 siblings, 1 reply; 29+ messages in thread
From: Patrick Steinhardt @ 2025-07-07 6:08 UTC (permalink / raw)
To: Ayush Chandekar
Cc: christian.couder, git, shejialuo, shyamthakkar001, gitster,
usmanakinyemi202
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository'
2025-07-07 6:08 ` Patrick Steinhardt
@ 2025-07-08 13:52 ` Ayush Chandekar
0 siblings, 0 replies; 29+ messages in thread
From: Ayush Chandekar @ 2025-07-08 13:52 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: christian.couder, git, shejialuo, shyamthakkar001, gitster,
usmanakinyemi202
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
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2025-07-08 13:52 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-08 1:06 [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
2025-06-08 1:06 ` [GSOC PATCH 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
2025-06-28 7:26 ` shejialuo
2025-06-28 13:14 ` Ayush Chandekar
2025-06-08 1:06 ` [GSOC PATCH 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
2025-06-28 7:33 ` shejialuo
2025-06-28 13:21 ` Ayush Chandekar
2025-06-25 15:59 ` [GSOC PATCH 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
2025-06-30 16:41 ` [GSOC PATCH v2 " Ayush Chandekar
2025-06-30 16:41 ` [GSOC PATCH v2 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
2025-07-01 13:01 ` Patrick Steinhardt
2025-07-01 18:24 ` Ayush Chandekar
2025-07-02 2:23 ` Patrick Steinhardt
2025-06-30 16:41 ` [GSOC PATCH v2 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
2025-07-01 13:01 ` Patrick Steinhardt
2025-07-01 16:42 ` Junio C Hamano
2025-07-01 18:09 ` Ayush Chandekar
2025-07-01 19:44 ` Usman Akinyemi
2025-07-01 22:04 ` Ayush Chandekar
2025-07-02 2:23 ` Patrick Steinhardt
2025-07-02 11:18 ` Usman Akinyemi
2025-07-02 16:53 ` Ben Knoble
2025-07-02 17:06 ` Junio C Hamano
2025-07-02 23:51 ` Ayush Chandekar
2025-07-04 14:12 ` [GSOC PATCH v3 0/2] builtin/prune: remove dependency on global variables and 'the_repository' Ayush Chandekar
2025-07-04 14:12 ` [GSOC PATCH v3 1/2] repository: move 'repository_format_precious_objects' to repo scope Ayush Chandekar
2025-07-04 14:12 ` [GSOC PATCH v3 2/2] builtin/prune: stop depending on 'the_repository' Ayush Chandekar
2025-07-07 6:08 ` Patrick Steinhardt
2025-07-08 13:52 ` Ayush Chandekar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).