Re: [GSoC][PATCH v4 5/9] builtin/pack-refs: factor out core logic into a shared library
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 24, 2025, 06:18 UTC
- Message-ID
- <aNONRY10f6R-3Il0@pks.im>
- In-Reply-To
- <20250919082647.535213-6-meetsoni3017@gmail.com>
On Fri, Sep 19, 2025 at 01:56:43PM +0530, Meet Soni wrote:
Show 10 quoted lines
> The implementation of `git pack-refs` is monolithic within > `cmd_pack_refs()`, making it impossible to share its logic with other > commands. To enable code reuse for the upcoming `git refs optimize` > subcommand, refactor the core logic into a shared helper function. > > Split the original `builtin/pack-refs.c` file into two parts: > > - A new shared library file, `pack-refs.c`, which contains the > core option parsing and packing logic in a new `pack_refs_core()` > helper function.
Should we maybe host that file in "refs/pack.c"? This ensures that all ref-related infra continues to sit in one place. We could also build on your previous steps and call it "refs/optimize.c" right away.
Show 15 quoted lines
> diff --git a/pack-refs.c b/pack-refs.c > new file mode 100644 > index 0000000000..1a5e07d8b8 > --- /dev/null > +++ b/pack-refs.c > @@ -0,0 +1,56 @@ > +#include "builtin.h" > +#include "config.h" > +#include "environment.h" > +#include "pack-refs.h" > +#include "parse-options.h" > +#include "refs.h" > +#include "revision.h" > + > +int pack_refs_core(int argc,
If we want to go with "refs/optimize.c" I'd call this `refs_optimize_core()`.
Show 17 quoted lines
> diff --git a/pack-refs.h b/pack-refs.h > new file mode 100644 > index 0000000000..5de27e7da8 > --- /dev/null > +++ b/pack-refs.h > @@ -0,0 +1,23 @@ > +#ifndef PACK_REFS_H > +#define PACK_REFS_H > + > +struct repository; > + > +/* > + * Shared usage string for options common to git-pack-refs(1) > + * and git-refs-optimize(1). The command-specific part (e.g., "git refs optimize ") > + * must be prepended by the caller. > + */ > +#define PACK_REFS_OPTS \
This would become `REFS_OPTIMIZE_OPTS`.
Show 12 quoted lines
> + "[--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]" > + > +/* > + * The core logic for pack-refs and its clones. > + */ > +int pack_refs_core(int argc, > + const char **argv, > + const char *prefix, > + struct repository *repo, > + const char * const *usage_opts); > + > +#endif /* PACK_REFS_H */
Patrick