{"thread":{"id":"55000","subject":"[PATCH 0/2] Two cleanups around 'prefetch' refs","startedAt":"2021-01-18T03:24:32Z","lastAt":"2021-01-21T05:14:28Z","messageCount":13,"participants":["Derrick Stolee via GitGitGadget","Taylor Blau","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"414575","messageId":"pull.838.git.1610940216.gitgitgadget@gmail.com","threadId":"55000","inReplyTo":null,"subject":"[PATCH 0/2] Two cleanups around 'prefetch' refs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-18T03:23:34Z","receivedAt":"2021-01-18T03:24:32Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Here are a couple things that caught my eye during a recent evaluation of\nthe maintenance feature:\n\n 1. 'refs/prefetch/' refs show up in 'git log' decorations. Auto-hide these.\n 2. t7900-maintenance.sh had some scary warnings that end up being\n unimportant.\n\nThis is based on 'master' at 66e871b (The third batch, 2021-01-15).\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n maintenance: set log.excludeDecoration durin prefetch\n t7900: clean up some broken refs\n\n builtin/gc.c | 6 ++++++\n t/t7900-maintenance.sh | 31 ++++++++++++++++++++++++++++++-\n 2 files changed, 36 insertions(+), 1 deletion(-)\n\n\nbase-commit: 66e871b6647ffea61a77a0f82c7ef3415f1ee79c\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-838%2Fderrickstolee%2Fprefetch-refs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-838/derrickstolee/prefetch-refs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/838\n-- \ngitgitgadget\n"},{"id":"414576","messageId":"5b2ce9049a69d4c450093433e4fa15c4e5e0c412.1610940216.git.gitgitgadget@gmail.com","threadId":"55000","inReplyTo":"pull.838.git.1610940216.gitgitgadget@gmail.com","subject":"[PATCH 1/2] maintenance: set log.excludeDecoration durin prefetch","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-18T03:23:35Z","receivedAt":"2021-01-18T03:24:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'prefetch' task fetches refs from all remotes and places them in the\nrefs/prefetch// refspace. As this task is intended to run in the\nbackground, this allows users to keep their local data very close to the\nremote servers' data while not updating the users' understanding of the\nremote refs in refs/remotes//.\n\nHowever, this can clutter 'git log' decorations with copies of the refs\nwith the full name 'refs/prefetch//'.\n\nThe log.excludeDecoration config option was added in a6be5e67 (log: add\nlog.excludeDecoration config option, 2020-05-16) for exactly this\npurpose.\n\nEnsure we set this only for users that would benefit from it by\nassigning it at the beginning of the prefetch task. Other alternatives\nwould be during 'git maintenance register' or 'git maintenance start',\nbut those might assign the config even when the prefetch task is\ndisabled by existing config. Further, users could run 'git maintenance\nrun --task=prefetch' using their own scripting or scheduling. This\nprovides the best coverage to automatically update the config when\nvaluable.\n\nIt is improbable, but possible, that users might want to run the\nprefetch task _and_ see these refs in their log decorations. This seems\nincredibly unlikely to me, but users can always opt-in on a\ncommand-by-command basis using --decorate-refs=refs/prefetch/.\n\nTest that this works in a few cases. In particular, ensure that our\nassignment of log.excludeDecoration=refs/prefetch/ is additive to other\nexisting exclusions. Further, ensure we do not add multiple copies in\nmultiple runs.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/gc.c | 6 ++++++\n t/t7900-maintenance.sh | 26 +++++++++++++++++++++++++-\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex b315b2ad588..54bae7f0c4c 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -897,6 +897,12 @@ static int maintenance_task_prefetch(struct maintenance_run_opts *opts)\n \tstruct string_list_item *item;\n \tstruct string_list remotes = STRING_LIST_INIT_DUP;\n \n+\tgit_config_set_multivar_gently(\"log.excludedecoration\",\n+\t\t\t\t\t\"refs/prefetch/\",\n+\t\t\t\t\t\"refs/prefetch/\",\n+\t\t\t\t\tCONFIG_FLAGS_FIXED_VALUE |\n+\t\t\t\t\tCONFIG_FLAGS_MULTI_REPLACE);\n+\n \tif (for_each_remote(append_remote, &remotes)) {\n \t\terror(_(\"failed to fill remotes\"));\n \t\tresult = 1;\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 1074009cc05..f9031cbb44b 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -149,7 +149,31 @@ test_expect_success 'prefetch multiple remotes' '\n \tgit log prefetch/remote2/two &&\n \tgit fetch --all &&\n \ttest_cmp_rev refs/remotes/remote1/one refs/prefetch/remote1/one &&\n-\ttest_cmp_rev refs/remotes/remote2/two refs/prefetch/remote2/two\n+\ttest_cmp_rev refs/remotes/remote2/two refs/prefetch/remote2/two &&\n+\n+\ttest_cmp_config refs/prefetch/ log.excludedecoration &&\n+\tgit log --oneline --decorate --all >log &&\n+\t! grep \"prefetch\" log\n+'\n+\n+test_expect_success 'prefetch and existing log.excludeDecoration values' '\n+\tgit config --unset-all log.excludeDecoration &&\n+\tgit config log.excludeDecoration refs/remotes/remote1/ &&\n+\tgit maintenance run --task=prefetch &&\n+\n+\tgit config --get-all log.excludeDecoration >out &&\n+\tgrep refs/remotes/remote1/ out &&\n+\tgrep refs/prefetch/ out &&\n+\n+\tgit log --oneline --decorate --all >log &&\n+\t! grep \"prefetch\" log &&\n+\t! grep \"remote1\" log &&\n+\tgrep \"remote2\" log &&\n+\n+\t# a second run does not change the config\n+\tgit maintenance run --task=prefetch &&\n+\tgit log --oneline --decorate --all >log2 &&\n+\ttest_cmp log log2\n '\n \n test_expect_success 'loose-objects task' '\n-- \ngitgitgadget\n\n"},{"id":"414577","messageId":"616b73a6556824fb94753cfcc62bf01d36b8b311.1610940216.git.gitgitgadget@gmail.com","threadId":"55000","inReplyTo":"pull.838.git.1610940216.gitgitgadget@gmail.com","subject":"[PATCH 2/2] t7900: clean up some broken refs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-18T03:23:36Z","receivedAt":"2021-01-18T03:24:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe tests for the 'prefetch' task create remotes and fetch refs into\n'refs/prefetch//' and tags into 'refs/tags/'. These tests use\nthe remotes to create objects not intended to be seen by the \"local\"\nrepository.\n\nIn that sense, the incrmental-repack tasks did not have these objects\nand refs in mind. That test replaces the object directory with a\nspecific pack-file layout for testing the batch-size logic. However,\nthis causes some operations to start showing warnings such as:\n\n error: refs/prefetch/remote1/one does not point to a valid object!\n error: refs/tags/one does not point to a valid object!\n\nThis only shows up if you run the tests verbosely and watch the output.\nIt caught my eye and I _thought_ that there was a bug where 'git gc' or\n'git repack' wouldn't check 'refs/prefetch/' before pruning objects.\nThat is incorrect. Those commands do handle 'refs/prefetch/' correctly.\n\nAll that is left is to clean up the tests in t7900-maintenance.sh to\nremove these tags and refs that are not being repacked for the\nincremental-repack tests.\n\nSigned-off-by: Derrick Stolee \n---\n t/t7900-maintenance.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex f9031cbb44b..6be9d42767a 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -256,6 +256,11 @@ test_expect_success 'incremental-repack task' '\n \tHEAD\n \t^HEAD~1\n \tEOF\n+\n+\t# Replace the object directory with this pack layout.\n+\t# However, it does not include all objects from the remotes.\n+\trm -rf .git/refs/prefetch &&\n+\trm -rf .git/refs/tags &&\n \trm -f $packDir/pack-* &&\n \trm -f $packDir/loose-* &&\n \tls $packDir/*.pack >packs-before &&\n-- \ngitgitgadget\n"},{"id":"414596","messageId":"YAWv+5HDxEvBK+8R@nand.local","threadId":"55000","inReplyTo":"5b2ce9049a69d4c450093433e4fa15c4e5e0c412.1610940216.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] maintenance: set log.excludeDecoration durin prefetch","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-01-18T15:57:47Z","receivedAt":"2021-01-18T15:59:53Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Not related to the patch below, but in the subject line: s/durin/\\0g/.\n\nOn Mon, Jan 18, 2021 at 03:23:35AM +0000, Derrick Stolee via GitGitGadget wrote:\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index b315b2ad588..54bae7f0c4c 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -897,6 +897,12 @@ static int maintenance_task_prefetch(struct maintenance_run_opts *opts)\n> \tstruct string_list_item *item;\n> \tstruct string_list remotes = STRING_LIST_INIT_DUP;\n>\n> +\tgit_config_set_multivar_gently(\"log.excludedecoration\",\n> +\t\t\t\t\t\"refs/prefetch/\",\n> +\t\t\t\t\t\"refs/prefetch/\",\n> +\t\t\t\t\tCONFIG_FLAGS_FIXED_VALUE |\n> +\t\t\t\t\tCONFIG_FLAGS_MULTI_REPLACE);\n> +\n\nOK; this is a good way to ensure that you're not constantly appending\n'refs/prefetch' into the config.\n\nI did notice that we have a 'remotes' string list just above, so I\nsuppose we could only ignore 'refs/prefetch/' for just the\nremotes that we know about, but I doubt that this would be all that\nuseful. (I.e., are there really users that are using refs/prefetch\nalready and don't want to hide the parts of it that aren't managed by\nmaintenance? Doubtful.)\n\n> +test_expect_success 'prefetch and existing log.excludeDecoration values' '\n> +\tgit config --unset-all log.excludeDecoration &&\n> +\tgit config log.excludeDecoration refs/remotes/remote1/ &&\n> +\tgit maintenance run --task=prefetch &&\n> +\n> +\tgit config --get-all log.excludeDecoration >out &&\n> +\tgrep refs/remotes/remote1/ out &&\n> +\tgrep refs/prefetch/ out &&\n> +\n> +\tgit log --oneline --decorate --all >log &&\n> +\t! grep \"prefetch\" log &&\n> +\t! grep \"remote1\" log &&\n> +\tgrep \"remote2\" log &&\n> +\n> +\t# a second run does not change the config\n> +\tgit maintenance run --task=prefetch &&\n> +\tgit log --oneline --decorate --all >log2 &&\n> +\ttest_cmp log log2\n\nGreat, this test matches what I would expect. Thank you!\n\nThanks,\nTaylor\n"},{"id":"414597","messageId":"YAWxh8O/XlxIfcv3@nand.local","threadId":"55000","inReplyTo":"616b73a6556824fb94753cfcc62bf01d36b8b311.1610940216.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] t7900: clean up some broken refs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-01-18T16:04:23Z","receivedAt":"2021-01-18T16:10:13Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Jan 18, 2021 at 03:23:36AM +0000, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n>\n> The tests for the 'prefetch' task create remotes and fetch refs into\n> 'refs/prefetch//' and tags into 'refs/tags/'. These tests use\n> the remotes to create objects not intended to be seen by the \"local\"\n> repository.\n>\n> In that sense, the incrmental-repack tasks did not have these objects\n> and refs in mind. That test replaces the object directory with a\n> specific pack-file layout for testing the batch-size logic. However,\n> this causes some operations to start showing warnings such as:\n>\n> error: refs/prefetch/remote1/one does not point to a valid object!\n> error: refs/tags/one does not point to a valid object!\n>\n> This only shows up if you run the tests verbosely and watch the output.\n> It caught my eye and I _thought_ that there was a bug where 'git gc' or\n> 'git repack' wouldn't check 'refs/prefetch/' before pruning objects.\n> That is incorrect. Those commands do handle 'refs/prefetch/' correctly.\n\nDo you think it would be worth checking that 'does not point to a valid\nobject' doesn't appear in the output?\n\n> All that is left is to clean up the tests in t7900-maintenance.sh to\n> remove these tags and refs that are not being repacked for the\n> incremental-repack tests.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> t/t7900-maintenance.sh | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> index f9031cbb44b..6be9d42767a 100755\n> --- a/t/t7900-maintenance.sh\n> +++ b/t/t7900-maintenance.sh\n> @@ -256,6 +256,11 @@ test_expect_success 'incremental-repack task' '\n> \tHEAD\n> \t^HEAD~1\n> \tEOF\n> +\n> +\t# Replace the object directory with this pack layout.\n> +\t# However, it does not include all objects from the remotes.\n> +\trm -rf .git/refs/prefetch &&\n> +\trm -rf .git/refs/tags &&\n\nHmm. Makes sense, but this will certainly need to be updated to work\nwith reftables, and it would break if you ran 'git pack-refs'.\n\nPerhaps instead:\n\n git for-each-ref --format='delete %(refname)' \\\n refs/prefetch refs/tags >refs &&\n git update-ref --stdin On Mon, Jan 18, 2021 at 03:23:36AM +0000, Derrick Stolee via GitGitGadget wrote:\n>> This only shows up if you run the tests verbosely and watch the output.\n>> It caught my eye and I _thought_ that there was a bug where 'git gc' or\n>> 'git repack' wouldn't check 'refs/prefetch/' before pruning objects.\n>> That is incorrect. Those commands do handle 'refs/prefetch/' correctly.\n> \n> Do you think it would be worth checking that 'does not point to a valid\n> object' doesn't appear in the output?\n\nPerhaps, but I think this isn't an actually interesting behavior\nto test. It's really the test that was broken, not Git itself.\n\n>> All that is left is to clean up the tests in t7900-maintenance.sh to\n>> remove these tags and refs that are not being repacked for the\n>> incremental-repack tests.\n>>\n>> Signed-off-by: Derrick Stolee \n>> ---\n>> t/t7900-maintenance.sh | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>>\n>> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n>> index f9031cbb44b..6be9d42767a 100755\n>> --- a/t/t7900-maintenance.sh\n>> +++ b/t/t7900-maintenance.sh\n>> @@ -256,6 +256,11 @@ test_expect_success 'incremental-repack task' '\n>> \tHEAD\n>> \t^HEAD~1\n>> \tEOF\n>> +\n>> +\t# Replace the object directory with this pack layout.\n>> +\t# However, it does not include all objects from the remotes.\n>> +\trm -rf .git/refs/prefetch &&\n>> +\trm -rf .git/refs/tags &&\n> \n> Hmm. Makes sense, but this will certainly need to be updated to work\n> with reftables, and it would break if you ran 'git pack-refs'.\n> \n> Perhaps instead:\n> \n> git for-each-ref --format='delete %(refname)' \\\n> refs/prefetch refs/tags >refs &&\n> git update-ref --stdin \n\nThe 'prefetch' task fetches refs from all remotes and places them in the\nrefs/prefetch// refspace. As this task is intended to run in the\nbackground, this allows users to keep their local data very close to the\nremote servers' data while not updating the users' understanding of the\nremote refs in refs/remotes//.\n\nHowever, this can clutter 'git log' decorations with copies of the refs\nwith the full name 'refs/prefetch//'.\n\nThe log.excludeDecoration config option was added in a6be5e67 (log: add\nlog.excludeDecoration config option, 2020-05-16) for exactly this\npurpose.\n\nEnsure we set this only for users that would benefit from it by\nassigning it at the beginning of the prefetch task. Other alternatives\nwould be during 'git maintenance register' or 'git maintenance start',\nbut those might assign the config even when the prefetch task is\ndisabled by existing config. Further, users could run 'git maintenance\nrun --task=prefetch' using their own scripting or scheduling. This\nprovides the best coverage to automatically update the config when\nvaluable.\n\nIt is improbable, but possible, that users might want to run the\nprefetch task _and_ see these refs in their log decorations. This seems\nincredibly unlikely to me, but users can always opt-in on a\ncommand-by-command basis using --decorate-refs=refs/prefetch/.\n\nTest that this works in a few cases. In particular, ensure that our\nassignment of log.excludeDecoration=refs/prefetch/ is additive to other\nexisting exclusions. Further, ensure we do not add multiple copies in\nmultiple runs.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/gc.c | 6 ++++++\n t/t7900-maintenance.sh | 26 +++++++++++++++++++++++++-\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex b315b2ad588..54bae7f0c4c 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -897,6 +897,12 @@ static int maintenance_task_prefetch(struct maintenance_run_opts *opts)\n \tstruct string_list_item *item;\n \tstruct string_list remotes = STRING_LIST_INIT_DUP;\n \n+\tgit_config_set_multivar_gently(\"log.excludedecoration\",\n+\t\t\t\t\t\"refs/prefetch/\",\n+\t\t\t\t\t\"refs/prefetch/\",\n+\t\t\t\t\tCONFIG_FLAGS_FIXED_VALUE |\n+\t\t\t\t\tCONFIG_FLAGS_MULTI_REPLACE);\n+\n \tif (for_each_remote(append_remote, &remotes)) {\n \t\terror(_(\"failed to fill remotes\"));\n \t\tresult = 1;\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 1074009cc05..f9031cbb44b 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -149,7 +149,31 @@ test_expect_success 'prefetch multiple remotes' '\n \tgit log prefetch/remote2/two &&\n \tgit fetch --all &&\n \ttest_cmp_rev refs/remotes/remote1/one refs/prefetch/remote1/one &&\n-\ttest_cmp_rev refs/remotes/remote2/two refs/prefetch/remote2/two\n+\ttest_cmp_rev refs/remotes/remote2/two refs/prefetch/remote2/two &&\n+\n+\ttest_cmp_config refs/prefetch/ log.excludedecoration &&\n+\tgit log --oneline --decorate --all >log &&\n+\t! grep \"prefetch\" log\n+'\n+\n+test_expect_success 'prefetch and existing log.excludeDecoration values' '\n+\tgit config --unset-all log.excludeDecoration &&\n+\tgit config log.excludeDecoration refs/remotes/remote1/ &&\n+\tgit maintenance run --task=prefetch &&\n+\n+\tgit config --get-all log.excludeDecoration >out &&\n+\tgrep refs/remotes/remote1/ out &&\n+\tgrep refs/prefetch/ out &&\n+\n+\tgit log --oneline --decorate --all >log &&\n+\t! grep \"prefetch\" log &&\n+\t! grep \"remote1\" log &&\n+\tgrep \"remote2\" log &&\n+\n+\t# a second run does not change the config\n+\tgit maintenance run --task=prefetch &&\n+\tgit log --oneline --decorate --all >log2 &&\n+\ttest_cmp log log2\n '\n \n test_expect_success 'loose-objects task' '\n-- \ngitgitgadget\n\n"},{"id":"414680","messageId":"35038dfd037f6da1e21c1d9f1a24b27cb7100598.1611060724.git.gitgitgadget@gmail.com","threadId":"55000","inReplyTo":"pull.838.v2.git.1611060724.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] t7900: clean up some broken refs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-19T12:52:04Z","receivedAt":"2021-01-19T14:52:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe tests for the 'prefetch' task create remotes and fetch refs into\n'refs/prefetch//' and tags into 'refs/tags/'. These tests use\nthe remotes to create objects not intended to be seen by the \"local\"\nrepository.\n\nIn that sense, the incrmental-repack tasks did not have these objects\nand refs in mind. That test replaces the object directory with a\nspecific pack-file layout for testing the batch-size logic. However,\nthis causes some operations to start showing warnings such as:\n\n error: refs/prefetch/remote1/one does not point to a valid object!\n error: refs/tags/one does not point to a valid object!\n\nThis only shows up if you run the tests verbosely and watch the output.\nIt caught my eye and I _thought_ that there was a bug where 'git gc' or\n'git repack' wouldn't check 'refs/prefetch/' before pruning objects.\nThat is incorrect. Those commands do handle 'refs/prefetch/' correctly.\n\nAll that is left is to clean up the tests in t7900-maintenance.sh to\nremove these tags and refs that are not being repacked for the\nincremental-repack tests. Use update-ref to ensure this works with all\nref backends.\n\nHelped-by: Taylor Blau \nSigned-off-by: Derrick Stolee \n---\n t/t7900-maintenance.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex f9031cbb44b..78ccf4b33f8 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -256,6 +256,13 @@ test_expect_success 'incremental-repack task' '\n \tHEAD\n \t^HEAD~1\n \tEOF\n+\n+\t# Delete refs that have not been repacked in these packs.\n+\tgit for-each-ref --format=\"delete %(refname)\" \\\n+\t\trefs/prefetch refs/tags >refs &&\n+\tgit update-ref --stdin packs-before &&\n-- \ngitgitgadget\n"},{"id":"414690","messageId":"pull.838.v2.git.1611060724.gitgitgadget@gmail.com","threadId":"55000","inReplyTo":"pull.838.git.1610940216.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Two cleanups around 'prefetch' refs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-19T12:52:02Z","receivedAt":"2021-01-19T18:27:13Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Here are a couple things that caught my eye during a recent evaluation of\nthe maintenance feature:\n\n 1. 'refs/prefetch/' refs show up in 'git log' decorations. Auto-hide these.\n 2. t7900-maintenance.sh had some scary warnings that end up being\n unimportant.\n\nThis is based on 'master' at 66e871b (The third batch, 2021-01-15).\n\nUpdate in v2: deleting refs more safely for alternate ref backends. (Thanks,\nTaylor!)\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n maintenance: set log.excludeDecoration durin prefetch\n t7900: clean up some broken refs\n\n builtin/gc.c | 6 ++++++\n t/t7900-maintenance.sh | 33 ++++++++++++++++++++++++++++++++-\n 2 files changed, 38 insertions(+), 1 deletion(-)\n\n\nbase-commit: 66e871b6647ffea61a77a0f82c7ef3415f1ee79c\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-838%2Fderrickstolee%2Fprefetch-refs-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-838/derrickstolee/prefetch-refs-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/838\n\nRange-diff vs v1:\n\n 1: 5b2ce9049a6 = 1: 5b2ce9049a6 maintenance: set log.excludeDecoration durin prefetch\n 2: 616b73a6556 ! 2: 35038dfd037 t7900: clean up some broken refs\n @@ Commit message\n \n All that is left is to clean up the tests in t7900-maintenance.sh to\n remove these tags and refs that are not being repacked for the\n - incremental-repack tests.\n + incremental-repack tests. Use update-ref to ensure this works with all\n + ref backends.\n \n + Helped-by: Taylor Blau \n Signed-off-by: Derrick Stolee \n \n ## t/t7900-maintenance.sh ##\n @@ t/t7900-maintenance.sh: test_expect_success 'incremental-repack task' '\n \t^HEAD~1\n \tEOF\n +\n ++\t# Delete refs that have not been repacked in these packs.\n ++\tgit for-each-ref --format=\"delete %(refname)\" \\\n ++\t\trefs/prefetch refs/tags >refs &&\n ++\tgit update-ref --stdin packs-before &&\n\n-- \ngitgitgadget\n"},{"id":"414742","messageId":"YAbrsO1uIuE/CTrP@nand.local","threadId":"55000","inReplyTo":"pull.838.v2.git.1611060724.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/2] Two cleanups around 'prefetch' refs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-01-19T14:24:48Z","receivedAt":"2021-01-19T22:57:52Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Jan 19, 2021 at 12:52:02PM +0000, Derrick Stolee via GitGitGadget wrote:\n> Update in v2: deleting refs more safely for alternate ref backends. (Thanks,\n> Taylor!)\n\nThe range-diff looks good to me, thanks!\n\n Reviewed-by: Taylor Blau \n\nThanks,\nTaylor\n"},{"id":"414876","messageId":"xmqqh7nbdow2.fsf@gitster.c.googlers.com","threadId":"55000","inReplyTo":"5b2ce9049a69d4c450093433e4fa15c4e5e0c412.1611060724.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/2] maintenance: set log.excludeDecoration durin prefetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-21T02:06:53Z","receivedAt":"2021-01-21T03:46:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> The 'prefetch' task fetches refs from all remotes and places them in the\n> refs/prefetch// refspace. As this task is intended to run in the\n> background, this allows users to keep their local data very close to the\n> remote servers' data while not updating the users' understanding of the\n> remote refs in refs/remotes//.\n>\n> However, this can clutter 'git log' decorations with copies of the refs\n> with the full name 'refs/prefetch//'.\n>\n> The log.excludeDecoration config option was added in a6be5e67 (log: add\n> log.excludeDecoration config option, 2020-05-16) for exactly this\n> purpose.\n>\n> Ensure we set this only for users that would benefit from it by\n> assigning it at the beginning of the prefetch task. Other alternatives\n> would be during 'git maintenance register' or 'git maintenance start',\n> but those might assign the config even when the prefetch task is\n> disabled by existing config. Further, users could run 'git maintenance\n> run --task=prefetch' using their own scripting or scheduling. This\n> provides the best coverage to automatically update the config when\n> valuable.\n\nOK. I think those users who keep distance from \"git maintenance\"\nare different story but all others cannot be using refs/prefetch/\nhierarchy for purposes other than \"git maintenance\" dictates, so\n\"git maintenance [register|start]\", or even when the user first runs\n\"git maintenance\" for that matter, would be acceptable point to add\nthe configuration, but at the beginning of a prefetch task, we know\nthe hierarchy is being used for what \"git maintenance\" wants to do,\nso it is a good place to do so.\n\nBut playing devil's advocate, I do not think throwing refs/prefetch\ninto a \"hardcoded list of hierarchies that would never be used for\ndecoration purposes\" would upset any end-users in practice. The only\nreason why I do not make such a suggestion is it is more work (such\na hardcoded reject list does not exist, if I recall correctly).\n\n> It is improbable, but possible, that users might want to run the\n> prefetch task _and_ see these refs in their log decorations. This seems\n> incredibly unlikely to me, but users can always opt-in on a\n> command-by-command basis using --decorate-refs=refs/prefetch/.\n\nIt is not a viable workaround to add \"--decorate-refs=refs/prefetch\"\nthat is a mouthful; configuration options are to reduce such typing,\nnot to force more of them. But because I agree with that\n\"incredibly unlikely\" assessment, I do not care.\n\nThanks.\n"},{"id":"414877","messageId":"xmqqczxzdney.fsf@gitster.c.googlers.com","threadId":"55000","inReplyTo":"35038dfd037f6da1e21c1d9f1a24b27cb7100598.1611060724.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] t7900: clean up some broken refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-21T02:38:45Z","receivedAt":"2021-01-21T03:47:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> The tests for the 'prefetch' task create remotes and fetch refs into\n> 'refs/prefetch//' and tags into 'refs/tags/'. These tests use\n> the remotes to create objects not intended to be seen by the \"local\"\n> repository.\n>\n> In that sense, the incrmental-repack tasks did not have these objects\n> and refs in mind. That test replaces the object directory with a\n> specific pack-file layout for testing the batch-size logic. However,\n> this causes some operations to start showing warnings such as:\n>\n> error: refs/prefetch/remote1/one does not point to a valid object!\n> error: refs/tags/one does not point to a valid object!\n\nSo we use prefetch refs but craft a repacking strategy in such a way\nthat these prefetch refs do not act as anchors, deliberately\nbreaking the repository and invalidating the prefetch refs so to\nspeak?\n\nWow, that sounds like an evil and brittle test set-up.\n\n> This only shows up if you run the tests verbosely and watch the output.\n> It caught my eye and I _thought_ that there was a bug where 'git gc' or\n> 'git repack' wouldn't check 'refs/prefetch/' before pruning objects.\n> That is incorrect. Those commands do handle 'refs/prefetch/' correctly.\n\n> All that is left is to clean up the tests in t7900-maintenance.sh to\n> remove these tags and refs that are not being repacked for the\n> incremental-repack tests. Use update-ref to ensure this works with all\n> ref backends.\n>\n> Helped-by: Taylor Blau \n> Signed-off-by: Derrick Stolee \n> ---\n> t/t7900-maintenance.sh | 7 +++++++\n> 1 file changed, 7 insertions(+)\n>\n> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> index f9031cbb44b..78ccf4b33f8 100755\n> --- a/t/t7900-maintenance.sh\n> +++ b/t/t7900-maintenance.sh\n> @@ -256,6 +256,13 @@ test_expect_success 'incremental-repack task' '\n> \tHEAD\n> \t^HEAD~1\n> \tEOF\n> +\n> +\t# Delete refs that have not been repacked in these packs.\n> +\tgit for-each-ref --format=\"delete %(refname)\" \\\n> +\t\trefs/prefetch refs/tags >refs &&\n> +\tgit update-ref --stdin +\n> +\t# Replace the object directory with this pack layout.\n> \trm -f $packDir/pack-* &&\n> \trm -f $packDir/loose-* &&\n> \tls $packDir/*.pack >packs-before &&\n"},{"id":"414885","messageId":"xmqq8s8ndn3a.fsf@gitster.c.googlers.com","threadId":"55000","inReplyTo":"YAbrsO1uIuE/CTrP@nand.local","subject":"Re: [PATCH v2 0/2] Two cleanups around 'prefetch' refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-21T02:45:45Z","receivedAt":"2021-01-21T05:14:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> On Tue, Jan 19, 2021 at 12:52:02PM +0000, Derrick Stolee via GitGitGadget wrote:\n>> Update in v2: deleting refs more safely for alternate ref backends. (Thanks,\n>> Taylor!)\n>\n> The range-diff looks good to me, thanks!\n>\n> Reviewed-by: Taylor Blau \n\nThanks, both.\n"}]}