threads / patch / 61212

patchbuiltin/gc: Replace run_command with C function refs_pack_refs

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

## tl;dr

3 messages between Mar 28, 2024 and Mar 28, 2024. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

vk· Mar 28, 2024, 04:00 UTC · lore

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(-)

Show changes to builtin/gc.c +23 −6

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)

-- 
2.39.3 (Apple Git-145)

vk· Mar 28, 2024, 04:12 UTC · re: vk · lore

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

On 2024-03-28 12:00, vk wrote:

Show 31 quoted lines

> 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· Mar 28, 2024, 04:31 UTC · re: vk · lore

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

vk <g@vkabc.me> 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.