From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 11 May 2023 19:34:53 GMT
Subject: Re: [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed
Message-ID: <xmqqzg6avezm.fsf@gitster.g>
In-Reply-To: <8c5c66a3050ee1c0ce8a48a088f5ecc2df7d1e3a.1683828635.git.gitgitgadget@gmail.com>

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> At GitLab, we have a system that creates ephemeral internal refs that
> don't live long before getting deleted. Having an option to exclude
> certain refs from a packed-refs file allows these internal references to
> be deleted much more efficiently.
>
> Add an --exclude option to the pack-refs builtin, and use the ref
> exclusions API to exclude certain refs from being packed into the final
> packed-refs file
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  Documentation/git-pack-refs.txt | 12 +++++++++++-
>  builtin/pack-refs.c             | 20 ++++++++++++++++----
>  refs.c                          |  4 ++--
>  refs.h                          |  7 ++++++-
>  refs/debug.c                    |  4 ++--
>  refs/files-backend.c            | 16 ++++++++++------
>  refs/packed-backend.c           |  2 +-
>  refs/refs-internal.h            |  3 ++-
>  revision.h                      |  2 +-
>  t/helper/test-ref-store.c       |  3 ++-
>  t/t3210-pack-refs.sh            | 16 ++++++++++++++++
>  11 files changed, 69 insertions(+), 20 deletions(-)

Nice.

> diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
> index e011e5fead3..c0f7426e519 100644
> --- a/Documentation/git-pack-refs.txt
> +++ b/Documentation/git-pack-refs.txt
> @@ -8,7 +8,7 @@ git-pack-refs - Pack heads and tags for efficient repository access
>  SYNOPSIS
>  --------
>  [verse]
> -'git pack-refs' [--all] [--no-prune]
> +'git pack-refs' [--all] [--no-prune] [--exclude <pattern>]
>  
>  DESCRIPTION
>  -----------
> @@ -60,6 +60,16 @@ interests.
>  The command usually removes loose refs under `$GIT_DIR/refs`
>  hierarchy after packing them.  This option tells it not to.
>  
> +--exclude <pattern>::
> +
> +Do not pack refs matching the given `glob(7)` pattern. Repetitions of this option
> +accumulate exclusion patterns. Use `--no-exclude` to clear and reset the list of
> +patterns. If a ref is already packed, including it with `--exclude` will not
> +unpack it.
> +
> +When used with `--all`, it will use the difference between the set of all refs,
> +and what is provided to `--exclude`.
> +

Just one question.  Does the above get formatted correctly, or does
the lack of a line with a sole '+' on it between the paragraphs make
the second paragraph look as if it is unrelated to the description
of the "--exclude" option?

Other than that I saw nothing surprising or unexpected in the
patch.  Looking good.

Thanks.