[PATCH 0/2] Two cleanups around 'prefetch' refs

[PATCH 0/2] Two cleanups around 'prefetch' refs

[Derrick Stolee via GitGitGadget]

Here are a couple things that caught my eye during a recent evaluation of
the maintenance feature:

 1. 'refs/prefetch/' refs show up in 'git log' decorations. Auto-hide these.
 2. t7900-maintenance.sh had some scary warnings that end up being
    unimportant.

This is based on 'master' at 66e871b (The third batch, 2021-01-15).

Thanks, -Stolee

Derrick Stolee (2):
  maintenance: set log.excludeDecoration durin prefetch
  t7900: clean up some broken refs

 builtin/gc.c           |  6 ++++++
 t/t7900-maintenance.sh | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)


base-commit: 66e871b6647ffea61a77a0f82c7ef3415f1ee79c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-838%2Fderrickstolee%2Fprefetch-refs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-838/derrickstolee/prefetch-refs-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/838
-- 
gitgitgadget

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

[Derrick Stolee via GitGitGadget]

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

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' '
-- 
gitgitgadget

Re: [PATCH 1/2] maintenance: set log.excludeDecoration durin prefetch

[Taylor Blau]

Not related to the patch below, but in the subject line: s/durin/\0g/.

On Mon, Jan 18, 2021 at 03:23:35AM +0000, Derrick Stolee via GitGitGadget wrote:
> 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);
> +

OK; this is a good way to ensure that you're not constantly appending
'refs/prefetch' into the config.

I did notice that we have a 'remotes' string list just above, so I
suppose we could only ignore 'refs/prefetch/<remote>' for just the
remotes that we know about, but I doubt that this would be all that
useful. (I.e., are there really users that are using refs/prefetch
already and don't want to hide the parts of it that aren't managed by
maintenance? Doubtful.)

> +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

Great, this test matches what I would expect. Thank you!

Thanks,
Taylor

[PATCH 2/2] t7900: clean up some broken refs

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

The tests for the 'prefetch' task create remotes and fetch refs into
'refs/prefetch/<remote>/' and tags into 'refs/tags/'. These tests use
the remotes to create objects not intended to be seen by the "local"
repository.

In that sense, the incrmental-repack tasks did not have these objects
and refs in mind. That test replaces the object directory with a
specific pack-file layout for testing the batch-size logic. However,
this causes some operations to start showing warnings such as:

 error: refs/prefetch/remote1/one does not point to a valid object!
 error: refs/tags/one does not point to a valid object!

This only shows up if you run the tests verbosely and watch the output.
It caught my eye and I _thought_ that there was a bug where 'git gc' or
'git repack' wouldn't check 'refs/prefetch/' before pruning objects.
That is incorrect. Those commands do handle 'refs/prefetch/' correctly.

All that is left is to clean up the tests in t7900-maintenance.sh to
remove these tags and refs that are not being repacked for the
incremental-repack tests.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 t/t7900-maintenance.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index f9031cbb44b..6be9d42767a 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -256,6 +256,11 @@ test_expect_success 'incremental-repack task' '
 	HEAD
 	^HEAD~1
 	EOF
