pack-redundant: gauge the usage before proposing its removal

The subcommand is unusably slow and the reason why nobody reports it as a performance bug is suspected to be the absense of users. Let's show a big message that asks the user to tell us that they still care about the command when an attempt is made to run the command, with an escape hatch to override it with a command line option. In a few releases, we may turn it into an error and keep it for a few more releases before finally removing it (during the whole time, the plan to remove it would be interrupted by end user raising hand). Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Aug 25, 2020 at 15:45 UTC c3b58472be2c647cd0c5af328980b15bd3f43b69
2 files changed +28 -13
builtin/pack-redundant.c
+13
@@ -554,6 +554,7 @@ static void load_all(void)
554 int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
555 {
556 int i;
557 + int i_still_use_this = 0;
558 struct pack_list *min = NULL, *red, *pl;
559 struct llist *ignore;
560 struct object_id *oid;
@@ -580,12 +581,24 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
581 alt_odb = 1;
582 continue;
583 }
584 + if (!strcmp(arg, "--i-still-use-this")) {
585 + i_still_use_this = 1;
586 + continue;
587 + }
588 if (*arg == '-')
589 usage(pack_redundant_usage);
590 else
591 break;
592 }
593
594 + if (!i_still_use_this) {
595 + fputs(_("'git pack-redundant' is nominated for removal.\n"
596 + "If you still use this command, please add an extra\n"
597 + "option, '--i-still-use-this', on the command line\n"
598 + "and let us know you still use it by sending an e-mail\n"
599 + "to <git@vger.kernel.org>. Thanks.\n"), stderr);
600 + }
601 +
602 if (load_all_packs)
603 load_all();
604 else
t/t5323-pack-redundant.sh
+15 -13
@@ -39,6 +39,8 @@ relationship between packs and objects is as follows:
39 master_repo=master.git
40 shared_repo=shared.git
41
42 +git_pack_redundant='git pack-redundant --i-still-use-this'
43 +
44 # Create commits in <repo> and assign each commit's oid to shell variables
45 # given in the arguments (A, B, and C). E.g.:
46 #
@@ -154,7 +156,7 @@ test_expect_success 'master: no redundant for pack 1, 2, 3' '
156 EOF
157 (
158 cd "$master_repo" &&
157 - git pack-redundant --all >out &&
159 + $git_pack_redundant --all >out &&
160 test_must_be_empty out
161 )
162 '
@@ -192,7 +194,7 @@ test_expect_success 'master: one of pack-2/pack-3 is redundant' '
194 cat >expect <<-EOF &&
195 P3:$P3
196 EOF
195 - git pack-redundant --all >out &&
197 + $git_pack_redundant --all >out &&
198 format_packfiles <out >actual &&
199 test_cmp expect actual
200 )
@@ -231,7 +233,7 @@ test_expect_success 'master: pack 2, 4, and 6 are redundant' '
233 P4:$P4
234 P6:$P6
235 EOF
234 - git pack-redundant --all >out &&
236 + $git_pack_redundant --all >out &&
237 format_packfiles <out >actual &&
238 test_cmp expect actual
239 )
@@ -266,7 +268,7 @@ test_expect_success 'master: pack-8 (subset of pack-1) is also redundant' '
268 P6:$P6
269 P8:$P8
270 EOF
269 - git pack-redundant --all >out &&
271 + $git_pack_redundant --all >out &&
272 format_packfiles <out >actual &&
273 test_cmp expect actual
274 )
@@ -284,9 +286,9 @@ test_expect_success 'master: clean loose objects' '
286 test_expect_success 'master: remove redundant packs and pass fsck' '
287 (
288 cd "$master_repo" &&
287 - git pack-redundant --all | xargs rm &&
289 + $git_pack_redundant --all | xargs rm &&
290 git fsck &&
289 - git pack-redundant --all >out &&
291 + $git_pack_redundant --all >out &&
292 test_must_be_empty out
293 )
294 '
@@ -304,7 +306,7 @@ test_expect_success 'setup shared.git' '
306 test_expect_success 'shared: all packs are redundant, but no output without --alt-odb' '
307 (
308 cd "$shared_repo" &&
307 - git pack-redundant --all >out &&
309 + $git_pack_redundant --all >out &&
310 test_must_be_empty out
311 )
312 '
@@ -343,7 +345,7 @@ test_expect_success 'shared: show redundant packs in stderr for verbose mode' '
345 P5:$P5
346 P7:$P7
347 EOF
346 - git pack-redundant --all --verbose >out 2>out.err &&
348 + $git_pack_redundant --all --verbose >out 2>out.err &&
349 test_must_be_empty out &&
350 grep "pack$" out.err | format_packfiles >actual &&
351 test_cmp expect actual
@@ -356,9 +358,9 @@ test_expect_success 'shared: remove redundant packs, no packs left' '
358 cat >expect <<-EOF &&
359 fatal: Zero packs found!
360 EOF
359 - git pack-redundant --all --alt-odb | xargs rm &&
361 + $git_pack_redundant --all --alt-odb | xargs rm &&
362 git fsck &&
361 - test_must_fail git pack-redundant --all --alt-odb >actual 2>&1 &&
363 + test_must_fail $git_pack_redundant --all --alt-odb >actual 2>&1 &&
364 test_cmp expect actual
365 )
366 '
@@ -386,7 +388,7 @@ test_expect_success 'shared: create new objects and packs' '
388 test_expect_success 'shared: no redundant without --alt-odb' '
389 (
390 cd "$shared_repo" &&
389 - git pack-redundant --all >out &&
391 + $git_pack_redundant --all >out &&
392 test_must_be_empty out
393 )
394 '
@@ -417,7 +419,7 @@ test_expect_success 'shared: no redundant without --alt-odb' '
419 test_expect_success 'shared: one pack is redundant with --alt-odb' '
420 (
421 cd "$shared_repo" &&
420 - git pack-redundant --all --alt-odb >out &&
422 + $git_pack_redundant --all --alt-odb >out &&
423 format_packfiles <out >actual &&
424 test_line_count = 1 actual
425 )
@@ -454,7 +456,7 @@ test_expect_success 'shared: ignore unique objects and all two packs are redunda
456 Px1:$Px1
457 Px2:$Px2
458 EOF
457 - git pack-redundant --all --alt-odb >out <<-EOF &&
459 + $git_pack_redundant --all --alt-odb >out <<-EOF &&
460 $X
461 $Y
462 $Z