On Fri, May 2, 2025 at 5:58 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> A command slated for removal like "git pack-redundant" gains a
> command line option "--i-still-use-this", and refuses to work when
> the option is not given. The message and the instruction upon
> seeing what to do are both rather long, so before letting another
> command to use the same mechanism, factor out the message+die part
> into a small helper function, and use that.
>
> The existing pack-redundant test lacked a test to make sure that we
> require the --i-still-use-this option. Add one while we are at it.
The "gains a command line option" made me think you were discussing a
change made by this patch, rather than discussing an existing
mechanism. Could I spitball an alternative? Maybe something like...
Commands slated for removal, like "git pack-redundant", now require an
explicit --i-still-use-this option to run. This discourages casual use and
surfaces their pending deprecation to users.
The warning message is long, so we factor it into a helper function
(you_still_use_that()) to simplify reuse by other commands.
Also add a missing test to ensure this enforcement works for
"pack-redundant".
>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> builtin/pack-redundant.c | 10 ++--------
> git-compat-util.h | 2 ++
> t/t5323-pack-redundant.sh | 5 +++++
> usage.c | 12 ++++++++++++
> 4 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
> index 3febe732f8..6dc9e020c7 100644
> --- a/builtin/pack-redundant.c
> +++ b/builtin/pack-redundant.c
> @@ -625,14 +625,8 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, s
> break;
> }
>
> - if (!i_still_use_this) {
> - fputs(_("'git pack-redundant' is nominated for removal.\n"
> - "If you still use this command, please add an extra\n"
> - "option, '--i-still-use-this', on the command line\n"
> - "and let us know you still use it by sending an e-mail\n"
> - "to <git@xxxxxxxxxxxxxxx>. Thanks.\n"), stderr);
> - die(_("refusing to run without --i-still-use-this"));
> - }
> + if (!i_still_use_this)
> + you_still_use_that("git pack-redundant");
>
> if (load_all_packs)
> load_all();
> diff --git a/git-compat-util.h b/git-compat-util.h
> index e123288e8f..21cab99567 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -703,6 +703,8 @@ void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
>
> void show_usage_if_asked(int ac, const char **av, const char *err);
>
> +NORETURN void you_still_use_that(const char *command_name);
> +
> #ifndef NO_OPENSSL
> #ifdef APPLE_COMMON_CRYPTO
> #include "compat/apple-common-crypto.h"
> diff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh
> index 688cd9706c..f2f20cfa40 100755
> --- a/t/t5323-pack-redundant.sh
> +++ b/t/t5323-pack-redundant.sh
> @@ -45,6 +45,11 @@ fi
> main_repo=main.git
> shared_repo=shared.git
>
> +test_expect_success 'pack-redundant needs --i-still-use-this' '
> + test_must_fail git pack-redundant >message 2>&1 &&
> + test_grep "nominated for removal" message
> +'
> +
> git_pack_redundant='git pack-redundant --i-still-use-this'
>
> # Create commits in <repo> and assign each commit's oid to shell variables
> diff --git a/usage.c b/usage.c
> index 38b46bbbfe..4aaad2b553 100644
> --- a/usage.c
> +++ b/usage.c
> @@ -372,3 +372,15 @@ void bug_fl(const char *file, int line, const char *fmt, ...)
> trace2_cmd_error_va(fmt, ap);
> va_end(ap);
> }
> +
> +NORETURN void you_still_use_that(const char *command_name)
> +{
> + fprintf(stderr,
> + _("'%s' is nominated for removal.\n"
> + "If you still use this command, please add an extra\n"
> + "option, '--i-still-use-this', on the command line\n"
> + "and let us know you still use it by sending an e-mail\n"
> + "to <git@xxxxxxxxxxxxxxx>. Thanks.\n"),
> + command_name);
> + die(_("refusing to run without --i-still-use-this"));
> +}
> --
> 2.49.0-601-ga5925c3955
Patch looks good.
