Re: Consider adding pruning of refs to git maintenance

From

Shubham Kanodia <shubham.kanodia10@gmail.com>

Date

Dec 17, 2024, 11:24 UTC

Message-ID

<CAG=Um+0yFYeBQGznkVG6TJeN-U+qySbt-0EbvM6Vd-BcvSCT4g@mail.gmail.com>

In-Reply-To

<CAG=Um+1NwB=ymwg+oM62f_W8G=3Gt14UFGe+S2MM3gTOdUcuHg@mail.gmail.com>

>
> On Tue, Dec 17, 2024 at 1:11 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > If we want to have such a feature I'd thus claim that it would be most
> > > sensible to make it opt-in rather than opt-out. I wouldn't want to be
> > > surprised by remote refs vanishing after going to bed, but may be okay
> > > with it when I explicitly ask for it.
> > >
> > > At that point one has to raise the question whether it is still all that
> > > useful compared to running `git remote prune` manually every now and
> > > then. Mostly because explicitly configuring maintenance is probably
> > > something that only power users would do, and those power users would
> > > likely know to prune manually.
> >
> > I am 100% in agreement with your reasoning.  If this thing is to
> > exist, it has to be opt-in.  We also need to add ample warning why
> > doing this asynchronously behind user's back while the user could be
> > working in the same repository is prone to cause surprises in the
> > documentation in big red letters.
> >
> > I however am OK with the idea of having it as an optional "task"
> > that is disabled by default in "git maintenance".  "git maintenance"
> > can be viewed as a platform-neutral way to set up scheduled tasks.
> >
> > A user may be a Git expert who knows what `git remote prune` does,
> > and understands and accepts the risk of confusion doing so without
> > explicit "do it now" from the end-user, but may not be a Linux or a
> > macOS or a Windows expert to know how to write crontab or its
> > equivalents on various schemes to define scheduled tasks.
> >
> > Thanks.
>
> Thanks for sharing your thoughts here. For some context — I currently
> look after git performance for a very large repository (60k+ refs)
> with a lot of active developers.
> I've observed that while power git users keep their repository tidy
> even without runnining maintenance, the majority of users infact don't
> know (or execute these), and
> run into performance issues. Easier fleet management was perhaps was
> one of the motivations behind adding this to `git-maintenance`.
>
> While its pretty rare that someone relies on a `refs/remote/*`
> reference (without creating a local copy of it in `refs/heads`), I can
> see why it can be surprising and an opt-in
> should be okay for me.
>
> Thanks,
> Shubham K

From aed935055aff819cce591eecbf6697f8e8e7b99e Mon Sep 17 00:00:00 2001
From: Shubham Kanodia <shubham.kanodia10@gmail.com>
Date: Tue, 17 Dec 2024 16:05:16 +0530
Subject: [PATCH] maintenance: add prune-remote-refs task

Signed-off-by: Shubham Kanodia <shubham.kanodia10@gmail.com>
---
 Documentation/git-maintenance.txt | 14 ++++++++++
 builtin/gc.c                      | 42 +++++++++++++++++++++++++++++
 t/t7900-maintenance.sh            | 44 +++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+)

diff --git a/Documentation/git-maintenance.txt
b/Documentation/git-maintenance.txt
index 6e6651309d..3e0ca84de2 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -158,6 +158,20 @@ pack-refs::
  need to iterate across many references. See linkgit:git-pack-refs[1]
  for more information.

+prune-remote-refs::
+ The `prune-remote-refs` task runs `git remote prune` on each remote
+ repository registered in the local repository. This task helps clean
+ up deleted remote branches, improving the performance of operations
+ that iterate through the refs. See linkgit:git-remote[1] for more
+ information. This task is disabled by default and must be enabled
+ manually.
++
+NOTE: This task is opt-in to prevent unexpected removal of remote refs
+for users of git-maintenance. It is recommended for power users who
+are comfortable with configuring maintenance tasks and understand the
+implications of automatic pruning. For others, running `git remote prune`
+manually might be more appropriate.
+
 OPTIONS
 -------
 --auto::
diff --git a/builtin/gc.c b/builtin/gc.c
index 4ae5196aed..9acf1d2989 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -20,6 +20,7 @@
 #include "lockfile.h"
 #include "parse-options.h"
 #include "run-command.h"
+#include "remote.h"
 #include "sigchain.h"
 #include "strvec.h"
 #include "commit.h"
@@ -913,6 +914,40 @@ static int maintenance_opt_schedule(const struct
option *opt, const char *arg,
  return 0;
 }

