[GSoC] builtin/gc: Replace run_command with C function refs_pack_refs

Commit Message

vk March 28, 2024, 4 a.m. UTC

Replace "git pack-refs" with C function refs_pack_refs
for maintenance_task_pack_refs(). C function refs_pack_refs
can be called directly, hence saving an external process.
---
 builtin/gc.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

Comments

vk March 28, 2024, 4:12 a.m. UTC | #1

On 2024-03-28 12:00, vk wrote:
> Replace "git pack-refs" with C function refs_pack_refs
> for maintenance_task_pack_refs(). C function refs_pack_refs
> can be called directly, hence saving an external process.
> ---
>  builtin/gc.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 3c874b248b..b55c5f5225 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -42,6 +42,8 @@
>  #include "setup.h"
>  #include "trace2.h"
> 
> +#include <revision.h>
> +
>  #define FAILED_RUN "failed to run %s"
> 
>  static const char * const builtin_gc_usage[] = {
> @@ -218,14 +220,29 @@ static int pack_refs_condition(void)
> 
>  static int maintenance_task_pack_refs(MAYBE_UNUSED struct 
> maintenance_run_opts *opts)
>  {
> -	struct child_process cmd = CHILD_PROCESS_INIT;
> 
> -	cmd.git_cmd = 1;
> -	strvec_pushl(&cmd.args, "pack-refs", "--all", "--prune", NULL);
> -	if (opts->auto_flag)
> -		strvec_push(&cmd.args, "--auto");

I forgot to add this --auto optional logic. Will add it in the following 
patch.

Thanks

Junio C Hamano March 28, 2024, 4:31 a.m. UTC | #2

vk <[e-mail]> writes:

> I forgot to add this --auto optional logic. Will add it in the
> following patch.

Please slow down a bit.  If you notice a problem in the patch you
just posted within 7 minutes after posting it, there is a good
chance that you are rushing it without carefully proofreading.

You'd be better off if you give yourself some time to think about
what you wrote, after drafting your patch.  Sleep on it, if needed.

There are issues in the patch.  Trying to spot them yourself would
give you a good exercise.  Documentation/SubmittingPatches and
Documentation/CodingGuidelines would be useful.

Thanks for your interest in Git development.

Patch

diff --git a/builtin/gc.c b/builtin/gc.c
index 3c874b248b..b55c5f5225 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -42,6 +42,8 @@ 
 #include "setup.h"
 #include "trace2.h"
 
+#include <revision.h>
+
 #define FAILED_RUN "failed to run %s"
 
 static const char * const builtin_gc_usage[] = {
@@ -218,14 +220,29 @@  static int pack_refs_condition(void)
 
 static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts)
 {
-	struct child_process cmd = CHILD_PROCESS_INIT;
 
-	cmd.git_cmd = 1;
-	strvec_pushl(&cmd.args, "pack-refs", "--all", "--prune", NULL);
-	if (opts->auto_flag)
-		strvec_push(&cmd.args, "--auto");
+	//following coding convention in pack-refs.c
+	int ret;
+
+	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+	struct string_list included_refs = STRING_LIST_INIT_NODUP;
+
+	//default flag is to prune
+	struct pack_refs_opts pack_refs_opts = {
+		.exclusions = &excludes,
+		.includes = &included_refs,
+		.flags = PACK_REFS_PRUNE,
+	};
+
+
+	//--all flag
+	string_list_append(pack_refs_opts.includes, "*");
+
+	ret = refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
+
+	//same as run_command return since ret is directly returned in cmd_pack_refs
+	return ret;
 
-	return run_command(&cmd);
 }
 
 static int too_many_loose_objects(void)