{"thread":{"id":"59461","subject":"[RFC/PATCH] pack-redundant: escalate deprecation warning to an error","startedAt":"2023-03-23T20:40:55Z","lastAt":"2023-03-28T19:06:50Z","messageCount":5,"participants":["Jeff King","Junio C Hamano","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"474042","messageId":"20230323204047.GA9290@coredump.intra.peff.net","threadId":"59461","inReplyTo":null,"subject":"[RFC/PATCH] pack-redundant: escalate deprecation warning to an error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-23T20:40:47Z","receivedAt":"2023-03-23T20:40:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In c3b58472be2 (pack-redundant: gauge the usage before proposing its\nremoval, 2020-08-25), we added a big, ugly warning when pack-redundant\nis run. The plan there indicated that we would ratchet that up to an\nerror before finally removing it. Since it has been 2.5 years (and 9\nreleases) since then, let's continue with the plan.\n\nNote that we did get one bite on the warning, which was somebody asking\nabout alternatives:\n\n https://lore.kernel.org/git/CAKvOHKAFXQwt4D8yUCCkf_TQL79mYaJ=KAKhtpDNTvHJFuX1NA@mail.gmail.com/\n\nbut we didn't undo the ugly warning (and the advice continues to be \"use\nrepack -d\" instead).\n\nThere was also some discussion around the time of the deprecation that\npack-redundant was invoked by the bitbake tool, and it still seems to do\nso now:\n\n https://git.openembedded.org/bitbake\n\nThat use should probably just go away in favor of an occasional repack\n(which probably even happens via auto-gc after fetch these days).\n\nBut since neither of those data points caused us to cancel the\ndeprecation plan by dropping the warning, it seems like we should\nproceed with the next step.\n\nSigned-off-by: Jeff King \n---\nI was looking in this file recently, and was reminded of the deprecation\nplan. The two data points above do give me a little bit of pause, but it\nseems like the current state is the worst of both worlds: we do not have\nthe benefit of dropping the code, and people who try to use the command\nhave a bad experience. So we should probably either proceed (as with\nthis patch), or decide we need to keep pack-redundant.\n\n builtin/pack-redundant.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c\nindex 82115c5808c..5e93d873208 100644\n--- a/builtin/pack-redundant.c\n+++ b/builtin/pack-redundant.c\n@@ -604,6 +604,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)\n \t\t\t\"option, '--i-still-use-this', on the command line\\n\"\n \t\t\t\"and let us know you still use it by sending an e-mail\\n\"\n \t\t\t\"to . Thanks.\\n\"), stderr);\n+\t\tdie(_(\"refusing to run without --i-still-use-this\"));\n \t}\n \n \tif (load_all_packs)\n-- \n2.40.0.597.ge37e901cd8f\n"},{"id":"474045","messageId":"xmqqlejn6vb9.fsf@gitster.g","threadId":"59461","inReplyTo":"20230323204047.GA9290@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] pack-redundant: escalate deprecation warning to an error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-23T20:56:26Z","receivedAt":"2023-03-23T20:57:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I was looking in this file recently, and was reminded of the deprecation\n> plan. The two data points above do give me a little bit of pause, but it\n> seems like the current state is the worst of both worlds: we do not have\n> the benefit of dropping the code, and people who try to use the command\n> have a bad experience. So we should probably either proceed (as with\n> this patch), or decide we need to keep pack-redundant.\n\nSounds like a good thing to do. Will queue. Thanks.\n\n>\n> builtin/pack-redundant.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c\n> index 82115c5808c..5e93d873208 100644\n> --- a/builtin/pack-redundant.c\n> +++ b/builtin/pack-redundant.c\n> @@ -604,6 +604,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)\n> \t\t\t\"option, '--i-still-use-this', on the command line\\n\"\n> \t\t\t\"and let us know you still use it by sending an e-mail\\n\"\n> \t\t\t\"to . Thanks.\\n\"), stderr);\n> +\t\tdie(_(\"refusing to run without --i-still-use-this\"));\n> \t}\n> \n> \tif (load_all_packs)\n"},{"id":"474049","messageId":"ZBzBr1EAXoqBwmVo@nand.local","threadId":"59461","inReplyTo":"xmqqlejn6vb9.fsf@gitster.g","subject":"Re: [RFC/PATCH] pack-redundant: escalate deprecation warning to an error","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-03-23T21:16:31Z","receivedAt":"2023-03-23T21:16:41Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Mar 23, 2023 at 01:56:26PM -0700, Junio C Hamano wrote:\n> Jeff King writes:\n>\n> > I was looking in this file recently, and was reminded of the deprecation\n> > plan. The two data points above do give me a little bit of pause, but it\n> > seems like the current state is the worst of both worlds: we do not have\n> > the benefit of dropping the code, and people who try to use the command\n> > have a bad experience. So we should probably either proceed (as with\n> > this patch), or decide we need to keep pack-redundant.\n>\n> Sounds like a good thing to do. Will queue. Thanks.\n\nYeah, I agree with and am persuaded by the \"worst of both worlds\"\nargument. I think that changing this to a die() is sensible for now.\n\nAt what point would it be fair to drop this builtin entirely from the\ntree?\n\nThanks,\nTaylor\n"},{"id":"474051","messageId":"xmqqh6ub6u1p.fsf@gitster.g","threadId":"59461","inReplyTo":"ZBzBr1EAXoqBwmVo@nand.local","subject":"Re: [RFC/PATCH] pack-redundant: escalate deprecation warning to an error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-23T21:23:46Z","receivedAt":"2023-03-23T21:23:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> On Thu, Mar 23, 2023 at 01:56:26PM -0700, Junio C Hamano wrote:\n>> Jeff King writes:\n>>\n>> > I was looking in this file recently, and was reminded of the deprecation\n>> > plan. The two data points above do give me a little bit of pause, but it\n>> > seems like the current state is the worst of both worlds: we do not have\n>> > the benefit of dropping the code, and people who try to use the command\n>> > have a bad experience. So we should probably either proceed (as with\n>> > this patch), or decide we need to keep pack-redundant.\n>>\n>> Sounds like a good thing to do. Will queue. Thanks.\n>\n> Yeah, I agree with and am persuaded by the \"worst of both worlds\"\n> argument. I think that changing this to a die() is sensible for now.\n>\n> At what point would it be fair to drop this builtin entirely from the\n> tree?\n\nI notice that \"git pack-redundant --help\" does not say anything\nabout its deprecation. We probably should add one together with\nthe patch in question, and then consider that the count-down timer\nhas finally started.\n\nThe timer should last probably for at least a few cycles.\n"},{"id":"474355","messageId":"20230328190641.GF18558@coredump.intra.peff.net","threadId":"59461","inReplyTo":"xmqqh6ub6u1p.fsf@gitster.g","subject":"[PATCH] pack-redundant: document deprecation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-28T19:06:41Z","receivedAt":"2023-03-28T19:06:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 23, 2023 at 02:23:46PM -0700, Junio C Hamano wrote:\n\n> I notice that \"git pack-redundant --help\" does not say anything\n> about its deprecation. We probably should add one together with\n> the patch in question, and then consider that the count-down timer\n> has finally started.\n\nGood thinking. Here is a patch.\n\n-- >8 --\nSubject: [PATCH] pack-redundant: document deprecation\n\nRunning the command itself has generated a warning for several versions,\nwhich has recently been upgraded to an error. Let's also make sure the\ndocumentation mentions what is going on. This also gives us a good spot\nto explain the reasoning and recommend alternatives.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-pack-redundant.txt | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/Documentation/git-pack-redundant.txt b/Documentation/git-pack-redundant.txt\nindex 99ef13839d4..13c3eb5ec96 100644\n--- a/Documentation/git-pack-redundant.txt\n+++ b/Documentation/git-pack-redundant.txt\n@@ -11,6 +11,20 @@ SYNOPSIS\n [verse]\n 'git pack-redundant' [--verbose] [--alt-odb] (--all | ...)\n \n+WARNING\n+-------\n+`git pack-redundant` has been deprecated and is scheduled for removal in\n+a future version of Git. Because it can only remove entire duplicate\n+packs and not individual duplicate objects, it is generally not a useful\n+tool for reducing repository size. You are better off using `git gc` to\n+do so, which will put objects into a new pack, removing duplicates.\n+\n+Running `pack-redundant` without the `--i-still-use-this` flag will fail\n+in this release. If you believe you have a use case for which\n+`pack-redundant` is better suited and oppose this removal, please\n+contact the Git mailing list at git@vger.kernel.org. More information\n+about the list is available at https://git-scm.com/community.\n+\n DESCRIPTION\n -----------\n This program computes which packs in your repository\n-- \n2.40.0.616.gf524ec75088\n\n"}]}