diff mbox series

[v2,1/2] maintenance: set log.excludeDecoration durin prefetch

Message ID 5b2ce9049a69d4c450093433e4fa15c4e5e0c412.1611060724.git.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 96eaffebbf3d00a498c28bc689e70d9d94bc9911
Headers show
Series Two cleanups around 'prefetch' refs | expand

Commit Message

Derrick Stolee Jan. 19, 2021, 12:52 p.m. UTC
From: Derrick Stolee <dstolee@microsoft.com>

The 'prefetch' task fetches refs from all remotes and places them in the
refs/prefetch/<remote>/ refspace. As this task is intended to run in the
background, this allows users to keep their local data very close to the
remote servers' data while not updating the users' understanding of the
remote refs in refs/remotes/<remote>/.

However, this can clutter 'git log' decorations with copies of the refs
with the full name 'refs/prefetch/<remote>/<branch>'.

The log.excludeDecoration config option was added in a6be5e67 (log: add
log.excludeDecoration config option, 2020-05-16) for exactly this
purpose.

Ensure we set this only for users that would benefit from it by
assigning it at the beginning of the prefetch task. Other alternatives
would be during 'git maintenance register' or 'git maintenance start',
but those might assign the config even when the prefetch task is
disabled by existing config. Further, users could run 'git maintenance
run --task=prefetch' using their own scripting or scheduling. This
provides the best coverage to automatically update the config when
valuable.

It is improbable, but possible, that users might want to run the
prefetch task _and_ see these refs in their log decorations. This seems
incredibly unlikely to me, but users can always opt-in on a
command-by-command basis using --decorate-refs=refs/prefetch/.

Test that this works in a few cases. In particular, ensure that our
assignment of log.excludeDecoration=refs/prefetch/ is additive to other
existing exclusions. Further, ensure we do not add multiple copies in
multiple runs.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/gc.c           |  6 ++++++
 t/t7900-maintenance.sh | 26 +++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

Comments

Junio C Hamano Jan. 21, 2021, 2:06 a.m. UTC | #1
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> The 'prefetch' task fetches refs from all remotes and places them in the
> refs/prefetch/<remote>/ refspace. As this task is intended to run in the
> background, this allows users to keep their local data very close to the
> remote servers' data while not updating the users' understanding of the
> remote refs in refs/remotes/<remote>/.
>
> However, this can clutter 'git log' decorations with copies of the refs
> with the full name 'refs/prefetch/<remote>/<branch>'.
>
> The log.excludeDecoration config option was added in a6be5e67 (log: add
> log.excludeDecoration config option, 2020-05-16) for exactly this
> purpose.
>
> Ensure we set this only for users that would benefit from it by
> assigning it at the beginning of the prefetch task. Other alternatives
> would be during 'git maintenance register' or 'git maintenance start',
> but those might assign the config even when the prefetch task is
> disabled by existing config. Further, users could run 'git maintenance
> run --task=prefetch' using their own scripting or scheduling. This
> provides the best coverage to automatically update the config when
> valuable.

OK.  I think those users who keep distance from "git maintenance"
are different story but all others cannot be using refs/prefetch/
hierarchy for purposes other than "git maintenance" dictates, so
"git maintenance [register|start]", or even when the user first runs
"git maintenance" for that matter, would be acceptable point to add
the configuration, but at the beginning of a prefetch task, we know
the hierarchy is being used for what "git maintenance" wants to do,
so it is a good place to do so.

But playing devil's advocate, I do not think throwing refs/prefetch
into a "hardcoded list of hierarchies that would never be used for
decoration purposes" would upset any end-users in practice.  The only
reason why I do not make such a suggestion is it is more work (such
a hardcoded reject list does not exist, if I recall correctly).

> It is improbable, but possible, that users might want to run the
> prefetch task _and_ see these refs in their log decorations. This seems
> incredibly unlikely to me, but users can always opt-in on a
> command-by-command basis using --decorate-refs=refs/prefetch/.

It is not a viable workaround to add "--decorate-refs=refs/prefetch"
that is a mouthful; configuration options are to reduce such typing,
not to force more of them.  But because I agree with that
"incredibly unlikely" assessment, I do not care.

Thanks.
diff mbox series

Patch

diff --git a/builtin/gc.c b/builtin/gc.c
index b315b2ad588..54bae7f0c4c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -897,6 +897,12 @@  static int maintenance_task_prefetch(struct maintenance_run_opts *opts)
 	struct string_list_item *item;
 	struct string_list remotes = STRING_LIST_INIT_DUP;
 
+	git_config_set_multivar_gently("log.excludedecoration",
+					"refs/prefetch/",
+					"refs/prefetch/",
+					CONFIG_FLAGS_FIXED_VALUE |
+					CONFIG_FLAGS_MULTI_REPLACE);
+
 	if (for_each_remote(append_remote, &remotes)) {
 		error(_("failed to fill remotes"));
 		result = 1;
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 1074009cc05..f9031cbb44b 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -149,7 +149,31 @@  test_expect_success 'prefetch multiple remotes' '
 	git log prefetch/remote2/two &&
 	git fetch --all &&
 	test_cmp_rev refs/remotes/remote1/one refs/prefetch/remote1/one &&
-	test_cmp_rev refs/remotes/remote2/two refs/prefetch/remote2/two
+	test_cmp_rev refs/remotes/remote2/two refs/prefetch/remote2/two &&
+
+	test_cmp_config refs/prefetch/ log.excludedecoration &&
+	git log --oneline --decorate --all >log &&
+	! grep "prefetch" log
+'
+
+test_expect_success 'prefetch and existing log.excludeDecoration values' '
+	git config --unset-all log.excludeDecoration &&
+	git config log.excludeDecoration refs/remotes/remote1/ &&
+	git maintenance run --task=prefetch &&
+
+	git config --get-all log.excludeDecoration >out &&
+	grep refs/remotes/remote1/ out &&
+	grep refs/prefetch/ out &&
+
+	git log --oneline --decorate --all >log &&
+	! grep "prefetch" log &&
+	! grep "remote1" log &&
+	grep "remote2" log &&
+
+	# a second run does not change the config
+	git maintenance run --task=prefetch &&
+	git log --oneline --decorate --all >log2 &&
+	test_cmp log log2
 '
 
 test_expect_success 'loose-objects task' '