"John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: John Cai <johncai86@xxxxxxxxx>
>
> At GitLab, we have a system that creates ephemeral internal refs that
> don't live long before getting deleted. Having an option to not include
"to not include" -> "to exclude", perhaps.
> +--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`.
> +
Clearly written.
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index d0581ee41ac..6a51267f379 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -19,6 +19,7 @@
> #include "../worktree.h"
> #include "../wrapper.h"
> #include "../write-or-die.h"
> +#include "../revision.h"
>
> /*
> * This backend uses the following flags in `ref_update::flags` for
> @@ -1173,7 +1174,7 @@ static void prune_refs(struct files_ref_store *refs, struct ref_to_prune **refs_
> */
> static int should_pack_ref(const char *refname,
> const struct object_id *oid, unsigned int ref_flags,
> - unsigned int pack_flags)
> + struct pack_refs_opts *opts)
> {
> /* Do not pack per-worktree refs: */
> if (parse_worktree_ref(refname, NULL, NULL, NULL) !=
> @@ -1181,7 +1182,7 @@ static int should_pack_ref(const char *refname,
> return 0;
>
> /* Do not pack non-tags unless PACK_REFS_ALL is set: */
> - if (!(pack_flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
> + if (!(opts->flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
> return 0;
>
> /* Do not pack symbolic refs: */
> @@ -1192,10 +1193,14 @@ static int should_pack_ref(const char *refname,
> if (!ref_resolves_to_object(refname, the_repository, oid, ref_flags))
> return 0;
>
> + if (opts->exclusions && ref_excluded(opts->exclusions, refname))
> + return 0;
It is _not_ _wrong_ per-se, but I would have expected for this to go
between "what do we include" and "symrefs and broken ones are not
packed", given that we will tweak the "what to include" logic in the
next step.
Other changes to the code in this patch are all naturally expected
to pass the new information through the callchain and looked good.
> diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
> index 07a0ff93def..31b9f72e84a 100755
> --- a/t/t3210-pack-refs.sh
> +++ b/t/t3210-pack-refs.sh
> @@ -108,6 +108,24 @@ test_expect_success \
> git branch -d n/o/p &&
> git branch n'
>
> +test_expect_success \
> + 'test excluded refs are not packed' '
> + git branch dont_pack1 &&
> + git branch dont_pack2 &&
> + git branch pack_this &&
> + git pack-refs --all --exclude refs/heads/dont_pack* &&
Let's quote the value given to "--exclude" to make it clear that we
do not expect glob to affect the shell, i.e.
git pack-refs --all --exclude "refs/heads/dont_pack*" &&
Some (early and ancient) parts, but not all parts, of this file use
4-space indentation and have the title of the test on a line that is
separate from test_expect_success, which is an ancient deprecated
style.
You do not want to imitate them in new tests you add. After the
dust from this topic settles, somebody should go in and clean the
style of this file. Let's not create more work for them.
Thanks.
> + test -f .git/refs/heads/dont_pack1 &&
> + test -f .git/refs/heads/dont_pack2 &&
> + ! test -f ./git/refs/heads/pack_this'
> +
> +test_expect_success \
> + 'test --no-exclude refs clears excluded refs' '
> + git branch dont_pack3 &&
> + git branch dont_pack4 &&
> + git pack-refs --all --exclude refs/heads/dont_pack* --no-exclude &&
> + ! test -f .git/refs/heads/dont_pack3 &&
> + ! test -f .git/refs/heads/dont_pack4'
> +
> test_expect_success \
> 'see if up-to-date packed refs are preserved' \
> 'git branch q &&