+static int collect_remote(struct remote *remote, void *cb_data)
+{
+ struct string_list *list = cb_data;
+
+ if (!remote->url.nr)
+ return 0;
+
+ string_list_append(list, remote->name);
+ return 0;
+}
+
+static int maintenance_task_prune_remote(struct maintenance_run_opts
*opts UNUSED,
+ struct gc_config *cfg UNUSED)
+{
+ struct string_list_item *item;
+ struct string_list remotes_list = STRING_LIST_INIT_NODUP;
+ struct child_process child = CHILD_PROCESS_INIT;
+ int result = 0;
+
+ for_each_remote(collect_remote, &remotes_list);
+
+ for_each_string_list_item (item, &remotes_list) {
+ const char *remote_name = item->string;
+ child.git_cmd = 1;
+ strvec_pushl(&child.args, "remote", "prune", remote_name, NULL);
+
+ if (run_command(&child))
+ result = error(_("failed to prune '%s'"), remote_name);
+ }
+
+ string_list_clear(&remotes_list, 0);
+ return result;
+}
+
 /* Remember to update object flag allocation in object.h */
 #define SEEN (1u<<0)

@@ -1375,6 +1410,7 @@ enum maintenance_task_label {
  TASK_GC,
  TASK_COMMIT_GRAPH,
  TASK_PACK_REFS,
+ TASK_PRUNE_REMOTE_REFS,

  /* Leave as final value */
  TASK__COUNT
@@ -1411,6 +1447,10 @@ static struct maintenance_task tasks[] = {
  maintenance_task_pack_refs,
  pack_refs_condition,
  },
+ [TASK_PRUNE_REMOTE_REFS] = {
+ "prune-remote-refs",
+ maintenance_task_prune_remote,
+ },
 };

 static int compare_tasks_by_selection(const void *a_, const void *b_)
@@ -1505,6 +1545,8 @@ static void initialize_maintenance_strategy(void)
  tasks[TASK_LOOSE_OBJECTS].schedule = SCHEDULE_DAILY;
  tasks[TASK_PACK_REFS].enabled = 1;
  tasks[TASK_PACK_REFS].schedule = SCHEDULE_WEEKLY;
+ tasks[TASK_PRUNE_REMOTE_REFS].enabled = 0;
+ tasks[TASK_PRUNE_REMOTE_REFS].schedule = SCHEDULE_DAILY;
  }
 }

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 0ce4ba1cbe..60a0c3f835 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -446,6 +446,50 @@ test_expect_success 'pack-refs task' '
  test_subcommand git pack-refs --all --prune <pack-refs.txt
 '

+test_expect_success 'prune-remote-refs task not enabled by default' '
+ git clone . prune-test &&
+ (
+ cd prune-test &&
+ GIT_TRACE2_EVENT="$(pwd)/prune.txt" git maintenance run 2>err &&
+ test_subcommand ! git remote prune origin <prune.txt
+ )
+'
+
+test_expect_success 'prune-remote-refs task cleans stale remote refs' '
+ test_commit initial &&
+
+ # Create two separate remote repos
+ git clone . remote1 &&
+ git clone . remote2 &&
+
+ git clone . prune-test-clean &&
+ (
+ cd prune-test-clean &&
+ git config maintenance.prune-remote-refs.enabled true &&
+
+ # Add both remotes
+ git remote add remote1 "../remote1" &&
+ git remote add remote2 "../remote2" &&
+
+ # Create and push branches to both remotes
+ git branch -f side2 HEAD &&
+ git push remote1 side2 &&
+ git push remote2 side2 &&
+
+ # Rename branches in each remote to simulate a stale branch
+ git -C ../remote1 branch -m side2 side3 &&
+ git -C ../remote2 branch -m side2 side4 &&
+
+ GIT_TRACE2_EVENT="$(pwd)/prune.txt" git maintenance run
--task=prune-remote-refs &&
+
+ # Verify pruning happened for both remotes
+ test_subcommand git remote prune remote1 <prune.txt &&
+ test_subcommand git remote prune remote2 <prune.txt &&
+ test_must_fail git rev-parse refs/remotes/remote1/side2 &&
+ test_must_fail git rev-parse refs/remotes/remote2/side2
+ )
+'
+
 test_expect_success '--auto and --schedule incompatible' '
  test_must_fail git maintenance run --auto --schedule=daily 2>err &&
  test_grep "at most one" err

-- 
2.46.2