+
+	# Replace the object directory with this pack layout.
+	# However, it does not include all objects from the remotes.
+	rm -rf .git/refs/prefetch &&
+	rm -rf .git/refs/tags &&
 	rm -f $packDir/pack-* &&
 	rm -f $packDir/loose-* &&
 	ls $packDir/*.pack >packs-before &&
-- 
gitgitgadget

Re: [PATCH 2/2] t7900: clean up some broken refs

[Taylor Blau]

On Mon, Jan 18, 2021 at 03:23:36AM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@microsoft.com>
>
> The tests for the 'prefetch' task create remotes and fetch refs into
> 'refs/prefetch/<remote>/' and tags into 'refs/tags/'. These tests use
> the remotes to create objects not intended to be seen by the "local"
> repository.
>
> In that sense, the incrmental-repack tasks did not have these objects
> and refs in mind. That test replaces the object directory with a
> specific pack-file layout for testing the batch-size logic. However,
> this causes some operations to start showing warnings such as:
>
>  error: refs/prefetch/remote1/one does not point to a valid object!
>  error: refs/tags/one does not point to a valid object!
>
> This only shows up if you run the tests verbosely and watch the output.
> It caught my eye and I _thought_ that there was a bug where 'git gc' or
> 'git repack' wouldn't check 'refs/prefetch/' before pruning objects.
> That is incorrect. Those commands do handle 'refs/prefetch/' correctly.

Do you think it would be worth checking that 'does not point to a valid
object' doesn't appear in the output?

> All that is left is to clean up the tests in t7900-maintenance.sh to
> remove these tags and refs that are not being repacked for the
> incremental-repack tests.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  t/t7900-maintenance.sh | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index f9031cbb44b..6be9d42767a 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -256,6 +256,11 @@ test_expect_success 'incremental-repack task' '
>  	HEAD
>  	^HEAD~1
>  	EOF
> +
> +	# Replace the object directory with this pack layout.
> +	# However, it does not include all objects from the remotes.
> +	rm -rf .git/refs/prefetch &&
> +	rm -rf .git/refs/tags &&

Hmm. Makes sense, but this will certainly need to be updated to work
with reftables, and it would break if you ran 'git pack-refs'.

Perhaps instead:

  git for-each-ref --format='delete %(refname)' \
    refs/prefetch refs/tags >refs &&
  git update-ref --stdin <refs

?

Thanks,
Taylor

Re: [PATCH 2/2] t7900: clean up some broken refs

[Derrick Stolee]

On 1/18/2021 11:04 AM, Taylor Blau wrote:
> On Mon, Jan 18, 2021 at 03:23:36AM +0000, Derrick Stolee via GitGitGadget wrote:
>> This only shows up if you run the tests verbosely and watch the output.
>> It caught my eye and I _thought_ that there was a bug where 'git gc' or
>> 'git repack' wouldn't check 'refs/prefetch/' before pruning objects.
>> That is incorrect. Those commands do handle 'refs/prefetch/' correctly.
> 
> Do you think it would be worth checking that 'does not point to a valid
> object' doesn't appear in the output?

Perhaps, but I think this isn't an actually interesting behavior
to test. It's really the test that was broken, not Git itself.

>> All that is left is to clean up the tests in t7900-maintenance.sh to
>> remove these tags and refs that are not being repacked for the
>> incremental-repack tests.
>>
>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>> ---
>>  t/t7900-maintenance.sh | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
>> index f9031cbb44b..6be9d42767a 100755
>> --- a/t/t7900-maintenance.sh
>> +++ b/t/t7900-maintenance.sh
>> @@ -256,6 +256,11 @@ test_expect_success 'incremental-repack task' '
>>  	HEAD
>>  	^HEAD~1
>>  	EOF
>> +
>> +	# Replace the object directory with this pack layout.
>> +	# However, it does not include all objects from the remotes.
>> +	rm -rf .git/refs/prefetch &&
>> +	rm -rf .git/refs/tags &&
> 
> Hmm. Makes sense, but this will certainly need to be updated to work
> with reftables, and it would break if you ran 'git pack-refs'.
> 
> Perhaps instead:
> 
>   git for-each-ref --format='delete %(refname)' \
>     refs/prefetch refs/tags >refs &&
>   git update-ref --stdin <refs

Deleting refs in a clean way is probably good to do.
I'm guessing you didn't use a pipe because it can be
hard to diagnose a failure in the chain? That's
probably reasonable.

Thanks,
-Stolee

[PATCH v2 0/2] Two cleanups around 'prefetch' refs

[Derrick Stolee via GitGitGadget]

Here are a couple things that caught my eye during a recent evaluation of
the maintenance feature:

 1. 'refs/prefetch/' refs show up in 'git log' decorations. Auto-hide these.
 2. t7900-maintenance.sh had some scary warnings that end up being
    unimportant.

This is based on 'master' at 66e871b (The third batch, 2021-01-15).

Update in v2: deleting refs more safely for alternate ref backends. (Thanks,
Taylor!)

Thanks, -Stolee

Derrick Stolee (2):
  maintenance: set log.excludeDecoration durin prefetch
  t7900: clean up some broken refs

 builtin/gc.c           |  6 ++++++
 t/t7900-maintenance.sh | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 1 deletion(-)


base-commit: 66e871b6647ffea61a77a0f82c7ef3415f1ee79c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-838%2Fderrickstolee%2Fprefetch-refs-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-838/derrickstolee/prefetch-refs-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/838

Range-diff vs v1:

 1:  5b2ce9049a6 = 1:  5b2ce9049a6 maintenance: set log.excludeDecoration durin prefetch
 2:  616b73a6556 ! 2:  35038dfd037 t7900: clean up some broken refs
     @@ Commit message
      
          All that is left is to clean up the tests in t7900-maintenance.sh to
          remove these tags and refs that are not being repacked for the
     -    incremental-repack tests.
     +    incremental-repack tests. Use update-ref to ensure this works with all
     +    ref backends.
      
     +    Helped-by: Taylor Blau <me@ttaylorr.com>
          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
      
       ## t/t7900-maintenance.sh ##
     @@ t/t7900-maintenance.sh: test_expect_success 'incremental-repack task' '
       	^HEAD~1
       	EOF
      +
     ++	# Delete refs that have not been repacked in these packs.
     ++	git for-each-ref --format="delete %(refname)" \
     ++		refs/prefetch refs/tags >refs &&
     ++	git update-ref --stdin <refs &&
     ++
      +	# Replace the object directory with this pack layout.
     -+	# However, it does not include all objects from the remotes.
     -+	rm -rf .git/refs/prefetch &&
     -+	rm -rf .git/refs/tags &&
       	rm -f $packDir/pack-* &&
       	rm -f $packDir/loose-* &&
       	ls $packDir/*.pack >packs-before &&

-- 
gitgitgadget

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

[Derrick Stolee via GitGitGadget]

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

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' '
-- 
gitgitgadget

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

[Junio C Hamano]

"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.

[PATCH v2 2/2] t7900: clean up some broken refs

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

The tests for the 'prefetch' task create remotes and fetch refs into
'refs/prefetch/<remote>/' and tags into 'refs/tags/'. These tests use
the remotes to create objects not intended to be seen by the "local"
repository.

In that sense, the incrmental-repack tasks did not have these objects
and refs in mind. That test replaces the object directory with a
specific pack-file layout for testing the batch-size logic. However,
this causes some operations to start showing warnings such as:

 error: refs/prefetch/remote1/one does not point to a valid object!
 error: refs/tags/one does not point to a valid object!

This only shows up if you run the tests verbosely and watch the output.
It caught my eye and I _thought_ that there was a bug where 'git gc' or
'git repack' wouldn't check 'refs/prefetch/' before pruning objects.
That is incorrect. Those commands do handle 'refs/prefetch/' correctly.

All that is left is to clean up the tests in t7900-maintenance.sh to
remove these tags and refs that are not being repacked for the
incremental-repack tests. Use update-ref to ensure this works with all
ref backends.

Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 t/t7900-maintenance.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index f9031cbb44b..78ccf4b33f8 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -256,6 +256,13 @@ test_expect_success 'incremental-repack task' '
 	HEAD
 	^HEAD~1
 	EOF
+
+	# Delete refs that have not been repacked in these packs.
+	git for-each-ref --format="delete %(refname)" \
+		refs/prefetch refs/tags >refs &&
+	git update-ref --stdin <refs &&
+
+	# Replace the object directory with this pack layout.
 	rm -f $packDir/pack-* &&
 	rm -f $packDir/loose-* &&
 	ls $packDir/*.pack >packs-before &&
-- 
gitgitgadget

Re: [PATCH v2 2/2] t7900: clean up some broken refs

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> The tests for the 'prefetch' task create remotes and fetch refs into
> 'refs/prefetch/<remote>/' and tags into 'refs/tags/'. These tests use
> the remotes to create objects not intended to be seen by the "local"
> repository.
>
> In that sense, the incrmental-repack tasks did not have these objects
> and refs in mind. That test replaces the object directory with a
> specific pack-file layout for testing the batch-size logic. However,
> this causes some operations to start showing warnings such as:
>
>  error: refs/prefetch/remote1/one does not point to a valid object!
>  error: refs/tags/one does not point to a valid object!

So we use prefetch refs but craft a repacking strategy in such a way
that these prefetch refs do not act as anchors, deliberately
breaking the repository and invalidating the prefetch refs so to
speak?

Wow, that sounds like an evil and brittle test set-up.

> This only shows up if you run the tests verbosely and watch the output.
> It caught my eye and I _thought_ that there was a bug where 'git gc' or
> 'git repack' wouldn't check 'refs/prefetch/' before pruning objects.
> That is incorrect. Those commands do handle 'refs/prefetch/' correctly.

> All that is left is to clean up the tests in t7900-maintenance.sh to
> remove these tags and refs that are not being repacked for the
> incremental-repack tests. Use update-ref to ensure this works with all
> ref backends.
>
> Helped-by: Taylor Blau <me@ttaylorr.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  t/t7900-maintenance.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index f9031cbb44b..78ccf4b33f8 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -256,6 +256,13 @@ test_expect_success 'incremental-repack task' '
>  	HEAD
>  	^HEAD~1
>  	EOF
> +
> +	# Delete refs that have not been repacked in these packs.
> +	git for-each-ref --format="delete %(refname)" \
> +		refs/prefetch refs/tags >refs &&
> +	git update-ref --stdin <refs &&
> +
> +	# Replace the object directory with this pack layout.
>  	rm -f $packDir/pack-* &&
>  	rm -f $packDir/loose-* &&
>  	ls $packDir/*.pack >packs-before &&

Re: [PATCH v2 0/2] Two cleanups around 'prefetch' refs

[Taylor Blau]

On Tue, Jan 19, 2021 at 12:52:02PM +0000, Derrick Stolee via GitGitGadget wrote:
> Update in v2: deleting refs more safely for alternate ref backends. (Thanks,
> Taylor!)

The range-diff looks good to me, thanks!

  Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor

Re: [PATCH v2 0/2] Two cleanups around 'prefetch' refs

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> On Tue, Jan 19, 2021 at 12:52:02PM +0000, Derrick Stolee via GitGitGadget wrote:
>> Update in v2: deleting refs more safely for alternate ref backends. (Thanks,
>> Taylor!)
>
> The range-diff looks good to me, thanks!
>
>   Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks, both.