Re: [PATCH 7/7] builtin/maintenance: fix auto-detach with non-standard tasks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Hi Patrick

On 13/08/2024 08:18, Patrick Steinhardt wrote:


Fix this bug by asking git-gc(1) to not detach when it is being invoked
via git-maintenance(1). Instead, the latter command now respects a new
config "maintenance.autoDetach", the equivalent of "gc.autoDetach", and
detaches itself into the background if not told otherwise. This should
continue to behave the same for all users which use the git-gc(1) task,
only. For others though, it means that we now properly perform all tasks
in the background.
I fear that users who are running "git maintenance" from a scheduler such as cron are likely to be surprised by this change in behavior. At the very least "git maintenance" will no-longer return a meaningful exit code. Perhaps we could switch the logic to be opt in and pass "--detach" (or "-c maintenance.autoDetach=true") when running "git maintenance" automatically from "git rebase" etc. 

Best Wishes

Phillip


Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
  builtin/gc.c             |  1 +
  run-command.c            | 12 ++++++++++-
  t/t5616-partial-clone.sh |  6 +++---
  t/t7900-maintenance.sh   | 43 +++++++++++++++++++++++++++++++---------
  4 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 63106e2028..bafee330a2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1063,6 +1063,7 @@ static int maintenance_task_gc(struct maintenance_run_opts *opts,
  		strvec_push(&child.args, "--quiet");
  	else
  		strvec_push(&child.args, "--no-quiet");
+	strvec_push(&child.args, "--no-detach");
return run_command(&child); 
  }
diff --git a/run-command.c b/run-command.c
index 45ba544932..94f2f3079f 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1808,16 +1808,26 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
int prepare_auto_maintenance(int quiet, struct child_process *maint) 
  {
-	int enabled;
+	int enabled, auto_detach;
if (!git_config_get_bool("maintenance.auto", &enabled) && 
  	    !enabled)
  		return 0;
+ /* 
+	 * When `maintenance.autoDetach` isn't set, then we fall back to
+	 * honoring `gc.autoDetach`. This is somewhat weird, but required to
+	 * retain behaviour from when we used to run git-gc(1) here.
+	 */
+	if (git_config_get_bool("maintenance.autodetach", &auto_detach) &&
+	    git_config_get_bool("gc.autodetach", &auto_detach))
+		auto_detach = 1;
+
  	maint->git_cmd = 1;
  	maint->close_object_store = 1;
  	strvec_pushl(&maint->args, "maintenance", "run", "--auto", NULL);
  	strvec_push(&maint->args, quiet ? "--quiet" : "--no-quiet");
+	strvec_push(&maint->args, auto_detach ? "--detach" : "--no-detach");
return 1; 
  }
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 2da7291e37..8415884754 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -229,7 +229,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
GIT_TRACE2_EVENT="$PWD/trace1.event" \ 
  	git -C pc1 fetch --refetch origin &&
-	test_subcommand git maintenance run --auto --no-quiet <trace1.event &&
+	test_subcommand git maintenance run --auto --no-quiet --detach <trace1.event &&
  	grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace1.event &&
  	grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace1.event &&
@@ -238,7 +238,7 @@ test_expect_success 'fetch --refetch triggers repacking' ' 
  		-c gc.autoPackLimit=0 \
  		-c maintenance.incremental-repack.auto=1234 \
  		-C pc1 fetch --refetch origin &&
-	test_subcommand git maintenance run --auto --no-quiet <trace2.event &&
+	test_subcommand git maintenance run --auto --no-quiet --detach <trace2.event &&
  	grep \"param\":\"gc.autopacklimit\",\"value\":\"0\" trace2.event &&
  	grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace2.event &&
@@ -247,7 +247,7 @@ test_expect_success 'fetch --refetch triggers repacking' ' 
  		-c gc.autoPackLimit=1234 \
  		-c maintenance.incremental-repack.auto=0 \
  		-C pc1 fetch --refetch origin &&
