diff mbox series

[v2] promisor-remote: add promisor.quiet configuration option

Message ID 20240524090937.2448229-1-tom@compton.nu (mailing list archive)
State Superseded
Headers show
Series [v2] promisor-remote: add promisor.quiet configuration option | expand

Commit Message

Tom Hughes May 24, 2024, 9:09 a.m. UTC
Add a configuration option to allow output from the promisor
fetching objects to be suppressed.

This allows us to stop commands like 'git blame' being swamped
with progress messages and gc notifications from the promisor
when used in a partial clone.

Signed-off-by: Tom Hughes <tom@compton.nu>
---
 Documentation/config.txt          |  2 ++
 Documentation/config/promisor.txt |  3 +++
 promisor-remote.c                 |  3 +++
 t/t0410-partial-clone.sh          | 37 +++++++++++++++++++++++++++++++
 4 files changed, 45 insertions(+)
 create mode 100644 Documentation/config/promisor.txt

Comments

Junio C Hamano May 24, 2024, 6:06 p.m. UTC | #1
Tom Hughes <tom@compton.nu> writes:

> +test_expect_success TTY 'promisor.quiet=false shows progress messages' '
> +	rm -rf server repo &&
> +	test_create_repo server &&
> +	test_commit -C server foo &&
> +	git -C server rm foo.t &&
> +	git -C server commit -m remove &&
> +	git -C server config uploadpack.allowanysha1inwant 1 &&
> +	git -C server config uploadpack.allowfilter 1 &&
> +
> +	git clone --filter=blob:none "file://$(pwd)/server" repo &&
> +	git -C repo config promisor.quiet "false" &&
> +
> +	test_terminal git -C repo cat-file -p foo:foo.t 2>err &&
> +
> +	# Ensure that progress messages are written
> +	grep "Receiving objects" err
> +'
> +
> +test_expect_success TTY 'promisor.quiet=true does not show progress messages' '
> +	rm -rf server repo &&
> +	test_create_repo server &&
> +	test_commit -C server foo &&
> +	git -C server rm foo.t &&
> +	git -C server commit -m remove &&
> +	git -C server config uploadpack.allowanysha1inwant 1 &&
> +	git -C server config uploadpack.allowfilter 1 &&
> +
> +	git clone --filter=blob:none "file://$(pwd)/server" repo &&
> +	git -C repo config promisor.quiet "true" &&
> +
> +	test_terminal git -C repo cat-file -p foo:foo.t 2>err &&
> +
> +	# Ensure that no progress messages are written
> +	! grep "Receiving objects" err
> +'

Wouldn't we want to see this as three (or four) tests,

 (1) "setup" that prepares the server end

 (2) "quiet=false" test that
     - makes a partial clone, 
     - sets .quiet to false, 
     - runs cat-file -p,
     - makes sure that the lazy fetching is chatty.

 (3) "quiet=true" test, which is the same as (2) except that it sets
     .quiet to true and expects the lazy fetching to be silent.

 (4) "quiet=unconfigured" test, which is the same as (2) except that
     it leaves .quiet unconfigured.

Other than that, looking very much nicer than the previous round
that manually mucked with internal implementation on the client end.

Thanks.
Tom Hughes May 25, 2024, 10:10 a.m. UTC | #2
On 24/05/2024 19:06, Junio C Hamano wrote:

> Wouldn't we want to see this as three (or four) tests,
> 
>   (1) "setup" that prepares the server end
> 
>   (2) "quiet=false" test that
>       - makes a partial clone,
>       - sets .quiet to false,
>       - runs cat-file -p,
>       - makes sure that the lazy fetching is chatty.
> 
>   (3) "quiet=true" test, which is the same as (2) except that it sets
>       .quiet to true and expects the lazy fetching to be silent.
> 
>   (4) "quiet=unconfigured" test, which is the same as (2) except that
>       it leaves .quiet unconfigured.

Sounds good to me. I've sent a v3 with those changes.

Tom
diff mbox series

Patch

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 70b448b132..6cae835db9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -487,6 +487,8 @@  include::config/pager.txt[]
 
 include::config/pretty.txt[]
 
+include::config/promisor.txt[]
+
 include::config/protocol.txt[]
 
 include::config/pull.txt[]
diff --git a/Documentation/config/promisor.txt b/Documentation/config/promisor.txt
new file mode 100644
index 0000000000..98c5cb2ec2
--- /dev/null
+++ b/Documentation/config/promisor.txt
@@ -0,0 +1,3 @@ 
+promisor.quiet::
+	If set to "true" assume `--quiet` when fetching additional
+	objects for a partial clone.
diff --git a/promisor-remote.c b/promisor-remote.c
index b414922c44..2ca7c2ae48 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -23,6 +23,7 @@  static int fetch_objects(struct repository *repo,
 	struct child_process child = CHILD_PROCESS_INIT;
 	int i;
 	FILE *child_in;
+	int quiet;
 
 	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0)) {
 		static int warning_shown;
@@ -41,6 +42,8 @@  static int fetch_objects(struct repository *repo,
 		     "fetch", remote_name, "--no-tags",
 		     "--no-write-fetch-head", "--recurse-submodules=no",
 		     "--filter=blob:none", "--stdin", NULL);
+	if (!git_config_get_bool("promisor.quiet", &quiet) && quiet)
+		strvec_push(&child.args, "--quiet");
 	if (start_command(&child))
 		die(_("promisor-remote: unable to fork off fetch subprocess"));
 	child_in = xfdopen(child.in, "w");
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 88a66f0904..2957160efa 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -3,6 +3,7 @@ 
 test_description='partial clone'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
 
 # missing promisor objects cause repacks which write bitmaps to fail
 GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0
@@ -689,6 +690,42 @@  test_expect_success 'lazy-fetch when accessing object not in the_repository' '
 	! grep "[?]$FILE_HASH" out
 '
 
+test_expect_success TTY 'promisor.quiet=false shows progress messages' '
+	rm -rf server repo &&
+	test_create_repo server &&
+	test_commit -C server foo &&
+	git -C server rm foo.t &&
+	git -C server commit -m remove &&
+	git -C server config uploadpack.allowanysha1inwant 1 &&
+	git -C server config uploadpack.allowfilter 1 &&
+
+	git clone --filter=blob:none "file://$(pwd)/server" repo &&
+	git -C repo config promisor.quiet "false" &&
+
+	test_terminal git -C repo cat-file -p foo:foo.t 2>err &&
+
+	# Ensure that progress messages are written
+	grep "Receiving objects" err
+'
+
+test_expect_success TTY 'promisor.quiet=true does not show progress messages' '
+	rm -rf server repo &&
+	test_create_repo server &&
+	test_commit -C server foo &&
+	git -C server rm foo.t &&
+	git -C server commit -m remove &&
+	git -C server config uploadpack.allowanysha1inwant 1 &&
+	git -C server config uploadpack.allowfilter 1 &&
+
+	git clone --filter=blob:none "file://$(pwd)/server" repo &&
+	git -C repo config promisor.quiet "true" &&
+
+	test_terminal git -C repo cat-file -p foo:foo.t 2>err &&
+
+	# Ensure that no progress messages are written
+	! grep "Receiving objects" err
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd