* [PATCH] pack-refs: teach --exclude option to exclude refs from being packed
@ 2023-05-04 15:48 John Cai via GitGitGadget
2023-05-04 16:48 ` Junio C Hamano
2023-05-09 19:18 ` [PATCH v2 0/3] pack-refs: Teach " John Cai via GitGitGadget
0 siblings, 2 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-04 15:48 UTC (permalink / raw)
To: git; +Cc: John Cai, John Cai
From: John Cai <jcai@gitlab.com>
Currently once refs are packed into a pack-refs file, deleting them can be
slow since it involves a full file scan of the entire pack-refs file. 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 certain
refs from a pack-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
pack-refs file
Signed-off-by: John Cai <johncai86@gmail.com>
---
pack-refs: Teach --exclude option to exclude refs from being packed
Add an --exclude option to the pack-refs builtin, to prevent certain
refs from being packed into the final packrefs file. At GitLab we keep
ephemeral internal references that quickly get deleted. Being able to
exclude them from the packrefs file will avoid incurring the performance
hit from ref deletions--especially since it scales with packrefs size.
It's worth noting that [1] discussed a proposal for a pack refs v2
format that would improve deletion speeds. The ref-table backend would
also improve performance of deletions. However, both of those proposals
are still being discussed.
1. https://lore.kernel.org/git/pull.1408.git.1667846164.gitgitgadget@gmail.com/
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1501%2Fjohn-cai%2Fjc%2Fexclude-refs-from-pack-refs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1501/john-cai/jc/exclude-refs-from-pack-refs-v1
Pull-Request: https://github.com/git/git/pull/1501
Documentation/git-pack-refs.txt | 8 +++++++-
builtin/pack-refs.c | 17 +++++++++++++++--
refs.c | 4 ++--
refs.h | 6 +++++-
refs/debug.c | 4 ++--
refs/files-backend.c | 13 ++++++++++---
refs/packed-backend.c | 3 ++-
refs/refs-internal.h | 4 +++-
t/helper/test-ref-store.c | 3 ++-
t/t3210-pack-refs.sh | 18 ++++++++++++++++++
10 files changed, 66 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
index 154081f2de2..d80e0a1562d 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
-----------
@@ -59,6 +59,12 @@ a repository with many branches of historical 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.
+
BUGS
----
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 9833815fb30..bc2db41c2cb 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -4,22 +4,35 @@
#include "parse-options.h"
#include "refs.h"
#include "repository.h"
+#include "revision.h"
static char const * const pack_refs_usage[] = {
- N_("git pack-refs [--all] [--no-prune]"),
+ N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
NULL
};
int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
+ static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+ struct pack_refs_opts pack_opts = {.exclusions = &excludes};
+ static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
+ struct string_list_item *item;
+
struct option opts[] = {
OPT_BIT(0, "all", &flags, N_("pack everything"), PACK_REFS_ALL),
OPT_BIT(0, "prune", &flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
+ N_("references to exclude")),
OPT_END(),
};
+
git_config(git_default_config, NULL);
if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
- return refs_pack_refs(get_main_ref_store(the_repository), flags);
+
+ for_each_string_list_item(item, &option_excluded_refs)
+ add_ref_exclusion(pack_opts.exclusions, item->string);
+
+ return refs_pack_refs(get_main_ref_store(the_repository), flags, &pack_opts);
}
diff --git a/refs.c b/refs.c
index d2a98e1c21f..637810347a0 100644
--- a/refs.c
+++ b/refs.c
@@ -2132,9 +2132,9 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
}
/* backend functions */
-int refs_pack_refs(struct ref_store *refs, unsigned int flags)
+int refs_pack_refs(struct ref_store *refs, unsigned int flags, struct pack_refs_opts *pack_opts)
{
- return refs->be->pack_refs(refs, flags);
+ return refs->be->pack_refs(refs, flags, pack_opts);
}
int peel_iterated_oid(const struct object_id *base, struct object_id *peeled)
diff --git a/refs.h b/refs.h
index 123cfa44244..a76dbb2d3f0 100644
--- a/refs.h
+++ b/refs.h
@@ -63,6 +63,10 @@ struct worktree;
#define RESOLVE_REF_NO_RECURSE 0x02
#define RESOLVE_REF_ALLOW_BAD_NAME 0x04
+struct pack_refs_opts {
+ struct ref_exclusions *exclusions;
+};
+
const char *refs_resolve_ref_unsafe(struct ref_store *refs,
const char *refname,
int resolve_flags,
@@ -405,7 +409,7 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt,
* Write a packed-refs file for the current repository.
* flags: Combination of the above PACK_REFS_* flags.
*/
-int refs_pack_refs(struct ref_store *refs, unsigned int flags);
+int refs_pack_refs(struct ref_store *refs, unsigned int flags, struct pack_refs_opts *opts);
/*
* Setup reflog before using. Fill in err and return -1 on failure.
diff --git a/refs/debug.c b/refs/debug.c
index adc34c836fc..93f6168052a 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -122,10 +122,10 @@ static int debug_initial_transaction_commit(struct ref_store *refs,
return res;
}
-static int debug_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int debug_pack_refs(struct ref_store *ref_store, unsigned int flags, struct pack_refs_opts *opts)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
- int res = drefs->refs->be->pack_refs(drefs->refs, flags);
+ int res = drefs->refs->be->pack_refs(drefs->refs, flags, opts);
trace_printf_key(&trace_refs, "pack_refs: %d\n", res);
return res;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index d0581ee41ac..b77c474919f 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,8 @@ 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)
+ unsigned int pack_flags,
+ struct pack_refs_opts *opts)
{
/* Do not pack per-worktree refs: */
if (parse_worktree_ref(refname, NULL, NULL, NULL) !=
@@ -1192,10 +1194,15 @@ 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;
+
return 1;
}
-static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int files_pack_refs(struct ref_store *ref_store,
+ unsigned int flags,
+ struct pack_refs_opts *pack_opts)
{
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,
@@ -1221,7 +1228,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
* pruned, also add it to refs_to_prune.
*/
if (!should_pack_ref(iter->refname, iter->oid, iter->flags,
- flags))
+ flags, pack_opts))
continue;
/*
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 2333ed5a1f7..8a611dd92b4 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1576,7 +1576,8 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
}
static int packed_pack_refs(struct ref_store *ref_store UNUSED,
- unsigned int flags UNUSED)
+ unsigned int flags UNUSED,
+ struct pack_refs_opts *pack_opts UNUSED)
{
/*
* Packed refs are already packed. It might be that loose refs
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index a85d113123c..7b4852104f0 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -547,7 +547,9 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err);
-typedef int pack_refs_fn(struct ref_store *ref_store, unsigned int flags);
+typedef int pack_refs_fn(struct ref_store *ref_store,
+ unsigned int flags,
+ struct pack_refs_opts *opts);
typedef int create_symref_fn(struct ref_store *ref_store,
const char *ref_target,
const char *refs_heads_master,
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 6d8f844e9c7..0405cacc99b 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -116,8 +116,9 @@ static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
static int cmd_pack_refs(struct ref_store *refs, const char **argv)
{
unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
+ struct pack_refs_opts pack_opts = { 0 };
- return refs_pack_refs(refs, flags);
+ return refs_pack_refs(refs, flags, &pack_opts);
}
static int cmd_create_symref(struct ref_store *refs, const char **argv)
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* &&
+ 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 &&
base-commit: f85cd430b12b0d3e4f1a30ef3239a1b73d5f6331
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-04 15:48 [PATCH] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
@ 2023-05-04 16:48 ` Junio C Hamano
2023-05-04 21:26 ` John Cai
2023-05-09 19:18 ` [PATCH v2 0/3] pack-refs: Teach " John Cai via GitGitGadget
1 sibling, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2023-05-04 16:48 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, John Cai, John Cai
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: John Cai <jcai@gitlab.com>
>
> Currently once refs are packed into a pack-refs file, deleting them can be
> slow since it involves a full file scan of the entire pack-refs file. At
"pack-refs" -> "packed-refs".
But performing a full file scan of 100MB file would take how many
milliseconds? Having to remove many refs from a single packed-refs
file would be costly if it is done one-by-one, though. I wonder how
effective our batched update mechanism is these days...
Sorry for straying to a tangent. In any case, I do not think the
first sentence is necessary; just start it with "At GitLab, ..." to
say that you have a need to be more selective than "is it a tag or
not?" to choose refs to be and not to be packed. The reason why we
may want to leave a ref loose is not limited to ref deletion
performance, and the benefit of this new feature is not limited to
it, either.
> GitLab, we have a system that creates ephemeral internal refs that don't
> live long before getting deleted. Having an option to not include certain
> refs from a pack-refs file allows these internal references to be deleted
> much more efficiently.
I think that is a valid use case.
If we step back a bit, we can see "pack-refs" has an option "--all"
that was added by b3d4204f (git-pack-refs --all, 2006-10-08), to
allow us pack only tags by default, because packing branches that
are meant to be updated often and also removed more often than tags
were found to be detrimental. We can view this new option a
follow-up work for the commit, to allow the users to define what to
be and what not to be packed, depending on their workflow.
This observation also makes us realize that we should consider the
opposite. It would benefit us to include some refs that we normally
do not pack and be more selective than "--all" ("--exclude" proposed
here is a way to leave some refs that we normally pack and be more
selective than not running pack-refs at all). A set of branches
that are only occasionally used may want to be packed while the rest
of branches want to be left loose because they are more active, or
something.
> 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
> pack-refs file
"pack-refs" appears here, too.
> Documentation/git-pack-refs.txt | 8 +++++++-
> builtin/pack-refs.c | 17 +++++++++++++++--
> refs.c | 4 ++--
> refs.h | 6 +++++-
> refs/debug.c | 4 ++--
> refs/files-backend.c | 13 ++++++++++---
> refs/packed-backend.c | 3 ++-
> refs/refs-internal.h | 4 +++-
> t/helper/test-ref-store.c | 3 ++-
> t/t3210-pack-refs.sh | 18 ++++++++++++++++++
> 10 files changed, 66 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
> index 154081f2de2..d80e0a1562d 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
> -----------
> @@ -59,6 +59,12 @@ a repository with many branches of historical 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.
The interaction of this option with "--all" needs to be described
somewhere. If we are to be adding "--include" for completeness,
that one also needs to interact with "--all".
> diff --git a/refs.c b/refs.c
> index d2a98e1c21f..637810347a0 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2132,9 +2132,9 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
> }
>
> /* backend functions */
> -int refs_pack_refs(struct ref_store *refs, unsigned int flags)
> +int refs_pack_refs(struct ref_store *refs, unsigned int flags, struct pack_refs_opts *pack_opts)
> {
> - return refs->be->pack_refs(refs, flags);
> + return refs->be->pack_refs(refs, flags, pack_opts);
That's a curious choice of the API. It is not like backend methods
all share the same "flags" bitset (they share "refs" pointer to the
ref_store), so I would have expected that it would become part of
the pack_refs_opts structure. Do not call the parameter pack_opts;
either spell it out as "pack_refs_opts", or just use "opts". The
latter is probably more preferrable as I do not expect it to be
ambiguous with other kinds of "opts".
The rest of the pack I found nothing unexpected or surprising.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-04 16:48 ` Junio C Hamano
@ 2023-05-04 21:26 ` John Cai
0 siblings, 0 replies; 29+ messages in thread
From: John Cai @ 2023-05-04 21:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Cai via GitGitGadget, git, John Cai
On 23/05/04 09:48AM, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
Hi Junio,
> > From: John Cai <jcai@gitlab.com>
> >
> > Currently once refs are packed into a pack-refs file, deleting them can be
> > slow since it involves a full file scan of the entire pack-refs file. At
>
> "pack-refs" -> "packed-refs".
>
> But performing a full file scan of 100MB file would take how many
> milliseconds? Having to remove many refs from a single packed-refs
> file would be costly if it is done one-by-one, though. I wonder how
> effective our batched update mechanism is these days...
>
> Sorry for straying to a tangent. In any case, I do not think the
> first sentence is necessary; just start it with "At GitLab, ..." to
> say that you have a need to be more selective than "is it a tag or
> not?" to choose refs to be and not to be packed. The reason why we
> may want to leave a ref loose is not limited to ref deletion
> performance, and the benefit of this new feature is not limited to
> it, either.
Good point
>
> > GitLab, we have a system that creates ephemeral internal refs that don't
> > live long before getting deleted. Having an option to not include certain
> > refs from a pack-refs file allows these internal references to be deleted
> > much more efficiently.
>
> I think that is a valid use case.
>
> If we step back a bit, we can see "pack-refs" has an option "--all"
> that was added by b3d4204f (git-pack-refs --all, 2006-10-08), to
> allow us pack only tags by default, because packing branches that
> are meant to be updated often and also removed more often than tags
> were found to be detrimental. We can view this new option a
> follow-up work for the commit, to allow the users to define what to
> be and what not to be packed, depending on their workflow.
>
> This observation also makes us realize that we should consider the
> opposite. It would benefit us to include some refs that we normally
> do not pack and be more selective than "--all" ("--exclude" proposed
> here is a way to leave some refs that we normally pack and be more
> selective than not running pack-refs at all). A set of branches
> that are only occasionally used may want to be packed while the rest
> of branches want to be left loose because they are more active, or
> something.
Yeah, that's a good observation. It would be nice to add an --include option to
give the user full flexibility of which refs to include.
>
> > 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
> > pack-refs file
>
> "pack-refs" appears here, too.
>
> > Documentation/git-pack-refs.txt | 8 +++++++-
> > builtin/pack-refs.c | 17 +++++++++++++++--
> > refs.c | 4 ++--
> > refs.h | 6 +++++-
> > refs/debug.c | 4 ++--
> > refs/files-backend.c | 13 ++++++++++---
> > refs/packed-backend.c | 3 ++-
> > refs/refs-internal.h | 4 +++-
> > t/helper/test-ref-store.c | 3 ++-
> > t/t3210-pack-refs.sh | 18 ++++++++++++++++++
> > 10 files changed, 66 insertions(+), 14 deletions(-)
> >
> > diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
> > index 154081f2de2..d80e0a1562d 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
> > -----------
> > @@ -59,6 +59,12 @@ a repository with many branches of historical 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.
>
> The interaction of this option with "--all" needs to be described
> somewhere. If we are to be adding "--include" for completeness,
> that one also needs to interact with "--all".
Sounds good
>
> > diff --git a/refs.c b/refs.c
> > index d2a98e1c21f..637810347a0 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -2132,9 +2132,9 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
> > }
> >
> > /* backend functions */
> > -int refs_pack_refs(struct ref_store *refs, unsigned int flags)
> > +int refs_pack_refs(struct ref_store *refs, unsigned int flags, struct pack_refs_opts *pack_opts)
> > {
> > - return refs->be->pack_refs(refs, flags);
> > + return refs->be->pack_refs(refs, flags, pack_opts);
>
> That's a curious choice of the API. It is not like backend methods
> all share the same "flags" bitset (they share "refs" pointer to the
> ref_store), so I would have expected that it would become part of
> the pack_refs_opts structure. Do not call the parameter pack_opts;
> either spell it out as "pack_refs_opts", or just use "opts". The
> latter is probably more preferrable as I do not expect it to be
> ambiguous with other kinds of "opts".
I didn't notice this, but it makes sense. We can move flags into the
pack_refs_opts struct.
>
> The rest of the pack I found nothing unexpected or surprising.
thanks
John
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 0/3] pack-refs: Teach --exclude option to exclude refs from being packed
2023-05-04 15:48 [PATCH] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
2023-05-04 16:48 ` Junio C Hamano
@ 2023-05-09 19:18 ` John Cai via GitGitGadget
2023-05-09 19:18 ` [PATCH v2 1/3] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
` (3 more replies)
1 sibling, 4 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-09 19:18 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, John Cai
git-pack-refs does not currently give much control over which refs are
packed. By default, all tags and already packed refs are packed. --all
allows the user to pack all refs. Beyond this the user does not have
control. Introduce a pair of options --include and --exclude that will allow
users full control over which refs end up in the packed-refs file.
Changes since v1:
* Clarify that --all packs not just branch tips but all refs under refs/ in
the documentation in patch 1
* Add --include in patch 3
It's worth noting that [1] discussed a proposal for a pack refs v2 format
that would improve deletion speeds. The ref-table backend would also improve
performance of deletions. However, both of those proposals are still being
discussed.
1. https://lore.kernel.org/git/pull.1408.git.1667846164.gitgitgadget@gmail.com/
John Cai (3):
docs: clarify git-pack-refs --all will pack all refs
pack-refs: teach --exclude option to exclude refs from being packed
pack-refs: teach pack-refs --include option
Documentation/git-pack-refs.txt | 32 ++++++++++++++++++++++++++++----
builtin/pack-refs.c | 27 +++++++++++++++++++++++----
refs.c | 4 ++--
refs.h | 8 +++++++-
refs/debug.c | 4 ++--
refs/files-backend.c | 24 ++++++++++++++++++------
refs/packed-backend.c | 2 +-
refs/refs-internal.h | 3 ++-
t/helper/test-ref-store.c | 3 ++-
t/t3210-pack-refs.sh | 28 ++++++++++++++++++++++++++++
10 files changed, 113 insertions(+), 22 deletions(-)
base-commit: f85cd430b12b0d3e4f1a30ef3239a1b73d5f6331
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1501%2Fjohn-cai%2Fjc%2Fexclude-refs-from-pack-refs-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1501/john-cai/jc/exclude-refs-from-pack-refs-v2
Pull-Request: https://github.com/git/git/pull/1501
Range-diff vs v1:
-: ----------- > 1: 0b40b24b95d docs: clarify git-pack-refs --all will pack all refs
1: fb4d5b911ae ! 2: 027b3f85a0b pack-refs: teach --exclude option to exclude refs from being packed
@@
## Metadata ##
-Author: John Cai <jcai@gitlab.com>
+Author: John Cai <johncai86@gmail.com>
## Commit message ##
pack-refs: teach --exclude option to exclude refs from being packed
- Currently once refs are packed into a pack-refs file, deleting them can be
- slow since it involves a full file scan of the entire pack-refs file. 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 certain
- refs from a pack-refs file allows these internal references to be deleted
- much more efficiently.
+ 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
+ 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
- pack-refs file
+ packed-refs file
Signed-off-by: John Cai <johncai86@gmail.com>
@@ Documentation/git-pack-refs.txt: git-pack-refs - Pack heads and tags for efficie
DESCRIPTION
-----------
-@@ Documentation/git-pack-refs.txt: a repository with many branches of historical interests.
+@@ Documentation/git-pack-refs.txt: interests.
The command usually removes loose refs under `$GIT_DIR/refs`
hierarchy after packing them. This option tells it not to.
@@ Documentation/git-pack-refs.txt: a repository with many branches of historical i
+
+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.
++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`.
+
BUGS
@@ builtin/pack-refs.c
{
unsigned int flags = PACK_REFS_PRUNE;
+ static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
-+ struct pack_refs_opts pack_opts = {.exclusions = &excludes};
++ struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
+ static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
+ struct string_list_item *item;
+
struct option opts[] = {
- OPT_BIT(0, "all", &flags, N_("pack everything"), PACK_REFS_ALL),
- OPT_BIT(0, "prune", &flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+- OPT_BIT(0, "all", &flags, N_("pack everything"), PACK_REFS_ALL),
+- OPT_BIT(0, "prune", &flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
++ OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
++ OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
+ N_("references to exclude")),
OPT_END(),
@@ builtin/pack-refs.c
- return refs_pack_refs(get_main_ref_store(the_repository), flags);
+
+ for_each_string_list_item(item, &option_excluded_refs)
-+ add_ref_exclusion(pack_opts.exclusions, item->string);
++ add_ref_exclusion(pack_refs_opts.exclusions, item->string);
+
-+ return refs_pack_refs(get_main_ref_store(the_repository), flags, &pack_opts);
++ return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
}
## refs.c ##
@@ refs.c: void base_ref_store_init(struct ref_store *refs, struct repository *repo
/* backend functions */
-int refs_pack_refs(struct ref_store *refs, unsigned int flags)
-+int refs_pack_refs(struct ref_store *refs, unsigned int flags, struct pack_refs_opts *pack_opts)
++int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)
{
- return refs->be->pack_refs(refs, flags);
-+ return refs->be->pack_refs(refs, flags, pack_opts);
++ return refs->be->pack_refs(refs, opts);
}
int peel_iterated_oid(const struct object_id *base, struct object_id *peeled)
@@ refs.h: struct worktree;
#define RESOLVE_REF_ALLOW_BAD_NAME 0x04
+struct pack_refs_opts {
++ unsigned int flags;
+ struct ref_exclusions *exclusions;
+};
+
@@ refs.h: void warn_dangling_symrefs(FILE *fp, const char *msg_fmt,
* flags: Combination of the above PACK_REFS_* flags.
*/
-int refs_pack_refs(struct ref_store *refs, unsigned int flags);
-+int refs_pack_refs(struct ref_store *refs, unsigned int flags, struct pack_refs_opts *opts);
++int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts);
/*
* Setup reflog before using. Fill in err and return -1 on failure.
@@ refs/debug.c: static int debug_initial_transaction_commit(struct ref_store *refs
}
-static int debug_pack_refs(struct ref_store *ref_store, unsigned int flags)
-+static int debug_pack_refs(struct ref_store *ref_store, unsigned int flags, struct pack_refs_opts *opts)
++static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
- int res = drefs->refs->be->pack_refs(drefs->refs, flags);
-+ int res = drefs->refs->be->pack_refs(drefs->refs, flags, opts);
++ int res = drefs->refs->be->pack_refs(drefs->refs, opts);
trace_printf_key(&trace_refs, "pack_refs: %d\n", res);
return res;
}
@@ refs/files-backend.c: static void prune_refs(struct files_ref_store *refs, struc
static int should_pack_ref(const char *refname,
const struct object_id *oid, unsigned int ref_flags,
- unsigned int pack_flags)
-+ unsigned int pack_flags,
+ struct pack_refs_opts *opts)
{
/* Do not pack per-worktree refs: */
if (parse_worktree_ref(refname, NULL, NULL, NULL) !=
+@@ refs/files-backend.c: 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: */
@@ refs/files-backend.c: static int should_pack_ref(const char *refname,
if (!ref_resolves_to_object(refname, the_repository, oid, ref_flags))
return 0;
@@ refs/files-backend.c: static int should_pack_ref(const char *refname,
-static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int files_pack_refs(struct ref_store *ref_store,
-+ unsigned int flags,
-+ struct pack_refs_opts *pack_opts)
++ struct pack_refs_opts *opts)
{
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,
@@ refs/files-backend.c: static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
+ * in the packed ref cache. If the reference should be
* pruned, also add it to refs_to_prune.
*/
- if (!should_pack_ref(iter->refname, iter->oid, iter->flags,
+- if (!should_pack_ref(iter->refname, iter->oid, iter->flags,
- flags))
-+ flags, pack_opts))
++ if (!should_pack_ref(iter->refname, iter->oid, iter->flags, opts))
continue;
/*
+@@ refs/files-backend.c: static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
+ iter->refname, err.buf);
+
+ /* Schedule the loose reference for pruning if requested. */
+- if ((flags & PACK_REFS_PRUNE)) {
++ if ((opts->flags & PACK_REFS_PRUNE)) {
+ struct ref_to_prune *n;
+ FLEX_ALLOC_STR(n, name, iter->refname);
+ oidcpy(&n->oid, iter->oid);
## refs/packed-backend.c ##
@@ refs/packed-backend.c: static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
@@ refs/packed-backend.c: static int packed_delete_refs(struct ref_store *ref_store
static int packed_pack_refs(struct ref_store *ref_store UNUSED,
- unsigned int flags UNUSED)
-+ unsigned int flags UNUSED,
+ struct pack_refs_opts *pack_opts UNUSED)
{
/*
@@ refs/refs-internal.h: typedef int ref_transaction_commit_fn(struct ref_store *re
-typedef int pack_refs_fn(struct ref_store *ref_store, unsigned int flags);
+typedef int pack_refs_fn(struct ref_store *ref_store,
-+ unsigned int flags,
+ struct pack_refs_opts *opts);
typedef int create_symref_fn(struct ref_store *ref_store,
const char *ref_target,
@@ t/helper/test-ref-store.c: static struct flag_definition pack_flags[] = { FLAG_D
static int cmd_pack_refs(struct ref_store *refs, const char **argv)
{
unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
-+ struct pack_refs_opts pack_opts = { 0 };
++ struct pack_refs_opts pack_opts = { .flags = flags };
- return refs_pack_refs(refs, flags);
-+ return refs_pack_refs(refs, flags, &pack_opts);
++ return refs_pack_refs(refs, &pack_opts);
}
static int cmd_create_symref(struct ref_store *refs, const char **argv)
-: ----------- > 3: 03950e8f120 pack-refs: teach pack-refs --include option
--
gitgitgadget
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 1/3] docs: clarify git-pack-refs --all will pack all refs
2023-05-09 19:18 ` [PATCH v2 0/3] pack-refs: Teach " John Cai via GitGitGadget
@ 2023-05-09 19:18 ` John Cai via GitGitGadget
2023-05-09 19:18 ` [PATCH v2 2/3] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
` (2 subsequent siblings)
3 siblings, 0 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-09 19:18 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, John Cai, John Cai
From: John Cai <johncai86@gmail.com>
--all packs not just branch tips but anything under refs/ with the
exception of hidden refs and broken refs. Clarify this in the
documentation.
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-pack-refs.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
index 154081f2de2..e011e5fead3 100644
--- a/Documentation/git-pack-refs.txt
+++ b/Documentation/git-pack-refs.txt
@@ -49,10 +49,11 @@ OPTIONS
The command by default packs all tags and refs that are already
packed, and leaves other refs
-alone. This is because branches are expected to be actively
+alone. This is because branches are expected to be actively
developed and packing their tips does not help performance.
-This option causes branch tips to be packed as well. Useful for
-a repository with many branches of historical interests.
+This option causes all refs to be packed as well, with the exception of hidden
+and broken refs. Useful for a repository with many branches of historical
+interests.
--no-prune::
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 2/3] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-09 19:18 ` [PATCH v2 0/3] pack-refs: Teach " John Cai via GitGitGadget
2023-05-09 19:18 ` [PATCH v2 1/3] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
@ 2023-05-09 19:18 ` John Cai via GitGitGadget
2023-05-09 21:04 ` Junio C Hamano
2023-05-09 19:18 ` [PATCH v2 3/3] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
2023-05-11 18:10 ` [PATCH v3 0/4] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
3 siblings, 1 reply; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-09 19:18 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, John Cai, John Cai
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 not include
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 | 21 +++++++++++++++++----
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 ++-
t/helper/test-ref-store.c | 3 ++-
t/t3210-pack-refs.sh | 18 ++++++++++++++++++
10 files changed, 71 insertions(+), 19 deletions(-)
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`.
+
BUGS
----
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 9833815fb30..3dcbd6e2421 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -4,22 +4,35 @@
#include "parse-options.h"
#include "refs.h"
#include "repository.h"
+#include "revision.h"
static char const * const pack_refs_usage[] = {
- N_("git pack-refs [--all] [--no-prune]"),
+ N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
NULL
};
int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
+ static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+ struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
+ static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
+ struct string_list_item *item;
+
struct option opts[] = {
- OPT_BIT(0, "all", &flags, N_("pack everything"), PACK_REFS_ALL),
- OPT_BIT(0, "prune", &flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
+ OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
+ N_("references to exclude")),
OPT_END(),
};
+
git_config(git_default_config, NULL);
if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
- return refs_pack_refs(get_main_ref_store(the_repository), flags);
+
+ for_each_string_list_item(item, &option_excluded_refs)
+ add_ref_exclusion(pack_refs_opts.exclusions, item->string);
+
+ return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
}
diff --git a/refs.c b/refs.c
index d2a98e1c21f..881a0da65cf 100644
--- a/refs.c
+++ b/refs.c
@@ -2132,9 +2132,9 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
}
/* backend functions */
-int refs_pack_refs(struct ref_store *refs, unsigned int flags)
+int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)
{
- return refs->be->pack_refs(refs, flags);
+ return refs->be->pack_refs(refs, opts);
}
int peel_iterated_oid(const struct object_id *base, struct object_id *peeled)
diff --git a/refs.h b/refs.h
index 123cfa44244..46020bd335c 100644
--- a/refs.h
+++ b/refs.h
@@ -63,6 +63,11 @@ struct worktree;
#define RESOLVE_REF_NO_RECURSE 0x02
#define RESOLVE_REF_ALLOW_BAD_NAME 0x04
+struct pack_refs_opts {
+ unsigned int flags;
+ struct ref_exclusions *exclusions;
+};
+
const char *refs_resolve_ref_unsafe(struct ref_store *refs,
const char *refname,
int resolve_flags,
@@ -405,7 +410,7 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt,
* Write a packed-refs file for the current repository.
* flags: Combination of the above PACK_REFS_* flags.
*/
-int refs_pack_refs(struct ref_store *refs, unsigned int flags);
+int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts);
/*
* Setup reflog before using. Fill in err and return -1 on failure.
diff --git a/refs/debug.c b/refs/debug.c
index adc34c836fc..63f434ea26b 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -122,10 +122,10 @@ static int debug_initial_transaction_commit(struct ref_store *refs,
return res;
}
-static int debug_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
- int res = drefs->refs->be->pack_refs(drefs->refs, flags);
+ int res = drefs->refs->be->pack_refs(drefs->refs, opts);
trace_printf_key(&trace_refs, "pack_refs: %d\n", res);
return res;
}
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;
+
return 1;
}
-static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int files_pack_refs(struct ref_store *ref_store,
+ struct pack_refs_opts *opts)
{
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,
@@ -1220,8 +1225,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
* in the packed ref cache. If the reference should be
* pruned, also add it to refs_to_prune.
*/
- if (!should_pack_ref(iter->refname, iter->oid, iter->flags,
- flags))
+ if (!should_pack_ref(iter->refname, iter->oid, iter->flags, opts))
continue;
/*
@@ -1235,7 +1239,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
iter->refname, err.buf);
/* Schedule the loose reference for pruning if requested. */
- if ((flags & PACK_REFS_PRUNE)) {
+ if ((opts->flags & PACK_REFS_PRUNE)) {
struct ref_to_prune *n;
FLEX_ALLOC_STR(n, name, iter->refname);
oidcpy(&n->oid, iter->oid);
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 2333ed5a1f7..9a2dd10dba7 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1576,7 +1576,7 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
}
static int packed_pack_refs(struct ref_store *ref_store UNUSED,
- unsigned int flags UNUSED)
+ struct pack_refs_opts *pack_opts UNUSED)
{
/*
* Packed refs are already packed. It might be that loose refs
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index a85d113123c..f72b7be8941 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -547,7 +547,8 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err);
-typedef int pack_refs_fn(struct ref_store *ref_store, unsigned int flags);
+typedef int pack_refs_fn(struct ref_store *ref_store,
+ struct pack_refs_opts *opts);
typedef int create_symref_fn(struct ref_store *ref_store,
const char *ref_target,
const char *refs_heads_master,
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 6d8f844e9c7..de4197708d9 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -116,8 +116,9 @@ static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
static int cmd_pack_refs(struct ref_store *refs, const char **argv)
{
unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
+ struct pack_refs_opts pack_opts = { .flags = flags };
- return refs_pack_refs(refs, flags);
+ return refs_pack_refs(refs, &pack_opts);
}
static int cmd_create_symref(struct ref_store *refs, const char **argv)
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* &&
+ 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 &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 3/3] pack-refs: teach pack-refs --include option
2023-05-09 19:18 ` [PATCH v2 0/3] pack-refs: Teach " John Cai via GitGitGadget
2023-05-09 19:18 ` [PATCH v2 1/3] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
2023-05-09 19:18 ` [PATCH v2 2/3] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
@ 2023-05-09 19:18 ` John Cai via GitGitGadget
2023-05-09 21:25 ` Junio C Hamano
2023-05-11 18:10 ` [PATCH v3 0/4] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
3 siblings, 1 reply; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-09 19:18 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, John Cai, John Cai
From: John Cai <johncai86@gmail.com>
Allow users to be more selective over which refs to pack by adding an
--include option to git-pack-refs.
The existing options allow some measure of selectivity. By default
git-pack-refs packs all tags. --all can be used to include all refs,
and the previous commit added the ability to exclude certain refs with
--exclude.
While these knobs give the user some selection over which refs to pack,
it could be useful to give more control. For instance, a repository may
have a set of branches that are rarely updated and would benefit from
being packed. --include would allow the user to easily include a set of
branches to be packed while leaving everything else unpacked.
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-pack-refs.txt | 15 ++++++++++++++-
builtin/pack-refs.c | 10 ++++++++--
refs.h | 1 +
refs/files-backend.c | 14 +++++++++++---
t/t3210-pack-refs.sh | 10 ++++++++++
5 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
index c0f7426e519..f187925bdc0 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] [--exclude <pattern>]
+'git pack-refs' [--all] [--no-prune] [--include <pattern>] [--exclude <pattern>]
DESCRIPTION
-----------
@@ -60,6 +60,15 @@ interests.
The command usually removes loose refs under `$GIT_DIR/refs`
hierarchy after packing them. This option tells it not to.
+--include <pattern>::
+
+Pack refs based on a `glob(7)` pattern. Repetitions of this option
+accumulate inclusion patterns. If a ref is both included in `--include` and
+`--exclude`, `--exclude` takes precedence. Using `--include` does not preclude
+all tags from being included by default. Symbolic refs and broken refs will never
+be packed. When used with `--all`, it will be a noop. Use `--no-include` to clear
+and reset the list of patterns.
+
--exclude <pattern>::
Do not pack refs matching the given `glob(7)` pattern. Repetitions of this option
@@ -70,6 +79,10 @@ unpack it.
When used with `--all`, it will use the difference between the set of all refs,
and what is provided to `--exclude`.
+When used with `--include`, it will use what is provided to `--include` as well
+as the the default of all tags and already packed refs, minus refs that are
+provided to `--exclude`.
+
BUGS
----
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 3dcbd6e2421..293cdd17181 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -7,7 +7,7 @@
#include "revision.h"
static char const * const pack_refs_usage[] = {
- N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
+ N_("git pack-refs [--all] [--no-prune] [--include <pattern>] [--exclude <pattern>]"),
NULL
};
@@ -15,13 +15,19 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
- struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
+ static struct string_list included_refs = STRING_LIST_INIT_NODUP;
+ struct pack_refs_opts pack_refs_opts = { .exclusions = &excludes,
+ .flags = flags,
+ .included_refs = &included_refs };
+
static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
struct string_list_item *item;
struct option opts[] = {
OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_STRING_LIST(0, "include", pack_refs_opts.included_refs, N_("pattern"),
+ N_("references to include")),
OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
N_("references to exclude")),
OPT_END(),
diff --git a/refs.h b/refs.h
index 46020bd335c..2f91f4c4a90 100644
--- a/refs.h
+++ b/refs.h
@@ -66,6 +66,7 @@ struct worktree;
struct pack_refs_opts {
unsigned int flags;
struct ref_exclusions *exclusions;
+ struct string_list *included_refs;
};
const char *refs_resolve_ref_unsafe(struct ref_store *refs,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 6a51267f379..3f8974a4a32 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1181,6 +1181,17 @@ static int should_pack_ref(const char *refname,
REF_WORKTREE_SHARED)
return 0;
+ if (opts->exclusions && ref_excluded(opts->exclusions, refname))
+ return 0;
+
+ if (opts->included_refs && opts->included_refs->nr) {
+ struct string_list_item *item;
+
+ for_each_string_list_item(item, opts->included_refs)
+ if (!wildmatch(item->string, refname, 0))
+ return 1;
+ }
+
/* Do not pack non-tags unless PACK_REFS_ALL is set: */
if (!(opts->flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
return 0;
@@ -1193,9 +1204,6 @@ 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;
-
return 1;
}
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index 31b9f72e84a..86b3ff6a3e0 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -126,6 +126,16 @@ test_expect_success \
! test -f .git/refs/heads/dont_pack3 &&
! test -f .git/refs/heads/dont_pack4'
+test_expect_success \
+ 'test only included refs are packed' '
+ git branch pack_this1 &&
+ git branch pack_this2 &&
+ git tag dont_pack5 &&
+ git pack-refs --include refs/tags/pack_this* --exclude refs/tags/dont_pack* &&
+ test -f .git/refs/tags/dont_pack5 &&
+ ! test -f ./git/refs/heads/pack_this1 &&
+ ! test -f ./git/refs/heads/pack_this2'
+
test_expect_success \
'see if up-to-date packed refs are preserved' \
'git branch q &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-09 19:18 ` [PATCH v2 2/3] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
@ 2023-05-09 21:04 ` Junio C Hamano
0 siblings, 0 replies; 29+ messages in thread
From: Junio C Hamano @ 2023-05-09 21:04 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, Christian Couder, John Cai
"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 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 &&
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 3/3] pack-refs: teach pack-refs --include option
2023-05-09 19:18 ` [PATCH v2 3/3] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
@ 2023-05-09 21:25 ` Junio C Hamano
2023-05-10 19:52 ` John Cai
0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2023-05-09 21:25 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, Christian Couder, John Cai
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +--include <pattern>::
> +
> +Pack refs based on a `glob(7)` pattern. Repetitions of this option
> +accumulate inclusion patterns. If a ref is both included in `--include` and
> +`--exclude`, `--exclude` takes precedence. Using `--include` does not preclude
> +all tags from being included by default. Symbolic refs and broken refs will never
> +be packed. When used with `--all`, it will be a noop. Use `--no-include` to clear
> +and reset the list of patterns.
Hmph, that was a bit unexpected. exclude taking precedence over
include is very much in line with how negative pathspec works and
the end-users should be familiar with it, but when the user bothers
to specify with --include what to include, I would have expected
that the "pack tags by default" would be defeated.
In other words, I would have expected that the program acts as if
the machinery works this way (iow, the code does not have to exactly
implement it this way---it just has to behave as if it did):
- it maintains two pattern list, positive and negative,
both start empty.
- "--exclude" are accumulated to the negative list.
- "--include" are accumulated to the positive list.
- "--all" adds "*" to the positive list.
- after parsing command line options, if the positive list is
empty, then "refs/tags/*" is added to the positive list.
- refs that match positive list but does not match negative list
are shown.
> +When used with `--include`, it will use what is provided to `--include` as well
> +as the the default of all tags and already packed refs, minus refs that are
> +provided to `--exclude`.
IOW, I would expect that the use of "--include" alone is enough to
defeat the default; the end user does not have to figure out that
they have to pass "--exclude=refs/tags/*" to do so when they are
specifying a specific hierarchy to include.
> @@ -66,6 +66,7 @@ struct worktree;
> struct pack_refs_opts {
> unsigned int flags;
> struct ref_exclusions *exclusions;
> + struct string_list *included_refs;
It is unfortunate that the struct is called ref_exclusions to imply
as if it is only usable for excluding refs from listing. It has
other members for handling hidden refs, and it would have been very
natural to extend it to also add included_refs pattern next to
excluded_refs string list. After all, the struct is used to tweak
which refs are included and which refs are excluded, and
historically everything was included unless listed on the excluded
pattern. We are now allowing the "everything is included" part to
be customizable with this step. If the struct were named with a
more neutral term, like ref_visibility to hint that it is about
setting visibility, then this patch wouldn't have added a separate
string list to this structure---instead it would have extended the
ref_exclusions (with a better name) struct and placed included_refs
string list there.
> };
>
> const char *refs_resolve_ref_unsafe(struct ref_store *refs,
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 6a51267f379..3f8974a4a32 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1181,6 +1181,17 @@ static int should_pack_ref(const char *refname,
> REF_WORKTREE_SHARED)
> return 0;
>
> + if (opts->exclusions && ref_excluded(opts->exclusions, refname))
> + return 0;
> +
> + if (opts->included_refs && opts->included_refs->nr) {
> + struct string_list_item *item;
> +
> + for_each_string_list_item(item, opts->included_refs)
> + if (!wildmatch(item->string, refname, 0))
> + return 1;
> + }
We can see why the initial placement of exclusion logic in the
earlier step was suboptimal here.
> /* Do not pack non-tags unless PACK_REFS_ALL is set: */
> if (!(opts->flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
> return 0;
> @@ -1193,9 +1204,6 @@ 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;
> -
> return 1;
> }
Other than that, the changes look mostly expected and no surprises.
Thanks.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 3/3] pack-refs: teach pack-refs --include option
2023-05-09 21:25 ` Junio C Hamano
@ 2023-05-10 19:52 ` John Cai
0 siblings, 0 replies; 29+ messages in thread
From: John Cai @ 2023-05-10 19:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Cai via GitGitGadget, git, Christian Couder
On 23/05/09 02:25PM, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +--include <pattern>::
> > +
> > +Pack refs based on a `glob(7)` pattern. Repetitions of this option
> > +accumulate inclusion patterns. If a ref is both included in `--include` and
> > +`--exclude`, `--exclude` takes precedence. Using `--include` does not preclude
> > +all tags from being included by default. Symbolic refs and broken refs will never
> > +be packed. When used with `--all`, it will be a noop. Use `--no-include` to clear
> > +and reset the list of patterns.
>
> Hmph, that was a bit unexpected. exclude taking precedence over
> include is very much in line with how negative pathspec works and
> the end-users should be familiar with it, but when the user bothers
> to specify with --include what to include, I would have expected
> that the "pack tags by default" would be defeated.
>
> In other words, I would have expected that the program acts as if
> the machinery works this way (iow, the code does not have to exactly
> implement it this way---it just has to behave as if it did):
>
> - it maintains two pattern list, positive and negative,
> both start empty.
> - "--exclude" are accumulated to the negative list.
> - "--include" are accumulated to the positive list.
> - "--all" adds "*" to the positive list.
> - after parsing command line options, if the positive list is
> empty, then "refs/tags/*" is added to the positive list.
> - refs that match positive list but does not match negative list
> are shown.
>
> > +When used with `--include`, it will use what is provided to `--include` as well
> > +as the the default of all tags and already packed refs, minus refs that are
> > +provided to `--exclude`.
>
> IOW, I would expect that the use of "--include" alone is enough to
> defeat the default; the end user does not have to figure out that
> they have to pass "--exclude=refs/tags/*" to do so when they are
> specifying a specific hierarchy to include.
Hm yeah, I think that is a nicer user experience.
>
> > @@ -66,6 +66,7 @@ struct worktree;
> > struct pack_refs_opts {
> > unsigned int flags;
> > struct ref_exclusions *exclusions;
> > + struct string_list *included_refs;
>
> It is unfortunate that the struct is called ref_exclusions to imply
> as if it is only usable for excluding refs from listing. It has
> other members for handling hidden refs, and it would have been very
> natural to extend it to also add included_refs pattern next to
> excluded_refs string list. After all, the struct is used to tweak
> which refs are included and which refs are excluded, and
> historically everything was included unless listed on the excluded
> pattern. We are now allowing the "everything is included" part to
> be customizable with this step. If the struct were named with a
> more neutral term, like ref_visibility to hint that it is about
> setting visibility, then this patch wouldn't have added a separate
> string list to this structure---instead it would have extended the
> ref_exclusions (with a better name) struct and placed included_refs
> string list there.
Thanks for calling this out. I was thinking along very similar lines when
working on this patch, but was too lazy to make the change :)
>
> > };
> >
> > const char *refs_resolve_ref_unsafe(struct ref_store *refs,
> > diff --git a/refs/files-backend.c b/refs/files-backend.c
> > index 6a51267f379..3f8974a4a32 100644
> > --- a/refs/files-backend.c
> > +++ b/refs/files-backend.c
> > @@ -1181,6 +1181,17 @@ static int should_pack_ref(const char *refname,
> > REF_WORKTREE_SHARED)
> > return 0;
> >
> > + if (opts->exclusions && ref_excluded(opts->exclusions, refname))
> > + return 0;
> > +
> > + if (opts->included_refs && opts->included_refs->nr) {
> > + struct string_list_item *item;
> > +
> > + for_each_string_list_item(item, opts->included_refs)
> > + if (!wildmatch(item->string, refname, 0))
> > + return 1;
> > + }
>
> We can see why the initial placement of exclusion logic in the
> earlier step was suboptimal here.
>
> > /* Do not pack non-tags unless PACK_REFS_ALL is set: */
> > if (!(opts->flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
> > return 0;
> > @@ -1193,9 +1204,6 @@ 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;
> > -
> > return 1;
> > }
>
>
> Other than that, the changes look mostly expected and no surprises.
>
> Thanks.
thanks
John
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 0/4] pack-refs: allow users control over which refs to pack
2023-05-09 19:18 ` [PATCH v2 0/3] pack-refs: Teach " John Cai via GitGitGadget
` (2 preceding siblings ...)
2023-05-09 19:18 ` [PATCH v2 3/3] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
@ 2023-05-11 18:10 ` John Cai via GitGitGadget
2023-05-11 18:10 ` [PATCH v3 1/4] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
` (4 more replies)
3 siblings, 5 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-11 18:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, John Cai
git-pack-refs does not currently give much control over which refs are
packed. By default, all tags and already packed refs are packed. --all
allows the user to pack all refs. Beyond this the user does not have
control. Introduce a pair of options --include and --exclude that will allow
users full control over which refs end up in the packed-refs file.
Changes since v2:
* repurpose ref_exclusions to be used for ref inclusions
* fixed test formatting
* adjusted --include to not include default of all tags
Changes since v1:
* Clarify that --all packs not just branch tips but all refs under refs/ in
the documentation in patch 1
* Add --include in patch 3
It's worth noting that [1] discussed a proposal for a pack refs v2 format
that would improve deletion speeds. The ref-table backend would also improve
performance of deletions. However, both of those proposals are still being
discussed.
1. https://lore.kernel.org/git/pull.1408.git.1667846164.gitgitgadget@gmail.com/
John Cai (4):
docs: clarify git-pack-refs --all will pack all refs
pack-refs: teach --exclude option to exclude refs from being packed
revision: modify ref_exclusions to handle inclusions
pack-refs: teach pack-refs --include option
Documentation/git-pack-refs.txt | 31 ++++++++++++--
builtin/pack-refs.c | 34 ++++++++++++++--
builtin/rev-parse.c | 18 ++++-----
refs.c | 4 +-
refs.h | 7 +++-
refs/debug.c | 4 +-
refs/files-backend.c | 23 ++++++-----
refs/packed-backend.c | 2 +-
refs/refs-internal.h | 3 +-
revision.c | 71 ++++++++++++++++++++-------------
revision.h | 28 ++++++++-----
t/helper/test-ref-store.c | 9 ++++-
t/t3210-pack-refs.sh | 37 +++++++++++++++++
13 files changed, 199 insertions(+), 72 deletions(-)
base-commit: 91428f078b8a4fe6948a4c955af1a693841e3985
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1501%2Fjohn-cai%2Fjc%2Fexclude-refs-from-pack-refs-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1501/john-cai/jc/exclude-refs-from-pack-refs-v3
Pull-Request: https://github.com/git/git/pull/1501
Range-diff vs v2:
1: 0b40b24b95d = 1: 0d462010b79 docs: clarify git-pack-refs --all will pack all refs
2: 027b3f85a0b ! 2: 8c5c66a3050 pack-refs: teach --exclude option to exclude refs from being packed
@@ Commit message
pack-refs: teach --exclude option to exclude refs from being packed
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
+ 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.
@@ builtin/pack-refs.c
+ N_("references to exclude")),
OPT_END(),
};
-+
git_config(git_default_config, NULL);
if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
@@ refs/files-backend.c: static void prune_refs(struct files_ref_store *refs, struc
{
/* Do not pack per-worktree refs: */
if (parse_worktree_ref(refname, NULL, NULL, NULL) !=
-@@ refs/files-backend.c: static int should_pack_ref(const char *refname,
+ REF_WORKTREE_SHARED)
return 0;
++ if (opts->exclusions && ref_excluded(opts->exclusions, 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/"))
@@ refs/files-backend.c: static int should_pack_ref(const char *refname,
/* Do not pack symbolic refs: */
@@ refs/files-backend.c: 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;
-+
return 1;
}
@@ refs/refs-internal.h: typedef int ref_transaction_commit_fn(struct ref_store *re
const char *ref_target,
const char *refs_heads_master,
+ ## revision.h ##
+@@ revision.h: struct rev_cmdline_info {
+ struct ref_exclusions {
+ /*
+ * Excluded refs is a list of wildmatch patterns. If any of the
+- * patterns matches, the reference will be excluded.
++ * patterns match, the reference will be excluded.
+ */
+ struct string_list excluded_refs;
+
+
## t/helper/test-ref-store.c ##
@@ t/helper/test-ref-store.c: static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
static int cmd_pack_refs(struct ref_store *refs, const char **argv)
@@ t/t3210-pack-refs.sh: 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* &&
-+ 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 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*" &&
++ 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 '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' \
-: ----------- > 3: 0a0693ad612 revision: modify ref_exclusions to handle inclusions
3: 03950e8f120 ! 4: b2f3b98cd24 pack-refs: teach pack-refs --include option
@@ Documentation/git-pack-refs.txt: interests.
+
+Pack refs based on a `glob(7)` pattern. Repetitions of this option
+accumulate inclusion patterns. If a ref is both included in `--include` and
-+`--exclude`, `--exclude` takes precedence. Using `--include` does not preclude
-+all tags from being included by default. Symbolic refs and broken refs will never
++`--exclude`, `--exclude` takes precedence. Using `--include` will preclude all
++tags from being included by default. Symbolic refs and broken refs will never
+be packed. When used with `--all`, it will be a noop. Use `--no-include` to clear
+and reset the list of patterns.
+
@@ Documentation/git-pack-refs.txt: unpack it.
When used with `--all`, it will use the difference between the set of all refs,
and what is provided to `--exclude`.
-+When used with `--include`, it will use what is provided to `--include` as well
-+as the the default of all tags and already packed refs, minus refs that are
-+provided to `--exclude`.
++When used with `--include`, refs provided to `--include`, minus refs that are
++provided to `--exclude` will be packed.
+
BUGS
@@ Documentation/git-pack-refs.txt: unpack it.
## builtin/pack-refs.c ##
@@
+ #include "refs.h"
+ #include "repository.h"
#include "revision.h"
++#include "trace.h"
static char const * const pack_refs_usage[] = {
- N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
@@ builtin/pack-refs.c
@@ builtin/pack-refs.c: int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
- static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
-- struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
-+ static struct string_list included_refs = STRING_LIST_INIT_NODUP;
-+ struct pack_refs_opts pack_refs_opts = { .exclusions = &excludes,
-+ .flags = flags,
-+ .included_refs = &included_refs };
-+
+ static struct ref_visibility visibility = REF_VISIBILITY_INIT;
+- struct pack_refs_opts pack_refs_opts = {.visibility = &visibility, .flags = flags};
++ struct pack_refs_opts pack_refs_opts = { .visibility = &visibility,
++ .flags = flags };
static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
++ static struct string_list option_included_refs = STRING_LIST_INIT_NODUP;
struct string_list_item *item;
struct option opts[] = {
OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
-+ OPT_STRING_LIST(0, "include", pack_refs_opts.included_refs, N_("pattern"),
++ OPT_STRING_LIST(0, "include", &option_included_refs, N_("pattern"),
+ N_("references to include")),
OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
N_("references to exclude")),
OPT_END(),
-
- ## refs.h ##
-@@ refs.h: struct worktree;
- struct pack_refs_opts {
- unsigned int flags;
- struct ref_exclusions *exclusions;
-+ struct string_list *included_refs;
- };
+@@ builtin/pack-refs.c: int cmd_pack_refs(int argc, const char **argv, const char *prefix)
+ for_each_string_list_item(item, &option_excluded_refs)
+ add_ref_exclusion(pack_refs_opts.visibility, item->string);
- const char *refs_resolve_ref_unsafe(struct ref_store *refs,
++ for_each_string_list_item(item, &option_included_refs)
++ add_ref_inclusion(pack_refs_opts.visibility, item->string);
++
++ if (pack_refs_opts.flags & PACK_REFS_ALL)
++ add_ref_inclusion(pack_refs_opts.visibility, "*");
++
++ if (!pack_refs_opts.visibility->included_refs.nr)
++ add_ref_inclusion(pack_refs_opts.visibility, "refs/tags/*");
++
+ return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
+ }
## refs/files-backend.c ##
@@ refs/files-backend.c: static int should_pack_ref(const char *refname,
REF_WORKTREE_SHARED)
return 0;
-+ if (opts->exclusions && ref_excluded(opts->exclusions, refname))
-+ return 0;
-+
-+ if (opts->included_refs && opts->included_refs->nr) {
-+ struct string_list_item *item;
-+
-+ for_each_string_list_item(item, opts->included_refs)
-+ if (!wildmatch(item->string, refname, 0))
-+ return 1;
-+ }
-+
- /* Do not pack non-tags unless PACK_REFS_ALL is set: */
- if (!(opts->flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
+- if (opts->visibility && ref_excluded(opts->visibility, refname))
+- return 0;
+-
+- /* Do not pack non-tags unless PACK_REFS_ALL is set: */
+- if (!(opts->flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
+- return 0;
+-
+ /* Do not pack symbolic refs: */
+ if (ref_flags & REF_ISSYMREF)
return 0;
@@ refs/files-backend.c: 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;
--
- return 1;
+- return 1;
++ if (opts->visibility && ref_excluded(opts->visibility, refname))
++ return 0;
++
++ if (opts->visibility && ref_included(opts->visibility, refname))
++ return 1;
++
++ return 0;
}
+ static int files_pack_refs(struct ref_store *ref_store,
+
+ ## t/helper/test-ref-store.c ##
+@@
+ #include "worktree.h"
+ #include "object-store.h"
+ #include "repository.h"
++#include "revision.h"
+
+ struct flag_definition {
+ const char *name;
+@@ t/helper/test-ref-store.c: static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
+ static int cmd_pack_refs(struct ref_store *refs, const char **argv)
+ {
+ unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
+- struct pack_refs_opts pack_opts = { .flags = flags };
++ static struct ref_visibility visibility = REF_VISIBILITY_INIT;
++ struct pack_refs_opts pack_opts = { .flags = flags,
++ .visibility = &visibility };
++
++ if (pack_opts.flags & PACK_REFS_ALL)
++ add_ref_inclusion(pack_opts.visibility, "*");
+
+ return refs_pack_refs(refs, &pack_opts);
+ }
## t/t3210-pack-refs.sh ##
-@@ t/t3210-pack-refs.sh: test_expect_success \
- ! test -f .git/refs/heads/dont_pack3 &&
- ! test -f .git/refs/heads/dont_pack4'
+@@ t/t3210-pack-refs.sh: test_expect_success 'test --no-exclude refs clears excluded refs' '
+ ! test -f .git/refs/heads/dont_pack3 &&
+ ! test -f .git/refs/heads/dont_pack4'
-+test_expect_success \
-+ 'test only included refs are packed' '
-+ git branch pack_this1 &&
-+ git branch pack_this2 &&
-+ git tag dont_pack5 &&
-+ git pack-refs --include refs/tags/pack_this* --exclude refs/tags/dont_pack* &&
-+ test -f .git/refs/tags/dont_pack5 &&
-+ ! test -f ./git/refs/heads/pack_this1 &&
-+ ! test -f ./git/refs/heads/pack_this2'
++test_expect_success 'test only included refs are packed' '
++ git branch pack_this1 &&
++ git branch pack_this2 &&
++ git tag dont_pack5 &&
++ git pack-refs --include "refs/heads/pack_this*" &&
++ test -f .git/refs/tags/dont_pack5 &&
++ ! test -f ./git/refs/heads/pack_this1 &&
++ ! test -f ./git/refs/heads/pack_this2'
++
++test_expect_success 'test --no-include refs clears included refs' '
++ git branch pack1 &&
++ git branch pack2 &&
++ git pack-refs --include "refs/heads/pack*" --no-include &&
++ test -f .git/refs/heads/pack1 &&
++ test -f .git/refs/heads/pack2'
++
++test_expect_success 'test --exclude takes precedence over --include' '
++ git branch dont_pack5 &&
++ git pack-refs --include "refs/heads/pack*" --exclude "refs/heads/pack*" &&
++ test -f .git/refs/heads/dont_pack5'
+
test_expect_success \
'see if up-to-date packed refs are preserved' \
--
gitgitgadget
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 1/4] docs: clarify git-pack-refs --all will pack all refs
2023-05-11 18:10 ` [PATCH v3 0/4] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
@ 2023-05-11 18:10 ` John Cai via GitGitGadget
2023-05-11 23:53 ` Taylor Blau
2023-05-11 18:10 ` [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
` (3 subsequent siblings)
4 siblings, 1 reply; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-11 18:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, John Cai, John Cai
From: John Cai <johncai86@gmail.com>
--all packs not just branch tips but anything under refs/ with the
exception of hidden refs and broken refs. Clarify this in the
documentation.
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-pack-refs.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
index 154081f2de2..e011e5fead3 100644
--- a/Documentation/git-pack-refs.txt
+++ b/Documentation/git-pack-refs.txt
@@ -49,10 +49,11 @@ OPTIONS
The command by default packs all tags and refs that are already
packed, and leaves other refs
-alone. This is because branches are expected to be actively
+alone. This is because branches are expected to be actively
developed and packing their tips does not help performance.
-This option causes branch tips to be packed as well. Useful for
-a repository with many branches of historical interests.
+This option causes all refs to be packed as well, with the exception of hidden
+and broken refs. Useful for a repository with many branches of historical
+interests.
--no-prune::
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-11 18:10 ` [PATCH v3 0/4] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
2023-05-11 18:10 ` [PATCH v3 1/4] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
@ 2023-05-11 18:10 ` John Cai via GitGitGadget
2023-05-11 19:34 ` Junio C Hamano
2023-05-12 0:00 ` Taylor Blau
2023-05-11 18:10 ` [PATCH v3 3/4] revision: modify ref_exclusions to handle inclusions John Cai via GitGitGadget
` (2 subsequent siblings)
4 siblings, 2 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-11 18:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, John Cai, John Cai
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(-)
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`.
+
BUGS
----
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 9833815fb30..1d1a64fe386 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -4,22 +4,34 @@
#include "parse-options.h"
#include "refs.h"
#include "repository.h"
+#include "revision.h"
static char const * const pack_refs_usage[] = {
- N_("git pack-refs [--all] [--no-prune]"),
+ N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
NULL
};
int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
+ static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+ struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
+ static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
+ struct string_list_item *item;
+
struct option opts[] = {
- OPT_BIT(0, "all", &flags, N_("pack everything"), PACK_REFS_ALL),
- OPT_BIT(0, "prune", &flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
+ OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
+ N_("references to exclude")),
OPT_END(),
};
git_config(git_default_config, NULL);
if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
- return refs_pack_refs(get_main_ref_store(the_repository), flags);
+
+ for_each_string_list_item(item, &option_excluded_refs)
+ add_ref_exclusion(pack_refs_opts.exclusions, item->string);
+
+ return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
}
diff --git a/refs.c b/refs.c
index d2a98e1c21f..881a0da65cf 100644
--- a/refs.c
+++ b/refs.c
@@ -2132,9 +2132,9 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
}
/* backend functions */
-int refs_pack_refs(struct ref_store *refs, unsigned int flags)
+int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)
{
- return refs->be->pack_refs(refs, flags);
+ return refs->be->pack_refs(refs, opts);
}
int peel_iterated_oid(const struct object_id *base, struct object_id *peeled)
diff --git a/refs.h b/refs.h
index 123cfa44244..46020bd335c 100644
--- a/refs.h
+++ b/refs.h
@@ -63,6 +63,11 @@ struct worktree;
#define RESOLVE_REF_NO_RECURSE 0x02
#define RESOLVE_REF_ALLOW_BAD_NAME 0x04
+struct pack_refs_opts {
+ unsigned int flags;
+ struct ref_exclusions *exclusions;
+};
+
const char *refs_resolve_ref_unsafe(struct ref_store *refs,
const char *refname,
int resolve_flags,
@@ -405,7 +410,7 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt,
* Write a packed-refs file for the current repository.
* flags: Combination of the above PACK_REFS_* flags.
*/
-int refs_pack_refs(struct ref_store *refs, unsigned int flags);
+int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts);
/*
* Setup reflog before using. Fill in err and return -1 on failure.
diff --git a/refs/debug.c b/refs/debug.c
index 6f11e6de46c..c0fa707a1da 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -123,10 +123,10 @@ static int debug_initial_transaction_commit(struct ref_store *refs,
return res;
}
-static int debug_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
- int res = drefs->refs->be->pack_refs(drefs->refs, flags);
+ int res = drefs->refs->be->pack_refs(drefs->refs, opts);
trace_printf_key(&trace_refs, "pack_refs: %d\n", res);
return res;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index bca7b851c5a..76aebfde695 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -21,6 +21,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
@@ -1175,15 +1176,18 @@ 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) !=
REF_WORKTREE_SHARED)
return 0;
+ if (opts->exclusions && ref_excluded(opts->exclusions, 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: */
@@ -1197,7 +1201,8 @@ static int should_pack_ref(const char *refname,
return 1;
}
-static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int files_pack_refs(struct ref_store *ref_store,
+ struct pack_refs_opts *opts)
{
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,
@@ -1222,8 +1227,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
* in the packed ref cache. If the reference should be
* pruned, also add it to refs_to_prune.
*/
- if (!should_pack_ref(iter->refname, iter->oid, iter->flags,
- flags))
+ if (!should_pack_ref(iter->refname, iter->oid, iter->flags, opts))
continue;
/*
@@ -1237,7 +1241,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
iter->refname, err.buf);
/* Schedule the loose reference for pruning if requested. */
- if ((flags & PACK_REFS_PRUNE)) {
+ if ((opts->flags & PACK_REFS_PRUNE)) {
struct ref_to_prune *n;
FLEX_ALLOC_STR(n, name, iter->refname);
oidcpy(&n->oid, iter->oid);
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 5b412a133be..291e53f5cfe 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1577,7 +1577,7 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
}
static int packed_pack_refs(struct ref_store *ref_store UNUSED,
- unsigned int flags UNUSED)
+ struct pack_refs_opts *pack_opts UNUSED)
{
/*
* Packed refs are already packed. It might be that loose refs
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index a85d113123c..f72b7be8941 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -547,7 +547,8 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err);
-typedef int pack_refs_fn(struct ref_store *ref_store, unsigned int flags);
+typedef int pack_refs_fn(struct ref_store *ref_store,
+ struct pack_refs_opts *opts);
typedef int create_symref_fn(struct ref_store *ref_store,
const char *ref_target,
const char *refs_heads_master,
diff --git a/revision.h b/revision.h
index 31828748dc0..25776af3815 100644
--- a/revision.h
+++ b/revision.h
@@ -87,7 +87,7 @@ struct rev_cmdline_info {
struct ref_exclusions {
/*
* Excluded refs is a list of wildmatch patterns. If any of the
- * patterns matches, the reference will be excluded.
+ * patterns match, the reference will be excluded.
*/
struct string_list excluded_refs;
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 6d8f844e9c7..de4197708d9 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -116,8 +116,9 @@ static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
static int cmd_pack_refs(struct ref_store *refs, const char **argv)
{
unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
+ struct pack_refs_opts pack_opts = { .flags = flags };
- return refs_pack_refs(refs, flags);
+ return refs_pack_refs(refs, &pack_opts);
}
static int cmd_create_symref(struct ref_store *refs, const char **argv)
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index 07a0ff93def..ddfc1b6e5f1 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -108,6 +108,22 @@ 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*" &&
+ 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 &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v3 3/4] revision: modify ref_exclusions to handle inclusions
2023-05-11 18:10 ` [PATCH v3 0/4] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
2023-05-11 18:10 ` [PATCH v3 1/4] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
2023-05-11 18:10 ` [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
@ 2023-05-11 18:10 ` John Cai via GitGitGadget
2023-05-11 19:54 ` Junio C Hamano
2023-05-11 18:10 ` [PATCH v3 4/4] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 0/3] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
4 siblings, 1 reply; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-11 18:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, John Cai, John Cai
From: John Cai <johncai86@gmail.com>
The ref_exclusions API provides the ability to check if certain refs are
to be excluded. We can easily extend this API to check if certain refs
are included, which a subsequent commit will make use of.
Rename ref_exclusions to ref_visibility and add a member to keep track
of inclusions. Add add_ref_inclusion(), ref_included() to be used for
refs to explicitly include.
Signed-off-by: John Cai <johncai86@gmail.com>
---
builtin/pack-refs.c | 6 ++--
builtin/rev-parse.c | 18 +++++------
refs.h | 2 +-
refs/files-backend.c | 2 +-
revision.c | 71 +++++++++++++++++++++++++++-----------------
revision.h | 26 ++++++++++------
6 files changed, 75 insertions(+), 50 deletions(-)
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 1d1a64fe386..2464575a665 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -14,8 +14,8 @@ static char const * const pack_refs_usage[] = {
int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
- static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
- struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
+ static struct ref_visibility visibility = REF_VISIBILITY_INIT;
+ struct pack_refs_opts pack_refs_opts = {.visibility = &visibility, .flags = flags};
static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
struct string_list_item *item;
@@ -31,7 +31,7 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
usage_with_options(pack_refs_usage, opts);
for_each_string_list_item(item, &option_excluded_refs)
- add_ref_exclusion(pack_refs_opts.exclusions, item->string);
+ add_ref_exclusion(pack_refs_opts.visibility, item->string);
return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
}
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 852e49e3403..9036d8876fc 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -46,7 +46,7 @@ static int abbrev_ref_strict;
static int output_sq;
static int stuck_long;
-static struct ref_exclusions ref_excludes = REF_EXCLUSIONS_INIT;
+static struct ref_visibility ref_visibility = REF_VISIBILITY_INIT;
/*
* Some arguments are relevant "revision" arguments,
@@ -208,7 +208,7 @@ static int show_default(void)
static int show_reference(const char *refname, const struct object_id *oid,
int flag UNUSED, void *cb_data UNUSED)
{
- if (ref_excluded(&ref_excludes, refname))
+ if (ref_excluded(&ref_visibility, refname))
return 0;
show_rev(NORMAL, oid, refname);
return 0;
@@ -596,7 +596,7 @@ static void handle_ref_opt(const char *pattern, const char *prefix)
for_each_glob_ref_in(show_reference, pattern, prefix, NULL);
else
for_each_ref_in(prefix, show_reference, NULL);
- clear_ref_exclusions(&ref_excludes);
+ clear_ref_visibility(&ref_visibility);
}
enum format_type {
@@ -874,7 +874,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
}
if (!strcmp(arg, "--all")) {
for_each_ref(show_reference, NULL);
- clear_ref_exclusions(&ref_excludes);
+ clear_ref_visibility(&ref_visibility);
continue;
}
if (skip_prefix(arg, "--disambiguate=", &arg)) {
@@ -888,13 +888,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
}
if (opt_with_value(arg, "--branches", &arg)) {
- if (ref_excludes.hidden_refs_configured)
+ if (ref_visibility.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --branches"));
handle_ref_opt(arg, "refs/heads/");
continue;
}
if (opt_with_value(arg, "--tags", &arg)) {
- if (ref_excludes.hidden_refs_configured)
+ if (ref_visibility.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --tags"));
handle_ref_opt(arg, "refs/tags/");
continue;
@@ -904,17 +904,17 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
}
if (opt_with_value(arg, "--remotes", &arg)) {
- if (ref_excludes.hidden_refs_configured)
+ if (ref_visibility.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --remotes"));
handle_ref_opt(arg, "refs/remotes/");
continue;
}
if (skip_prefix(arg, "--exclude=", &arg)) {
- add_ref_exclusion(&ref_excludes, arg);
+ add_ref_exclusion(&ref_visibility, arg);
continue;
}
if (skip_prefix(arg, "--exclude-hidden=", &arg)) {
- exclude_hidden_refs(&ref_excludes, arg);
+ exclude_hidden_refs(&ref_visibility, arg);
continue;
}
if (!strcmp(arg, "--show-toplevel")) {
diff --git a/refs.h b/refs.h
index 46020bd335c..57949a1726b 100644
--- a/refs.h
+++ b/refs.h
@@ -65,7 +65,7 @@ struct worktree;
struct pack_refs_opts {
unsigned int flags;
- struct ref_exclusions *exclusions;
+ struct ref_visibility *visibility;
};
const char *refs_resolve_ref_unsafe(struct ref_store *refs,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 76aebfde695..3ef19199788 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1183,7 +1183,7 @@ static int should_pack_ref(const char *refname,
REF_WORKTREE_SHARED)
return 0;
- if (opts->exclusions && ref_excluded(opts->exclusions, refname))
+ if (opts->visibility && ref_excluded(opts->visibility, refname))
return 0;
/* Do not pack non-tags unless PACK_REFS_ALL is set: */
diff --git a/revision.c b/revision.c
index b33cc1d106a..d59cd728e9e 100644
--- a/revision.c
+++ b/revision.c
@@ -1533,54 +1533,71 @@ static void add_rev_cmdline_list(struct rev_info *revs,
}
}
-int ref_excluded(const struct ref_exclusions *exclusions, const char *path)
+static int ref_matched(const char *path,
+ const struct string_list *l,
+ const struct string_list *hidden_refs)
{
const char *stripped_path = strip_namespace(path);
struct string_list_item *item;
- for_each_string_list_item(item, &exclusions->excluded_refs) {
+ for_each_string_list_item(item, l) {
if (!wildmatch(item->string, path, 0))
return 1;
}
- if (ref_is_hidden(stripped_path, path, &exclusions->hidden_refs))
+ if (ref_is_hidden(stripped_path, path, hidden_refs))
return 1;
return 0;
}
-void init_ref_exclusions(struct ref_exclusions *exclusions)
+int ref_excluded(const struct ref_visibility *visibility, const char *path)
{
- struct ref_exclusions blank = REF_EXCLUSIONS_INIT;
- memcpy(exclusions, &blank, sizeof(*exclusions));
+ return ref_matched(path, &visibility->excluded_refs, &visibility->hidden_refs);
}
-void clear_ref_exclusions(struct ref_exclusions *exclusions)
+int ref_included(const struct ref_visibility *visibility, const char *path)
{
- string_list_clear(&exclusions->excluded_refs, 0);
- string_list_clear(&exclusions->hidden_refs, 0);
- exclusions->hidden_refs_configured = 0;
+ return ref_matched(path, &visibility->included_refs, &visibility->hidden_refs);
}
-void add_ref_exclusion(struct ref_exclusions *exclusions, const char *exclude)
+void init_ref_visibility(struct ref_visibility *visibility)
{
- string_list_append(&exclusions->excluded_refs, exclude);
+ struct ref_visibility blank = REF_VISIBILITY_INIT;
+ memcpy(visibility, &blank, sizeof(*visibility));
+}
+
+void clear_ref_visibility(struct ref_visibility *visibility)
+{
+ string_list_clear(&visibility->excluded_refs, 0);
+ string_list_clear(&visibility->hidden_refs, 0);
+ visibility->hidden_refs_configured = 0;
+}
+
+void add_ref_exclusion(struct ref_visibility *visibility, const char *exclude)
+{
+ string_list_append(&visibility->excluded_refs, exclude);
+}
+
+void add_ref_inclusion(struct ref_visibility *visibility, const char *include)
+{
+ string_list_append(&visibility->included_refs, include);
}
struct exclude_hidden_refs_cb {
- struct ref_exclusions *exclusions;
+ struct ref_visibility *visibility;
const char *section;
};
static int hide_refs_config(const char *var, const char *value, void *cb_data)
{
struct exclude_hidden_refs_cb *cb = cb_data;
- cb->exclusions->hidden_refs_configured = 1;
+ cb->visibility->hidden_refs_configured = 1;
return parse_hide_refs_config(var, value, cb->section,
- &cb->exclusions->hidden_refs);
+ &cb->visibility->hidden_refs);
}
-void exclude_hidden_refs(struct ref_exclusions *exclusions, const char *section)
+void exclude_hidden_refs(struct ref_visibility *visibility, const char *section)
{
struct exclude_hidden_refs_cb cb;
@@ -1588,10 +1605,10 @@ void exclude_hidden_refs(struct ref_exclusions *exclusions, const char *section)
strcmp(section, "uploadpack"))
die(_("unsupported section for hidden refs: %s"), section);
- if (exclusions->hidden_refs_configured)
+ if (visibility->hidden_refs_configured)
die(_("--exclude-hidden= passed more than once"));
- cb.exclusions = exclusions;
+ cb.visibility = visibility;
cb.section = section;
git_config(hide_refs_config, &cb);
@@ -1935,7 +1952,7 @@ void repo_init_revisions(struct repository *r,
init_display_notes(&revs->notes_opt);
list_objects_filter_init(&revs->filter);
- init_ref_exclusions(&revs->ref_excludes);
+ init_ref_visibility(&revs->ref_excludes);
}
static void add_pending_commit_list(struct rev_info *revs,
@@ -2724,12 +2741,12 @@ static int handle_revision_pseudo_opt(struct rev_info *revs,
init_all_refs_cb(&cb, revs, *flags);
other_head_refs(handle_one_ref, &cb);
}
- clear_ref_exclusions(&revs->ref_excludes);
+ clear_ref_visibility(&revs->ref_excludes);
} else if (!strcmp(arg, "--branches")) {
if (revs->ref_excludes.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --branches"));
handle_refs(refs, revs, *flags, refs_for_each_branch_ref);
- clear_ref_exclusions(&revs->ref_excludes);
+ clear_ref_visibility(&revs->ref_excludes);
} else if (!strcmp(arg, "--bisect")) {
read_bisect_terms(&term_bad, &term_good);
handle_refs(refs, revs, *flags, for_each_bad_bisect_ref);
@@ -2740,17 +2757,17 @@ static int handle_revision_pseudo_opt(struct rev_info *revs,
if (revs->ref_excludes.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --tags"));
handle_refs(refs, revs, *flags, refs_for_each_tag_ref);
- clear_ref_exclusions(&revs->ref_excludes);
+ clear_ref_visibility(&revs->ref_excludes);
} else if (!strcmp(arg, "--remotes")) {
if (revs->ref_excludes.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --remotes"));
handle_refs(refs, revs, *flags, refs_for_each_remote_ref);
- clear_ref_exclusions(&revs->ref_excludes);
+ clear_ref_visibility(&revs->ref_excludes);
} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
struct all_refs_cb cb;
init_all_refs_cb(&cb, revs, *flags);
for_each_glob_ref(handle_one_ref, optarg, &cb);
- clear_ref_exclusions(&revs->ref_excludes);
+ clear_ref_visibility(&revs->ref_excludes);
return argcount;
} else if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
add_ref_exclusion(&revs->ref_excludes, optarg);
@@ -2764,21 +2781,21 @@ static int handle_revision_pseudo_opt(struct rev_info *revs,
return error(_("--exclude-hidden cannot be used together with --branches"));
init_all_refs_cb(&cb, revs, *flags);
for_each_glob_ref_in(handle_one_ref, optarg, "refs/heads/", &cb);
- clear_ref_exclusions(&revs->ref_excludes);
+ clear_ref_visibility(&revs->ref_excludes);
} else if (skip_prefix(arg, "--tags=", &optarg)) {
struct all_refs_cb cb;
if (revs->ref_excludes.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --tags"));
init_all_refs_cb(&cb, revs, *flags);
for_each_glob_ref_in(handle_one_ref, optarg, "refs/tags/", &cb);
- clear_ref_exclusions(&revs->ref_excludes);
+ clear_ref_visibility(&revs->ref_excludes);
} else if (skip_prefix(arg, "--remotes=", &optarg)) {
struct all_refs_cb cb;
if (revs->ref_excludes.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --remotes"));
init_all_refs_cb(&cb, revs, *flags);
for_each_glob_ref_in(handle_one_ref, optarg, "refs/remotes/", &cb);
- clear_ref_exclusions(&revs->ref_excludes);
+ clear_ref_visibility(&revs->ref_excludes);
} else if (!strcmp(arg, "--reflog")) {
add_reflogs_to_pending(revs, *flags);
} else if (!strcmp(arg, "--indexed-objects")) {
diff --git a/revision.h b/revision.h
index 25776af3815..296c520a003 100644
--- a/revision.h
+++ b/revision.h
@@ -84,7 +84,12 @@ struct rev_cmdline_info {
} *rev;
};
-struct ref_exclusions {
+struct ref_visibility {
+ /*
+ * Included refs is a list of wildmatch patterns. If any of the
+ * patterns match, the reference will be included.
+ */
+ struct string_list included_refs;
/*
* Excluded refs is a list of wildmatch patterns. If any of the
* patterns match, the reference will be excluded.
@@ -106,9 +111,10 @@ struct ref_exclusions {
};
/**
- * Initialize a `struct ref_exclusions` with a macro.
+ * Initialize a `struct ref_visibility` with a macro.
*/
-#define REF_EXCLUSIONS_INIT { \
+#define REF_VISIBILITY_INIT { \
+ .included_refs = STRING_LIST_INIT_DUP, \
.excluded_refs = STRING_LIST_INIT_DUP, \
.hidden_refs = STRING_LIST_INIT_DUP, \
}
@@ -135,7 +141,7 @@ struct rev_info {
struct list_objects_filter_options filter;
/* excluding from --branches, --refs, etc. expansion */
- struct ref_exclusions ref_excludes;
+ struct ref_visibility ref_excludes;
/* Basic information */
const char *prefix;
@@ -487,11 +493,13 @@ void show_object_with_name(FILE *, struct object *, const char *);
* Helpers to check if a reference should be excluded.
*/
-int ref_excluded(const struct ref_exclusions *exclusions, const char *path);
-void init_ref_exclusions(struct ref_exclusions *);
-void clear_ref_exclusions(struct ref_exclusions *);
-void add_ref_exclusion(struct ref_exclusions *, const char *exclude);
-void exclude_hidden_refs(struct ref_exclusions *, const char *section);
+int ref_excluded(const struct ref_visibility *exclusions, const char *path);
+int ref_included(const struct ref_visibility *exclusions, const char *path);
+void init_ref_visibility(struct ref_visibility *);
+void clear_ref_visibility(struct ref_visibility *);
+void add_ref_exclusion(struct ref_visibility *, const char *exclude);
+void add_ref_inclusion(struct ref_visibility *, const char *exclude);
+void exclude_hidden_refs(struct ref_visibility *, const char *section);
/**
* This function can be used if you want to add commit objects as revision
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v3 4/4] pack-refs: teach pack-refs --include option
2023-05-11 18:10 ` [PATCH v3 0/4] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
` (2 preceding siblings ...)
2023-05-11 18:10 ` [PATCH v3 3/4] revision: modify ref_exclusions to handle inclusions John Cai via GitGitGadget
@ 2023-05-11 18:10 ` John Cai via GitGitGadget
2023-05-11 20:06 ` Junio C Hamano
2023-05-12 19:03 ` John Cai
2023-05-12 21:34 ` [PATCH v4 0/3] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
4 siblings, 2 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-11 18:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, John Cai, John Cai
From: John Cai <johncai86@gmail.com>
Allow users to be more selective over which refs to pack by adding an
--include option to git-pack-refs.
The existing options allow some measure of selectivity. By default
git-pack-refs packs all tags. --all can be used to include all refs,
and the previous commit added the ability to exclude certain refs with
--exclude.
While these knobs give the user some selection over which refs to pack,
it could be useful to give more control. For instance, a repository may
have a set of branches that are rarely updated and would benefit from
being packed. --include would allow the user to easily include a set of
branches to be packed while leaving everything else unpacked.
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-pack-refs.txt | 14 +++++++++++++-
builtin/pack-refs.c | 18 ++++++++++++++++--
refs/files-backend.c | 15 +++++++--------
t/helper/test-ref-store.c | 8 +++++++-
t/t3210-pack-refs.sh | 21 +++++++++++++++++++++
5 files changed, 64 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
index c0f7426e519..85874a5f5dc 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] [--exclude <pattern>]
+'git pack-refs' [--all] [--no-prune] [--include <pattern>] [--exclude <pattern>]
DESCRIPTION
-----------
@@ -60,6 +60,15 @@ interests.
The command usually removes loose refs under `$GIT_DIR/refs`
hierarchy after packing them. This option tells it not to.
+--include <pattern>::
+
+Pack refs based on a `glob(7)` pattern. Repetitions of this option
+accumulate inclusion patterns. If a ref is both included in `--include` and
+`--exclude`, `--exclude` takes precedence. Using `--include` will preclude all
+tags from being included by default. Symbolic refs and broken refs will never
+be packed. When used with `--all`, it will be a noop. Use `--no-include` to clear
+and reset the list of patterns.
+
--exclude <pattern>::
Do not pack refs matching the given `glob(7)` pattern. Repetitions of this option
@@ -70,6 +79,9 @@ unpack it.
When used with `--all`, it will use the difference between the set of all refs,
and what is provided to `--exclude`.
+When used with `--include`, refs provided to `--include`, minus refs that are
+provided to `--exclude` will be packed.
+
BUGS
----
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 2464575a665..5062206f22e 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -5,9 +5,10 @@
#include "refs.h"
#include "repository.h"
#include "revision.h"
+#include "trace.h"
static char const * const pack_refs_usage[] = {
- N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
+ N_("git pack-refs [--all] [--no-prune] [--include <pattern>] [--exclude <pattern>]"),
NULL
};
@@ -15,13 +16,17 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
static struct ref_visibility visibility = REF_VISIBILITY_INIT;
- struct pack_refs_opts pack_refs_opts = {.visibility = &visibility, .flags = flags};
+ struct pack_refs_opts pack_refs_opts = { .visibility = &visibility,
+ .flags = flags };
static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
+ static struct string_list option_included_refs = STRING_LIST_INIT_NODUP;
struct string_list_item *item;
struct option opts[] = {
OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_STRING_LIST(0, "include", &option_included_refs, N_("pattern"),
+ N_("references to include")),
OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
N_("references to exclude")),
OPT_END(),
@@ -33,5 +38,14 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
for_each_string_list_item(item, &option_excluded_refs)
add_ref_exclusion(pack_refs_opts.visibility, item->string);
+ for_each_string_list_item(item, &option_included_refs)
+ add_ref_inclusion(pack_refs_opts.visibility, item->string);
+
+ if (pack_refs_opts.flags & PACK_REFS_ALL)
+ add_ref_inclusion(pack_refs_opts.visibility, "*");
+
+ if (!pack_refs_opts.visibility->included_refs.nr)
+ add_ref_inclusion(pack_refs_opts.visibility, "refs/tags/*");
+
return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 3ef19199788..c669cf8001a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1183,13 +1183,6 @@ static int should_pack_ref(const char *refname,
REF_WORKTREE_SHARED)
return 0;
- if (opts->visibility && ref_excluded(opts->visibility, refname))
- return 0;
-
- /* Do not pack non-tags unless PACK_REFS_ALL is set: */
- if (!(opts->flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
- return 0;
-
/* Do not pack symbolic refs: */
if (ref_flags & REF_ISSYMREF)
return 0;
@@ -1198,7 +1191,13 @@ static int should_pack_ref(const char *refname,
if (!ref_resolves_to_object(refname, the_repository, oid, ref_flags))
return 0;
- return 1;
+ if (opts->visibility && ref_excluded(opts->visibility, refname))
+ return 0;
+
+ if (opts->visibility && ref_included(opts->visibility, refname))
+ return 1;
+
+ return 0;
}
static int files_pack_refs(struct ref_store *ref_store,
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index de4197708d9..0dec1223362 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -5,6 +5,7 @@
#include "worktree.h"
#include "object-store.h"
#include "repository.h"
+#include "revision.h"
struct flag_definition {
const char *name;
@@ -116,7 +117,12 @@ static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
static int cmd_pack_refs(struct ref_store *refs, const char **argv)
{
unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
- struct pack_refs_opts pack_opts = { .flags = flags };
+ static struct ref_visibility visibility = REF_VISIBILITY_INIT;
+ struct pack_refs_opts pack_opts = { .flags = flags,
+ .visibility = &visibility };
+
+ if (pack_opts.flags & PACK_REFS_ALL)
+ add_ref_inclusion(pack_opts.visibility, "*");
return refs_pack_refs(refs, &pack_opts);
}
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index ddfc1b6e5f1..9ff6326b646 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -124,6 +124,27 @@ test_expect_success 'test --no-exclude refs clears excluded refs' '
! test -f .git/refs/heads/dont_pack3 &&
! test -f .git/refs/heads/dont_pack4'
+test_expect_success 'test only included refs are packed' '
+ git branch pack_this1 &&
+ git branch pack_this2 &&
+ git tag dont_pack5 &&
+ git pack-refs --include "refs/heads/pack_this*" &&
+ test -f .git/refs/tags/dont_pack5 &&
+ ! test -f ./git/refs/heads/pack_this1 &&
+ ! test -f ./git/refs/heads/pack_this2'
+
+test_expect_success 'test --no-include refs clears included refs' '
+ git branch pack1 &&
+ git branch pack2 &&
+ git pack-refs --include "refs/heads/pack*" --no-include &&
+ test -f .git/refs/heads/pack1 &&
+ test -f .git/refs/heads/pack2'
+
+test_expect_success 'test --exclude takes precedence over --include' '
+ git branch dont_pack5 &&
+ git pack-refs --include "refs/heads/pack*" --exclude "refs/heads/pack*" &&
+ test -f .git/refs/heads/dont_pack5'
+
test_expect_success \
'see if up-to-date packed refs are preserved' \
'git branch q &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-11 18:10 ` [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
@ 2023-05-11 19:34 ` Junio C Hamano
2023-05-12 0:00 ` Taylor Blau
1 sibling, 0 replies; 29+ messages in thread
From: Junio C Hamano @ 2023-05-11 19:34 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, Christian Couder, John Cai
"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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 3/4] revision: modify ref_exclusions to handle inclusions
2023-05-11 18:10 ` [PATCH v3 3/4] revision: modify ref_exclusions to handle inclusions John Cai via GitGitGadget
@ 2023-05-11 19:54 ` Junio C Hamano
2023-05-12 14:56 ` John Cai
0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2023-05-11 19:54 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, Christian Couder, John Cai
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +static int ref_matched(const char *path,
> + const struct string_list *l,
> + const struct string_list *hidden_refs)
> {
> const char *stripped_path = strip_namespace(path);
> struct string_list_item *item;
>
> - for_each_string_list_item(item, &exclusions->excluded_refs) {
> + for_each_string_list_item(item, l) {
> if (!wildmatch(item->string, path, 0))
> return 1;
> }
>
> - if (ref_is_hidden(stripped_path, path, &exclusions->hidden_refs))
> + if (ref_is_hidden(stripped_path, path, hidden_refs))
> return 1;
>
> return 0;
> }
>
> -void init_ref_exclusions(struct ref_exclusions *exclusions)
> +int ref_excluded(const struct ref_visibility *visibility, const char *path)
> {
> - struct ref_exclusions blank = REF_EXCLUSIONS_INIT;
> - memcpy(exclusions, &blank, sizeof(*exclusions));
> + return ref_matched(path, &visibility->excluded_refs, &visibility->hidden_refs);
> }
>
> -void clear_ref_exclusions(struct ref_exclusions *exclusions)
> +int ref_included(const struct ref_visibility *visibility, const char *path)
> {
> - string_list_clear(&exclusions->excluded_refs, 0);
> - string_list_clear(&exclusions->hidden_refs, 0);
> - exclusions->hidden_refs_configured = 0;
> + return ref_matched(path, &visibility->included_refs, &visibility->hidden_refs);
> }
It is unexpected that we do "hidden" inside ref_matched(). I would
have expected that no matter what exclusion or inclusion patterns
say, hidden things are to be kept hidden. I.e. I expected
- ref_matched(), which takes a path and a list of patterns, checks
if the path matches any of the given patterns;
- ref_excluded(), whcih takes a path and a visibility, asks
ref_matched() if the path matches visibility->excluded_refs and
relays its answer to the caller.
- ref_included(), which takes a path and a visibility, asks
ref_matched() if the path matches visibility->included_refs and
relays its answer to the caller.
- ref_visibility(), which takes a path and a visibility, goes
through the following sequence:
- if ref_is_hidden() says that the path is hidden, it is not
visible;
- if ref_excluded() says the path is excluded, it is not visible;
- if ref_included() says the path is included, it is visible;
- if none of the above triggers, the fate of the path is
determined by some default logic.
or something along that line. That would also allow us to avoid
having to call ref_is_hidden() more than once when we need to check
both inclusion and exclusion list.
But perhaps I am missing some requirements to be taken into
account, so let me read on.
To be honest, I didn't expect the "exclusions can be extended",
which I alluded to as a future, possible, follow-on work, to be
included as a part of this topic. I appreciate your going an extra
mile, but I am not sure if it was a good change in the context of
this series. With this patch, it is not trivial to even validate
that there is no behaviour change to any users of "struct
ref_exclusions" when the included_refs list is empty, which is an
absolute must to avoid regressions.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 4/4] pack-refs: teach pack-refs --include option
2023-05-11 18:10 ` [PATCH v3 4/4] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
@ 2023-05-11 20:06 ` Junio C Hamano
2023-05-12 14:48 ` John Cai
2023-05-12 19:03 ` John Cai
1 sibling, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2023-05-11 20:06 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, Christian Couder, John Cai
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> @@ -1198,7 +1191,13 @@ static int should_pack_ref(const char *refname,
> if (!ref_resolves_to_object(refname, the_repository, oid, ref_flags))
> return 0;
>
> - return 1;
> + if (opts->visibility && ref_excluded(opts->visibility, refname))
> + return 0;
> +
> + if (opts->visibility && ref_included(opts->visibility, refname))
> + return 1;
> +
> + return 0;
> }
When the user did not say --exclude or --include, can we not have
opts->visibility? IOW, can opts->visiblity be NULL?
Even if it can be NULL, shouldn't we be defaulting to "pack", as we
rejected refs to be excluded already?
> @@ -33,5 +38,14 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
> for_each_string_list_item(item, &option_excluded_refs)
> add_ref_exclusion(pack_refs_opts.visibility, item->string);
>
> + for_each_string_list_item(item, &option_included_refs)
> + add_ref_inclusion(pack_refs_opts.visibility, item->string);
> +
> + if (pack_refs_opts.flags & PACK_REFS_ALL)
> + add_ref_inclusion(pack_refs_opts.visibility, "*");
> +
> + if (!pack_refs_opts.visibility->included_refs.nr)
> + add_ref_inclusion(pack_refs_opts.visibility, "refs/tags/*");
Given the above code, I think visibility is always non-NULL, and the
inclusion list has at least one element. So the above "what should
be the default?" question is theoretical. But it would be nicer if
we do not check opts->visibility there. By writing
if (opts->visibility && ref_included(opts->visibility, refname))
return 1;
you are saying "if visibility is defined and it is included, say
YES", and logically it follows that, if we did not return true from
here, we do not know if the end-user supplied inclusion list did not
match (i.e. ref_included() said no), or we skipped the check because
the end-user did not supply the visibility rule. It is easy to
mistake that the next statement after this, i.e. "return 0;", is the
default action when the end-user did not give any rule.
But that is not what is going on. Because visibility is always
given,
The last part would become
if (ref_included(opts->visibility, refname))
return 1;
return 0;
and the "return 0" is no longer confusing. If inclusion check says
yes, the result is "to include", otherwise the result is "not to
include". Even shorter, we could say
return !ref_excluded(opts->visibility, refname) &&
ref_included(opts->visibility, refname) &&
which we can trivially read the design decision: exclude list has
priority, and include list is consulted only after exclude list does
not ban it.
Thanks.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/4] docs: clarify git-pack-refs --all will pack all refs
2023-05-11 18:10 ` [PATCH v3 1/4] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
@ 2023-05-11 23:53 ` Taylor Blau
0 siblings, 0 replies; 29+ messages in thread
From: Taylor Blau @ 2023-05-11 23:53 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, Junio C Hamano, Christian Couder, John Cai
On Thu, May 11, 2023 at 06:10:31PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
>
> --all packs not just branch tips but anything under refs/ with the
> exception of hidden refs and broken refs. Clarify this in the
> documentation.
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> Documentation/git-pack-refs.txt | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
> index 154081f2de2..e011e5fead3 100644
> --- a/Documentation/git-pack-refs.txt
> +++ b/Documentation/git-pack-refs.txt
> @@ -49,10 +49,11 @@ OPTIONS
>
> The command by default packs all tags and refs that are already
> packed, and leaves other refs
> -alone. This is because branches are expected to be actively
> +alone. This is because branches are expected to be actively
This looks like a stray change.
> developed and packing their tips does not help performance.
> -This option causes branch tips to be packed as well. Useful for
> -a repository with many branches of historical interests.
> +This option causes all refs to be packed as well, with the exception of hidden
> +and broken refs. Useful for a repository with many branches of historical
> +interests.
The new wording looks good, though you may want to mention that `--all`
does not pack symrefs, either. That's not the fault of your patch,
though, since it was the case in the pre-image, as well.
The wrapping on the new text looks a little off, though.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-11 18:10 ` [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
2023-05-11 19:34 ` Junio C Hamano
@ 2023-05-12 0:00 ` Taylor Blau
2023-05-12 12:53 ` John Cai
1 sibling, 1 reply; 29+ messages in thread
From: Taylor Blau @ 2023-05-12 0:00 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, Junio C Hamano, Christian Couder, John Cai
On Thu, May 11, 2023 at 06:10:32PM +0000, John Cai via GitGitGadget wrote:
> 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(-)
>
> 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`.
> +
I think this last paragraph could be simplified, though feel free to
discard my suggestion if you think it makes things less clear.
When used with `--all`, pack only loose refs which do not match any of
the provided `--exclude` patterns.
> int cmd_pack_refs(int argc, const char **argv, const char *prefix)
> {
> unsigned int flags = PACK_REFS_PRUNE;
> + static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
> + struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
> + static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
> + struct string_list_item *item;
Since this list does not appear to be sensitive to its order, have you
considered using the strvec API instead of the string_list one?
> diff --git a/refs.c b/refs.c
> index d2a98e1c21f..881a0da65cf 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2132,9 +2132,9 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
> }
>
> /* backend functions */
> -int refs_pack_refs(struct ref_store *refs, unsigned int flags)
> +int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)
> {
> - return refs->be->pack_refs(refs, flags);
> + return refs->be->pack_refs(refs, opts);
> }
>
> int peel_iterated_oid(const struct object_id *base, struct object_id *peeled)
> diff --git a/refs.h b/refs.h
> index 123cfa44244..46020bd335c 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -63,6 +63,11 @@ struct worktree;
> #define RESOLVE_REF_NO_RECURSE 0x02
> #define RESOLVE_REF_ALLOW_BAD_NAME 0x04
>
> +struct pack_refs_opts {
> + unsigned int flags;
> + struct ref_exclusions *exclusions;
I think this would be OK to include directly in the struct instead of
via a pointer, but either is fine.
> @@ -1175,15 +1176,18 @@ 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) !=
> REF_WORKTREE_SHARED)
> return 0;
>
> + if (opts->exclusions && ref_excluded(opts->exclusions, refname))
> + return 0;
Looks good, here is where we throw out refs that we don't want. I wonder
if ref_excluded() does the right thing with a zero-initialized argument
(i.e. that it behaves as if nothing matches).
I wonder if it's possible to skip over certain loose references by
avoiding traversal into the sub-directories for simple prefixes. That
may be a premature optimization, though, so I don't think you
necessarily need to worry about it in this round.
> +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*" &&
> + 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'
Tests look good. The trailing quote is a little odd to be placed on the
last line of the test instead of off on its own, but I suppose that is
imitating existing style, which is OK.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-12 0:00 ` Taylor Blau
@ 2023-05-12 12:53 ` John Cai
2023-05-12 21:11 ` John Cai
0 siblings, 1 reply; 29+ messages in thread
From: John Cai @ 2023-05-12 12:53 UTC (permalink / raw)
To: Taylor Blau
Cc: John Cai via GitGitGadget, git, Junio C Hamano, Christian Couder
Hey Taylor,
On 11 May 2023, at 20:00, Taylor Blau wrote:
> On Thu, May 11, 2023 at 06:10:32PM +0000, John Cai via GitGitGadget wrote:
>> 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(-)
>>
>> 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`.
>> +
>
> I think this last paragraph could be simplified, though feel free to
> discard my suggestion if you think it makes things less clear.
>
> When used with `--all`, pack only loose refs which do not match any of
> the provided `--exclude` patterns.
I like the wording here, thanks
>
>> int cmd_pack_refs(int argc, const char **argv, const char *prefix)
>> {
>> unsigned int flags = PACK_REFS_PRUNE;
>> + static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
>> + struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
>> + static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
>> + struct string_list_item *item;
>
> Since this list does not appear to be sensitive to its order, have you
> considered using the strvec API instead of the string_list one?
Thanks for this suggestion--you're right in that the order doesn't matter here.
The only thing is, the only option parsing macro I could find that operates on
strvec is OPT_PASSTHRU_ARGV. I tried it out, and it seem to work just fine.
>
>> diff --git a/refs.c b/refs.c
>> index d2a98e1c21f..881a0da65cf 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -2132,9 +2132,9 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
>> }
>>
>> /* backend functions */
>> -int refs_pack_refs(struct ref_store *refs, unsigned int flags)
>> +int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)
>> {
>> - return refs->be->pack_refs(refs, flags);
>> + return refs->be->pack_refs(refs, opts);
>> }
>>
>> int peel_iterated_oid(const struct object_id *base, struct object_id *peeled)
>> diff --git a/refs.h b/refs.h
>> index 123cfa44244..46020bd335c 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -63,6 +63,11 @@ struct worktree;
>> #define RESOLVE_REF_NO_RECURSE 0x02
>> #define RESOLVE_REF_ALLOW_BAD_NAME 0x04
>>
>> +struct pack_refs_opts {
>> + unsigned int flags;
>> + struct ref_exclusions *exclusions;
>
> I think this would be OK to include directly in the struct instead of
> via a pointer, but either is fine.
>
>> @@ -1175,15 +1176,18 @@ 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) !=
>> REF_WORKTREE_SHARED)
>> return 0;
>>
>> + if (opts->exclusions && ref_excluded(opts->exclusions, refname))
>> + return 0;
>
> Looks good, here is where we throw out refs that we don't want. I wonder
> if ref_excluded() does the right thing with a zero-initialized argument
> (i.e. that it behaves as if nothing matches).
Yes, I think we can skip checking if opt->exclusions is not null. Junio had
feedback around this as well.
>
> I wonder if it's possible to skip over certain loose references by
> avoiding traversal into the sub-directories for simple prefixes. That
> may be a premature optimization, though, so I don't think you
> necessarily need to worry about it in this round.
>
>> +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*" &&
>> + 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'
>
> Tests look good. The trailing quote is a little odd to be placed on the
> last line of the test instead of off on its own, but I suppose that is
> imitating existing style, which is OK.
thanks for the feedback!
John
>
> Thanks,
> Taylor
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 4/4] pack-refs: teach pack-refs --include option
2023-05-11 20:06 ` Junio C Hamano
@ 2023-05-12 14:48 ` John Cai
0 siblings, 0 replies; 29+ messages in thread
From: John Cai @ 2023-05-12 14:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Cai via GitGitGadget, git, Christian Couder
Hi Junio,
On 11 May 2023, at 16:06, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> @@ -1198,7 +1191,13 @@ static int should_pack_ref(const char *refname,
>> if (!ref_resolves_to_object(refname, the_repository, oid, ref_flags))
>> return 0;
>>
>> - return 1;
>> + if (opts->visibility && ref_excluded(opts->visibility, refname))
>> + return 0;
>> +
>> + if (opts->visibility && ref_included(opts->visibility, refname))
>> + return 1;
>> +
>> + return 0;
>> }
>
> When the user did not say --exclude or --include, can we not have
> opts->visibility? IOW, can opts->visiblity be NULL?
>
> Even if it can be NULL, shouldn't we be defaulting to "pack", as we
> rejected refs to be excluded already?
>
>> @@ -33,5 +38,14 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
>> for_each_string_list_item(item, &option_excluded_refs)
>> add_ref_exclusion(pack_refs_opts.visibility, item->string);
>>
>> + for_each_string_list_item(item, &option_included_refs)
>> + add_ref_inclusion(pack_refs_opts.visibility, item->string);
>> +
>> + if (pack_refs_opts.flags & PACK_REFS_ALL)
>> + add_ref_inclusion(pack_refs_opts.visibility, "*");
>> +
>> + if (!pack_refs_opts.visibility->included_refs.nr)
>> + add_ref_inclusion(pack_refs_opts.visibility, "refs/tags/*");
>
> Given the above code, I think visibility is always non-NULL, and the
> inclusion list has at least one element. So the above "what should
> be the default?" question is theoretical. But it would be nicer if
> we do not check opts->visibility there. By writing
>
> if (opts->visibility && ref_included(opts->visibility, refname))
> return 1;
>
> you are saying "if visibility is defined and it is included, say
> YES", and logically it follows that, if we did not return true from
> here, we do not know if the end-user supplied inclusion list did not
> match (i.e. ref_included() said no), or we skipped the check because
> the end-user did not supply the visibility rule. It is easy to
> mistake that the next statement after this, i.e. "return 0;", is the
> default action when the end-user did not give any rule.
>
> But that is not what is going on. Because visibility is always
> given,
>
> The last part would become
>
> if (ref_included(opts->visibility, refname))
> return 1;
> return 0;
>
> and the "return 0" is no longer confusing. If inclusion check says
> yes, the result is "to include", otherwise the result is "not to
> include". Even shorter, we could say
>
> return !ref_excluded(opts->visibility, refname) &&
> ref_included(opts->visibility, refname) &&
>
> which we can trivially read the design decision: exclude list has
> priority, and include list is consulted only after exclude list does
> not ban it.
Yes, this is the logic. I agree that getting rid of the opts->visibility check
would make it more clear.
thanks
John
>
> Thanks.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 3/4] revision: modify ref_exclusions to handle inclusions
2023-05-11 19:54 ` Junio C Hamano
@ 2023-05-12 14:56 ` John Cai
0 siblings, 0 replies; 29+ messages in thread
From: John Cai @ 2023-05-12 14:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Cai via GitGitGadget, git, Christian Couder
Hi Junio,
On 11 May 2023, at 15:54, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> +static int ref_matched(const char *path,
>> + const struct string_list *l,
>> + const struct string_list *hidden_refs)
>> {
>> const char *stripped_path = strip_namespace(path);
>> struct string_list_item *item;
>>
>> - for_each_string_list_item(item, &exclusions->excluded_refs) {
>> + for_each_string_list_item(item, l) {
>> if (!wildmatch(item->string, path, 0))
>> return 1;
>> }
>>
>> - if (ref_is_hidden(stripped_path, path, &exclusions->hidden_refs))
>> + if (ref_is_hidden(stripped_path, path, hidden_refs))
>> return 1;
>>
>> return 0;
>> }
>>
>> -void init_ref_exclusions(struct ref_exclusions *exclusions)
>> +int ref_excluded(const struct ref_visibility *visibility, const char *path)
>> {
>> - struct ref_exclusions blank = REF_EXCLUSIONS_INIT;
>> - memcpy(exclusions, &blank, sizeof(*exclusions));
>> + return ref_matched(path, &visibility->excluded_refs, &visibility->hidden_refs);
>> }
>>
>> -void clear_ref_exclusions(struct ref_exclusions *exclusions)
>> +int ref_included(const struct ref_visibility *visibility, const char *path)
>> {
>> - string_list_clear(&exclusions->excluded_refs, 0);
>> - string_list_clear(&exclusions->hidden_refs, 0);
>> - exclusions->hidden_refs_configured = 0;
>> + return ref_matched(path, &visibility->included_refs, &visibility->hidden_refs);
>> }
>
> It is unexpected that we do "hidden" inside ref_matched(). I would
> have expected that no matter what exclusion or inclusion patterns
> say, hidden things are to be kept hidden. I.e. I expected
Oops. Yes this was an oversight.
>
> - ref_matched(), which takes a path and a list of patterns, checks
> if the path matches any of the given patterns;
>
> - ref_excluded(), whcih takes a path and a visibility, asks
> ref_matched() if the path matches visibility->excluded_refs and
> relays its answer to the caller.
>
> - ref_included(), which takes a path and a visibility, asks
> ref_matched() if the path matches visibility->included_refs and
> relays its answer to the caller.
>
> - ref_visibility(), which takes a path and a visibility, goes
> through the following sequence:
>
> - if ref_is_hidden() says that the path is hidden, it is not
> visible;
>
> - if ref_excluded() says the path is excluded, it is not visible;
>
> - if ref_included() says the path is included, it is visible;
>
> - if none of the above triggers, the fate of the path is
> determined by some default logic.
That's a good point. I didn't think about adding a ref_visibility() function
that conslidates inclusion and exclusion.
>
> or something along that line. That would also allow us to avoid
> having to call ref_is_hidden() more than once when we need to check
> both inclusion and exclusion list.
>
> But perhaps I am missing some requirements to be taken into
> account, so let me read on.
>
> To be honest, I didn't expect the "exclusions can be extended",
> which I alluded to as a future, possible, follow-on work, to be
> included as a part of this topic. I appreciate your going an extra
> mile, but I am not sure if it was a good change in the context of
> this series. With this patch, it is not trivial to even validate
> that there is no behaviour change to any users of "struct
> ref_exclusions" when the included_refs list is empty, which is an
> absolute must to avoid regressions.
Okay, maybe we can include this refactor in a subsequent series then.
thanks
John
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 4/4] pack-refs: teach pack-refs --include option
2023-05-11 18:10 ` [PATCH v3 4/4] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
2023-05-11 20:06 ` Junio C Hamano
@ 2023-05-12 19:03 ` John Cai
1 sibling, 0 replies; 29+ messages in thread
From: John Cai @ 2023-05-12 19:03 UTC (permalink / raw)
To: git; +Cc: Christian Couder, Junio C Hamano, Taylor Blau
On 11 May 2023, at 14:10, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
>
> Allow users to be more selective over which refs to pack by adding an
> --include option to git-pack-refs.
>
> The existing options allow some measure of selectivity. By default
> git-pack-refs packs all tags. --all can be used to include all refs,
> and the previous commit added the ability to exclude certain refs with
> --exclude.
>
> While these knobs give the user some selection over which refs to pack,
> it could be useful to give more control. For instance, a repository may
> have a set of branches that are rarely updated and would benefit from
> being packed. --include would allow the user to easily include a set of
> branches to be packed while leaving everything else unpacked.
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> Documentation/git-pack-refs.txt | 14 +++++++++++++-
> builtin/pack-refs.c | 18 ++++++++++++++++--
> refs/files-backend.c | 15 +++++++--------
> t/helper/test-ref-store.c | 8 +++++++-
> t/t3210-pack-refs.sh | 21 +++++++++++++++++++++
> 5 files changed, 64 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
> index c0f7426e519..85874a5f5dc 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] [--exclude <pattern>]
> +'git pack-refs' [--all] [--no-prune] [--include <pattern>] [--exclude <pattern>]
>
> DESCRIPTION
> -----------
> @@ -60,6 +60,15 @@ interests.
> The command usually removes loose refs under `$GIT_DIR/refs`
> hierarchy after packing them. This option tells it not to.
>
> +--include <pattern>::
> +
> +Pack refs based on a `glob(7)` pattern. Repetitions of this option
> +accumulate inclusion patterns. If a ref is both included in `--include` and
> +`--exclude`, `--exclude` takes precedence. Using `--include` will preclude all
> +tags from being included by default. Symbolic refs and broken refs will never
> +be packed. When used with `--all`, it will be a noop. Use `--no-include` to clear
> +and reset the list of patterns.
> +
> --exclude <pattern>::
>
> Do not pack refs matching the given `glob(7)` pattern. Repetitions of this option
> @@ -70,6 +79,9 @@ unpack it.
> When used with `--all`, it will use the difference between the set of all refs,
> and what is provided to `--exclude`.
>
> +When used with `--include`, refs provided to `--include`, minus refs that are
> +provided to `--exclude` will be packed.
> +
>
> BUGS
> ----
> diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
> index 2464575a665..5062206f22e 100644
> --- a/builtin/pack-refs.c
> +++ b/builtin/pack-refs.c
> @@ -5,9 +5,10 @@
> #include "refs.h"
> #include "repository.h"
> #include "revision.h"
> +#include "trace.h"
>
> static char const * const pack_refs_usage[] = {
> - N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
> + N_("git pack-refs [--all] [--no-prune] [--include <pattern>] [--exclude <pattern>]"),
> NULL
> };
>
> @@ -15,13 +16,17 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
> {
> unsigned int flags = PACK_REFS_PRUNE;
> static struct ref_visibility visibility = REF_VISIBILITY_INIT;
> - struct pack_refs_opts pack_refs_opts = {.visibility = &visibility, .flags = flags};
> + struct pack_refs_opts pack_refs_opts = { .visibility = &visibility,
> + .flags = flags };
> static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
> + static struct string_list option_included_refs = STRING_LIST_INIT_NODUP;
> struct string_list_item *item;
>
> struct option opts[] = {
> OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
> OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
> + OPT_STRING_LIST(0, "include", &option_included_refs, N_("pattern"),
> + N_("references to include")),
> OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
> N_("references to exclude")),
> OPT_END(),
> @@ -33,5 +38,14 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
> for_each_string_list_item(item, &option_excluded_refs)
> add_ref_exclusion(pack_refs_opts.visibility, item->string);
>
> + for_each_string_list_item(item, &option_included_refs)
> + add_ref_inclusion(pack_refs_opts.visibility, item->string);
> +
> + if (pack_refs_opts.flags & PACK_REFS_ALL)
> + add_ref_inclusion(pack_refs_opts.visibility, "*");
> +
> + if (!pack_refs_opts.visibility->included_refs.nr)
> + add_ref_inclusion(pack_refs_opts.visibility, "refs/tags/*");
> +
> return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
> }
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 3ef19199788..c669cf8001a 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1183,13 +1183,6 @@ static int should_pack_ref(const char *refname,
> REF_WORKTREE_SHARED)
> return 0;
>
> - if (opts->visibility && ref_excluded(opts->visibility, refname))
> - return 0;
> -
> - /* Do not pack non-tags unless PACK_REFS_ALL is set: */
> - if (!(opts->flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
> - return 0;
> -
> /* Do not pack symbolic refs: */
> if (ref_flags & REF_ISSYMREF)
> return 0;
> @@ -1198,7 +1191,13 @@ static int should_pack_ref(const char *refname,
> if (!ref_resolves_to_object(refname, the_repository, oid, ref_flags))
> return 0;
>
> - return 1;
> + if (opts->visibility && ref_excluded(opts->visibility, refname))
> + return 0;
> +
> + if (opts->visibility && ref_included(opts->visibility, refname))
> + return 1;
> +
> + return 0;
> }
>
> static int files_pack_refs(struct ref_store *ref_store,
> diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
> index de4197708d9..0dec1223362 100644
> --- a/t/helper/test-ref-store.c
> +++ b/t/helper/test-ref-store.c
> @@ -5,6 +5,7 @@
> #include "worktree.h"
> #include "object-store.h"
> #include "repository.h"
> +#include "revision.h"
>
> struct flag_definition {
> const char *name;
> @@ -116,7 +117,12 @@ static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
> static int cmd_pack_refs(struct ref_store *refs, const char **argv)
> {
> unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
> - struct pack_refs_opts pack_opts = { .flags = flags };
> + static struct ref_visibility visibility = REF_VISIBILITY_INIT;
> + struct pack_refs_opts pack_opts = { .flags = flags,
> + .visibility = &visibility };
> +
> + if (pack_opts.flags & PACK_REFS_ALL)
> + add_ref_inclusion(pack_opts.visibility, "*");
I was wondering about this test function. I had to add this in because now we
are no longer checking the PACK_REFS_ALL flag in refs/files-backend.c and
instead relying on the inclusions data structure.
However, this does not support --exclude and --include options. With the current
plumbing code in this file, it doesn't look like it supports options with values
as it uses a bitmask.
My question is, do we need to add support for every option we add to
git-pack-refs here? Or are the changes in this patch sufficient.
thanks
John
>
> return refs_pack_refs(refs, &pack_opts);
> }
> diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
> index ddfc1b6e5f1..9ff6326b646 100755
> --- a/t/t3210-pack-refs.sh
> +++ b/t/t3210-pack-refs.sh
> @@ -124,6 +124,27 @@ test_expect_success 'test --no-exclude refs clears excluded refs' '
> ! test -f .git/refs/heads/dont_pack3 &&
> ! test -f .git/refs/heads/dont_pack4'
>
> +test_expect_success 'test only included refs are packed' '
> + git branch pack_this1 &&
> + git branch pack_this2 &&
> + git tag dont_pack5 &&
> + git pack-refs --include "refs/heads/pack_this*" &&
> + test -f .git/refs/tags/dont_pack5 &&
> + ! test -f ./git/refs/heads/pack_this1 &&
> + ! test -f ./git/refs/heads/pack_this2'
> +
> +test_expect_success 'test --no-include refs clears included refs' '
> + git branch pack1 &&
> + git branch pack2 &&
> + git pack-refs --include "refs/heads/pack*" --no-include &&
> + test -f .git/refs/heads/pack1 &&
> + test -f .git/refs/heads/pack2'
> +
> +test_expect_success 'test --exclude takes precedence over --include' '
> + git branch dont_pack5 &&
> + git pack-refs --include "refs/heads/pack*" --exclude "refs/heads/pack*" &&
> + test -f .git/refs/heads/dont_pack5'
> +
> test_expect_success \
> 'see if up-to-date packed refs are preserved' \
> 'git branch q &&
> --
> gitgitgadget
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-12 12:53 ` John Cai
@ 2023-05-12 21:11 ` John Cai
0 siblings, 0 replies; 29+ messages in thread
From: John Cai @ 2023-05-12 21:11 UTC (permalink / raw)
To: Taylor Blau
Cc: John Cai via GitGitGadget, git, Junio C Hamano, Christian Couder
On 12 May 2023, at 8:53, John Cai wrote:
> Hey Taylor,
>
> On 11 May 2023, at 20:00, Taylor Blau wrote:
>
>> On Thu, May 11, 2023 at 06:10:32PM +0000, John Cai via GitGitGadget wrote:
>>> 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(-)
>>>
>>> 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`.
>>> +
>>
>> I think this last paragraph could be simplified, though feel free to
>> discard my suggestion if you think it makes things less clear.
>>
>> When used with `--all`, pack only loose refs which do not match any of
>> the provided `--exclude` patterns.
>
> I like the wording here, thanks
>
>>
>>> int cmd_pack_refs(int argc, const char **argv, const char *prefix)
>>> {
>>> unsigned int flags = PACK_REFS_PRUNE;
>>> + static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
>>> + struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
>>> + static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
>>> + struct string_list_item *item;
>>
>> Since this list does not appear to be sensitive to its order, have you
>> considered using the strvec API instead of the string_list one?
>
> Thanks for this suggestion--you're right in that the order doesn't matter here.
> The only thing is, the only option parsing macro I could find that operates on
> strvec is OPT_PASSTHRU_ARGV. I tried it out, and it seem to work just fine.
Actually I had a bug in my test that made it seem like it worked. I realize that
OPT_PASSTHRU_ARGV passes through the entire option flag eg
--include=refs/tags/dont_pack* so we would have to strip the flag name. It's not
as user friendly as OPT_STRING_LIST that is just plug and play.
Unless there is a significant performance improvement in using strvec, I'm
thinking maybe I'll just stick to string_list.
thanks
John
>
>>
>>> diff --git a/refs.c b/refs.c
>>> index d2a98e1c21f..881a0da65cf 100644
>>> --- a/refs.c
>>> +++ b/refs.c
>>> @@ -2132,9 +2132,9 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
>>> }
>>>
>>> /* backend functions */
>>> -int refs_pack_refs(struct ref_store *refs, unsigned int flags)
>>> +int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)
>>> {
>>> - return refs->be->pack_refs(refs, flags);
>>> + return refs->be->pack_refs(refs, opts);
>>> }
>>>
>>> int peel_iterated_oid(const struct object_id *base, struct object_id *peeled)
>>> diff --git a/refs.h b/refs.h
>>> index 123cfa44244..46020bd335c 100644
>>> --- a/refs.h
>>> +++ b/refs.h
>>> @@ -63,6 +63,11 @@ struct worktree;
>>> #define RESOLVE_REF_NO_RECURSE 0x02
>>> #define RESOLVE_REF_ALLOW_BAD_NAME 0x04
>>>
>>> +struct pack_refs_opts {
>>> + unsigned int flags;
>>> + struct ref_exclusions *exclusions;
>>
>> I think this would be OK to include directly in the struct instead of
>> via a pointer, but either is fine.
>>
>>> @@ -1175,15 +1176,18 @@ 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) !=
>>> REF_WORKTREE_SHARED)
>>> return 0;
>>>
>>> + if (opts->exclusions && ref_excluded(opts->exclusions, refname))
>>> + return 0;
>>
>> Looks good, here is where we throw out refs that we don't want. I wonder
>> if ref_excluded() does the right thing with a zero-initialized argument
>> (i.e. that it behaves as if nothing matches).
>
> Yes, I think we can skip checking if opt->exclusions is not null. Junio had
> feedback around this as well.
>
>>
>> I wonder if it's possible to skip over certain loose references by
>> avoiding traversal into the sub-directories for simple prefixes. That
>> may be a premature optimization, though, so I don't think you
>> necessarily need to worry about it in this round.
>>
>>> +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*" &&
>>> + 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'
>>
>> Tests look good. The trailing quote is a little odd to be placed on the
>> last line of the test instead of off on its own, but I suppose that is
>> imitating existing style, which is OK.
>
> thanks for the feedback!
> John
>>
>> Thanks,
>> Taylor
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 0/3] pack-refs: allow users control over which refs to pack
2023-05-11 18:10 ` [PATCH v3 0/4] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
` (3 preceding siblings ...)
2023-05-11 18:10 ` [PATCH v3 4/4] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
@ 2023-05-12 21:34 ` John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 1/3] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
` (2 more replies)
4 siblings, 3 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-12 21:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, Taylor Blau, John Cai
git-pack-refs does not currently give much control over which refs are
packed. By default, all tags and already packed refs are packed. --all
allows the user to pack all refs. Beyond this the user does not have
control. Introduce a pair of options --include and --exclude that will allow
users full control over which refs end up in the packed-refs file.
Changes since v3:
* got rid of extending ref_exclusions
* use a strvec instead of a string_list to keep track of included refs
Changes since v2:
* repurpose ref_exclusions to be used for ref inclusions
* fixed test formatting
* adjusted --include to not include default of all tags
Changes since v1:
* Clarify that --all packs not just branch tips but all refs under refs/ in
the documentation in patch 1
* Add --include in patch 3
It's worth noting that [1] discussed a proposal for a pack refs v2 format
that would improve deletion speeds. The ref-table backend would also improve
performance of deletions. However, both of those proposals are still being
discussed.
1. https://lore.kernel.org/git/pull.1408.git.1667846164.gitgitgadget@gmail.com/
John Cai (3):
docs: clarify git-pack-refs --all will pack all refs
pack-refs: teach --exclude option to exclude refs from being packed
pack-refs: teach pack-refs --include option
Documentation/git-pack-refs.txt | 29 +++++++++++++++++++++++---
builtin/pack-refs.c | 31 +++++++++++++++++++++++----
refs.c | 4 ++--
refs.h | 8 ++++++-
refs/debug.c | 4 ++--
refs/files-backend.c | 26 ++++++++++++++---------
refs/packed-backend.c | 2 +-
refs/refs-internal.h | 3 ++-
revision.h | 2 +-
t/helper/test-ref-store.c | 11 +++++++++-
t/t3210-pack-refs.sh | 37 +++++++++++++++++++++++++++++++++
11 files changed, 131 insertions(+), 26 deletions(-)
base-commit: 91428f078b8a4fe6948a4c955af1a693841e3985
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1501%2Fjohn-cai%2Fjc%2Fexclude-refs-from-pack-refs-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1501/john-cai/jc/exclude-refs-from-pack-refs-v4
Pull-Request: https://github.com/git/git/pull/1501
Range-diff vs v3:
1: 0d462010b79 ! 1: 554da1dc705 docs: clarify git-pack-refs --all will pack all refs
@@ Commit message
Signed-off-by: John Cai <johncai86@gmail.com>
## Documentation/git-pack-refs.txt ##
-@@ Documentation/git-pack-refs.txt: OPTIONS
-
- The command by default packs all tags and refs that are already
+@@ Documentation/git-pack-refs.txt: The command by default packs all tags and refs that are already
packed, and leaves other refs
--alone. This is because branches are expected to be actively
-+alone. This is because branches are expected to be actively
+ alone. This is because branches are expected to be actively
developed and packing their tips does not help performance.
-This option causes branch tips to be packed as well. Useful for
-a repository with many branches of historical interests.
-+This option causes all refs to be packed as well, with the exception of hidden
-+and broken refs. Useful for a repository with many branches of historical
-+interests.
++This option causes all refs to be packed as well, with the exception
++of hidden refs, broken refs, and symbolic refs. Useful for a repository
++with many branches of historical interests.
--no-prune::
2: 8c5c66a3050 ! 2: 69300845df1 pack-refs: teach --exclude option to exclude refs from being packed
@@ Documentation/git-pack-refs.txt: git-pack-refs - Pack heads and tags for efficie
DESCRIPTION
-----------
-@@ Documentation/git-pack-refs.txt: interests.
+@@ Documentation/git-pack-refs.txt: with many branches of historical interests.
The command usually removes loose refs under `$GIT_DIR/refs`
hierarchy after packing them. This option tells it not to.
@@ Documentation/git-pack-refs.txt: interests.
+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`.
++When used with `--all`, pack only loose refs which do not match any of
++the provided `--exclude` patterns.
+
BUGS
@@ refs/files-backend.c: static void prune_refs(struct files_ref_store *refs, struc
REF_WORKTREE_SHARED)
return 0;
-+ if (opts->exclusions && ref_excluded(opts->exclusions, refname))
++ if (ref_excluded(opts->exclusions, refname))
+ return 0;
+
/* Do not pack non-tags unless PACK_REFS_ALL is set: */
@@ t/t3210-pack-refs.sh: test_expect_success \
+ git pack-refs --all --exclude "refs/heads/dont_pack*" &&
+ test -f .git/refs/heads/dont_pack1 &&
+ test -f .git/refs/heads/dont_pack2 &&
-+ ! test -f ./git/refs/heads/pack_this'
++ ! test -f .git/refs/heads/pack_this'
+
+test_expect_success 'test --no-exclude refs clears excluded refs' '
+ git branch dont_pack3 &&
3: 0a0693ad612 < -: ----------- revision: modify ref_exclusions to handle inclusions
4: b2f3b98cd24 ! 3: 4bbe4c05ceb pack-refs: teach pack-refs --include option
@@ Documentation/git-pack-refs.txt: git-pack-refs - Pack heads and tags for efficie
DESCRIPTION
-----------
-@@ Documentation/git-pack-refs.txt: interests.
+@@ Documentation/git-pack-refs.txt: with many branches of historical interests.
The command usually removes loose refs under `$GIT_DIR/refs`
hierarchy after packing them. This option tells it not to.
@@ Documentation/git-pack-refs.txt: interests.
Do not pack refs matching the given `glob(7)` pattern. Repetitions of this option
@@ Documentation/git-pack-refs.txt: unpack it.
- When used with `--all`, it will use the difference between the set of all refs,
- and what is provided to `--exclude`.
+ When used with `--all`, pack only loose refs which do not match any of
+ the provided `--exclude` patterns.
+When used with `--include`, refs provided to `--include`, minus refs that are
+provided to `--exclude` will be packed.
@@ Documentation/git-pack-refs.txt: unpack it.
## builtin/pack-refs.c ##
@@
- #include "refs.h"
- #include "repository.h"
#include "revision.h"
-+#include "trace.h"
static char const * const pack_refs_usage[] = {
- N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
@@ builtin/pack-refs.c
@@ builtin/pack-refs.c: int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
- static struct ref_visibility visibility = REF_VISIBILITY_INIT;
-- struct pack_refs_opts pack_refs_opts = {.visibility = &visibility, .flags = flags};
-+ struct pack_refs_opts pack_refs_opts = { .visibility = &visibility,
+ static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+- struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
++ static struct string_list included_refs = STRING_LIST_INIT_NODUP;
++ struct pack_refs_opts pack_refs_opts = { .exclusions = &excludes,
++ .includes = &included_refs,
+ .flags = flags };
static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
-+ static struct string_list option_included_refs = STRING_LIST_INIT_NODUP;
struct string_list_item *item;
struct option opts[] = {
OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
-+ OPT_STRING_LIST(0, "include", &option_included_refs, N_("pattern"),
++ OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"),
+ N_("references to include")),
OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
N_("references to exclude")),
OPT_END(),
@@ builtin/pack-refs.c: int cmd_pack_refs(int argc, const char **argv, const char *prefix)
for_each_string_list_item(item, &option_excluded_refs)
- add_ref_exclusion(pack_refs_opts.visibility, item->string);
+ add_ref_exclusion(pack_refs_opts.exclusions, item->string);
-+ for_each_string_list_item(item, &option_included_refs)
-+ add_ref_inclusion(pack_refs_opts.visibility, item->string);
-+
+ if (pack_refs_opts.flags & PACK_REFS_ALL)
-+ add_ref_inclusion(pack_refs_opts.visibility, "*");
++ string_list_append(pack_refs_opts.includes, "*");
+
-+ if (!pack_refs_opts.visibility->included_refs.nr)
-+ add_ref_inclusion(pack_refs_opts.visibility, "refs/tags/*");
++ if (!pack_refs_opts.includes->nr)
++ string_list_append(pack_refs_opts.includes, "refs/tags/*");
+
return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
}
+ ## refs.h ##
+@@ refs.h: struct worktree;
+ struct pack_refs_opts {
+ unsigned int flags;
+ struct ref_exclusions *exclusions;
++ struct string_list *includes;
+ };
+
+ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
+
## refs/files-backend.c ##
@@ refs/files-backend.c: static int should_pack_ref(const char *refname,
+ const struct object_id *oid, unsigned int ref_flags,
+ struct pack_refs_opts *opts)
+ {
++ struct string_list_item *item;
++
+ /* Do not pack per-worktree refs: */
+ if (parse_worktree_ref(refname, NULL, NULL, NULL) !=
REF_WORKTREE_SHARED)
return 0;
-- if (opts->visibility && ref_excluded(opts->visibility, refname))
+- if (ref_excluded(opts->exclusions, refname))
- return 0;
-
- /* Do not pack non-tags unless PACK_REFS_ALL is set: */
@@ refs/files-backend.c: static int should_pack_ref(const char *refname,
return 0;
- return 1;
-+ if (opts->visibility && ref_excluded(opts->visibility, refname))
++ if (ref_excluded(opts->exclusions, refname))
+ return 0;
+
-+ if (opts->visibility && ref_included(opts->visibility, refname))
-+ return 1;
++ for_each_string_list_item(item, opts->includes)
++ if (!wildmatch(item->string, refname, 0))
++ return 1;
+
+ return 0;
}
@@ t/helper/test-ref-store.c: static struct flag_definition pack_flags[] = { FLAG_D
{
unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
- struct pack_refs_opts pack_opts = { .flags = flags };
-+ static struct ref_visibility visibility = REF_VISIBILITY_INIT;
++ static struct ref_exclusions exclusions = REF_EXCLUSIONS_INIT;
++ static struct string_list included_refs = STRING_LIST_INIT_NODUP;
+ struct pack_refs_opts pack_opts = { .flags = flags,
-+ .visibility = &visibility };
++ .exclusions = &exclusions,
++ .includes = &included_refs };
+
+ if (pack_opts.flags & PACK_REFS_ALL)
-+ add_ref_inclusion(pack_opts.visibility, "*");
++ string_list_append(pack_opts.includes, "*");
return refs_pack_refs(refs, &pack_opts);
}
@@ t/t3210-pack-refs.sh: test_expect_success 'test --no-exclude refs clears exclude
+ git tag dont_pack5 &&
+ git pack-refs --include "refs/heads/pack_this*" &&
+ test -f .git/refs/tags/dont_pack5 &&
-+ ! test -f ./git/refs/heads/pack_this1 &&
-+ ! test -f ./git/refs/heads/pack_this2'
++ ! test -f .git/refs/heads/pack_this1 &&
++ ! test -f .git/refs/heads/pack_this2'
+
+test_expect_success 'test --no-include refs clears included refs' '
+ git branch pack1 &&
--
gitgitgadget
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 1/3] docs: clarify git-pack-refs --all will pack all refs
2023-05-12 21:34 ` [PATCH v4 0/3] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
@ 2023-05-12 21:34 ` John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 2/3] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 3/3] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
2 siblings, 0 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-12 21:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, Taylor Blau, John Cai, John Cai
From: John Cai <johncai86@gmail.com>
--all packs not just branch tips but anything under refs/ with the
exception of hidden refs and broken refs. Clarify this in the
documentation.
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-pack-refs.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
index 154081f2de2..22f00665006 100644
--- a/Documentation/git-pack-refs.txt
+++ b/Documentation/git-pack-refs.txt
@@ -51,8 +51,9 @@ The command by default packs all tags and refs that are already
packed, and leaves other refs
alone. This is because branches are expected to be actively
developed and packing their tips does not help performance.
-This option causes branch tips to be packed as well. Useful for
-a repository with many branches of historical interests.
+This option causes all refs to be packed as well, with the exception
+of hidden refs, broken refs, and symbolic refs. Useful for a repository
+with many branches of historical interests.
--no-prune::
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 2/3] pack-refs: teach --exclude option to exclude refs from being packed
2023-05-12 21:34 ` [PATCH v4 0/3] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 1/3] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
@ 2023-05-12 21:34 ` John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 3/3] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
2 siblings, 0 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-12 21:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, Taylor Blau, John Cai, John Cai
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(-)
diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
index 22f00665006..546aa122dff 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 @@ with many branches of historical 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`, pack only loose refs which do not match any of
+the provided `--exclude` patterns.
+
BUGS
----
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 9833815fb30..1d1a64fe386 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -4,22 +4,34 @@
#include "parse-options.h"
#include "refs.h"
#include "repository.h"
+#include "revision.h"
static char const * const pack_refs_usage[] = {
- N_("git pack-refs [--all] [--no-prune]"),
+ N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
NULL
};
int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
+ static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+ struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
+ static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
+ struct string_list_item *item;
+
struct option opts[] = {
- OPT_BIT(0, "all", &flags, N_("pack everything"), PACK_REFS_ALL),
- OPT_BIT(0, "prune", &flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
+ OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
+ N_("references to exclude")),
OPT_END(),
};
git_config(git_default_config, NULL);
if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
- return refs_pack_refs(get_main_ref_store(the_repository), flags);
+
+ for_each_string_list_item(item, &option_excluded_refs)
+ add_ref_exclusion(pack_refs_opts.exclusions, item->string);
+
+ return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
}
diff --git a/refs.c b/refs.c
index d2a98e1c21f..881a0da65cf 100644
--- a/refs.c
+++ b/refs.c
@@ -2132,9 +2132,9 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
}
/* backend functions */
-int refs_pack_refs(struct ref_store *refs, unsigned int flags)
+int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)
{
- return refs->be->pack_refs(refs, flags);
+ return refs->be->pack_refs(refs, opts);
}
int peel_iterated_oid(const struct object_id *base, struct object_id *peeled)
diff --git a/refs.h b/refs.h
index 123cfa44244..46020bd335c 100644
--- a/refs.h
+++ b/refs.h
@@ -63,6 +63,11 @@ struct worktree;
#define RESOLVE_REF_NO_RECURSE 0x02
#define RESOLVE_REF_ALLOW_BAD_NAME 0x04
+struct pack_refs_opts {
+ unsigned int flags;
+ struct ref_exclusions *exclusions;
+};
+
const char *refs_resolve_ref_unsafe(struct ref_store *refs,
const char *refname,
int resolve_flags,
@@ -405,7 +410,7 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt,
* Write a packed-refs file for the current repository.
* flags: Combination of the above PACK_REFS_* flags.
*/
-int refs_pack_refs(struct ref_store *refs, unsigned int flags);
+int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts);
/*
* Setup reflog before using. Fill in err and return -1 on failure.
diff --git a/refs/debug.c b/refs/debug.c
index 6f11e6de46c..c0fa707a1da 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -123,10 +123,10 @@ static int debug_initial_transaction_commit(struct ref_store *refs,
return res;
}
-static int debug_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
- int res = drefs->refs->be->pack_refs(drefs->refs, flags);
+ int res = drefs->refs->be->pack_refs(drefs->refs, opts);
trace_printf_key(&trace_refs, "pack_refs: %d\n", res);
return res;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index bca7b851c5a..5075e6c0d28 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -21,6 +21,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
@@ -1175,15 +1176,18 @@ 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) !=
REF_WORKTREE_SHARED)
return 0;
+ if (ref_excluded(opts->exclusions, 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: */
@@ -1197,7 +1201,8 @@ static int should_pack_ref(const char *refname,
return 1;
}
-static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int files_pack_refs(struct ref_store *ref_store,
+ struct pack_refs_opts *opts)
{
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,
@@ -1222,8 +1227,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
* in the packed ref cache. If the reference should be
* pruned, also add it to refs_to_prune.
*/
- if (!should_pack_ref(iter->refname, iter->oid, iter->flags,
- flags))
+ if (!should_pack_ref(iter->refname, iter->oid, iter->flags, opts))
continue;
/*
@@ -1237,7 +1241,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
iter->refname, err.buf);
/* Schedule the loose reference for pruning if requested. */
- if ((flags & PACK_REFS_PRUNE)) {
+ if ((opts->flags & PACK_REFS_PRUNE)) {
struct ref_to_prune *n;
FLEX_ALLOC_STR(n, name, iter->refname);
oidcpy(&n->oid, iter->oid);
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 5b412a133be..291e53f5cfe 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1577,7 +1577,7 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
}
static int packed_pack_refs(struct ref_store *ref_store UNUSED,
- unsigned int flags UNUSED)
+ struct pack_refs_opts *pack_opts UNUSED)
{
/*
* Packed refs are already packed. It might be that loose refs
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index a85d113123c..f72b7be8941 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -547,7 +547,8 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err);
-typedef int pack_refs_fn(struct ref_store *ref_store, unsigned int flags);
+typedef int pack_refs_fn(struct ref_store *ref_store,
+ struct pack_refs_opts *opts);
typedef int create_symref_fn(struct ref_store *ref_store,
const char *ref_target,
const char *refs_heads_master,
diff --git a/revision.h b/revision.h
index 31828748dc0..25776af3815 100644
--- a/revision.h
+++ b/revision.h
@@ -87,7 +87,7 @@ struct rev_cmdline_info {
struct ref_exclusions {
/*
* Excluded refs is a list of wildmatch patterns. If any of the
- * patterns matches, the reference will be excluded.
+ * patterns match, the reference will be excluded.
*/
struct string_list excluded_refs;
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 6d8f844e9c7..de4197708d9 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -116,8 +116,9 @@ static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
static int cmd_pack_refs(struct ref_store *refs, const char **argv)
{
unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
+ struct pack_refs_opts pack_opts = { .flags = flags };
- return refs_pack_refs(refs, flags);
+ return refs_pack_refs(refs, &pack_opts);
}
static int cmd_create_symref(struct ref_store *refs, const char **argv)
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index 07a0ff93def..925b90cd3ba 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -108,6 +108,22 @@ 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*" &&
+ 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 &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 3/3] pack-refs: teach pack-refs --include option
2023-05-12 21:34 ` [PATCH v4 0/3] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 1/3] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 2/3] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
@ 2023-05-12 21:34 ` John Cai via GitGitGadget
2 siblings, 0 replies; 29+ messages in thread
From: John Cai via GitGitGadget @ 2023-05-12 21:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Christian Couder, Taylor Blau, John Cai, John Cai
From: John Cai <johncai86@gmail.com>
Allow users to be more selective over which refs to pack by adding an
--include option to git-pack-refs.
The existing options allow some measure of selectivity. By default
git-pack-refs packs all tags. --all can be used to include all refs,
and the previous commit added the ability to exclude certain refs with
--exclude.
While these knobs give the user some selection over which refs to pack,
it could be useful to give more control. For instance, a repository may
have a set of branches that are rarely updated and would benefit from
being packed. --include would allow the user to easily include a set of
branches to be packed while leaving everything else unpacked.
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-pack-refs.txt | 14 +++++++++++++-
builtin/pack-refs.c | 15 +++++++++++++--
refs.h | 1 +
refs/files-backend.c | 18 ++++++++++--------
t/helper/test-ref-store.c | 10 +++++++++-
t/t3210-pack-refs.sh | 21 +++++++++++++++++++++
6 files changed, 67 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
index 546aa122dff..284956acb3c 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] [--exclude <pattern>]
+'git pack-refs' [--all] [--no-prune] [--include <pattern>] [--exclude <pattern>]
DESCRIPTION
-----------
@@ -60,6 +60,15 @@ with many branches of historical interests.
The command usually removes loose refs under `$GIT_DIR/refs`
hierarchy after packing them. This option tells it not to.
+--include <pattern>::
+
+Pack refs based on a `glob(7)` pattern. Repetitions of this option
+accumulate inclusion patterns. If a ref is both included in `--include` and
+`--exclude`, `--exclude` takes precedence. Using `--include` will preclude all
+tags from being included by default. Symbolic refs and broken refs will never
+be packed. When used with `--all`, it will be a noop. Use `--no-include` to clear
+and reset the list of patterns.
+
--exclude <pattern>::
Do not pack refs matching the given `glob(7)` pattern. Repetitions of this option
@@ -70,6 +79,9 @@ unpack it.
When used with `--all`, pack only loose refs which do not match any of
the provided `--exclude` patterns.
+When used with `--include`, refs provided to `--include`, minus refs that are
+provided to `--exclude` will be packed.
+
BUGS
----
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 1d1a64fe386..bcf383cac9d 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -7,7 +7,7 @@
#include "revision.h"
static char const * const pack_refs_usage[] = {
- N_("git pack-refs [--all] [--no-prune] [--exclude <pattern>]"),
+ N_("git pack-refs [--all] [--no-prune] [--include <pattern>] [--exclude <pattern>]"),
NULL
};
@@ -15,13 +15,18 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
static struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
- struct pack_refs_opts pack_refs_opts = {.exclusions = &excludes, .flags = flags};
+ static struct string_list included_refs = STRING_LIST_INIT_NODUP;
+ struct pack_refs_opts pack_refs_opts = { .exclusions = &excludes,
+ .includes = &included_refs,
+ .flags = flags };
static struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
struct string_list_item *item;
struct option opts[] = {
OPT_BIT(0, "all", &pack_refs_opts.flags, N_("pack everything"), PACK_REFS_ALL),
OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
+ OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"),
+ N_("references to include")),
OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
N_("references to exclude")),
OPT_END(),
@@ -33,5 +38,11 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
for_each_string_list_item(item, &option_excluded_refs)
add_ref_exclusion(pack_refs_opts.exclusions, item->string);
+ if (pack_refs_opts.flags & PACK_REFS_ALL)
+ string_list_append(pack_refs_opts.includes, "*");
+
+ if (!pack_refs_opts.includes->nr)
+ string_list_append(pack_refs_opts.includes, "refs/tags/*");
+
return refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
}
diff --git a/refs.h b/refs.h
index 46020bd335c..933fdebe584 100644
--- a/refs.h
+++ b/refs.h
@@ -66,6 +66,7 @@ struct worktree;
struct pack_refs_opts {
unsigned int flags;
struct ref_exclusions *exclusions;
+ struct string_list *includes;
};
const char *refs_resolve_ref_unsafe(struct ref_store *refs,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 5075e6c0d28..629e95ff857 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1178,18 +1178,13 @@ static int should_pack_ref(const char *refname,
const struct object_id *oid, unsigned int ref_flags,
struct pack_refs_opts *opts)
{
+ struct string_list_item *item;
+
/* Do not pack per-worktree refs: */
if (parse_worktree_ref(refname, NULL, NULL, NULL) !=
REF_WORKTREE_SHARED)
return 0;
- if (ref_excluded(opts->exclusions, refname))
- return 0;
-
- /* Do not pack non-tags unless PACK_REFS_ALL is set: */
- if (!(opts->flags & PACK_REFS_ALL) && !starts_with(refname, "refs/tags/"))
- return 0;
-
/* Do not pack symbolic refs: */
if (ref_flags & REF_ISSYMREF)
return 0;
@@ -1198,7 +1193,14 @@ static int should_pack_ref(const char *refname,
if (!ref_resolves_to_object(refname, the_repository, oid, ref_flags))
return 0;
- return 1;
+ if (ref_excluded(opts->exclusions, refname))
+ return 0;
+
+ for_each_string_list_item(item, opts->includes)
+ if (!wildmatch(item->string, refname, 0))
+ return 1;
+
+ return 0;
}
static int files_pack_refs(struct ref_store *ref_store,
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index de4197708d9..a6977b5e839 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -5,6 +5,7 @@
#include "worktree.h"
#include "object-store.h"
#include "repository.h"
+#include "revision.h"
struct flag_definition {
const char *name;
@@ -116,7 +117,14 @@ static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
static int cmd_pack_refs(struct ref_store *refs, const char **argv)
{
unsigned int flags = arg_flags(*argv++, "flags", pack_flags);
- struct pack_refs_opts pack_opts = { .flags = flags };
+ static struct ref_exclusions exclusions = REF_EXCLUSIONS_INIT;
+ static struct string_list included_refs = STRING_LIST_INIT_NODUP;
+ struct pack_refs_opts pack_opts = { .flags = flags,
+ .exclusions = &exclusions,
+ .includes = &included_refs };
+
+ if (pack_opts.flags & PACK_REFS_ALL)
+ string_list_append(pack_opts.includes, "*");
return refs_pack_refs(refs, &pack_opts);
}
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index 925b90cd3ba..9f399d2f75a 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -124,6 +124,27 @@ test_expect_success 'test --no-exclude refs clears excluded refs' '
! test -f .git/refs/heads/dont_pack3 &&
! test -f .git/refs/heads/dont_pack4'
+test_expect_success 'test only included refs are packed' '
+ git branch pack_this1 &&
+ git branch pack_this2 &&
+ git tag dont_pack5 &&
+ git pack-refs --include "refs/heads/pack_this*" &&
+ test -f .git/refs/tags/dont_pack5 &&
+ ! test -f .git/refs/heads/pack_this1 &&
+ ! test -f .git/refs/heads/pack_this2'
+
+test_expect_success 'test --no-include refs clears included refs' '
+ git branch pack1 &&
+ git branch pack2 &&
+ git pack-refs --include "refs/heads/pack*" --no-include &&
+ test -f .git/refs/heads/pack1 &&
+ test -f .git/refs/heads/pack2'
+
+test_expect_success 'test --exclude takes precedence over --include' '
+ git branch dont_pack5 &&
+ git pack-refs --include "refs/heads/pack*" --exclude "refs/heads/pack*" &&
+ test -f .git/refs/heads/dont_pack5'
+
test_expect_success \
'see if up-to-date packed refs are preserved' \
'git branch q &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 29+ messages in thread
end of thread, other threads:[~2023-05-12 21:35 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-04 15:48 [PATCH] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
2023-05-04 16:48 ` Junio C Hamano
2023-05-04 21:26 ` John Cai
2023-05-09 19:18 ` [PATCH v2 0/3] pack-refs: Teach " John Cai via GitGitGadget
2023-05-09 19:18 ` [PATCH v2 1/3] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
2023-05-09 19:18 ` [PATCH v2 2/3] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
2023-05-09 21:04 ` Junio C Hamano
2023-05-09 19:18 ` [PATCH v2 3/3] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
2023-05-09 21:25 ` Junio C Hamano
2023-05-10 19:52 ` John Cai
2023-05-11 18:10 ` [PATCH v3 0/4] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
2023-05-11 18:10 ` [PATCH v3 1/4] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
2023-05-11 23:53 ` Taylor Blau
2023-05-11 18:10 ` [PATCH v3 2/4] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
2023-05-11 19:34 ` Junio C Hamano
2023-05-12 0:00 ` Taylor Blau
2023-05-12 12:53 ` John Cai
2023-05-12 21:11 ` John Cai
2023-05-11 18:10 ` [PATCH v3 3/4] revision: modify ref_exclusions to handle inclusions John Cai via GitGitGadget
2023-05-11 19:54 ` Junio C Hamano
2023-05-12 14:56 ` John Cai
2023-05-11 18:10 ` [PATCH v3 4/4] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
2023-05-11 20:06 ` Junio C Hamano
2023-05-12 14:48 ` John Cai
2023-05-12 19:03 ` John Cai
2023-05-12 21:34 ` [PATCH v4 0/3] pack-refs: allow users control over which refs to pack John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 1/3] docs: clarify git-pack-refs --all will pack all refs John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 2/3] pack-refs: teach --exclude option to exclude refs from being packed John Cai via GitGitGadget
2023-05-12 21:34 ` [PATCH v4 3/3] pack-refs: teach pack-refs --include option John Cai via GitGitGadget
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).