-	test_subcommand git maintenance run --auto --no-quiet <trace3.event &&
+	test_subcommand git maintenance run --auto --no-quiet --detach <trace3.event &&
  	grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace3.event &&
  	grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"0\" trace3.event
  '
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 771525aa4b..06ab43cfb5 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -49,22 +49,47 @@ test_expect_success 'run [--auto|--quiet]' '
  		git maintenance run --auto 2>/dev/null &&
  	GIT_TRACE2_EVENT="$(pwd)/run-no-quiet.txt" \
  		git maintenance run --no-quiet 2>/dev/null &&
-	test_subcommand git gc --quiet <run-no-auto.txt &&
-	test_subcommand ! git gc --auto --quiet <run-auto.txt &&
-	test_subcommand git gc --no-quiet <run-no-quiet.txt
+	test_subcommand git gc --quiet --no-detach <run-no-auto.txt &&
+	test_subcommand ! git gc --auto --quiet --no-detach <run-auto.txt &&
+	test_subcommand git gc --no-quiet --no-detach <run-no-quiet.txt
  '
test_expect_success 'maintenance.auto config option' ' 
  	GIT_TRACE2_EVENT="$(pwd)/default" git commit --quiet --allow-empty -m 1 &&
-	test_subcommand git maintenance run --auto --quiet <default &&
+	test_subcommand git maintenance run --auto --quiet --detach <default &&
  	GIT_TRACE2_EVENT="$(pwd)/true" \
  		git -c maintenance.auto=true \
  		commit --quiet --allow-empty -m 2 &&
-	test_subcommand git maintenance run --auto --quiet  <true &&
+	test_subcommand git maintenance run --auto --quiet --detach <true &&
  	GIT_TRACE2_EVENT="$(pwd)/false" \
  		git -c maintenance.auto=false \
  		commit --quiet --allow-empty -m 3 &&
-	test_subcommand ! git maintenance run --auto --quiet  <false
+	test_subcommand ! git maintenance run --auto --quiet --detach <false
+'
+
+for cfg in maintenance.autoDetach gc.autoDetach
+do
+	test_expect_success "$cfg=true config option" '
+		test_when_finished "rm -f trace" &&
+		test_config $cfg true &&
+		GIT_TRACE2_EVENT="$(pwd)/trace" git commit --quiet --allow-empty -m 1 &&
+		test_subcommand git maintenance run --auto --quiet --detach <trace
+	'
+
+	test_expect_success "$cfg=false config option" '
+		test_when_finished "rm -f trace" &&
+		test_config $cfg false &&
+		GIT_TRACE2_EVENT="$(pwd)/trace" git commit --quiet --allow-empty -m 1 &&
+		test_subcommand git maintenance run --auto --quiet --no-detach <trace
+	'
+done
+
+test_expect_success "maintenance.autoDetach overrides gc.autoDetach" '
+	test_when_finished "rm -f trace" &&
+	test_config maintenance.autoDetach false &&
+	test_config gc.autoDetach true &&
+	GIT_TRACE2_EVENT="$(pwd)/trace" git commit --quiet --allow-empty -m 1 &&
+	test_subcommand git maintenance run --auto --quiet --no-detach <trace
  '
test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' ' 
@@ -129,9 +154,9 @@ test_expect_success 'run --task=<task>' '
  		git maintenance run --task=commit-graph 2>/dev/null &&
  	GIT_TRACE2_EVENT="$(pwd)/run-both.txt" \
  		git maintenance run --task=commit-graph --task=gc 2>/dev/null &&
-	test_subcommand ! git gc --quiet <run-commit-graph.txt &&
-	test_subcommand git gc --quiet <run-gc.txt &&
-	test_subcommand git gc --quiet <run-both.txt &&
+	test_subcommand ! git gc --quiet --no-detach <run-commit-graph.txt &&
+	test_subcommand git gc --quiet --no-detach <run-gc.txt &&
+	test_subcommand git gc --quiet --no-detach <run-both.txt &&
  	test_subcommand git commit-graph write --split --reachable --no-progress <run-commit-graph.txt &&
  	test_subcommand ! git commit-graph write --split --reachable --no-progress <run-gc.txt &&
  	test_subcommand git commit-graph write --split --reachable --no-progress <run-both.txt
[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux