* [PATCH] prune-packed: remove USE_THE_REPOSITORY_VARIABLE
@ 2026-10-18 16:31 Shashank S via GitGitGadget
2026-10-19 7:09 ` Patrick Steinhardt
2026-10-19 16:53 ` [PATCH v2] [Outreachy Patch] " Shashank S via GitGitGadget
0 siblings, 2 replies; 5+ messages in thread
From: Shashank S via GitGitGadget @ 2026-10-18 16:31 UTC (permalink / raw)
To: git
Cc: Kaartic Sivaraam, Christian Couder, Siddharth Asthana,
Usman Akinyemi, Junio C Hamano, Shashank S, Shashank
From: Shashank <shashu8660@gmail.com>
Right now, the prune_packed_objects() function uses the global
the_repository variable. To help with the libification effort,
change this function so it takes a 'struct repository' pointer
as an argument instead.
The function for_each_loose_file_in_source() uses a callback
that only takes a single 'void *data' pointer. This means we
cannot pass both the repository pointer and the 'opts' integer
at the same time. To fix this, create a new 'struct prune_data'
to hold both of them together, and update the prune_subdir() and
prune_object() callbacks to read from this new struct.
Update the builtins that call this function (builtin/prune-packed.c,
builtin/prune.c, and builtin/repack.c) to pass their local `repo`
variables. Because we are now actually using the 'repo' parameter
in these files, we can safely remove the UNUSED macro from them.
finally, drop the USE_THE_REPOSITORY_VARIABLE macro from
prune-packed.c.
Signed-off-by: Shashank <shashu8660@gmail.com>
---
[Outreachy PATCH] prune-packed: remove use_the_repository_variable
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2266%2Fshashu8660%2Foutreachy2-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2266/shashu8660/outreachy2-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2266
builtin/prune-packed.c | 4 ++--
builtin/prune.c | 2 +-
builtin/repack.c | 2 +-
prune-packed.c | 32 ++++++++++++++++++++------------
prune-packed.h | 4 +++-
5 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index 4d63f26b0a..d44e7f81d4 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -11,7 +11,7 @@ static const char * const prune_packed_usage[] = {
int cmd_prune_packed(int argc,
const char **argv,
const char *prefix,
- struct repository *repo UNUSED)
+ struct repository *repo)
{
int opts = isatty(2) ? PRUNE_PACKED_VERBOSE : 0;
const struct option prune_packed_options[] = {
@@ -30,6 +30,6 @@ int cmd_prune_packed(int argc,
prune_packed_usage,
prune_packed_options);
- prune_packed_objects(opts);
+ prune_packed_objects(repo, opts);
return 0;
}
diff --git a/builtin/prune.c b/builtin/prune.c
index a7e4678d11..4eee5a2e27 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -201,7 +201,7 @@ int cmd_prune(int argc,
for_each_loose_file_in_source(repo->objects->sources,
prune_object, prune_cruft, prune_subdir, &revs);
- prune_packed_objects(show_only ? PRUNE_PACKED_DRY_RUN : 0);
+ prune_packed_objects(repo, show_only ? PRUNE_PACKED_DRY_RUN : 0);
remove_temporary_files(repo_get_object_directory(repo));
s = mkpathdup("%s/pack", repo_get_object_directory(repo));
remove_temporary_files(s);
diff --git a/builtin/repack.c b/builtin/repack.c
index c4360382c1..e8970e8eb7 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -757,7 +757,7 @@ int cmd_repack(int argc,
wrote_incremental_midx);
if (show_progress)
opts |= PRUNE_PACKED_VERBOSE;
- prune_packed_objects(opts);
+ prune_packed_objects(repo, opts);
if (!keep_unreachable &&
(!(pack_everything & LOOSEN_UNREACHABLE) ||
diff --git a/prune-packed.c b/prune-packed.c
index d49dc11957..a53b7031b5 100644
--- a/prune-packed.c
+++ b/prune-packed.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "gettext.h"
#include "object-file.h"
@@ -8,40 +6,50 @@
#include "prune-packed.h"
#include "repository.h"
+struct prune_data {
+ struct repository *r;
+ int opts;
+};
+
static struct progress *progress;
static int prune_subdir(unsigned int nr, const char *path, void *data)
{
- int *opts = data;
+ struct prune_data *d = data;
display_progress(progress, nr + 1);
- if (!(*opts & PRUNE_PACKED_DRY_RUN))
+ if (!(d->opts & PRUNE_PACKED_DRY_RUN))
rmdir(path);
return 0;
}
static int prune_object(const struct object_id *oid, const char *path,
- void *data)
+ void *data)
{
- int *opts = data;
+ struct prune_data *d = data;
- if (!has_object_pack(the_repository, oid))
+ if (!has_object_pack(d->r, oid))
return 0;
- if (*opts & PRUNE_PACKED_DRY_RUN)
+ if (d->opts & PRUNE_PACKED_DRY_RUN)
printf("rm -f %s\n", path);
else
unlink_or_warn(path);
return 0;
}
-void prune_packed_objects(int opts)
+void prune_packed_objects(struct repository *r, int opts)
{
+ struct prune_data cb_data = {
+ .r = r,
+ .opts = opts
+ };
+
if (opts & PRUNE_PACKED_VERBOSE)
- progress = start_delayed_progress(the_repository,
+ progress = start_delayed_progress(r,
_("Removing duplicate objects"), 256);
- for_each_loose_file_in_source(the_repository->objects->sources,
- prune_object, NULL, prune_subdir, &opts);
+ for_each_loose_file_in_source(r->objects->sources,
+ prune_object, NULL, prune_subdir, &cb_data);
/* Ensure we show 100% before finishing progress */
display_progress(progress, 256);
diff --git a/prune-packed.h b/prune-packed.h
index 936fa9df23..9ec7ff0d60 100644
--- a/prune-packed.h
+++ b/prune-packed.h
@@ -1,9 +1,11 @@
#ifndef PRUNE_PACKED_H
#define PRUNE_PACKED_H
+struct repository;
+
#define PRUNE_PACKED_DRY_RUN 01
#define PRUNE_PACKED_VERBOSE 02
-void prune_packed_objects(int);
+void prune_packed_objects(struct repository *r, int opts);
#endif
base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
--
gitgitgadget
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] prune-packed: remove USE_THE_REPOSITORY_VARIABLE
2026-10-18 16:31 [PATCH] prune-packed: remove USE_THE_REPOSITORY_VARIABLE Shashank S via GitGitGadget
@ 2026-10-19 7:09 ` Patrick Steinhardt
2026-10-19 16:38 ` Shashank S
2026-10-19 16:53 ` [PATCH v2] [Outreachy Patch] " Shashank S via GitGitGadget
1 sibling, 1 reply; 5+ messages in thread
From: Patrick Steinhardt @ 2026-10-19 7:09 UTC (permalink / raw)
To: Shashank S via GitGitGadget
Cc: git, Kaartic Sivaraam, Christian Couder, Siddharth Asthana,
Usman Akinyemi, Junio C Hamano, Shashank S
On Sun, Oct 18, 2026 at 04:31:12PM +0000, Shashank S via GitGitGadget wrote:
> From: Shashank <shashu8660@gmail.com>
>
> Right now, the prune_packed_objects() function uses the global
Nit: we typically refrain from saying "now", as it's implicitly clear
that you're describing the status quo in your commit message unless you
explicitly refer to an older commit.
> the_repository variable. To help with the libification effort,
> change this function so it takes a 'struct repository' pointer
> as an argument instead.
>
> The function for_each_loose_file_in_source() uses a callback
> that only takes a single 'void *data' pointer. This means we
> cannot pass both the repository pointer and the 'opts' integer
> at the same time. To fix this, create a new 'struct prune_data'
> to hold both of them together, and update the prune_subdir() and
> prune_object() callbacks to read from this new struct.
>
> Update the builtins that call this function (builtin/prune-packed.c,
> builtin/prune.c, and builtin/repack.c) to pass their local `repo`
> variables. Because we are now actually using the 'repo' parameter
> in these files, we can safely remove the UNUSED macro from them.
The question isn't really whether we can safely remove the UNUSED macro.
The question is rather whether we can assume that `repo` always points
to a proper repository. And for that you need to research whether these
commands can be invoked outside of a repository, because if so the repo
may be `NULL`.
> finally, drop the USE_THE_REPOSITORY_VARIABLE macro from
s/finally/Finally
> prune-packed.c.
>
> Signed-off-by: Shashank <shashu8660@gmail.com>
We typically ask contributors to spell out their full name, see
Documentation/SubmittingPatches [[real-name]].
The changes all look sensible to me, thanks!
Patrick
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] prune-packed: remove USE_THE_REPOSITORY_VARIABLE
2026-10-19 7:09 ` Patrick Steinhardt
@ 2026-10-19 16:38 ` Shashank S
2026-10-19 21:05 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: Shashank S @ 2026-10-19 16:38 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Shashank S via GitGitGadget, git, Kaartic Sivaraam,
Christian Couder, Siddharth Asthana, Usman Akinyemi,
Junio C Hamano
On Mon, 19 Oct 2026 at 12:39, Patrick Steinhardt <ps@pks.im> wrote:
>
> On Sun, Oct 18, 2026 at 04:31:12PM +0000, Shashank S via GitGitGadget wrote:
> > Right now, the prune_packed_objects() function uses the global
>
> Nit: we typically refrain from saying "now", as it's implicitly clear
> that you're describing the status quo in your commit message unless you
> explicitly refer to an older commit.
thank you for the review and point the mistakes i have done i will
correct all the mistake and i will submit the changed commit in the V2
patch.
> > in these files, we can safely remove the UNUSED macro from them.
>
> The question isn't really whether we can safely remove the UNUSED macro.
> The question is rather whether we can assume that `repo` always points
> to a proper repository. And for that you need to research whether these
> commands can be invoked outside of a repository, because if so the repo
> may be `NULL`.
>
I have checked git.c file in that file I went through this commands .
prune , prune-packed and repack which are .
{ "prune", cmd_prune, RUN_SETUP },
{ "prune-packed", cmd_prune_packed, RUN_SETUP },
{ "repack", cmd_repack, RUN_SETUP },
All this commands are all registered with a special flags '
RUN_SETUP'. This flags search for .git folder . If it finds a .git
folder Git successfully initializes the repo struct and passes that to
valid pointer to your builtin . If it does not find a .git folder (if
they ran it outside the repository) Git immediately stops and shows
the error . It never run the code .
> > finally, drop the USE_THE_REPOSITORY_VARIABLE macro from
>
> s/finally/Finally
i will change this too.
>
> > prune-packed.c.
> >
> > Signed-off-by: Shashank <shashu8660@gmail.com>
>
> We typically ask contributors to spell out their full name, see
> Documentation/SubmittingPatches [[real-name]].
>
> The changes all look sensible to me, thanks!
>
> Patrick
Thank you
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] prune-packed: remove USE_THE_REPOSITORY_VARIABLE
2026-10-19 16:38 ` Shashank S
@ 2026-10-19 21:05 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2026-10-19 21:05 UTC (permalink / raw)
To: Shashank S
Cc: Patrick Steinhardt, Shashank S via GitGitGadget, git,
Kaartic Sivaraam, Christian Couder, Siddharth Asthana,
Usman Akinyemi
Shashank S <shashu8660@gmail.com> writes:
>> The question isn't really whether we can safely remove the UNUSED macro.
>> The question is rather whether we can assume that `repo` always points
>> to a proper repository. And for that you need to research whether these
>> commands can be invoked outside of a repository, because if so the repo
>> may be `NULL`.
>
> I have checked git.c file in that file I went through this commands .
> prune , prune-packed and repack which are .
> { "prune", cmd_prune, RUN_SETUP },
> { "prune-packed", cmd_prune_packed, RUN_SETUP },
> { "repack", cmd_repack, RUN_SETUP },
> All this commands are all registered with a special flags '
> RUN_SETUP'. This flags search for .git folder . If it finds a .git
> folder Git successfully initializes the repo struct and passes that to
> valid pointer to your builtin . If it does not find a .git folder (if
> they ran it outside the repository) Git immediately stops and shows
> the error . It never run the code .
The analysis is not entirely correct. Assuming that your system
administrator is not insane enough and did not make the entire
filesystem as a working tree of a repository,
$ cd / && git repack -h
does call cmd_repack() with "repo" set to NULL.
The call to parse_options() will give the short help and exit.
Becaues the references to "repo" you added all happen after the code
already called parse_options(), your update will not make the code
dereference NULL. Some subcommands with tricky call sequence into
parse_options() _may_ have to be a bit more careful than just
checking "are we using RUN_SETUP?". Of course RUN_SETUP_GENTLY is
an explicit sign that you have to be prepared to see NULL in "repo".
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] [Outreachy Patch] prune-packed: remove USE_THE_REPOSITORY_VARIABLE
2026-10-18 16:31 [PATCH] prune-packed: remove USE_THE_REPOSITORY_VARIABLE Shashank S via GitGitGadget
2026-10-19 7:09 ` Patrick Steinhardt
@ 2026-10-19 16:53 ` Shashank S via GitGitGadget
1 sibling, 0 replies; 5+ messages in thread
From: Shashank S via GitGitGadget @ 2026-10-19 16:53 UTC (permalink / raw)
To: git
Cc: Kaartic Sivaraam, Christian Couder, Siddharth Asthana,
Usman Akinyemi, Junio C Hamano, Patrick Steinhardt, Shashank S,
Shashank S Mogaveera
From: Shashank S Mogaveera <shashu8660@gmail.com>
The prune_packed_objects() function uses the global
the_repository variable. To help with the libification effort,
change this function so it takes a 'struct repository' pointer
as an argument instead.
The function for_each_loose_file_in_source() uses a callback
that only takes a single 'void *data' pointer. This means we
cannot pass both the repository pointer and the 'opts' integer
at the same time. To fix this, create a new 'struct prune_data'
to hold both of them together, and update the prune_subdir() and
prune_object() callbacks to read from this new struct.
Update the builtins that call this function (builtin/prune-packed.c,
builtin/prune.c, and builtin/repack.c) to pass their local 'repo'
variables. We can safely remove the UNUSED macro from them because
these commands are all declared with the RUN_SETUP flag in git.c,
which guarantees that the 'repo' pointer will never be NULL.
Finally, drop the USE_THE_REPOSITORY_VARIABLE macro from
prune-packed.c.
Signed-off-by: Shashank S Mogaveera <shashu8660@gmail.com>
---
[Outreachy PATCH] prune-packed: remove use_the_repository_variable
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2266%2Fshashu8660%2Foutreachy2-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2266/shashu8660/outreachy2-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2266
Range-diff vs v1:
1: 5d9ba0cf3a ! 1: 6b583e1d46 prune-packed: remove USE_THE_REPOSITORY_VARIABLE
@@
## Metadata ##
-Author: Shashank <shashu8660@gmail.com>
+Author: Shashank S Mogaveera <shashu8660@gmail.com>
## Commit message ##
- prune-packed: remove USE_THE_REPOSITORY_VARIABLE
+ [Outreachy Patch] prune-packed: remove USE_THE_REPOSITORY_VARIABLE
- Right now, the prune_packed_objects() function uses the global
+ The prune_packed_objects() function uses the global
the_repository variable. To help with the libification effort,
change this function so it takes a 'struct repository' pointer
as an argument instead.
@@ Commit message
prune_object() callbacks to read from this new struct.
Update the builtins that call this function (builtin/prune-packed.c,
- builtin/prune.c, and builtin/repack.c) to pass their local `repo`
- variables. Because we are now actually using the 'repo' parameter
- in these files, we can safely remove the UNUSED macro from them.
+ builtin/prune.c, and builtin/repack.c) to pass their local 'repo'
+ variables. We can safely remove the UNUSED macro from them because
+ these commands are all declared with the RUN_SETUP flag in git.c,
+ which guarantees that the 'repo' pointer will never be NULL.
- finally, drop the USE_THE_REPOSITORY_VARIABLE macro from
+ Finally, drop the USE_THE_REPOSITORY_VARIABLE macro from
prune-packed.c.
- Signed-off-by: Shashank <shashu8660@gmail.com>
+ Signed-off-by: Shashank S Mogaveera <shashu8660@gmail.com>
## builtin/prune-packed.c ##
@@ builtin/prune-packed.c: static const char * const prune_packed_usage[] = {
builtin/prune-packed.c | 4 ++--
builtin/prune.c | 2 +-
builtin/repack.c | 2 +-
prune-packed.c | 32 ++++++++++++++++++++------------
prune-packed.h | 4 +++-
5 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index 4d63f26b0a..d44e7f81d4 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -11,7 +11,7 @@ static const char * const prune_packed_usage[] = {
int cmd_prune_packed(int argc,
const char **argv,
const char *prefix,
- struct repository *repo UNUSED)
+ struct repository *repo)
{
int opts = isatty(2) ? PRUNE_PACKED_VERBOSE : 0;
const struct option prune_packed_options[] = {
@@ -30,6 +30,6 @@ int cmd_prune_packed(int argc,
prune_packed_usage,
prune_packed_options);
- prune_packed_objects(opts);
+ prune_packed_objects(repo, opts);
return 0;
}
diff --git a/builtin/prune.c b/builtin/prune.c
index a7e4678d11..4eee5a2e27 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -201,7 +201,7 @@ int cmd_prune(int argc,
for_each_loose_file_in_source(repo->objects->sources,
prune_object, prune_cruft, prune_subdir, &revs);
- prune_packed_objects(show_only ? PRUNE_PACKED_DRY_RUN : 0);
+ prune_packed_objects(repo, show_only ? PRUNE_PACKED_DRY_RUN : 0);
remove_temporary_files(repo_get_object_directory(repo));
s = mkpathdup("%s/pack", repo_get_object_directory(repo));
remove_temporary_files(s);
diff --git a/builtin/repack.c b/builtin/repack.c
index c4360382c1..e8970e8eb7 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -757,7 +757,7 @@ int cmd_repack(int argc,
wrote_incremental_midx);
if (show_progress)
opts |= PRUNE_PACKED_VERBOSE;
- prune_packed_objects(opts);
+ prune_packed_objects(repo, opts);
if (!keep_unreachable &&
(!(pack_everything & LOOSEN_UNREACHABLE) ||
diff --git a/prune-packed.c b/prune-packed.c
index d49dc11957..a53b7031b5 100644
--- a/prune-packed.c
+++ b/prune-packed.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "gettext.h"
#include "object-file.h"
@@ -8,40 +6,50 @@
#include "prune-packed.h"
#include "repository.h"
+struct prune_data {
+ struct repository *r;
+ int opts;
+};
+
static struct progress *progress;
static int prune_subdir(unsigned int nr, const char *path, void *data)
{
- int *opts = data;
+ struct prune_data *d = data;
display_progress(progress, nr + 1);
- if (!(*opts & PRUNE_PACKED_DRY_RUN))
+ if (!(d->opts & PRUNE_PACKED_DRY_RUN))
rmdir(path);
return 0;
}
static int prune_object(const struct object_id *oid, const char *path,
- void *data)
+ void *data)
{
- int *opts = data;
+ struct prune_data *d = data;
- if (!has_object_pack(the_repository, oid))
+ if (!has_object_pack(d->r, oid))
return 0;
- if (*opts & PRUNE_PACKED_DRY_RUN)
+ if (d->opts & PRUNE_PACKED_DRY_RUN)
printf("rm -f %s\n", path);
else
unlink_or_warn(path);
return 0;
}
-void prune_packed_objects(int opts)
+void prune_packed_objects(struct repository *r, int opts)
{
+ struct prune_data cb_data = {
+ .r = r,
+ .opts = opts
+ };
+
if (opts & PRUNE_PACKED_VERBOSE)
- progress = start_delayed_progress(the_repository,
+ progress = start_delayed_progress(r,
_("Removing duplicate objects"), 256);
- for_each_loose_file_in_source(the_repository->objects->sources,
- prune_object, NULL, prune_subdir, &opts);
+ for_each_loose_file_in_source(r->objects->sources,
+ prune_object, NULL, prune_subdir, &cb_data);
/* Ensure we show 100% before finishing progress */
display_progress(progress, 256);
diff --git a/prune-packed.h b/prune-packed.h
index 936fa9df23..9ec7ff0d60 100644
--- a/prune-packed.h
+++ b/prune-packed.h
@@ -1,9 +1,11 @@
#ifndef PRUNE_PACKED_H
#define PRUNE_PACKED_H
+struct repository;
+
#define PRUNE_PACKED_DRY_RUN 01
#define PRUNE_PACKED_VERBOSE 02
-void prune_packed_objects(int);
+void prune_packed_objects(struct repository *r, int opts);
#endif
base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
--
gitgitgadget
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-19 21:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-18 16:31 [PATCH] prune-packed: remove USE_THE_REPOSITORY_VARIABLE Shashank S via GitGitGadget
2026-10-19 7:09 ` Patrick Steinhardt
2026-10-19 16:38 ` Shashank S
2026-10-19 21:05 ` Junio C Hamano
2026-10-19 16:53 ` [PATCH v2] [Outreachy Patch] " Shashank S via GitGitGadget
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.