* [PATCH v3 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow`
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
@ 2021-04-09 11:27 ` Patrick Steinhardt
2021-04-09 11:27 ` [PATCH v3 2/8] revision: mark commit parents as NOT_USER_GIVEN Patrick Steinhardt
` (8 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-09 11:27 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]
When `uploadpackfilter.allow` is set to `true`, it means that filters
are enabled by default except in the case where a filter is explicitly
disabled via `uploadpackilter.<filter>.allow`. This option will not only
enable the currently supported set of filters, but also any filters
which get added in the future. As such, an admin which wants to have
tight control over which filters are allowed and which aren't probably
shouldn't ever set `uploadpackfilter.allow=true`.
Amend the documentation to make the ramifications more explicit so that
admins are aware of this.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/config/uploadpack.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
index b0d761282c..6729a072ea 100644
--- a/Documentation/config/uploadpack.txt
+++ b/Documentation/config/uploadpack.txt
@@ -59,7 +59,8 @@ uploadpack.allowFilter::
uploadpackfilter.allow::
Provides a default value for unspecified object filters (see: the
- below configuration variable).
+ below configuration variable). If set to `true`, this will also
+ enable all filters which get added in the future.
Defaults to `true`.
uploadpackfilter.<filter>.allow::
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v3 2/8] revision: mark commit parents as NOT_USER_GIVEN
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
2021-04-09 11:27 ` [PATCH v3 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow` Patrick Steinhardt
@ 2021-04-09 11:27 ` Patrick Steinhardt
2021-04-09 11:28 ` [PATCH v3 3/8] list-objects: move tag processing into its own function Patrick Steinhardt
` (7 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-09 11:27 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 2338 bytes --]
The NOT_USER_GIVEN flag of an object marks whether a flag was explicitly
provided by the user or not. The most important use case for this is
when filtering objects: only objects that were not explicitly requested
will get filtered.
The flag is currently only set for blobs and trees, which has been fine
given that there are no filters for tags or commits currently. We're
about to extend filtering capabilities to add object type filter though,
which requires us to set up the NOT_USER_GIVEN flag correctly -- if it's
not set, the object wouldn't get filtered at all.
Mark unseen commit parents as NOT_USER_GIVEN when processing parents.
Like this, explicitly provided parents stay user-given and thus
unfiltered, while parents which get loaded as part of the graph walk
can be filtered.
This commit shouldn't have any user-visible impact yet as there is no
logic to filter commits yet.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
revision.c | 4 ++--
revision.h | 3 ---
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/revision.c b/revision.c
index 553c0faa9b..fd34c75e23 100644
--- a/revision.c
+++ b/revision.c
@@ -1123,7 +1123,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
mark_parents_uninteresting(p);
if (p->object.flags & SEEN)
continue;
- p->object.flags |= SEEN;
+ p->object.flags |= (SEEN | NOT_USER_GIVEN);
if (list)
commit_list_insert_by_date(p, list);
if (queue)
@@ -1165,7 +1165,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
}
p->object.flags |= left_flag;
if (!(p->object.flags & SEEN)) {
- p->object.flags |= SEEN;
+ p->object.flags |= (SEEN | NOT_USER_GIVEN);
if (list)
commit_list_insert_by_date(p, list);
if (queue)
diff --git a/revision.h b/revision.h
index a24f72dcd1..93aa012f51 100644
--- a/revision.h
+++ b/revision.h
@@ -44,9 +44,6 @@
/*
* Indicates object was reached by traversal. i.e. not given by user on
* command-line or stdin.
- * NEEDSWORK: NOT_USER_GIVEN doesn't apply to commits because we only support
- * filtering trees and blobs, but it may be useful to support filtering commits
- * in the future.
*/
#define NOT_USER_GIVEN (1u<<25)
#define TRACK_LINEAR (1u<<26)
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v3 3/8] list-objects: move tag processing into its own function
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
2021-04-09 11:27 ` [PATCH v3 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow` Patrick Steinhardt
2021-04-09 11:27 ` [PATCH v3 2/8] revision: mark commit parents as NOT_USER_GIVEN Patrick Steinhardt
@ 2021-04-09 11:28 ` Patrick Steinhardt
2021-04-09 11:28 ` [PATCH v3 4/8] list-objects: support filtering by tag and commit Patrick Steinhardt
` (6 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-09 11:28 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]
Move processing of tags into its own function to make the logic easier
to extend when we're going to implement filtering for tags. No change in
behaviour is expected from this commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
list-objects.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/list-objects.c b/list-objects.c
index e19589baa0..a5a60301cb 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -213,6 +213,14 @@ static void process_tree(struct traversal_context *ctx,
free_tree_buffer(tree);
}
+static void process_tag(struct traversal_context *ctx,
+ struct tag *tag,
+ const char *name)
+{
+ tag->object.flags |= SEEN;
+ ctx->show_object(&tag->object, name, ctx->show_data);
+}
+
static void mark_edge_parents_uninteresting(struct commit *commit,
struct rev_info *revs,
show_edge_fn show_edge)
@@ -334,8 +342,7 @@ static void traverse_trees_and_blobs(struct traversal_context *ctx,
if (obj->flags & (UNINTERESTING | SEEN))
continue;
if (obj->type == OBJ_TAG) {
- obj->flags |= SEEN;
- ctx->show_object(obj, name, ctx->show_data);
+ process_tag(ctx, (struct tag *)obj, name);
continue;
}
if (!path)
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v3 4/8] list-objects: support filtering by tag and commit
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
` (2 preceding siblings ...)
2021-04-09 11:28 ` [PATCH v3 3/8] list-objects: move tag processing into its own function Patrick Steinhardt
@ 2021-04-09 11:28 ` Patrick Steinhardt
2021-04-11 6:49 ` Junio C Hamano
2021-04-09 11:28 ` [PATCH v3 5/8] list-objects: implement object type filter Patrick Steinhardt
` (5 subsequent siblings)
9 siblings, 1 reply; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-09 11:28 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 4789 bytes --]
Object filters currently only support filtering blobs or trees based on
some criteria. This commit lays the foundation to also allow filtering
of tags and commits.
No change in behaviour is expected from this commit given that there are
no filters yet for those object types.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
list-objects-filter.c | 40 ++++++++++++++++++++++++++++++++++++++++
list-objects-filter.h | 2 ++
list-objects.c | 22 +++++++++++++++++++---
3 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 39e2f15333..0ebfa52966 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -82,6 +82,16 @@ static enum list_objects_filter_result filter_blobs_none(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
/* always include all tree objects */
@@ -173,6 +183,16 @@ static enum list_objects_filter_result filter_trees_depth(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_END_TREE:
assert(obj->type == OBJ_TREE);
filter_data->current_depth--;
@@ -267,6 +287,16 @@ static enum list_objects_filter_result filter_blobs_limit(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
/* always include all tree objects */
@@ -371,6 +401,16 @@ static enum list_objects_filter_result filter_sparse(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
dtype = DT_DIR;
diff --git a/list-objects-filter.h b/list-objects-filter.h
index cfd784e203..9e98814111 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -55,6 +55,8 @@ enum list_objects_filter_result {
};
enum list_objects_filter_situation {
+ LOFS_COMMIT,
+ LOFS_TAG,
LOFS_BEGIN_TREE,
LOFS_END_TREE,
LOFS_BLOB
diff --git a/list-objects.c b/list-objects.c
index a5a60301cb..0c524a81ac 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -217,8 +217,14 @@ static void process_tag(struct traversal_context *ctx,
struct tag *tag,
const char *name)
{
- tag->object.flags |= SEEN;
- ctx->show_object(&tag->object, name, ctx->show_data);
+ enum list_objects_filter_result r;
+
+ r = list_objects_filter__filter_object(ctx->revs->repo, LOFS_TAG,
+ &tag->object, "", 0, ctx->filter);
+ if (r & LOFR_MARK_SEEN)
+ tag->object.flags |= SEEN;
+ if (r & LOFR_DO_SHOW)
+ ctx->show_object(&tag->object, name, ctx->show_data);
}
static void mark_edge_parents_uninteresting(struct commit *commit,
@@ -368,6 +374,12 @@ static void do_traverse(struct traversal_context *ctx)
strbuf_init(&csp, PATH_MAX);
while ((commit = get_revision(ctx->revs)) != NULL) {
+ enum list_objects_filter_result r;
+
+ r = list_objects_filter__filter_object(ctx->revs->repo,
+ LOFS_COMMIT, &commit->object,
+ NULL, NULL, ctx->filter);
+
/*
* an uninteresting boundary commit may not have its tree
* parsed yet, but we are not going to show them anyway
@@ -382,7 +394,11 @@ static void do_traverse(struct traversal_context *ctx)
die(_("unable to load root tree for commit %s"),
oid_to_hex(&commit->object.oid));
}
- ctx->show_commit(commit, ctx->show_data);
+
+ if (r & LOFR_MARK_SEEN)
+ commit->object.flags |= SEEN;
+ if (r & LOFR_DO_SHOW)
+ ctx->show_commit(commit, ctx->show_data);
if (ctx->revs->tree_blobs_in_commit_order)
/*
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* Re: [PATCH v3 4/8] list-objects: support filtering by tag and commit
2021-04-09 11:28 ` [PATCH v3 4/8] list-objects: support filtering by tag and commit Patrick Steinhardt
@ 2021-04-11 6:49 ` Junio C Hamano
0 siblings, 0 replies; 97+ messages in thread
From: Junio C Hamano @ 2021-04-11 6:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King, Christian Couder, Taylor Blau
Patrick Steinhardt <ps@pks.im> writes:
> Object filters currently only support filtering blobs or trees based on
> some criteria. This commit lays the foundation to also allow filtering
> of tags and commits.
>
> No change in behaviour is expected from this commit given that there are
> no filters yet for those object types.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> list-objects-filter.c | 40 ++++++++++++++++++++++++++++++++++++++++
> list-objects-filter.h | 2 ++
> list-objects.c | 22 +++++++++++++++++++---
> 3 files changed, 61 insertions(+), 3 deletions(-)
>
> diff --git a/list-objects-filter.c b/list-objects-filter.c
> index 39e2f15333..0ebfa52966 100644
> --- a/list-objects-filter.c
> +++ b/list-objects-filter.c
> @@ -82,6 +82,16 @@ static enum list_objects_filter_result filter_blobs_none(
> default:
> BUG("unknown filter_situation: %d", filter_situation);
>
> + case LOFS_TAG:
> + assert(obj->type == OBJ_TAG);
> + /* always include all tag objects */
> + return LOFR_MARK_SEEN | LOFR_DO_SHOW;
> +
> + case LOFS_COMMIT:
> + assert(obj->type == OBJ_COMMIT);
> + /* always include all commit objects */
> + return LOFR_MARK_SEEN | LOFR_DO_SHOW;
> +
> case LOFS_BEGIN_TREE:
> assert(obj->type == OBJ_TREE);
> /* always include all tree objects */
> @@ -173,6 +183,16 @@ static enum list_objects_filter_result filter_trees_depth(
> default:
> BUG("unknown filter_situation: %d", filter_situation);
>
> + case LOFS_TAG:
> + assert(obj->type == OBJ_TAG);
> + /* always include all tag objects */
> + return LOFR_MARK_SEEN | LOFR_DO_SHOW;
> +
> + case LOFS_COMMIT:
> + assert(obj->type == OBJ_COMMIT);
> + /* always include all commit objects */
> + return LOFR_MARK_SEEN | LOFR_DO_SHOW;
> +
> case LOFS_END_TREE:
> assert(obj->type == OBJ_TREE);
> filter_data->current_depth--;
> @@ -267,6 +287,16 @@ static enum list_objects_filter_result filter_blobs_limit(
> default:
> BUG("unknown filter_situation: %d", filter_situation);
>
> + case LOFS_TAG:
> + assert(obj->type == OBJ_TAG);
> + /* always include all tag objects */
> + return LOFR_MARK_SEEN | LOFR_DO_SHOW;
> +
> + case LOFS_COMMIT:
> + assert(obj->type == OBJ_COMMIT);
> + /* always include all commit objects */
> + return LOFR_MARK_SEEN | LOFR_DO_SHOW;
> +
> case LOFS_BEGIN_TREE:
> assert(obj->type == OBJ_TREE);
> /* always include all tree objects */
> @@ -371,6 +401,16 @@ static enum list_objects_filter_result filter_sparse(
> default:
> BUG("unknown filter_situation: %d", filter_situation);
>
> + case LOFS_TAG:
> + assert(obj->type == OBJ_TAG);
> + /* always include all tag objects */
> + return LOFR_MARK_SEEN | LOFR_DO_SHOW;
> +
> + case LOFS_COMMIT:
> + assert(obj->type == OBJ_COMMIT);
> + /* always include all commit objects */
> + return LOFR_MARK_SEEN | LOFR_DO_SHOW;
> +
> case LOFS_BEGIN_TREE:
> assert(obj->type == OBJ_TREE);
> dtype = DT_DIR;
> diff --git a/list-objects-filter.h b/list-objects-filter.h
> index cfd784e203..9e98814111 100644
> --- a/list-objects-filter.h
> +++ b/list-objects-filter.h
> @@ -55,6 +55,8 @@ enum list_objects_filter_result {
> };
>
> enum list_objects_filter_situation {
> + LOFS_COMMIT,
> + LOFS_TAG,
> LOFS_BEGIN_TREE,
> LOFS_END_TREE,
> LOFS_BLOB
> diff --git a/list-objects.c b/list-objects.c
> index a5a60301cb..0c524a81ac 100644
> --- a/list-objects.c
> +++ b/list-objects.c
> @@ -217,8 +217,14 @@ static void process_tag(struct traversal_context *ctx,
> struct tag *tag,
> const char *name)
> {
> - tag->object.flags |= SEEN;
> - ctx->show_object(&tag->object, name, ctx->show_data);
> + enum list_objects_filter_result r;
> +
> + r = list_objects_filter__filter_object(ctx->revs->repo, LOFS_TAG,
> + &tag->object, "", 0, ctx->filter);
s/0/NULL/
> + if (r & LOFR_MARK_SEEN)
> + tag->object.flags |= SEEN;
> + if (r & LOFR_DO_SHOW)
> + ctx->show_object(&tag->object, name, ctx->show_data);
> }
>
> static void mark_edge_parents_uninteresting(struct commit *commit,
> @@ -368,6 +374,12 @@ static void do_traverse(struct traversal_context *ctx)
> strbuf_init(&csp, PATH_MAX);
>
> while ((commit = get_revision(ctx->revs)) != NULL) {
> + enum list_objects_filter_result r;
> +
> + r = list_objects_filter__filter_object(ctx->revs->repo,
> + LOFS_COMMIT, &commit->object,
> + NULL, NULL, ctx->filter);
> +
> /*
> * an uninteresting boundary commit may not have its tree
> * parsed yet, but we are not going to show them anyway
> @@ -382,7 +394,11 @@ static void do_traverse(struct traversal_context *ctx)
> die(_("unable to load root tree for commit %s"),
> oid_to_hex(&commit->object.oid));
> }
> - ctx->show_commit(commit, ctx->show_data);
> +
> + if (r & LOFR_MARK_SEEN)
> + commit->object.flags |= SEEN;
> + if (r & LOFR_DO_SHOW)
> + ctx->show_commit(commit, ctx->show_data);
>
> if (ctx->revs->tree_blobs_in_commit_order)
> /*
^ permalink raw reply [flat|nested] 97+ messages in thread
* [PATCH v3 5/8] list-objects: implement object type filter
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
` (3 preceding siblings ...)
2021-04-09 11:28 ` [PATCH v3 4/8] list-objects: support filtering by tag and commit Patrick Steinhardt
@ 2021-04-09 11:28 ` Patrick Steinhardt
2021-04-09 11:28 ` [PATCH v3 6/8] pack-bitmap: " Patrick Steinhardt
` (4 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-09 11:28 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 9502 bytes --]
While it already is possible to filter objects by some criteria in
git-rev-list(1), it is not yet possible to filter out only a specific
type of objects. This makes some filters less useful. The `blob:limit`
filter for example filters blobs such that only those which are smaller
than the given limit are returned. But it is unfit to ask only for these
smallish blobs, given that git-rev-list(1) will continue to print tags,
commits and trees.
Now that we have the infrastructure in place to also filter tags and
commits, we can improve this situation by implementing a new filter
which selects objects based on their type. Above query can thus
trivially be implemented with the following command:
$ git rev-list --objects --filter=object:type=blob \
--filter=blob:limit=200
Furthermore, this filter allows to optimize for certain other cases: if
for example only tags or commits have been selected, there is no need to
walk down trees.
The new filter is not yet supported in bitmaps. This is going to be
implemented in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/config/uploadpack.txt | 6 +--
Documentation/rev-list-options.txt | 3 ++
list-objects-filter-options.c | 14 ++++++
list-objects-filter-options.h | 2 +
list-objects-filter.c | 76 +++++++++++++++++++++++++++++
t/t6112-rev-list-filters-objects.sh | 48 ++++++++++++++++++
6 files changed, 146 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
index 6729a072ea..32fad5bbe8 100644
--- a/Documentation/config/uploadpack.txt
+++ b/Documentation/config/uploadpack.txt
@@ -66,9 +66,9 @@ uploadpackfilter.allow::
uploadpackfilter.<filter>.allow::
Explicitly allow or ban the object filter corresponding to
`<filter>`, where `<filter>` may be one of: `blob:none`,
- `blob:limit`, `tree`, `sparse:oid`, or `combine`. If using
- combined filters, both `combine` and all of the nested filter
- kinds must be allowed. Defaults to `uploadpackfilter.allow`.
+ `blob:limit`, `object:type`, `tree`, `sparse:oid`, or `combine`.
+ If using combined filters, both `combine` and all of the nested
+ filter kinds must be allowed. Defaults to `uploadpackfilter.allow`.
uploadpackfilter.tree.maxDepth::
Only allow `--filter=tree:<n>` when `<n>` is no more than the value of
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index b1c8f86c6e..3afa8fffbd 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -892,6 +892,9 @@ or units. n may be zero. The suffixes k, m, and g can be used to name
units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same
as 'blob:limit=1024'.
+
+The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects
+which are not of the requested type.
++
The form '--filter=sparse:oid=<blob-ish>' uses a sparse-checkout
specification contained in the blob (or blob-expression) '<blob-ish>'
to omit blobs that would not be not required for a sparse checkout on
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index d2d1c81caf..bb6f6577d5 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -29,6 +29,8 @@ const char *list_object_filter_config_name(enum list_objects_filter_choice c)
return "tree";
case LOFC_SPARSE_OID:
return "sparse:oid";
+ case LOFC_OBJECT_TYPE:
+ return "object:type";
case LOFC_COMBINE:
return "combine";
case LOFC__COUNT:
@@ -97,6 +99,18 @@ static int gently_parse_list_objects_filter(
}
return 1;
+ } else if (skip_prefix(arg, "object:type=", &v0)) {
+ int type = type_from_string_gently(v0, -1, 1);
+ if (type < 0) {
+ strbuf_addstr(errbuf, _("expected 'object:type=<type>'"));
+ return 1;
+ }
+
+ filter_options->object_type = type;
+ filter_options->choice = LOFC_OBJECT_TYPE;
+
+ return 0;
+
} else if (skip_prefix(arg, "combine:", &v0)) {
return parse_combine_filter(filter_options, v0, errbuf);
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index 01767c3c96..4d0d0588cc 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -13,6 +13,7 @@ enum list_objects_filter_choice {
LOFC_BLOB_LIMIT,
LOFC_TREE_DEPTH,
LOFC_SPARSE_OID,
+ LOFC_OBJECT_TYPE,
LOFC_COMBINE,
LOFC__COUNT /* must be last */
};
@@ -54,6 +55,7 @@ struct list_objects_filter_options {
char *sparse_oid_name;
unsigned long blob_limit_value;
unsigned long tree_exclude_depth;
+ enum object_type object_type;
/* LOFC_COMBINE values */
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 0ebfa52966..1c1ee3d1bb 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -545,6 +545,81 @@ static void filter_sparse_oid__init(
filter->free_fn = filter_sparse_free;
}
+/*
+ * A filter for list-objects to omit large blobs.
+ * And to OPTIONALLY collect a list of the omitted OIDs.
+ */
+struct filter_object_type_data {
+ enum object_type object_type;
+};
+
+static enum list_objects_filter_result filter_object_type(
+ struct repository *r,
+ enum list_objects_filter_situation filter_situation,
+ struct object *obj,
+ const char *pathname,
+ const char *filename,
+ struct oidset *omits,
+ void *filter_data_)
+{
+ struct filter_object_type_data *filter_data = filter_data_;
+
+ switch (filter_situation) {
+ default:
+ BUG("unknown filter_situation: %d", filter_situation);
+
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ if (filter_data->object_type == OBJ_TAG)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+ return LOFR_MARK_SEEN;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ if (filter_data->object_type == OBJ_COMMIT)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+ return LOFR_MARK_SEEN;
+
+ case LOFS_BEGIN_TREE:
+ assert(obj->type == OBJ_TREE);
+
+ /*
+ * If we only want to show commits or tags, then there is no
+ * need to walk down trees.
+ */
+ if (filter_data->object_type == OBJ_COMMIT ||
+ filter_data->object_type == OBJ_TAG)
+ return LOFR_SKIP_TREE;
+
+ if (filter_data->object_type == OBJ_TREE)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ return LOFR_MARK_SEEN;
+
+ case LOFS_BLOB:
+ assert(obj->type == OBJ_BLOB);
+
+ if (filter_data->object_type == OBJ_BLOB)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+ return LOFR_MARK_SEEN;
+
+ case LOFS_END_TREE:
+ return LOFR_ZERO;
+ }
+}
+
+static void filter_object_type__init(
+ struct list_objects_filter_options *filter_options,
+ struct filter *filter)
+{
+ struct filter_object_type_data *d = xcalloc(1, sizeof(*d));
+ d->object_type = filter_options->object_type;
+
+ filter->filter_data = d;
+ filter->filter_object_fn = filter_object_type;
+ filter->free_fn = free;
+}
+
/* A filter which only shows objects shown by all sub-filters. */
struct combine_filter_data {
struct subfilter *sub;
@@ -691,6 +766,7 @@ static filter_init_fn s_filters[] = {
filter_blobs_limit__init,
filter_trees_depth__init,
filter_sparse_oid__init,
+ filter_object_type__init,
filter_combine__init,
};
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index 31457d13b9..c79ec04060 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -159,6 +159,54 @@ test_expect_success 'verify blob:limit=1m' '
test_must_be_empty observed
'
+# Test object:type=<type> filter.
+
+test_expect_success 'setup object-type' '
+ git init object-type &&
+ echo contents >object-type/blob &&
+ git -C object-type add blob &&
+ git -C object-type commit -m commit-message &&
+ git -C object-type tag tag -m tag-message
+'
+
+test_expect_success 'verify object:type= fails with invalid type' '
+ test_must_fail git -C object-type rev-list --objects --filter=object:type= HEAD &&
+ test_must_fail git -C object-type rev-list --objects --filter=object:type=invalid HEAD
+'
+
+test_expect_success 'verify object:type=blob prints blob and commit' '
+ (
+ git -C object-type rev-parse HEAD &&
+ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob)
+ ) >expected &&
+ git -C object-type rev-list --objects --filter=object:type=blob HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tree prints tree and commit' '
+ (
+ git -C object-type rev-parse HEAD &&
+ printf "%s \n" $(git -C object-type rev-parse HEAD^{tree})
+ ) >expected &&
+ git -C object-type rev-list --objects --filter=object:type=tree HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=commit prints commit' '
+ git -C object-type rev-parse HEAD >expected &&
+ git -C object-type rev-list --objects --filter=object:type=commit HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tag prints tag' '
+ (
+ git -C object-type rev-parse HEAD &&
+ printf "%s tag\n" $(git -C object-type rev-parse tag)
+ ) >expected &&
+ git -C object-type rev-list --objects --filter=object:type=tag tag >actual &&
+ test_cmp expected actual
+'
+
# Test sparse:path=<path> filter.
# !!!!
# NOTE: sparse:path filter support has been dropped for security reasons,
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v3 6/8] pack-bitmap: implement object type filter
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
` (4 preceding siblings ...)
2021-04-09 11:28 ` [PATCH v3 5/8] list-objects: implement object type filter Patrick Steinhardt
@ 2021-04-09 11:28 ` Patrick Steinhardt
2021-04-09 11:28 ` [PATCH v3 7/8] pack-bitmap: implement combined filter Patrick Steinhardt
` (3 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-09 11:28 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 3827 bytes --]
The preceding commit has added a new object filter for git-rev-list(1)
which allows to filter objects by type. Implement the equivalent filter
for packfile bitmaps so that we can answer these queries fast.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 29 ++++++++++++++++++++++++++---
t/t6113-rev-list-bitmap-filters.sh | 25 ++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index b4513f8672..cd3f5c433e 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -779,9 +779,6 @@ static void filter_bitmap_exclude_type(struct bitmap_index *bitmap_git,
eword_t mask;
uint32_t i;
- if (type != OBJ_BLOB && type != OBJ_TREE)
- BUG("filter_bitmap_exclude_type: unsupported type '%d'", type);
-
/*
* The non-bitmap version of this filter never removes
* objects which the other side specifically asked for,
@@ -911,6 +908,24 @@ static void filter_bitmap_tree_depth(struct bitmap_index *bitmap_git,
OBJ_BLOB);
}
+static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
+ struct object_list *tip_objects,
+ struct bitmap *to_filter,
+ enum object_type object_type)
+{
+ if (object_type < OBJ_COMMIT || object_type > OBJ_TAG)
+ BUG("filter_bitmap_object_type given invalid object");
+
+ if (object_type != OBJ_TAG)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_TAG);
+ if (object_type != OBJ_COMMIT)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_COMMIT);
+ if (object_type != OBJ_TREE)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_TREE);
+ if (object_type != OBJ_BLOB)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_BLOB);
+}
+
static int filter_bitmap(struct bitmap_index *bitmap_git,
struct object_list *tip_objects,
struct bitmap *to_filter,
@@ -943,6 +958,14 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return 0;
}
+ if (filter->choice == LOFC_OBJECT_TYPE) {
+ if (bitmap_git)
+ filter_bitmap_object_type(bitmap_git, tip_objects,
+ to_filter,
+ filter->object_type);
+ return 0;
+ }
+
/* filter choice not handled */
return -1;
}
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index 3f889949ca..fb66735ac8 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -10,7 +10,8 @@ test_expect_success 'set up bitmapped repo' '
test_commit much-larger-blob-one &&
git repack -adb &&
test_commit two &&
- test_commit much-larger-blob-two
+ test_commit much-larger-blob-two &&
+ git tag tag
'
test_expect_success 'filters fallback to non-bitmap traversal' '
@@ -75,4 +76,26 @@ test_expect_success 'tree:1 filter' '
test_cmp expect actual
'
+test_expect_success 'object:type filter' '
+ git rev-list --objects --filter=object:type=tag tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=tag tag >actual &&
+ test_cmp expect actual &&
+
+ git rev-list --objects --filter=object:type=commit tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=commit tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter=object:type=tree tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=tree tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v3 7/8] pack-bitmap: implement combined filter
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
` (5 preceding siblings ...)
2021-04-09 11:28 ` [PATCH v3 6/8] pack-bitmap: " Patrick Steinhardt
@ 2021-04-09 11:28 ` Patrick Steinhardt
2021-04-09 11:28 ` [PATCH v3 8/8] rev-list: allow filtering of provided items Patrick Steinhardt
` (2 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-09 11:28 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 3298 bytes --]
When the user has multiple objects filters specified, then this is
internally represented by having a "combined" filter. These combined
filters aren't yet supported by bitmap indices and can thus not be
accelerated.
Fix this by implementing support for these combined filters. The
implementation is quite trivial: when there's a combined filter, we
simply recurse into `filter_bitmap()` for all of the sub-filters.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 41 +++++++++++++++++++++++++++---
t/t6113-rev-list-bitmap-filters.sh | 7 +++++
2 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index cd3f5c433e..4385f15828 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -926,6 +926,29 @@ static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_BLOB);
}
+static int filter_supported(struct list_objects_filter_options *filter)
+{
+ int i;
+
+ switch (filter->choice) {
+ case LOFC_BLOB_NONE:
+ case LOFC_BLOB_LIMIT:
+ case LOFC_OBJECT_TYPE:
+ return 1;
+ case LOFC_TREE_DEPTH:
+ if (filter->tree_exclude_depth == 0)
+ return 1;
+ return 0;
+ case LOFC_COMBINE:
+ for (i = 0; i < filter->sub_nr; i++)
+ if (!filter_supported(&filter->sub[i]))
+ return 0;
+ return 1;
+ default:
+ return 0;
+ }
+}
+
static int filter_bitmap(struct bitmap_index *bitmap_git,
struct object_list *tip_objects,
struct bitmap *to_filter,
@@ -933,6 +956,8 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
{
if (!filter || filter->choice == LOFC_DISABLED)
return 0;
+ if (!filter_supported(filter))
+ return -1;
if (filter->choice == LOFC_BLOB_NONE) {
if (bitmap_git)
@@ -949,8 +974,7 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return 0;
}
- if (filter->choice == LOFC_TREE_DEPTH &&
- filter->tree_exclude_depth == 0) {
+ if (filter->choice == LOFC_TREE_DEPTH) {
if (bitmap_git)
filter_bitmap_tree_depth(bitmap_git, tip_objects,
to_filter,
@@ -966,8 +990,17 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return 0;
}
- /* filter choice not handled */
- return -1;
+ if (filter->choice == LOFC_COMBINE) {
+ int i;
+ for (i = 0; i < filter->sub_nr; i++) {
+ if (filter_bitmap(bitmap_git, tip_objects, to_filter,
+ &filter->sub[i]) < 0)
+ return -1;
+ }
+ return 0;
+ }
+
+ BUG("unsupported filter choice");
}
static int can_filter_bitmap(struct list_objects_filter_options *filter)
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index fb66735ac8..cb9db7df6f 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -98,4 +98,11 @@ test_expect_success 'object:type filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'combine filter' '
+ git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v3 8/8] rev-list: allow filtering of provided items
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
` (6 preceding siblings ...)
2021-04-09 11:28 ` [PATCH v3 7/8] pack-bitmap: implement combined filter Patrick Steinhardt
@ 2021-04-09 11:28 ` Patrick Steinhardt
2021-04-09 11:32 ` [RESEND PATCH " Patrick Steinhardt
2021-04-09 15:00 ` [PATCH " Philip Oakley
2021-04-11 6:02 ` [PATCH v3 0/8] rev-parse: implement object type filter Junio C Hamano
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
9 siblings, 2 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-09 11:28 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 11964 bytes --]
When providing an object filter, it is currently impossible to also
filter provided items. E.g. when executing `git rev-list HEAD` , the
commit this reference points to will be treated as user-provided and is
thus excluded from the filtering mechanism. This makes it harder than
necessary to properly use the new `--filter=object:type` filter given
that even if the user wants to only see blobs, he'll still see commits
of provided references.
Improve this by introducing a new `--filter-provided` option to the
git-rev-parse(1) command. If given, then all user-provided references
will be subject to filtering.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/rev-list-options.txt | 5 ++++
builtin/pack-objects.c | 2 +-
builtin/rev-list.c | 36 +++++++++++++++++++++--------
pack-bitmap.c | 6 +++--
pack-bitmap.h | 3 ++-
reachable.c | 2 +-
t/t6112-rev-list-filters-objects.sh | 28 ++++++++++++++++++++++
t/t6113-rev-list-bitmap-filters.sh | 36 +++++++++++++++++++++++++++++
8 files changed, 104 insertions(+), 14 deletions(-)
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 3afa8fffbd..7fa18fc6e6 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -933,6 +933,11 @@ equivalent.
--no-filter::
Turn off any previous `--filter=` argument.
+--filter-provided-revisions::
+ Filter the list of explicitly provided revisions, which would otherwise
+ always be printed even if they did not match any of the filters. Only
+ useful with `--filter=`.
+
--filter-print-omitted::
Only useful with `--filter=`; prints a list of the objects omitted
by the filter. Object IDs are prefixed with a ``~'' character.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 525c2d8552..2f2026dc87 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3516,7 +3516,7 @@ static int pack_options_allow_reuse(void)
static int get_object_list_from_bitmap(struct rev_info *revs)
{
- if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options)))
+ if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options, 0)))
return -1;
if (pack_options_allow_reuse() &&
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index b4d8ea0a35..13f0ff3f8d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -398,7 +398,8 @@ static inline int parse_missing_action_value(const char *value)
}
static int try_bitmap_count(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs)
{
uint32_t commit_count = 0,
tag_count = 0,
@@ -433,7 +434,7 @@ static int try_bitmap_count(struct rev_info *revs,
*/
max_count = revs->max_count;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
if (!bitmap_git)
return -1;
@@ -450,7 +451,8 @@ static int try_bitmap_count(struct rev_info *revs,
}
static int try_bitmap_traversal(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs)
{
struct bitmap_index *bitmap_git;
@@ -461,7 +463,7 @@ static int try_bitmap_traversal(struct rev_info *revs,
if (revs->max_count >= 0)
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
if (!bitmap_git)
return -1;
@@ -471,14 +473,15 @@ static int try_bitmap_traversal(struct rev_info *revs,
}
static int try_bitmap_disk_usage(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs)
{
struct bitmap_index *bitmap_git;
if (!show_disk_usage)
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
if (!bitmap_git)
return -1;
@@ -499,6 +502,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
int bisect_show_vars = 0;
int bisect_find_all = 0;
int use_bitmap_index = 0;
+ int filter_provided_revs = 0;
const char *show_progress = NULL;
if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -599,6 +603,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
list_objects_filter_set_no_filter(&filter_options);
continue;
}
+ if (!strcmp(arg, "--filter-provided-revisions")) {
+ filter_provided_revs = 1;
+ continue;
+ }
if (!strcmp(arg, "--filter-print-omitted")) {
arg_print_omitted = 1;
continue;
@@ -665,11 +673,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
progress = start_delayed_progress(show_progress, 0);
if (use_bitmap_index) {
- if (!try_bitmap_count(&revs, &filter_options))
+ if (!try_bitmap_count(&revs, &filter_options, filter_provided_revs))
return 0;
- if (!try_bitmap_disk_usage(&revs, &filter_options))
+ if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_revs))
return 0;
- if (!try_bitmap_traversal(&revs, &filter_options))
+ if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_revs))
return 0;
}
@@ -694,6 +702,16 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
return show_bisect_vars(&info, reaches, all);
}
+ if (filter_provided_revs) {
+ struct commit_list *c;
+ for (i = 0; i < revs.pending.nr; i++) {
+ struct object_array_entry *pending = revs.pending.objects + i;
+ pending->item->flags |= NOT_USER_GIVEN;
+ }
+ for (c = revs.commits; c; c = c->next)
+ c->item->object.flags |= NOT_USER_GIVEN;
+ }
+
if (arg_print_omitted)
oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);
if (arg_missing_action == MA_PRINT)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 4385f15828..0576a19a28 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1009,7 +1009,8 @@ static int can_filter_bitmap(struct list_objects_filter_options *filter)
}
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs)
{
unsigned int i;
@@ -1104,7 +1105,8 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
if (haves_bitmap)
bitmap_and_not(wants_bitmap, haves_bitmap);
- filter_bitmap(bitmap_git, wants, wants_bitmap, filter);
+ filter_bitmap(bitmap_git, (filter && filter_provided_revs) ? NULL : wants,
+ wants_bitmap, filter);
bitmap_git->result = wants_bitmap;
bitmap_git->haves = haves_bitmap;
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 36d99930d8..5d8ae3b590 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -50,7 +50,8 @@ void traverse_bitmap_commit_list(struct bitmap_index *,
show_reachable_fn show_reachable);
void test_bitmap_walk(struct rev_info *revs);
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter);
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs);
int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
struct packed_git **packfile,
uint32_t *entries,
diff --git a/reachable.c b/reachable.c
index 77a60c70a5..fc833cae43 100644
--- a/reachable.c
+++ b/reachable.c
@@ -223,7 +223,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
cp.progress = progress;
cp.count = 0;
- bitmap_git = prepare_bitmap_walk(revs, NULL);
+ bitmap_git = prepare_bitmap_walk(revs, NULL, 0);
if (bitmap_git) {
traverse_bitmap_commit_list(bitmap_git, revs, mark_object_seen);
free_bitmap_index(bitmap_git);
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index c79ec04060..47c558ab0e 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -207,6 +207,34 @@ test_expect_success 'verify object:type=tag prints tag' '
test_cmp expected actual
'
+test_expect_success 'verify object:type=blob prints only blob with --filter-provided' '
+ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=blob --filter-provided HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tree prints only tree with --filter-provided' '
+ printf "%s \n" $(git -C object-type rev-parse HEAD^{tree}) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=tree HEAD --filter-provided >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=commit prints only commit with --filter-provided' '
+ git -C object-type rev-parse HEAD >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=commit --filter-provided HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tag prints only tag with --filter-provided' '
+ printf "%s tag\n" $(git -C object-type rev-parse tag) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=tag --filter-provided tag >actual &&
+ test_cmp expected actual
+'
+
# Test sparse:path=<path> filter.
# !!!!
# NOTE: sparse:path filter support has been dropped for security reasons,
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index cb9db7df6f..9053ac5059 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -98,6 +98,28 @@ test_expect_success 'object:type filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'object:type filter with --filter-provided' '
+ git rev-list --objects --filter-provided --filter=object:type=tag tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided --filter=object:type=tag tag >actual &&
+ test_cmp expect actual &&
+
+ git rev-list --objects --filter-provided --filter=object:type=commit tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided --filter=object:type=commit tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter-provided --filter=object:type=tree tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided --filter=object:type=tree tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter-provided --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_expect_success 'combine filter' '
git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
git rev-list --use-bitmap-index \
@@ -105,4 +127,18 @@ test_expect_success 'combine filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'combine filter with --filter-provided' '
+ git rev-list --objects --filter-provided --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git cat-file --batch-check="%(objecttype) %(objectsize)" <actual >objects &&
+ while read objecttype objectsize
+ do
+ test "$objecttype" = blob || return 1
+ test "$objectsize" -le 1000 || return 1
+ done <objects
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [RESEND PATCH v3 8/8] rev-list: allow filtering of provided items
2021-04-09 11:28 ` [PATCH v3 8/8] rev-list: allow filtering of provided items Patrick Steinhardt
@ 2021-04-09 11:32 ` Patrick Steinhardt
2021-04-09 15:00 ` [PATCH " Philip Oakley
1 sibling, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-09 11:32 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 12244 bytes --]
When providing an object filter, it is currently impossible to also
filter provided items. E.g. when executing `git rev-list HEAD` , the
commit this reference points to will be treated as user-provided and is
thus excluded from the filtering mechanism. This makes it harder than
necessary to properly use the new `--filter=object:type` filter given
that even if the user wants to only see blobs, he'll still see commits
of provided references.
Improve this by introducing a new `--filter-provided` option to the
git-rev-parse(1) command. If given, then all user-provided references
will be subject to filtering.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Forgot to stage and add test changes to adjust for the changed flag
name.
Documentation/rev-list-options.txt | 5 ++++
builtin/pack-objects.c | 2 +-
builtin/rev-list.c | 36 +++++++++++++++++++++--------
pack-bitmap.c | 6 +++--
pack-bitmap.h | 3 ++-
reachable.c | 2 +-
t/t6112-rev-list-filters-objects.sh | 28 ++++++++++++++++++++++
t/t6113-rev-list-bitmap-filters.sh | 36 +++++++++++++++++++++++++++++
8 files changed, 104 insertions(+), 14 deletions(-)
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 3afa8fffbd..7fa18fc6e6 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -933,6 +933,11 @@ equivalent.
--no-filter::
Turn off any previous `--filter=` argument.
+--filter-provided-revisions::
+ Filter the list of explicitly provided revisions, which would otherwise
+ always be printed even if they did not match any of the filters. Only
+ useful with `--filter=`.
+
--filter-print-omitted::
Only useful with `--filter=`; prints a list of the objects omitted
by the filter. Object IDs are prefixed with a ``~'' character.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 525c2d8552..2f2026dc87 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3516,7 +3516,7 @@ static int pack_options_allow_reuse(void)
static int get_object_list_from_bitmap(struct rev_info *revs)
{
- if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options)))
+ if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options, 0)))
return -1;
if (pack_options_allow_reuse() &&
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index b4d8ea0a35..13f0ff3f8d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -398,7 +398,8 @@ static inline int parse_missing_action_value(const char *value)
}
static int try_bitmap_count(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs)
{
uint32_t commit_count = 0,
tag_count = 0,
@@ -433,7 +434,7 @@ static int try_bitmap_count(struct rev_info *revs,
*/
max_count = revs->max_count;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
if (!bitmap_git)
return -1;
@@ -450,7 +451,8 @@ static int try_bitmap_count(struct rev_info *revs,
}
static int try_bitmap_traversal(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs)
{
struct bitmap_index *bitmap_git;
@@ -461,7 +463,7 @@ static int try_bitmap_traversal(struct rev_info *revs,
if (revs->max_count >= 0)
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
if (!bitmap_git)
return -1;
@@ -471,14 +473,15 @@ static int try_bitmap_traversal(struct rev_info *revs,
}
static int try_bitmap_disk_usage(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs)
{
struct bitmap_index *bitmap_git;
if (!show_disk_usage)
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
if (!bitmap_git)
return -1;
@@ -499,6 +502,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
int bisect_show_vars = 0;
int bisect_find_all = 0;
int use_bitmap_index = 0;
+ int filter_provided_revs = 0;
const char *show_progress = NULL;
if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -599,6 +603,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
list_objects_filter_set_no_filter(&filter_options);
continue;
}
+ if (!strcmp(arg, "--filter-provided-revisions")) {
+ filter_provided_revs = 1;
+ continue;
+ }
if (!strcmp(arg, "--filter-print-omitted")) {
arg_print_omitted = 1;
continue;
@@ -665,11 +673,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
progress = start_delayed_progress(show_progress, 0);
if (use_bitmap_index) {
- if (!try_bitmap_count(&revs, &filter_options))
+ if (!try_bitmap_count(&revs, &filter_options, filter_provided_revs))
return 0;
- if (!try_bitmap_disk_usage(&revs, &filter_options))
+ if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_revs))
return 0;
- if (!try_bitmap_traversal(&revs, &filter_options))
+ if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_revs))
return 0;
}
@@ -694,6 +702,16 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
return show_bisect_vars(&info, reaches, all);
}
+ if (filter_provided_revs) {
+ struct commit_list *c;
+ for (i = 0; i < revs.pending.nr; i++) {
+ struct object_array_entry *pending = revs.pending.objects + i;
+ pending->item->flags |= NOT_USER_GIVEN;
+ }
+ for (c = revs.commits; c; c = c->next)
+ c->item->object.flags |= NOT_USER_GIVEN;
+ }
+
if (arg_print_omitted)
oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);
if (arg_missing_action == MA_PRINT)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 4385f15828..0576a19a28 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1009,7 +1009,8 @@ static int can_filter_bitmap(struct list_objects_filter_options *filter)
}
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs)
{
unsigned int i;
@@ -1104,7 +1105,8 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
if (haves_bitmap)
bitmap_and_not(wants_bitmap, haves_bitmap);
- filter_bitmap(bitmap_git, wants, wants_bitmap, filter);
+ filter_bitmap(bitmap_git, (filter && filter_provided_revs) ? NULL : wants,
+ wants_bitmap, filter);
bitmap_git->result = wants_bitmap;
bitmap_git->haves = haves_bitmap;
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 36d99930d8..5d8ae3b590 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -50,7 +50,8 @@ void traverse_bitmap_commit_list(struct bitmap_index *,
show_reachable_fn show_reachable);
void test_bitmap_walk(struct rev_info *revs);
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter);
+ struct list_objects_filter_options *filter,
+ int filter_provided_revs);
int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
struct packed_git **packfile,
uint32_t *entries,
diff --git a/reachable.c b/reachable.c
index 77a60c70a5..fc833cae43 100644
--- a/reachable.c
+++ b/reachable.c
@@ -223,7 +223,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
cp.progress = progress;
cp.count = 0;
- bitmap_git = prepare_bitmap_walk(revs, NULL);
+ bitmap_git = prepare_bitmap_walk(revs, NULL, 0);
if (bitmap_git) {
traverse_bitmap_commit_list(bitmap_git, revs, mark_object_seen);
free_bitmap_index(bitmap_git);
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index c79ec04060..0a305c9c49 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -207,6 +207,34 @@ test_expect_success 'verify object:type=tag prints tag' '
test_cmp expected actual
'
+test_expect_success 'verify object:type=blob prints only blob with --filter-provided-revisions' '
+ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=blob --filter-provided-revisions HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tree prints only tree with --filter-provided-revisions' '
+ printf "%s \n" $(git -C object-type rev-parse HEAD^{tree}) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=tree HEAD --filter-provided-revisions >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=commit prints only commit with --filter-provided-revisions' '
+ git -C object-type rev-parse HEAD >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=commit --filter-provided-revisions HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tag prints only tag with --filter-provided-revisions' '
+ printf "%s tag\n" $(git -C object-type rev-parse tag) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=tag --filter-provided-revisions tag >actual &&
+ test_cmp expected actual
+'
+
# Test sparse:path=<path> filter.
# !!!!
# NOTE: sparse:path filter support has been dropped for security reasons,
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index cb9db7df6f..bfc9bdafa0 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -98,6 +98,28 @@ test_expect_success 'object:type filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'object:type filter with --filter-provided-revisions' '
+ git rev-list --objects --filter-provided-revisions --filter=object:type=tag tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-revisions --filter=object:type=tag tag >actual &&
+ test_cmp expect actual &&
+
+ git rev-list --objects --filter-provided-revisions --filter=object:type=commit tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-revisions --filter=object:type=commit tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter-provided-revisions --filter=object:type=tree tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-revisions --filter=object:type=tree tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter-provided-revisions --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-revisions --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_expect_success 'combine filter' '
git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
git rev-list --use-bitmap-index \
@@ -105,4 +127,18 @@ test_expect_success 'combine filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'combine filter with --filter-provided-revisions' '
+ git rev-list --objects --filter-provided-revisions --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-revisions --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git cat-file --batch-check="%(objecttype) %(objectsize)" <actual >objects &&
+ while read objecttype objectsize
+ do
+ test "$objecttype" = blob || return 1
+ test "$objectsize" -le 1000 || return 1
+ done <objects
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* Re: [PATCH v3 8/8] rev-list: allow filtering of provided items
2021-04-09 11:28 ` [PATCH v3 8/8] rev-list: allow filtering of provided items Patrick Steinhardt
2021-04-09 11:32 ` [RESEND PATCH " Patrick Steinhardt
@ 2021-04-09 15:00 ` Philip Oakley
2021-04-12 13:15 ` Patrick Steinhardt
1 sibling, 1 reply; 97+ messages in thread
From: Philip Oakley @ 2021-04-09 15:00 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Jeff King, Christian Couder, Taylor Blau
typo nit.
On 09/04/2021 12:28, Patrick Steinhardt wrote:
> When providing an object filter, it is currently impossible to also
> filter provided items. E.g. when executing `git rev-list HEAD` , the
> commit this reference points to will be treated as user-provided and is
> thus excluded from the filtering mechanism. This makes it harder than
> necessary to properly use the new `--filter=object:type` filter given
> that even if the user wants to only see blobs, he'll still see commits
> of provided references.
>
> Improve this by introducing a new `--filter-provided` option to the
s/--filter-provided/--filter-provided-revisions/
Also in some tests - I presume the option should be spelled out in full.
> git-rev-parse(1) command. If given, then all user-provided references
> will be subject to filtering.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Documentation/rev-list-options.txt | 5 ++++
> builtin/pack-objects.c | 2 +-
> builtin/rev-list.c | 36 +++++++++++++++++++++--------
> pack-bitmap.c | 6 +++--
> pack-bitmap.h | 3 ++-
> reachable.c | 2 +-
> t/t6112-rev-list-filters-objects.sh | 28 ++++++++++++++++++++++
> t/t6113-rev-list-bitmap-filters.sh | 36 +++++++++++++++++++++++++++++
> 8 files changed, 104 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 3afa8fffbd..7fa18fc6e6 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -933,6 +933,11 @@ equivalent.
> --no-filter::
> Turn off any previous `--filter=` argument.
>
> +--filter-provided-revisions::
> + Filter the list of explicitly provided revisions, which would otherwise
> + always be printed even if they did not match any of the filters. Only
> + useful with `--filter=`.
> +
> --filter-print-omitted::
> Only useful with `--filter=`; prints a list of the objects omitted
> by the filter. Object IDs are prefixed with a ``~'' character.
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 525c2d8552..2f2026dc87 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -3516,7 +3516,7 @@ static int pack_options_allow_reuse(void)
>
> static int get_object_list_from_bitmap(struct rev_info *revs)
> {
> - if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options)))
> + if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options, 0)))
> return -1;
>
> if (pack_options_allow_reuse() &&
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index b4d8ea0a35..13f0ff3f8d 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -398,7 +398,8 @@ static inline int parse_missing_action_value(const char *value)
> }
>
> static int try_bitmap_count(struct rev_info *revs,
> - struct list_objects_filter_options *filter)
> + struct list_objects_filter_options *filter,
> + int filter_provided_revs)
> {
> uint32_t commit_count = 0,
> tag_count = 0,
> @@ -433,7 +434,7 @@ static int try_bitmap_count(struct rev_info *revs,
> */
> max_count = revs->max_count;
>
> - bitmap_git = prepare_bitmap_walk(revs, filter);
> + bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
> if (!bitmap_git)
> return -1;
>
> @@ -450,7 +451,8 @@ static int try_bitmap_count(struct rev_info *revs,
> }
>
> static int try_bitmap_traversal(struct rev_info *revs,
> - struct list_objects_filter_options *filter)
> + struct list_objects_filter_options *filter,
> + int filter_provided_revs)
> {
> struct bitmap_index *bitmap_git;
>
> @@ -461,7 +463,7 @@ static int try_bitmap_traversal(struct rev_info *revs,
> if (revs->max_count >= 0)
> return -1;
>
> - bitmap_git = prepare_bitmap_walk(revs, filter);
> + bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
> if (!bitmap_git)
> return -1;
>
> @@ -471,14 +473,15 @@ static int try_bitmap_traversal(struct rev_info *revs,
> }
>
> static int try_bitmap_disk_usage(struct rev_info *revs,
> - struct list_objects_filter_options *filter)
> + struct list_objects_filter_options *filter,
> + int filter_provided_revs)
> {
> struct bitmap_index *bitmap_git;
>
> if (!show_disk_usage)
> return -1;
>
> - bitmap_git = prepare_bitmap_walk(revs, filter);
> + bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
> if (!bitmap_git)
> return -1;
>
> @@ -499,6 +502,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
> int bisect_show_vars = 0;
> int bisect_find_all = 0;
> int use_bitmap_index = 0;
> + int filter_provided_revs = 0;
> const char *show_progress = NULL;
>
> if (argc == 2 && !strcmp(argv[1], "-h"))
> @@ -599,6 +603,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
> list_objects_filter_set_no_filter(&filter_options);
> continue;
> }
> + if (!strcmp(arg, "--filter-provided-revisions")) {
> + filter_provided_revs = 1;
> + continue;
> + }
> if (!strcmp(arg, "--filter-print-omitted")) {
> arg_print_omitted = 1;
> continue;
> @@ -665,11 +673,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
> progress = start_delayed_progress(show_progress, 0);
>
> if (use_bitmap_index) {
> - if (!try_bitmap_count(&revs, &filter_options))
> + if (!try_bitmap_count(&revs, &filter_options, filter_provided_revs))
> return 0;
> - if (!try_bitmap_disk_usage(&revs, &filter_options))
> + if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_revs))
> return 0;
> - if (!try_bitmap_traversal(&revs, &filter_options))
> + if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_revs))
> return 0;
> }
>
> @@ -694,6 +702,16 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
> return show_bisect_vars(&info, reaches, all);
> }
>
> + if (filter_provided_revs) {
> + struct commit_list *c;
> + for (i = 0; i < revs.pending.nr; i++) {
> + struct object_array_entry *pending = revs.pending.objects + i;
> + pending->item->flags |= NOT_USER_GIVEN;
> + }
> + for (c = revs.commits; c; c = c->next)
> + c->item->object.flags |= NOT_USER_GIVEN;
> + }
> +
> if (arg_print_omitted)
> oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);
> if (arg_missing_action == MA_PRINT)
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 4385f15828..0576a19a28 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -1009,7 +1009,8 @@ static int can_filter_bitmap(struct list_objects_filter_options *filter)
> }
>
> struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
> - struct list_objects_filter_options *filter)
> + struct list_objects_filter_options *filter,
> + int filter_provided_revs)
> {
> unsigned int i;
>
> @@ -1104,7 +1105,8 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
> if (haves_bitmap)
> bitmap_and_not(wants_bitmap, haves_bitmap);
>
> - filter_bitmap(bitmap_git, wants, wants_bitmap, filter);
> + filter_bitmap(bitmap_git, (filter && filter_provided_revs) ? NULL : wants,
> + wants_bitmap, filter);
>
> bitmap_git->result = wants_bitmap;
> bitmap_git->haves = haves_bitmap;
> diff --git a/pack-bitmap.h b/pack-bitmap.h
> index 36d99930d8..5d8ae3b590 100644
> --- a/pack-bitmap.h
> +++ b/pack-bitmap.h
> @@ -50,7 +50,8 @@ void traverse_bitmap_commit_list(struct bitmap_index *,
> show_reachable_fn show_reachable);
> void test_bitmap_walk(struct rev_info *revs);
> struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
> - struct list_objects_filter_options *filter);
> + struct list_objects_filter_options *filter,
> + int filter_provided_revs);
> int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
> struct packed_git **packfile,
> uint32_t *entries,
> diff --git a/reachable.c b/reachable.c
> index 77a60c70a5..fc833cae43 100644
> --- a/reachable.c
> +++ b/reachable.c
> @@ -223,7 +223,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
> cp.progress = progress;
> cp.count = 0;
>
> - bitmap_git = prepare_bitmap_walk(revs, NULL);
> + bitmap_git = prepare_bitmap_walk(revs, NULL, 0);
> if (bitmap_git) {
> traverse_bitmap_commit_list(bitmap_git, revs, mark_object_seen);
> free_bitmap_index(bitmap_git);
> diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
> index c79ec04060..47c558ab0e 100755
> --- a/t/t6112-rev-list-filters-objects.sh
> +++ b/t/t6112-rev-list-filters-objects.sh
> @@ -207,6 +207,34 @@ test_expect_success 'verify object:type=tag prints tag' '
> test_cmp expected actual
> '
>
> +test_expect_success 'verify object:type=blob prints only blob with --filter-provided' '
> + printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob) >expected &&
> + git -C object-type rev-list --objects \
> + --filter=object:type=blob --filter-provided HEAD >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'verify object:type=tree prints only tree with --filter-provided' '
> + printf "%s \n" $(git -C object-type rev-parse HEAD^{tree}) >expected &&
> + git -C object-type rev-list --objects \
> + --filter=object:type=tree HEAD --filter-provided >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'verify object:type=commit prints only commit with --filter-provided' '
> + git -C object-type rev-parse HEAD >expected &&
> + git -C object-type rev-list --objects \
> + --filter=object:type=commit --filter-provided HEAD >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'verify object:type=tag prints only tag with --filter-provided' '
> + printf "%s tag\n" $(git -C object-type rev-parse tag) >expected &&
> + git -C object-type rev-list --objects \
> + --filter=object:type=tag --filter-provided tag >actual &&
> + test_cmp expected actual
> +'
> +
> # Test sparse:path=<path> filter.
> # !!!!
> # NOTE: sparse:path filter support has been dropped for security reasons,
> diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
> index cb9db7df6f..9053ac5059 100755
> --- a/t/t6113-rev-list-bitmap-filters.sh
> +++ b/t/t6113-rev-list-bitmap-filters.sh
> @@ -98,6 +98,28 @@ test_expect_success 'object:type filter' '
> test_bitmap_traversal expect actual
> '
>
> +test_expect_success 'object:type filter with --filter-provided' '
> + git rev-list --objects --filter-provided --filter=object:type=tag tag >expect &&
> + git rev-list --use-bitmap-index \
> + --objects --filter-provided --filter=object:type=tag tag >actual &&
> + test_cmp expect actual &&
> +
> + git rev-list --objects --filter-provided --filter=object:type=commit tag >expect &&
> + git rev-list --use-bitmap-index \
> + --objects --filter-provided --filter=object:type=commit tag >actual &&
> + test_bitmap_traversal expect actual &&
> +
> + git rev-list --objects --filter-provided --filter=object:type=tree tag >expect &&
> + git rev-list --use-bitmap-index \
> + --objects --filter-provided --filter=object:type=tree tag >actual &&
> + test_bitmap_traversal expect actual &&
> +
> + git rev-list --objects --filter-provided --filter=object:type=blob tag >expect &&
> + git rev-list --use-bitmap-index \
> + --objects --filter-provided --filter=object:type=blob tag >actual &&
> + test_bitmap_traversal expect actual
> +'
> +
> test_expect_success 'combine filter' '
> git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
> git rev-list --use-bitmap-index \
> @@ -105,4 +127,18 @@ test_expect_success 'combine filter' '
> test_bitmap_traversal expect actual
> '
>
> +test_expect_success 'combine filter with --filter-provided' '
> + git rev-list --objects --filter-provided --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
> + git rev-list --use-bitmap-index \
> + --objects --filter-provided --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
> + test_bitmap_traversal expect actual &&
> +
> + git cat-file --batch-check="%(objecttype) %(objectsize)" <actual >objects &&
> + while read objecttype objectsize
> + do
> + test "$objecttype" = blob || return 1
> + test "$objectsize" -le 1000 || return 1
> + done <objects
> +'
> +
> test_done
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v3 8/8] rev-list: allow filtering of provided items
2021-04-09 15:00 ` [PATCH " Philip Oakley
@ 2021-04-12 13:15 ` Patrick Steinhardt
0 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:15 UTC (permalink / raw)
To: Philip Oakley; +Cc: git, Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 965 bytes --]
On Fri, Apr 09, 2021 at 04:00:26PM +0100, Philip Oakley wrote:
> typo nit.
> On 09/04/2021 12:28, Patrick Steinhardt wrote:
> > When providing an object filter, it is currently impossible to also
> > filter provided items. E.g. when executing `git rev-list HEAD` , the
> > commit this reference points to will be treated as user-provided and is
> > thus excluded from the filtering mechanism. This makes it harder than
> > necessary to properly use the new `--filter=object:type` filter given
> > that even if the user wants to only see blobs, he'll still see commits
> > of provided references.
> >
> > Improve this by introducing a new `--filter-provided` option to the
> s/--filter-provided/--filter-provided-revisions/
>
> Also in some tests - I presume the option should be spelled out in full.
Right. I did fix these in the resend because I forgot to stage changes,
but still had it in the commit message.
Fixed now, thanks!
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v3 0/8] rev-parse: implement object type filter
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
` (7 preceding siblings ...)
2021-04-09 11:28 ` [PATCH v3 8/8] rev-list: allow filtering of provided items Patrick Steinhardt
@ 2021-04-11 6:02 ` Junio C Hamano
2021-04-12 13:12 ` Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
9 siblings, 1 reply; 97+ messages in thread
From: Junio C Hamano @ 2021-04-11 6:02 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King, Christian Couder, Taylor Blau
Patrick Steinhardt <ps@pks.im> writes:
> Subject: Re: [PATCH v3 0/8] rev-parse: implement object type filter
>
> this is the third version of my patch series which implements a new
> `object:type` filter for git-rev-parse(1) and git-upload-pack(1) and
> extends support for bitmap indices to work with combined filters.
Do you truly mean rev-parse, or is it just a typo for rev-list?
> This mostly addresses Peff's comments. Thanks for your feedback!
>
> - Removed the `base` parameter from `process_tag()`.
>
> - The object type filter doesn't assume ordering for the object type
> enum anymore.
>
> - Combined filters in the bitmap path now verify that
> `filter_bitmap` does not return any errors.
>
> - Renamed "--filter-provided" to "--filter-provided-revisions" and
> added documentation for it.
>
> - Refactored the code to not munge the `filter_provided` field in
> the filter options struct, but instead carry it in rev-list.c.
>
> Please see the attached range-diff for more details.
>
> Patrick
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v3 0/8] rev-parse: implement object type filter
2021-04-11 6:02 ` [PATCH v3 0/8] rev-parse: implement object type filter Junio C Hamano
@ 2021-04-12 13:12 ` Patrick Steinhardt
0 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Christian Couder, Taylor Blau
[-- Attachment #1: Type: text/plain, Size: 552 bytes --]
On Sat, Apr 10, 2021 at 11:02:55PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Subject: Re: [PATCH v3 0/8] rev-parse: implement object type filter
> >
> > this is the third version of my patch series which implements a new
> > `object:type` filter for git-rev-parse(1) and git-upload-pack(1) and
> > extends support for bitmap indices to work with combined filters.
>
> Do you truly mean rev-parse, or is it just a typo for rev-list?
It's a typo both in the series' title and here in the text.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 97+ messages in thread
* [PATCH v4 0/8] rev-list: implement object type filter
2021-04-09 11:27 ` [PATCH v3 " Patrick Steinhardt
` (8 preceding siblings ...)
2021-04-11 6:02 ` [PATCH v3 0/8] rev-parse: implement object type filter Junio C Hamano
@ 2021-04-12 13:37 ` Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow` Patrick Steinhardt
` (9 more replies)
9 siblings, 10 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 18246 bytes --]
Hi,
this is the fourth version of my patch series which implements a new
`object:type` filter for git-rev-list(1) and git-upload-pack(1) and
extends support for bitmap indices to work with combined filters.
Changes compared to v3:
- Small style fix to not pass an empty string and `0`, but instead
simply pass two `NULL` pointers to
`list_objects_filter__filter_object, pointed out by Junio.
- I've changed patch 7/8 as proposed by Peff: support of combined
filters is now determined directly in `filter_bitmap()`, without
having to mirror all filter types in the new `filter_supported()`
function.
- Renamed `--filter-provided-revisions` to
`--filter-provided-objects` as proposed by Peff and addressed both
commit message and tests as pointed out by Philip.
Thanks for all your feedback! As alawys, the range-diff is attached
below.
Patrick
Patrick Steinhardt (8):
uploadpack.txt: document implication of `uploadpackfilter.allow`
revision: mark commit parents as NOT_USER_GIVEN
list-objects: move tag processing into its own function
list-objects: support filtering by tag and commit
list-objects: implement object type filter
pack-bitmap: implement object type filter
pack-bitmap: implement combined filter
rev-list: allow filtering of provided items
Documentation/config/uploadpack.txt | 9 ++-
Documentation/rev-list-options.txt | 8 ++
builtin/pack-objects.c | 2 +-
builtin/rev-list.c | 36 ++++++---
list-objects-filter-options.c | 14 ++++
list-objects-filter-options.h | 2 +
list-objects-filter.c | 116 ++++++++++++++++++++++++++++
list-objects-filter.h | 2 +
list-objects.c | 30 ++++++-
pack-bitmap.c | 45 +++++++++--
pack-bitmap.h | 3 +-
reachable.c | 2 +-
revision.c | 4 +-
revision.h | 3 -
t/t6112-rev-list-filters-objects.sh | 76 ++++++++++++++++++
t/t6113-rev-list-bitmap-filters.sh | 68 +++++++++++++++-
16 files changed, 390 insertions(+), 30 deletions(-)
Range-diff against v3:
1: f80b9570d4 = 1: f80b9570d4 uploadpack.txt: document implication of `uploadpackfilter.allow`
2: 46c1952405 = 2: 46c1952405 revision: mark commit parents as NOT_USER_GIVEN
3: 3d792f6339 = 3: 3d792f6339 list-objects: move tag processing into its own function
4: 80193d6ba3 ! 4: 674da0f9ac list-objects: support filtering by tag and commit
@@ list-objects.c: static void process_tag(struct traversal_context *ctx,
+ enum list_objects_filter_result r;
+
+ r = list_objects_filter__filter_object(ctx->revs->repo, LOFS_TAG,
-+ &tag->object, "", 0, ctx->filter);
++ &tag->object, NULL, NULL,
++ ctx->filter);
+ if (r & LOFR_MARK_SEEN)
+ tag->object.flags |= SEEN;
+ if (r & LOFR_DO_SHOW)
5: e2a14abf92 = 5: d22a5fd37d list-objects: implement object type filter
6: 46d4450d38 = 6: 17c9f66bbc pack-bitmap: implement object type filter
7: 06a376399b ! 7: 759ac54bb2 pack-bitmap: implement combined filter
@@ Commit message
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## pack-bitmap.c ##
-@@ pack-bitmap.c: static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
- filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_BLOB);
- }
-
-+static int filter_supported(struct list_objects_filter_options *filter)
-+{
-+ int i;
-+
-+ switch (filter->choice) {
-+ case LOFC_BLOB_NONE:
-+ case LOFC_BLOB_LIMIT:
-+ case LOFC_OBJECT_TYPE:
-+ return 1;
-+ case LOFC_TREE_DEPTH:
-+ if (filter->tree_exclude_depth == 0)
-+ return 1;
-+ return 0;
-+ case LOFC_COMBINE:
-+ for (i = 0; i < filter->sub_nr; i++)
-+ if (!filter_supported(&filter->sub[i]))
-+ return 0;
-+ return 1;
-+ default:
-+ return 0;
-+ }
-+}
-+
- static int filter_bitmap(struct bitmap_index *bitmap_git,
- struct object_list *tip_objects,
- struct bitmap *to_filter,
-@@ pack-bitmap.c: static int filter_bitmap(struct bitmap_index *bitmap_git,
- {
- if (!filter || filter->choice == LOFC_DISABLED)
- return 0;
-+ if (!filter_supported(filter))
-+ return -1;
-
- if (filter->choice == LOFC_BLOB_NONE) {
- if (bitmap_git)
@@ pack-bitmap.c: static int filter_bitmap(struct bitmap_index *bitmap_git,
return 0;
}
-- if (filter->choice == LOFC_TREE_DEPTH &&
-- filter->tree_exclude_depth == 0) {
-+ if (filter->choice == LOFC_TREE_DEPTH) {
- if (bitmap_git)
- filter_bitmap_tree_depth(bitmap_git, tip_objects,
- to_filter,
-@@ pack-bitmap.c: static int filter_bitmap(struct bitmap_index *bitmap_git,
- return 0;
- }
-
-- /* filter choice not handled */
-- return -1;
+ if (filter->choice == LOFC_COMBINE) {
+ int i;
+ for (i = 0; i < filter->sub_nr; i++) {
@@ pack-bitmap.c: static int filter_bitmap(struct bitmap_index *bitmap_git,
+ return 0;
+ }
+
-+ BUG("unsupported filter choice");
+ /* filter choice not handled */
+ return -1;
}
-
- static int can_filter_bitmap(struct list_objects_filter_options *filter)
## t/t6113-rev-list-bitmap-filters.sh ##
@@ t/t6113-rev-list-bitmap-filters.sh: test_expect_success 'object:type filter' '
8: cf2297b413 ! 8: c779d222cf rev-list: allow filtering of provided items
@@ Commit message
that even if the user wants to only see blobs, he'll still see commits
of provided references.
- Improve this by introducing a new `--filter-provided` option to the
- git-rev-parse(1) command. If given, then all user-provided references
- will be subject to filtering.
+ Improve this by introducing a new `--filter-provided-objects` option
+ to the git-rev-parse(1) command. If given, then all user-provided
+ references will be subject to filtering.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
@@ Documentation/rev-list-options.txt: equivalent.
--no-filter::
Turn off any previous `--filter=` argument.
-+--filter-provided-revisions::
-+ Filter the list of explicitly provided revisions, which would otherwise
++--filter-provided-objects::
++ Filter the list of explicitly provided objects, which would otherwise
+ always be printed even if they did not match any of the filters. Only
+ useful with `--filter=`.
+
@@ builtin/rev-list.c: static inline int parse_missing_action_value(const char *val
static int try_bitmap_count(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
-+ int filter_provided_revs)
++ int filter_provided_objects)
{
uint32_t commit_count = 0,
tag_count = 0,
@@ builtin/rev-list.c: static int try_bitmap_count(struct rev_info *revs,
max_count = revs->max_count;
- bitmap_git = prepare_bitmap_walk(revs, filter);
-+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
++ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
if (!bitmap_git)
return -1;
@@ builtin/rev-list.c: static int try_bitmap_count(struct rev_info *revs,
static int try_bitmap_traversal(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
-+ int filter_provided_revs)
++ int filter_provided_objects)
{
struct bitmap_index *bitmap_git;
@@ builtin/rev-list.c: static int try_bitmap_traversal(struct rev_info *revs,
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
-+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
++ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
if (!bitmap_git)
return -1;
@@ builtin/rev-list.c: static int try_bitmap_traversal(struct rev_info *revs,
static int try_bitmap_disk_usage(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
-+ int filter_provided_revs)
++ int filter_provided_objects)
{
struct bitmap_index *bitmap_git;
@@ builtin/rev-list.c: static int try_bitmap_traversal(struct rev_info *revs,
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
-+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
++ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
if (!bitmap_git)
return -1;
@@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
int bisect_show_vars = 0;
int bisect_find_all = 0;
int use_bitmap_index = 0;
-+ int filter_provided_revs = 0;
++ int filter_provided_objects = 0;
const char *show_progress = NULL;
if (argc == 2 && !strcmp(argv[1], "-h"))
@@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
list_objects_filter_set_no_filter(&filter_options);
continue;
}
-+ if (!strcmp(arg, "--filter-provided-revisions")) {
-+ filter_provided_revs = 1;
++ if (!strcmp(arg, "--filter-provided-objects")) {
++ filter_provided_objects = 1;
+ continue;
+ }
if (!strcmp(arg, "--filter-print-omitted")) {
@@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
if (use_bitmap_index) {
- if (!try_bitmap_count(&revs, &filter_options))
-+ if (!try_bitmap_count(&revs, &filter_options, filter_provided_revs))
++ if (!try_bitmap_count(&revs, &filter_options, filter_provided_objects))
return 0;
- if (!try_bitmap_disk_usage(&revs, &filter_options))
-+ if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_revs))
++ if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_objects))
return 0;
- if (!try_bitmap_traversal(&revs, &filter_options))
-+ if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_revs))
++ if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_objects))
return 0;
}
@@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
return show_bisect_vars(&info, reaches, all);
}
-+ if (filter_provided_revs) {
++ if (filter_provided_objects) {
+ struct commit_list *c;
+ for (i = 0; i < revs.pending.nr; i++) {
+ struct object_array_entry *pending = revs.pending.objects + i;
@@ pack-bitmap.c: static int can_filter_bitmap(struct list_objects_filter_options *
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
-+ int filter_provided_revs)
++ int filter_provided_objects)
{
unsigned int i;
@@ pack-bitmap.c: struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
bitmap_and_not(wants_bitmap, haves_bitmap);
- filter_bitmap(bitmap_git, wants, wants_bitmap, filter);
-+ filter_bitmap(bitmap_git, (filter && filter_provided_revs) ? NULL : wants,
++ filter_bitmap(bitmap_git, (filter && filter_provided_objects) ? NULL : wants,
+ wants_bitmap, filter);
bitmap_git->result = wants_bitmap;
@@ pack-bitmap.h: void traverse_bitmap_commit_list(struct bitmap_index *,
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter);
+ struct list_objects_filter_options *filter,
-+ int filter_provided_revs);
++ int filter_provided_objects);
int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
struct packed_git **packfile,
uint32_t *entries,
@@ t/t6112-rev-list-filters-objects.sh: test_expect_success 'verify object:type=tag
test_cmp expected actual
'
-+test_expect_success 'verify object:type=blob prints only blob with --filter-provided-revisions' '
++test_expect_success 'verify object:type=blob prints only blob with --filter-provided-objects' '
+ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob) >expected &&
+ git -C object-type rev-list --objects \
-+ --filter=object:type=blob --filter-provided-revisions HEAD >actual &&
++ --filter=object:type=blob --filter-provided-objects HEAD >actual &&
+ test_cmp expected actual
+'
+
-+test_expect_success 'verify object:type=tree prints only tree with --filter-provided-revisions' '
++test_expect_success 'verify object:type=tree prints only tree with --filter-provided-objects' '
+ printf "%s \n" $(git -C object-type rev-parse HEAD^{tree}) >expected &&
+ git -C object-type rev-list --objects \
-+ --filter=object:type=tree HEAD --filter-provided-revisions >actual &&
++ --filter=object:type=tree HEAD --filter-provided-objects >actual &&
+ test_cmp expected actual
+'
+
-+test_expect_success 'verify object:type=commit prints only commit with --filter-provided-revisions' '
++test_expect_success 'verify object:type=commit prints only commit with --filter-provided-objects' '
+ git -C object-type rev-parse HEAD >expected &&
+ git -C object-type rev-list --objects \
-+ --filter=object:type=commit --filter-provided-revisions HEAD >actual &&
++ --filter=object:type=commit --filter-provided-objects HEAD >actual &&
+ test_cmp expected actual
+'
+
-+test_expect_success 'verify object:type=tag prints only tag with --filter-provided-revisions' '
++test_expect_success 'verify object:type=tag prints only tag with --filter-provided-objects' '
+ printf "%s tag\n" $(git -C object-type rev-parse tag) >expected &&
+ git -C object-type rev-list --objects \
-+ --filter=object:type=tag --filter-provided-revisions tag >actual &&
++ --filter=object:type=tag --filter-provided-objects tag >actual &&
+ test_cmp expected actual
+'
+
@@ t/t6113-rev-list-bitmap-filters.sh: test_expect_success 'object:type filter' '
test_bitmap_traversal expect actual
'
-+test_expect_success 'object:type filter with --filter-provided-revisions' '
-+ git rev-list --objects --filter-provided-revisions --filter=object:type=tag tag >expect &&
++test_expect_success 'object:type filter with --filter-provided-objects' '
++ git rev-list --objects --filter-provided-objects --filter=object:type=tag tag >expect &&
+ git rev-list --use-bitmap-index \
-+ --objects --filter-provided-revisions --filter=object:type=tag tag >actual &&
++ --objects --filter-provided-objects --filter=object:type=tag tag >actual &&
+ test_cmp expect actual &&
+
-+ git rev-list --objects --filter-provided-revisions --filter=object:type=commit tag >expect &&
++ git rev-list --objects --filter-provided-objects --filter=object:type=commit tag >expect &&
+ git rev-list --use-bitmap-index \
-+ --objects --filter-provided-revisions --filter=object:type=commit tag >actual &&
++ --objects --filter-provided-objects --filter=object:type=commit tag >actual &&
+ test_bitmap_traversal expect actual &&
+
-+ git rev-list --objects --filter-provided-revisions --filter=object:type=tree tag >expect &&
++ git rev-list --objects --filter-provided-objects --filter=object:type=tree tag >expect &&
+ git rev-list --use-bitmap-index \
-+ --objects --filter-provided-revisions --filter=object:type=tree tag >actual &&
++ --objects --filter-provided-objects --filter=object:type=tree tag >actual &&
+ test_bitmap_traversal expect actual &&
+
-+ git rev-list --objects --filter-provided-revisions --filter=object:type=blob tag >expect &&
++ git rev-list --objects --filter-provided-objects --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
-+ --objects --filter-provided-revisions --filter=object:type=blob tag >actual &&
++ --objects --filter-provided-objects --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
@@ t/t6113-rev-list-bitmap-filters.sh: test_expect_success 'combine filter' '
test_bitmap_traversal expect actual
'
-+test_expect_success 'combine filter with --filter-provided-revisions' '
-+ git rev-list --objects --filter-provided-revisions --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
++test_expect_success 'combine filter with --filter-provided-objects' '
++ git rev-list --objects --filter-provided-objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
-+ --objects --filter-provided-revisions --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
++ --objects --filter-provided-objects --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git cat-file --batch-check="%(objecttype) %(objectsize)" <actual >objects &&
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 97+ messages in thread
* [PATCH v4 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow`
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
@ 2021-04-12 13:37 ` Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 2/8] revision: mark commit parents as NOT_USER_GIVEN Patrick Steinhardt
` (8 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]
When `uploadpackfilter.allow` is set to `true`, it means that filters
are enabled by default except in the case where a filter is explicitly
disabled via `uploadpackilter.<filter>.allow`. This option will not only
enable the currently supported set of filters, but also any filters
which get added in the future. As such, an admin which wants to have
tight control over which filters are allowed and which aren't probably
shouldn't ever set `uploadpackfilter.allow=true`.
Amend the documentation to make the ramifications more explicit so that
admins are aware of this.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/config/uploadpack.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
index b0d761282c..6729a072ea 100644
--- a/Documentation/config/uploadpack.txt
+++ b/Documentation/config/uploadpack.txt
@@ -59,7 +59,8 @@ uploadpack.allowFilter::
uploadpackfilter.allow::
Provides a default value for unspecified object filters (see: the
- below configuration variable).
+ below configuration variable). If set to `true`, this will also
+ enable all filters which get added in the future.
Defaults to `true`.
uploadpackfilter.<filter>.allow::
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v4 2/8] revision: mark commit parents as NOT_USER_GIVEN
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow` Patrick Steinhardt
@ 2021-04-12 13:37 ` Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 3/8] list-objects: move tag processing into its own function Patrick Steinhardt
` (7 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 2338 bytes --]
The NOT_USER_GIVEN flag of an object marks whether a flag was explicitly
provided by the user or not. The most important use case for this is
when filtering objects: only objects that were not explicitly requested
will get filtered.
The flag is currently only set for blobs and trees, which has been fine
given that there are no filters for tags or commits currently. We're
about to extend filtering capabilities to add object type filter though,
which requires us to set up the NOT_USER_GIVEN flag correctly -- if it's
not set, the object wouldn't get filtered at all.
Mark unseen commit parents as NOT_USER_GIVEN when processing parents.
Like this, explicitly provided parents stay user-given and thus
unfiltered, while parents which get loaded as part of the graph walk
can be filtered.
This commit shouldn't have any user-visible impact yet as there is no
logic to filter commits yet.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
revision.c | 4 ++--
revision.h | 3 ---
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/revision.c b/revision.c
index 553c0faa9b..fd34c75e23 100644
--- a/revision.c
+++ b/revision.c
@@ -1123,7 +1123,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
mark_parents_uninteresting(p);
if (p->object.flags & SEEN)
continue;
- p->object.flags |= SEEN;
+ p->object.flags |= (SEEN | NOT_USER_GIVEN);
if (list)
commit_list_insert_by_date(p, list);
if (queue)
@@ -1165,7 +1165,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
}
p->object.flags |= left_flag;
if (!(p->object.flags & SEEN)) {
- p->object.flags |= SEEN;
+ p->object.flags |= (SEEN | NOT_USER_GIVEN);
if (list)
commit_list_insert_by_date(p, list);
if (queue)
diff --git a/revision.h b/revision.h
index a24f72dcd1..93aa012f51 100644
--- a/revision.h
+++ b/revision.h
@@ -44,9 +44,6 @@
/*
* Indicates object was reached by traversal. i.e. not given by user on
* command-line or stdin.
- * NEEDSWORK: NOT_USER_GIVEN doesn't apply to commits because we only support
- * filtering trees and blobs, but it may be useful to support filtering commits
- * in the future.
*/
#define NOT_USER_GIVEN (1u<<25)
#define TRACK_LINEAR (1u<<26)
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v4 3/8] list-objects: move tag processing into its own function
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow` Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 2/8] revision: mark commit parents as NOT_USER_GIVEN Patrick Steinhardt
@ 2021-04-12 13:37 ` Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 4/8] list-objects: support filtering by tag and commit Patrick Steinhardt
` (6 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]
Move processing of tags into its own function to make the logic easier
to extend when we're going to implement filtering for tags. No change in
behaviour is expected from this commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
list-objects.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/list-objects.c b/list-objects.c
index e19589baa0..a5a60301cb 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -213,6 +213,14 @@ static void process_tree(struct traversal_context *ctx,
free_tree_buffer(tree);
}
+static void process_tag(struct traversal_context *ctx,
+ struct tag *tag,
+ const char *name)
+{
+ tag->object.flags |= SEEN;
+ ctx->show_object(&tag->object, name, ctx->show_data);
+}
+
static void mark_edge_parents_uninteresting(struct commit *commit,
struct rev_info *revs,
show_edge_fn show_edge)
@@ -334,8 +342,7 @@ static void traverse_trees_and_blobs(struct traversal_context *ctx,
if (obj->flags & (UNINTERESTING | SEEN))
continue;
if (obj->type == OBJ_TAG) {
- obj->flags |= SEEN;
- ctx->show_object(obj, name, ctx->show_data);
+ process_tag(ctx, (struct tag *)obj, name);
continue;
}
if (!path)
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v4 4/8] list-objects: support filtering by tag and commit
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
` (2 preceding siblings ...)
2021-04-12 13:37 ` [PATCH v4 3/8] list-objects: move tag processing into its own function Patrick Steinhardt
@ 2021-04-12 13:37 ` Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 5/8] list-objects: implement object type filter Patrick Steinhardt
` (5 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 4809 bytes --]
Object filters currently only support filtering blobs or trees based on
some criteria. This commit lays the foundation to also allow filtering
of tags and commits.
No change in behaviour is expected from this commit given that there are
no filters yet for those object types.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
list-objects-filter.c | 40 ++++++++++++++++++++++++++++++++++++++++
list-objects-filter.h | 2 ++
list-objects.c | 23 ++++++++++++++++++++---
3 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 39e2f15333..0ebfa52966 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -82,6 +82,16 @@ static enum list_objects_filter_result filter_blobs_none(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
/* always include all tree objects */
@@ -173,6 +183,16 @@ static enum list_objects_filter_result filter_trees_depth(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_END_TREE:
assert(obj->type == OBJ_TREE);
filter_data->current_depth--;
@@ -267,6 +287,16 @@ static enum list_objects_filter_result filter_blobs_limit(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
/* always include all tree objects */
@@ -371,6 +401,16 @@ static enum list_objects_filter_result filter_sparse(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
dtype = DT_DIR;
diff --git a/list-objects-filter.h b/list-objects-filter.h
index cfd784e203..9e98814111 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -55,6 +55,8 @@ enum list_objects_filter_result {
};
enum list_objects_filter_situation {
+ LOFS_COMMIT,
+ LOFS_TAG,
LOFS_BEGIN_TREE,
LOFS_END_TREE,
LOFS_BLOB
diff --git a/list-objects.c b/list-objects.c
index a5a60301cb..7f404677d5 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -217,8 +217,15 @@ static void process_tag(struct traversal_context *ctx,
struct tag *tag,
const char *name)
{
- tag->object.flags |= SEEN;
- ctx->show_object(&tag->object, name, ctx->show_data);
+ enum list_objects_filter_result r;
+
+ r = list_objects_filter__filter_object(ctx->revs->repo, LOFS_TAG,
+ &tag->object, NULL, NULL,
+ ctx->filter);
+ if (r & LOFR_MARK_SEEN)
+ tag->object.flags |= SEEN;
+ if (r & LOFR_DO_SHOW)
+ ctx->show_object(&tag->object, name, ctx->show_data);
}
static void mark_edge_parents_uninteresting(struct commit *commit,
@@ -368,6 +375,12 @@ static void do_traverse(struct traversal_context *ctx)
strbuf_init(&csp, PATH_MAX);
while ((commit = get_revision(ctx->revs)) != NULL) {
+ enum list_objects_filter_result r;
+
+ r = list_objects_filter__filter_object(ctx->revs->repo,
+ LOFS_COMMIT, &commit->object,
+ NULL, NULL, ctx->filter);
+
/*
* an uninteresting boundary commit may not have its tree
* parsed yet, but we are not going to show them anyway
@@ -382,7 +395,11 @@ static void do_traverse(struct traversal_context *ctx)
die(_("unable to load root tree for commit %s"),
oid_to_hex(&commit->object.oid));
}
- ctx->show_commit(commit, ctx->show_data);
+
+ if (r & LOFR_MARK_SEEN)
+ commit->object.flags |= SEEN;
+ if (r & LOFR_DO_SHOW)
+ ctx->show_commit(commit, ctx->show_data);
if (ctx->revs->tree_blobs_in_commit_order)
/*
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v4 5/8] list-objects: implement object type filter
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
` (3 preceding siblings ...)
2021-04-12 13:37 ` [PATCH v4 4/8] list-objects: support filtering by tag and commit Patrick Steinhardt
@ 2021-04-12 13:37 ` Patrick Steinhardt
2021-04-13 9:57 ` Ævar Arnfjörð Bjarmason
2021-04-12 13:37 ` [PATCH v4 6/8] pack-bitmap: " Patrick Steinhardt
` (4 subsequent siblings)
9 siblings, 1 reply; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 9502 bytes --]
While it already is possible to filter objects by some criteria in
git-rev-list(1), it is not yet possible to filter out only a specific
type of objects. This makes some filters less useful. The `blob:limit`
filter for example filters blobs such that only those which are smaller
than the given limit are returned. But it is unfit to ask only for these
smallish blobs, given that git-rev-list(1) will continue to print tags,
commits and trees.
Now that we have the infrastructure in place to also filter tags and
commits, we can improve this situation by implementing a new filter
which selects objects based on their type. Above query can thus
trivially be implemented with the following command:
$ git rev-list --objects --filter=object:type=blob \
--filter=blob:limit=200
Furthermore, this filter allows to optimize for certain other cases: if
for example only tags or commits have been selected, there is no need to
walk down trees.
The new filter is not yet supported in bitmaps. This is going to be
implemented in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/config/uploadpack.txt | 6 +--
Documentation/rev-list-options.txt | 3 ++
list-objects-filter-options.c | 14 ++++++
list-objects-filter-options.h | 2 +
list-objects-filter.c | 76 +++++++++++++++++++++++++++++
t/t6112-rev-list-filters-objects.sh | 48 ++++++++++++++++++
6 files changed, 146 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
index 6729a072ea..32fad5bbe8 100644
--- a/Documentation/config/uploadpack.txt
+++ b/Documentation/config/uploadpack.txt
@@ -66,9 +66,9 @@ uploadpackfilter.allow::
uploadpackfilter.<filter>.allow::
Explicitly allow or ban the object filter corresponding to
`<filter>`, where `<filter>` may be one of: `blob:none`,
- `blob:limit`, `tree`, `sparse:oid`, or `combine`. If using
- combined filters, both `combine` and all of the nested filter
- kinds must be allowed. Defaults to `uploadpackfilter.allow`.
+ `blob:limit`, `object:type`, `tree`, `sparse:oid`, or `combine`.
+ If using combined filters, both `combine` and all of the nested
+ filter kinds must be allowed. Defaults to `uploadpackfilter.allow`.
uploadpackfilter.tree.maxDepth::
Only allow `--filter=tree:<n>` when `<n>` is no more than the value of
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index b1c8f86c6e..3afa8fffbd 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -892,6 +892,9 @@ or units. n may be zero. The suffixes k, m, and g can be used to name
units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same
as 'blob:limit=1024'.
+
+The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects
+which are not of the requested type.
++
The form '--filter=sparse:oid=<blob-ish>' uses a sparse-checkout
specification contained in the blob (or blob-expression) '<blob-ish>'
to omit blobs that would not be not required for a sparse checkout on
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index d2d1c81caf..bb6f6577d5 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -29,6 +29,8 @@ const char *list_object_filter_config_name(enum list_objects_filter_choice c)
return "tree";
case LOFC_SPARSE_OID:
return "sparse:oid";
+ case LOFC_OBJECT_TYPE:
+ return "object:type";
case LOFC_COMBINE:
return "combine";
case LOFC__COUNT:
@@ -97,6 +99,18 @@ static int gently_parse_list_objects_filter(
}
return 1;
+ } else if (skip_prefix(arg, "object:type=", &v0)) {
+ int type = type_from_string_gently(v0, -1, 1);
+ if (type < 0) {
+ strbuf_addstr(errbuf, _("expected 'object:type=<type>'"));
+ return 1;
+ }
+
+ filter_options->object_type = type;
+ filter_options->choice = LOFC_OBJECT_TYPE;
+
+ return 0;
+
} else if (skip_prefix(arg, "combine:", &v0)) {
return parse_combine_filter(filter_options, v0, errbuf);
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index 01767c3c96..4d0d0588cc 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -13,6 +13,7 @@ enum list_objects_filter_choice {
LOFC_BLOB_LIMIT,
LOFC_TREE_DEPTH,
LOFC_SPARSE_OID,
+ LOFC_OBJECT_TYPE,
LOFC_COMBINE,
LOFC__COUNT /* must be last */
};
@@ -54,6 +55,7 @@ struct list_objects_filter_options {
char *sparse_oid_name;
unsigned long blob_limit_value;
unsigned long tree_exclude_depth;
+ enum object_type object_type;
/* LOFC_COMBINE values */
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 0ebfa52966..1c1ee3d1bb 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -545,6 +545,81 @@ static void filter_sparse_oid__init(
filter->free_fn = filter_sparse_free;
}
+/*
+ * A filter for list-objects to omit large blobs.
+ * And to OPTIONALLY collect a list of the omitted OIDs.
+ */
+struct filter_object_type_data {
+ enum object_type object_type;
+};
+
+static enum list_objects_filter_result filter_object_type(
+ struct repository *r,
+ enum list_objects_filter_situation filter_situation,
+ struct object *obj,
+ const char *pathname,
+ const char *filename,
+ struct oidset *omits,
+ void *filter_data_)
+{
+ struct filter_object_type_data *filter_data = filter_data_;
+
+ switch (filter_situation) {
+ default:
+ BUG("unknown filter_situation: %d", filter_situation);
+
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ if (filter_data->object_type == OBJ_TAG)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+ return LOFR_MARK_SEEN;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ if (filter_data->object_type == OBJ_COMMIT)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+ return LOFR_MARK_SEEN;
+
+ case LOFS_BEGIN_TREE:
+ assert(obj->type == OBJ_TREE);
+
+ /*
+ * If we only want to show commits or tags, then there is no
+ * need to walk down trees.
+ */
+ if (filter_data->object_type == OBJ_COMMIT ||
+ filter_data->object_type == OBJ_TAG)
+ return LOFR_SKIP_TREE;
+
+ if (filter_data->object_type == OBJ_TREE)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ return LOFR_MARK_SEEN;
+
+ case LOFS_BLOB:
+ assert(obj->type == OBJ_BLOB);
+
+ if (filter_data->object_type == OBJ_BLOB)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+ return LOFR_MARK_SEEN;
+
+ case LOFS_END_TREE:
+ return LOFR_ZERO;
+ }
+}
+
+static void filter_object_type__init(
+ struct list_objects_filter_options *filter_options,
+ struct filter *filter)
+{
+ struct filter_object_type_data *d = xcalloc(1, sizeof(*d));
+ d->object_type = filter_options->object_type;
+
+ filter->filter_data = d;
+ filter->filter_object_fn = filter_object_type;
+ filter->free_fn = free;
+}
+
/* A filter which only shows objects shown by all sub-filters. */
struct combine_filter_data {
struct subfilter *sub;
@@ -691,6 +766,7 @@ static filter_init_fn s_filters[] = {
filter_blobs_limit__init,
filter_trees_depth__init,
filter_sparse_oid__init,
+ filter_object_type__init,
filter_combine__init,
};
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index 31457d13b9..c79ec04060 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -159,6 +159,54 @@ test_expect_success 'verify blob:limit=1m' '
test_must_be_empty observed
'
+# Test object:type=<type> filter.
+
+test_expect_success 'setup object-type' '
+ git init object-type &&
+ echo contents >object-type/blob &&
+ git -C object-type add blob &&
+ git -C object-type commit -m commit-message &&
+ git -C object-type tag tag -m tag-message
+'
+
+test_expect_success 'verify object:type= fails with invalid type' '
+ test_must_fail git -C object-type rev-list --objects --filter=object:type= HEAD &&
+ test_must_fail git -C object-type rev-list --objects --filter=object:type=invalid HEAD
+'
+
+test_expect_success 'verify object:type=blob prints blob and commit' '
+ (
+ git -C object-type rev-parse HEAD &&
+ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob)
+ ) >expected &&
+ git -C object-type rev-list --objects --filter=object:type=blob HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tree prints tree and commit' '
+ (
+ git -C object-type rev-parse HEAD &&
+ printf "%s \n" $(git -C object-type rev-parse HEAD^{tree})
+ ) >expected &&
+ git -C object-type rev-list --objects --filter=object:type=tree HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=commit prints commit' '
+ git -C object-type rev-parse HEAD >expected &&
+ git -C object-type rev-list --objects --filter=object:type=commit HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tag prints tag' '
+ (
+ git -C object-type rev-parse HEAD &&
+ printf "%s tag\n" $(git -C object-type rev-parse tag)
+ ) >expected &&
+ git -C object-type rev-list --objects --filter=object:type=tag tag >actual &&
+ test_cmp expected actual
+'
+
# Test sparse:path=<path> filter.
# !!!!
# NOTE: sparse:path filter support has been dropped for security reasons,
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* Re: [PATCH v4 5/8] list-objects: implement object type filter
2021-04-12 13:37 ` [PATCH v4 5/8] list-objects: implement object type filter Patrick Steinhardt
@ 2021-04-13 9:57 ` Ævar Arnfjörð Bjarmason
2021-04-13 10:43 ` Andreas Schwab
2021-04-14 11:32 ` Patrick Steinhardt
0 siblings, 2 replies; 97+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-04-13 9:57 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Jeff King, Christian Couder, Taylor Blau, Philip Oakley
On Mon, Apr 12 2021, Patrick Steinhardt wrote:
> + } else if (skip_prefix(arg, "object:type=", &v0)) {
> + int type = type_from_string_gently(v0, -1, 1);
> + if (type < 0) {
> + strbuf_addstr(errbuf, _("expected 'object:type=<type>'"));
Maybe:
strbuf_addstr(errbuf, _("'%s' for 'object:type=<type>' is not a valid object type"), v0);
> +# Test object:type=<type> filter.
> +
> +test_expect_success 'setup object-type' '
> + git init object-type &&
> + echo contents >object-type/blob &&
> + git -C object-type add blob &&
> + git -C object-type commit -m commit-message &&
> + git -C object-type tag tag -m tag-message
> +'
Does this really need to not be the shorter:
test_create_repo object-type &&
test_commit -C object-type blob
Or if it really needs the annotated tag maybe that + --no-tag + create
the tag after, and it can eventually use my
https://lore.kernel.org/git/patch-06.16-8d43fdd5865-20210412T110456Z-avarab@gmail.com/
> +test_expect_success 'verify object:type=blob prints blob and commit' '
> + (
> + git -C object-type rev-parse HEAD &&
> + printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob)
> + ) >expected &&
Just use > and >> or a single printf with two arguments instead of a
subshell?
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 5/8] list-objects: implement object type filter
2021-04-13 9:57 ` Ævar Arnfjörð Bjarmason
@ 2021-04-13 10:43 ` Andreas Schwab
2021-04-14 11:32 ` Patrick Steinhardt
1 sibling, 0 replies; 97+ messages in thread
From: Andreas Schwab @ 2021-04-13 10:43 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Patrick Steinhardt, git, Jeff King, Christian Couder,
Taylor Blau, Philip Oakley
On Apr 13 2021, Ævar Arnfjörð Bjarmason wrote:
> On Mon, Apr 12 2021, Patrick Steinhardt wrote:
>> +test_expect_success 'verify object:type=blob prints blob and commit' '
>> + (
>> + git -C object-type rev-parse HEAD &&
>> + printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob)
>> + ) >expected &&
>
> Just use > and >> or a single printf with two arguments instead of a
> subshell?
You can also use just { } for grouping.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 5/8] list-objects: implement object type filter
2021-04-13 9:57 ` Ævar Arnfjörð Bjarmason
2021-04-13 10:43 ` Andreas Schwab
@ 2021-04-14 11:32 ` Patrick Steinhardt
1 sibling, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-14 11:32 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]
On Tue, Apr 13, 2021 at 11:57:04AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> On Mon, Apr 12 2021, Patrick Steinhardt wrote:
>
> > + } else if (skip_prefix(arg, "object:type=", &v0)) {
> > + int type = type_from_string_gently(v0, -1, 1);
> > + if (type < 0) {
> > + strbuf_addstr(errbuf, _("expected 'object:type=<type>'"));
>
> Maybe:
>
> strbuf_addstr(errbuf, _("'%s' for 'object:type=<type>' is not a valid object type"), v0);
`strbuf_addf`, but otherwise I agree that it does make for a more
actionable error message.
> > +# Test object:type=<type> filter.
> > +
> > +test_expect_success 'setup object-type' '
> > + git init object-type &&
> > + echo contents >object-type/blob &&
> > + git -C object-type add blob &&
> > + git -C object-type commit -m commit-message &&
> > + git -C object-type tag tag -m tag-message
> > +'
>
> Does this really need to not be the shorter:
>
> test_create_repo object-type &&
> test_commit -C object-type blob
>
> Or if it really needs the annotated tag maybe that + --no-tag + create
> the tag after, and it can eventually use my
> https://lore.kernel.org/git/patch-06.16-8d43fdd5865-20210412T110456Z-avarab@gmail.com/
It must be an annotated tag because that's what the `--object` flag of
git-rev-list(1) is working on: objects. But anyway, as you say I can
still use these two helpers and then manually create the tag.
> > +test_expect_success 'verify object:type=blob prints blob and commit' '
> > + (
> > + git -C object-type rev-parse HEAD &&
> > + printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob)
> > + ) >expected &&
>
> Just use > and >> or a single printf with two arguments instead of a
> subshell?
I don't really mind, happy to take this proposal.
Thanks!
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 97+ messages in thread
* [PATCH v4 6/8] pack-bitmap: implement object type filter
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
` (4 preceding siblings ...)
2021-04-12 13:37 ` [PATCH v4 5/8] list-objects: implement object type filter Patrick Steinhardt
@ 2021-04-12 13:37 ` Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 7/8] pack-bitmap: implement combined filter Patrick Steinhardt
` (3 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 3827 bytes --]
The preceding commit has added a new object filter for git-rev-list(1)
which allows to filter objects by type. Implement the equivalent filter
for packfile bitmaps so that we can answer these queries fast.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 29 ++++++++++++++++++++++++++---
t/t6113-rev-list-bitmap-filters.sh | 25 ++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index b4513f8672..cd3f5c433e 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -779,9 +779,6 @@ static void filter_bitmap_exclude_type(struct bitmap_index *bitmap_git,
eword_t mask;
uint32_t i;
- if (type != OBJ_BLOB && type != OBJ_TREE)
- BUG("filter_bitmap_exclude_type: unsupported type '%d'", type);
-
/*
* The non-bitmap version of this filter never removes
* objects which the other side specifically asked for,
@@ -911,6 +908,24 @@ static void filter_bitmap_tree_depth(struct bitmap_index *bitmap_git,
OBJ_BLOB);
}
+static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
+ struct object_list *tip_objects,
+ struct bitmap *to_filter,
+ enum object_type object_type)
+{
+ if (object_type < OBJ_COMMIT || object_type > OBJ_TAG)
+ BUG("filter_bitmap_object_type given invalid object");
+
+ if (object_type != OBJ_TAG)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_TAG);
+ if (object_type != OBJ_COMMIT)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_COMMIT);
+ if (object_type != OBJ_TREE)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_TREE);
+ if (object_type != OBJ_BLOB)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_BLOB);
+}
+
static int filter_bitmap(struct bitmap_index *bitmap_git,
struct object_list *tip_objects,
struct bitmap *to_filter,
@@ -943,6 +958,14 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return 0;
}
+ if (filter->choice == LOFC_OBJECT_TYPE) {
+ if (bitmap_git)
+ filter_bitmap_object_type(bitmap_git, tip_objects,
+ to_filter,
+ filter->object_type);
+ return 0;
+ }
+
/* filter choice not handled */
return -1;
}
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index 3f889949ca..fb66735ac8 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -10,7 +10,8 @@ test_expect_success 'set up bitmapped repo' '
test_commit much-larger-blob-one &&
git repack -adb &&
test_commit two &&
- test_commit much-larger-blob-two
+ test_commit much-larger-blob-two &&
+ git tag tag
'
test_expect_success 'filters fallback to non-bitmap traversal' '
@@ -75,4 +76,26 @@ test_expect_success 'tree:1 filter' '
test_cmp expect actual
'
+test_expect_success 'object:type filter' '
+ git rev-list --objects --filter=object:type=tag tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=tag tag >actual &&
+ test_cmp expect actual &&
+
+ git rev-list --objects --filter=object:type=commit tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=commit tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter=object:type=tree tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=tree tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v4 7/8] pack-bitmap: implement combined filter
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
` (5 preceding siblings ...)
2021-04-12 13:37 ` [PATCH v4 6/8] pack-bitmap: " Patrick Steinhardt
@ 2021-04-12 13:37 ` Patrick Steinhardt
2021-04-12 13:37 ` [PATCH v4 8/8] rev-list: allow filtering of provided items Patrick Steinhardt
` (2 subsequent siblings)
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]
When the user has multiple objects filters specified, then this is
internally represented by having a "combined" filter. These combined
filters aren't yet supported by bitmap indices and can thus not be
accelerated.
Fix this by implementing support for these combined filters. The
implementation is quite trivial: when there's a combined filter, we
simply recurse into `filter_bitmap()` for all of the sub-filters.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 10 ++++++++++
t/t6113-rev-list-bitmap-filters.sh | 7 +++++++
2 files changed, 17 insertions(+)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index cd3f5c433e..7ce3ede7e4 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -966,6 +966,16 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return 0;
}
+ if (filter->choice == LOFC_COMBINE) {
+ int i;
+ for (i = 0; i < filter->sub_nr; i++) {
+ if (filter_bitmap(bitmap_git, tip_objects, to_filter,
+ &filter->sub[i]) < 0)
+ return -1;
+ }
+ return 0;
+ }
+
/* filter choice not handled */
return -1;
}
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index fb66735ac8..cb9db7df6f 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -98,4 +98,11 @@ test_expect_success 'object:type filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'combine filter' '
+ git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v4 8/8] rev-list: allow filtering of provided items
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
` (6 preceding siblings ...)
2021-04-12 13:37 ` [PATCH v4 7/8] pack-bitmap: implement combined filter Patrick Steinhardt
@ 2021-04-12 13:37 ` Patrick Steinhardt
2021-04-13 7:45 ` [PATCH v4 0/8] rev-list: implement object type filter Jeff King
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
9 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-12 13:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 12169 bytes --]
When providing an object filter, it is currently impossible to also
filter provided items. E.g. when executing `git rev-list HEAD` , the
commit this reference points to will be treated as user-provided and is
thus excluded from the filtering mechanism. This makes it harder than
necessary to properly use the new `--filter=object:type` filter given
that even if the user wants to only see blobs, he'll still see commits
of provided references.
Improve this by introducing a new `--filter-provided-objects` option
to the git-rev-parse(1) command. If given, then all user-provided
references will be subject to filtering.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/rev-list-options.txt | 5 ++++
builtin/pack-objects.c | 2 +-
builtin/rev-list.c | 36 +++++++++++++++++++++--------
pack-bitmap.c | 6 +++--
pack-bitmap.h | 3 ++-
reachable.c | 2 +-
t/t6112-rev-list-filters-objects.sh | 28 ++++++++++++++++++++++
t/t6113-rev-list-bitmap-filters.sh | 36 +++++++++++++++++++++++++++++
8 files changed, 104 insertions(+), 14 deletions(-)
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 3afa8fffbd..5bf2a85f69 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -933,6 +933,11 @@ equivalent.
--no-filter::
Turn off any previous `--filter=` argument.
+--filter-provided-objects::
+ Filter the list of explicitly provided objects, which would otherwise
+ always be printed even if they did not match any of the filters. Only
+ useful with `--filter=`.
+
--filter-print-omitted::
Only useful with `--filter=`; prints a list of the objects omitted
by the filter. Object IDs are prefixed with a ``~'' character.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 525c2d8552..2f2026dc87 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3516,7 +3516,7 @@ static int pack_options_allow_reuse(void)
static int get_object_list_from_bitmap(struct rev_info *revs)
{
- if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options)))
+ if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options, 0)))
return -1;
if (pack_options_allow_reuse() &&
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index b4d8ea0a35..7677b1af5a 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -398,7 +398,8 @@ static inline int parse_missing_action_value(const char *value)
}
static int try_bitmap_count(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects)
{
uint32_t commit_count = 0,
tag_count = 0,
@@ -433,7 +434,7 @@ static int try_bitmap_count(struct rev_info *revs,
*/
max_count = revs->max_count;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
if (!bitmap_git)
return -1;
@@ -450,7 +451,8 @@ static int try_bitmap_count(struct rev_info *revs,
}
static int try_bitmap_traversal(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects)
{
struct bitmap_index *bitmap_git;
@@ -461,7 +463,7 @@ static int try_bitmap_traversal(struct rev_info *revs,
if (revs->max_count >= 0)
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
if (!bitmap_git)
return -1;
@@ -471,14 +473,15 @@ static int try_bitmap_traversal(struct rev_info *revs,
}
static int try_bitmap_disk_usage(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects)
{
struct bitmap_index *bitmap_git;
if (!show_disk_usage)
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
if (!bitmap_git)
return -1;
@@ -499,6 +502,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
int bisect_show_vars = 0;
int bisect_find_all = 0;
int use_bitmap_index = 0;
+ int filter_provided_objects = 0;
const char *show_progress = NULL;
if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -599,6 +603,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
list_objects_filter_set_no_filter(&filter_options);
continue;
}
+ if (!strcmp(arg, "--filter-provided-objects")) {
+ filter_provided_objects = 1;
+ continue;
+ }
if (!strcmp(arg, "--filter-print-omitted")) {
arg_print_omitted = 1;
continue;
@@ -665,11 +673,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
progress = start_delayed_progress(show_progress, 0);
if (use_bitmap_index) {
- if (!try_bitmap_count(&revs, &filter_options))
+ if (!try_bitmap_count(&revs, &filter_options, filter_provided_objects))
return 0;
- if (!try_bitmap_disk_usage(&revs, &filter_options))
+ if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_objects))
return 0;
- if (!try_bitmap_traversal(&revs, &filter_options))
+ if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_objects))
return 0;
}
@@ -694,6 +702,16 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
return show_bisect_vars(&info, reaches, all);
}
+ if (filter_provided_objects) {
+ struct commit_list *c;
+ for (i = 0; i < revs.pending.nr; i++) {
+ struct object_array_entry *pending = revs.pending.objects + i;
+ pending->item->flags |= NOT_USER_GIVEN;
+ }
+ for (c = revs.commits; c; c = c->next)
+ c->item->object.flags |= NOT_USER_GIVEN;
+ }
+
if (arg_print_omitted)
oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);
if (arg_missing_action == MA_PRINT)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 7ce3ede7e4..6b790a834b 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -986,7 +986,8 @@ static int can_filter_bitmap(struct list_objects_filter_options *filter)
}
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects)
{
unsigned int i;
@@ -1081,7 +1082,8 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
if (haves_bitmap)
bitmap_and_not(wants_bitmap, haves_bitmap);
- filter_bitmap(bitmap_git, wants, wants_bitmap, filter);
+ filter_bitmap(bitmap_git, (filter && filter_provided_objects) ? NULL : wants,
+ wants_bitmap, filter);
bitmap_git->result = wants_bitmap;
bitmap_git->haves = haves_bitmap;
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 36d99930d8..bb45217d3b 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -50,7 +50,8 @@ void traverse_bitmap_commit_list(struct bitmap_index *,
show_reachable_fn show_reachable);
void test_bitmap_walk(struct rev_info *revs);
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter);
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects);
int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
struct packed_git **packfile,
uint32_t *entries,
diff --git a/reachable.c b/reachable.c
index 77a60c70a5..fc833cae43 100644
--- a/reachable.c
+++ b/reachable.c
@@ -223,7 +223,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
cp.progress = progress;
cp.count = 0;
- bitmap_git = prepare_bitmap_walk(revs, NULL);
+ bitmap_git = prepare_bitmap_walk(revs, NULL, 0);
if (bitmap_git) {
traverse_bitmap_commit_list(bitmap_git, revs, mark_object_seen);
free_bitmap_index(bitmap_git);
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index c79ec04060..de751b65b4 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -207,6 +207,34 @@ test_expect_success 'verify object:type=tag prints tag' '
test_cmp expected actual
'
+test_expect_success 'verify object:type=blob prints only blob with --filter-provided-objects' '
+ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=blob --filter-provided-objects HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tree prints only tree with --filter-provided-objects' '
+ printf "%s \n" $(git -C object-type rev-parse HEAD^{tree}) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=tree HEAD --filter-provided-objects >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=commit prints only commit with --filter-provided-objects' '
+ git -C object-type rev-parse HEAD >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=commit --filter-provided-objects HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tag prints only tag with --filter-provided-objects' '
+ printf "%s tag\n" $(git -C object-type rev-parse tag) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=tag --filter-provided-objects tag >actual &&
+ test_cmp expected actual
+'
+
# Test sparse:path=<path> filter.
# !!!!
# NOTE: sparse:path filter support has been dropped for security reasons,
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index cb9db7df6f..4d8e09167e 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -98,6 +98,28 @@ test_expect_success 'object:type filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'object:type filter with --filter-provided-objects' '
+ git rev-list --objects --filter-provided-objects --filter=object:type=tag tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=object:type=tag tag >actual &&
+ test_cmp expect actual &&
+
+ git rev-list --objects --filter-provided-objects --filter=object:type=commit tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=object:type=commit tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter-provided-objects --filter=object:type=tree tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=object:type=tree tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter-provided-objects --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_expect_success 'combine filter' '
git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
git rev-list --use-bitmap-index \
@@ -105,4 +127,18 @@ test_expect_success 'combine filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'combine filter with --filter-provided-objects' '
+ git rev-list --objects --filter-provided-objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git cat-file --batch-check="%(objecttype) %(objectsize)" <actual >objects &&
+ while read objecttype objectsize
+ do
+ test "$objecttype" = blob || return 1
+ test "$objectsize" -le 1000 || return 1
+ done <objects
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
` (7 preceding siblings ...)
2021-04-12 13:37 ` [PATCH v4 8/8] rev-list: allow filtering of provided items Patrick Steinhardt
@ 2021-04-13 7:45 ` Jeff King
2021-04-13 8:06 ` Patrick Steinhardt
2021-04-13 21:03 ` Junio C Hamano
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
9 siblings, 2 replies; 97+ messages in thread
From: Jeff King @ 2021-04-13 7:45 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Christian Couder, Taylor Blau, Philip Oakley
On Mon, Apr 12, 2021 at 03:37:14PM +0200, Patrick Steinhardt wrote:
> this is the fourth version of my patch series which implements a new
> `object:type` filter for git-rev-list(1) and git-upload-pack(1) and
> extends support for bitmap indices to work with combined filters.
>
> Changes compared to v3:
>
> - Small style fix to not pass an empty string and `0`, but instead
> simply pass two `NULL` pointers to
> `list_objects_filter__filter_object, pointed out by Junio.
>
> - I've changed patch 7/8 as proposed by Peff: support of combined
> filters is now determined directly in `filter_bitmap()`, without
> having to mirror all filter types in the new `filter_supported()`
> function.
>
> - Renamed `--filter-provided-revisions` to
> `--filter-provided-objects` as proposed by Peff and addressed both
> commit message and tests as pointed out by Philip.
Thanks. I have no more nits to pick. The only question left for me is
the big one of "do we like this with --filter, or should it be some kind
of rev-list option", as discussed in:
https://lore.kernel.org/git/YHB7R8hVRt+V+i2W@coredump.intra.peff.net/
As I said elsewhere, I'm OK going with this route. I just wanted to call
attention to that earlier response in case you had any final thoughts
(I'm guessing your final thoughts are "jeez, I already wrote it with
--filter, so let's just go with that" which is perfectly reasonable to
me. ;) ).
-Peff
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-13 7:45 ` [PATCH v4 0/8] rev-list: implement object type filter Jeff King
@ 2021-04-13 8:06 ` Patrick Steinhardt
2021-04-15 9:42 ` Jeff King
2021-04-13 21:03 ` Junio C Hamano
1 sibling, 1 reply; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-13 8:06 UTC (permalink / raw)
To: Jeff King; +Cc: git, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 2825 bytes --]
On Tue, Apr 13, 2021 at 03:45:21AM -0400, Jeff King wrote:
> On Mon, Apr 12, 2021 at 03:37:14PM +0200, Patrick Steinhardt wrote:
>
> > this is the fourth version of my patch series which implements a new
> > `object:type` filter for git-rev-list(1) and git-upload-pack(1) and
> > extends support for bitmap indices to work with combined filters.
> >
> > Changes compared to v3:
> >
> > - Small style fix to not pass an empty string and `0`, but instead
> > simply pass two `NULL` pointers to
> > `list_objects_filter__filter_object, pointed out by Junio.
> >
> > - I've changed patch 7/8 as proposed by Peff: support of combined
> > filters is now determined directly in `filter_bitmap()`, without
> > having to mirror all filter types in the new `filter_supported()`
> > function.
> >
> > - Renamed `--filter-provided-revisions` to
> > `--filter-provided-objects` as proposed by Peff and addressed both
> > commit message and tests as pointed out by Philip.
>
> Thanks. I have no more nits to pick. The only question left for me is
> the big one of "do we like this with --filter, or should it be some kind
> of rev-list option", as discussed in:
>
> https://lore.kernel.org/git/YHB7R8hVRt+V+i2W@coredump.intra.peff.net/
>
> As I said elsewhere, I'm OK going with this route. I just wanted to call
> attention to that earlier response in case you had any final thoughts
> (I'm guessing your final thoughts are "jeez, I already wrote it with
> --filter, so let's just go with that" which is perfectly reasonable to
> me. ;) ).
I don't think it would help usability to add new `--show-blobs` and
`--show-trees` options. The user interface to show this kind of
information exists already with `--objects`, and by adding another way
of asking a similar query would raise the question of how these two ways
interact with each other:
- Does `--show-blobs` have effect if `--objects` is not set?
- Is `--objects` redundant if we have `--show-blobs`, or would
`--objects --show-blobs` list all objects regardless of whether
they're blobs or not?
- What would happen if the user says `--show-blobs --no-objects`?
- Are these options mutually exclusive?
We avoid all these questions by just adding it as a filter.
Furthermore, the filter also allows future iterations which build on top
of this. If we had a combined OR filter in addition to the existing
combined AND filter, the user could say "Give me all blobs which aren't
bigger than a specific size PLUS all trees with a depth smaller than 5
PLUS all commits and tags". It's not like I'd know of a specific usecase
for this right now, but I think the potential of having such filters in
the future is a plus.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-13 8:06 ` Patrick Steinhardt
@ 2021-04-15 9:42 ` Jeff King
2021-04-16 22:06 ` Junio C Hamano
0 siblings, 1 reply; 97+ messages in thread
From: Jeff King @ 2021-04-15 9:42 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Christian Couder, Taylor Blau, Philip Oakley
On Tue, Apr 13, 2021 at 10:06:13AM +0200, Patrick Steinhardt wrote:
> I don't think it would help usability to add new `--show-blobs` and
> `--show-trees` options. The user interface to show this kind of
> information exists already with `--objects`, and by adding another way
> of asking a similar query would raise the question of how these two ways
> interact with each other:
>
> - Does `--show-blobs` have effect if `--objects` is not set?
>
> - Is `--objects` redundant if we have `--show-blobs`, or would
> `--objects --show-blobs` list all objects regardless of whether
> they're blobs or not?
>
> - What would happen if the user says `--show-blobs --no-objects`?
>
> - Are these options mutually exclusive?
>
> We avoid all these questions by just adding it as a filter.
I'm not too worried about those. I'd imagine that "--objects" becomes a
documented synonym for "--show-trees --show-blobs --show-commits
--show-tags", and then the usual interactions take over.
But...
> Furthermore, the filter also allows future iterations which build on top
> of this. If we had a combined OR filter in addition to the existing
> combined AND filter, the user could say "Give me all blobs which aren't
> bigger than a specific size PLUS all trees with a depth smaller than 5
> PLUS all commits and tags". It's not like I'd know of a specific usecase
> for this right now, but I think the potential of having such filters in
> the future is a plus.
Yeah, that's true. My biggest complaint is lack of an OR filter, but we
could add that later. And then we would be _more_ flexible, as you note,
since we could and/or more filters.
So I'm OK proceeding with this direction.
-Peff
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-15 9:42 ` Jeff King
@ 2021-04-16 22:06 ` Junio C Hamano
2021-04-16 23:15 ` Junio C Hamano
0 siblings, 1 reply; 97+ messages in thread
From: Junio C Hamano @ 2021-04-16 22:06 UTC (permalink / raw)
To: Patrick Steinhardt, Jeff King
Cc: git, Christian Couder, Taylor Blau, Philip Oakley
Jeff King <peff@peff.net> writes:
>> Furthermore, the filter also allows future iterations which build on top
>> of this. If we had a combined OR filter in addition to the existing
>> combined AND filter, the user could say "Give me all blobs which aren't
>> bigger than a specific size PLUS all trees with a depth smaller than 5
>> PLUS all commits and tags". It's not like I'd know of a specific usecase
>> for this right now, but I think the potential of having such filters in
>> the future is a plus.
>
> Yeah, that's true. My biggest complaint is lack of an OR filter, but we
> could add that later. And then we would be _more_ flexible, as you note,
> since we could and/or more filters.
>
> So I'm OK proceeding with this direction.
I think the only remaining issues are the comments on 5/8 on tests,
then? Hopefully we can have one more iteration to finalize the
topic and merge it down to 'next'?
Thanks.
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-16 22:06 ` Junio C Hamano
@ 2021-04-16 23:15 ` Junio C Hamano
2021-04-17 1:17 ` Ramsay Jones
0 siblings, 1 reply; 97+ messages in thread
From: Junio C Hamano @ 2021-04-16 23:15 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Jeff King, git, Christian Couder, Taylor Blau, Philip Oakley
Junio C Hamano <gitster@pobox.com> writes:
> I think the only remaining issues are the comments on 5/8 on tests,
> then? Hopefully we can have one more iteration to finalize the
> topic and merge it down to 'next'?
>
> Thanks.
I guess not. I am guessing this topic is responsible for
https://github.com/git/git/runs/2366364023?check_suite_focus=true#step:4:115
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-16 23:15 ` Junio C Hamano
@ 2021-04-17 1:17 ` Ramsay Jones
2021-04-17 9:01 ` Jeff King
0 siblings, 1 reply; 97+ messages in thread
From: Ramsay Jones @ 2021-04-17 1:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Patrick Steinhardt, Jeff King, git, Christian Couder,
Taylor Blau, Philip Oakley
On Fri, Apr 16, 2021 at 04:15:39PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > I think the only remaining issues are the comments on 5/8 on tests,
> > then? Hopefully we can have one more iteration to finalize the
> > topic and merge it down to 'next'?
> >
> > Thanks.
>
> I guess not. I am guessing this topic is responsible for
>
> https://github.com/git/git/runs/2366364023?check_suite_focus=true#step:4:115
Yes, I noticed this a few days ago, and tried the obvious fix (ie to
#include "cache.h" at the start of the list-objects-filter-options.h
header file) which does indeed work fine. However, I then thought that
moving the definition of 'enum object_type' (along with the TYPE_BITS
#define) to the 'object.h' header file (and #include "object.h" into
cache.h) would be a better idea...
Having done that, I wondered how many '#include "cache.h"' could be
replaced by "object.h", and ... well, that was a few days ago and
something came up...
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-17 1:17 ` Ramsay Jones
@ 2021-04-17 9:01 ` Jeff King
2021-04-17 21:45 ` Junio C Hamano
0 siblings, 1 reply; 97+ messages in thread
From: Jeff King @ 2021-04-17 9:01 UTC (permalink / raw)
To: Ramsay Jones
Cc: Junio C Hamano, Patrick Steinhardt, git, Christian Couder,
Taylor Blau, Philip Oakley
On Sat, Apr 17, 2021 at 02:17:32AM +0100, Ramsay Jones wrote:
> On Fri, Apr 16, 2021 at 04:15:39PM -0700, Junio C Hamano wrote:
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> > > I think the only remaining issues are the comments on 5/8 on tests,
> > > then? Hopefully we can have one more iteration to finalize the
> > > topic and merge it down to 'next'?
> > >
> > > Thanks.
> >
> > I guess not. I am guessing this topic is responsible for
> >
> > https://github.com/git/git/runs/2366364023?check_suite_focus=true#step:4:115
>
> Yes, I noticed this a few days ago, and tried the obvious fix (ie to
> #include "cache.h" at the start of the list-objects-filter-options.h
> header file) which does indeed work fine. However, I then thought that
> moving the definition of 'enum object_type' (along with the TYPE_BITS
> #define) to the 'object.h' header file (and #include "object.h" into
> cache.h) would be a better idea...
>
> Having done that, I wondered how many '#include "cache.h"' could be
> replaced by "object.h", and ... well, that was a few days ago and
> something came up...
I agree that would probably be nice, but let's not hold up the topic. It
can include "cache.h" like a bunch of other headers in the meantime.
(Too bad we can't just forward-declare the enum like we do for some
other types, but it's impossible in C).
-Peff
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-17 9:01 ` Jeff King
@ 2021-04-17 21:45 ` Junio C Hamano
0 siblings, 0 replies; 97+ messages in thread
From: Junio C Hamano @ 2021-04-17 21:45 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Jeff King, Ramsay Jones, git, Christian Couder, Taylor Blau,
Philip Oakley
Jeff King <peff@peff.net> writes:
> I agree that would probably be nice, but let's not hold up the topic. It
> can include "cache.h" like a bunch of other headers in the meantime.
>
> (Too bad we can't just forward-declare the enum like we do for some
> other types, but it's impossible in C).
For now...
From f08346ce05e5512e5914e760aba769d0ef8035bc Mon Sep 17 00:00:00 2001
From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 17 Apr 2021 14:44:37 -0700
Subject: [PATCH] fixup! list-objects: implement object type filter
---
list-objects-filter-options.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index 4d0d0588cc..da5b6737e2 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -1,6 +1,7 @@
#ifndef LIST_OBJECTS_FILTER_OPTIONS_H
#define LIST_OBJECTS_FILTER_OPTIONS_H
+#include "cache.h"
#include "parse-options.h"
#include "string-list.h"
--
2.31.1-450-gca40e2789b
^ permalink raw reply related [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-13 7:45 ` [PATCH v4 0/8] rev-list: implement object type filter Jeff King
2021-04-13 8:06 ` Patrick Steinhardt
@ 2021-04-13 21:03 ` Junio C Hamano
2021-04-14 11:59 ` Patrick Steinhardt
1 sibling, 1 reply; 97+ messages in thread
From: Junio C Hamano @ 2021-04-13 21:03 UTC (permalink / raw)
To: Jeff King
Cc: Patrick Steinhardt, git, Christian Couder, Taylor Blau, Philip Oakley
Jeff King <peff@peff.net> writes:
> Thanks. I have no more nits to pick. The only question left for me is
> the big one of "do we like this with --filter, or should it be some kind
> of rev-list option", as discussed in:
>
> https://lore.kernel.org/git/YHB7R8hVRt+V+i2W@coredump.intra.peff.net/
I do agree that adding "--blobs", "--trees", and "--tags" options to
the "--objects" (and implicit --commits) to rev-list parameters is a
lot more in line with the original design by Linus when we added
"--objects" (vs not giving it). We even internally have had the "do
we show trees?" "do we show blobs?" separate bits from the very
beginning of the "--objects" feature at 9de48752 (git-rev-list: add
option to list all objects (not just commits), 2005-06-24), which
was extended to cover tags at 3c90f03d (Prepare git-rev-list for
tracking tag objects too, 2005-06-29). The basic design principle
hasn't changed when the code was reorganized to be closer to the
current shape at ae563542 (First cut at libifying revlist
generation, 2006-02-25).
But back then, we didn't have mechanisms to filter rev-list output
using other criteria, which brought us the umbrella notation to use
"--filter=...", so as a notation, it might be OK, provided if
git rev-list \
--filter=object:type=tag \
--filter=object:type=commit \
--filter=object:type=tree \
--filter=object:type=blob "$@ther args"
is an equivalent to existing
git rev-list --objects "$@ther args"
I however have to wonder why this need so much code (in other words,
why do we need more than just adding something similar to this block
in the revision.c machinery:
} else if (!strcmp(arg, "--objects")) {
revs->tag_objects = 1;
revs->tree_objects = 1;
revs->blob_objects = 1;
that flips <type>_objects member bits?) though.
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-13 21:03 ` Junio C Hamano
@ 2021-04-14 11:59 ` Patrick Steinhardt
2021-04-14 21:07 ` Junio C Hamano
0 siblings, 1 reply; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-14 11:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, git, Christian Couder, Taylor Blau, Philip Oakley
[-- Attachment #1: Type: text/plain, Size: 3499 bytes --]
On Tue, Apr 13, 2021 at 02:03:12PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Thanks. I have no more nits to pick. The only question left for me is
> > the big one of "do we like this with --filter, or should it be some kind
> > of rev-list option", as discussed in:
> >
> > https://lore.kernel.org/git/YHB7R8hVRt+V+i2W@coredump.intra.peff.net/
>
> I do agree that adding "--blobs", "--trees", and "--tags" options to
> the "--objects" (and implicit --commits) to rev-list parameters is a
> lot more in line with the original design by Linus when we added
> "--objects" (vs not giving it). We even internally have had the "do
> we show trees?" "do we show blobs?" separate bits from the very
> beginning of the "--objects" feature at 9de48752 (git-rev-list: add
> option to list all objects (not just commits), 2005-06-24), which
> was extended to cover tags at 3c90f03d (Prepare git-rev-list for
> tracking tag objects too, 2005-06-29). The basic design principle
> hasn't changed when the code was reorganized to be closer to the
> current shape at ae563542 (First cut at libifying revlist
> generation, 2006-02-25).
>
> But back then, we didn't have mechanisms to filter rev-list output
> using other criteria, which brought us the umbrella notation to use
> "--filter=...", so as a notation, it might be OK, provided if
>
> git rev-list \
> --filter=object:type=tag \
> --filter=object:type=commit \
> --filter=object:type=tree \
> --filter=object:type=blob "$@ther args"
>
> is an equivalent to existing
>
> git rev-list --objects "$@ther args"
Filters are currently AND filters, so specifying them multiple times
would cause us to print nothing. And I don't think we should treat
object:type filters any different compared to the other filters, because
that could limit our ability to iterate in the future where we may want
to add OR filters.
I initially said that I didn't want to add `object:type=tag,commit`
filters as a way to say that all types which are either a tag or commit
should be printed because I thought that having the ability to combine
filters with an OR instead of an AND would be the better design here.
But on the other hand, there is no reason we cannot have both, and it
would implement your above usecase, even though syntax is different.
> I however have to wonder why this need so much code (in other words,
> why do we need more than just adding something similar to this block
> in the revision.c machinery:
>
> } else if (!strcmp(arg, "--objects")) {
> revs->tag_objects = 1;
> revs->tree_objects = 1;
> revs->blob_objects = 1;
>
> that flips <type>_objects member bits?) though.
So if we make this part of the rev machinery, I guess your idea is that
we can just put the logic into `show_object()`? That would indeed be a
lot simpler to implement with a lot less code. But on the other hand,
it's also less efficient: we cannot stop walking down the graph like we
can with the current design when e.g. saying "Only give me
{tags,commits,trees}". And with bitmap indices, we can even skip all
objects of a certain type at once, without having to load the object,
right inside the filtering logic. If this was part of `show_object()`,
we wouldn't be able to do so (easily).
Anyway, this is assuming that I'm not misinterpreting what you mean by
your above comment. So please let me know if I misunderstood.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-14 11:59 ` Patrick Steinhardt
@ 2021-04-14 21:07 ` Junio C Hamano
2021-04-15 9:57 ` Jeff King
0 siblings, 1 reply; 97+ messages in thread
From: Junio C Hamano @ 2021-04-14 21:07 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Jeff King, git, Christian Couder, Taylor Blau, Philip Oakley
Patrick Steinhardt <ps@pks.im> writes:
>> I however have to wonder why this need so much code (in other words,
>> why do we need more than just adding something similar to this block
>> in the revision.c machinery:
>>
>> } else if (!strcmp(arg, "--objects")) {
>> revs->tag_objects = 1;
>> revs->tree_objects = 1;
>> revs->blob_objects = 1;
>>
>> that flips <type>_objects member bits?) though.
>
> So if we make this part of the rev machinery, I guess your idea is that
> we can just put the logic into `show_object()`?
Not necessarily. I was making a lot more naïve observations.
* Even though we have 3 independent (tag|tree|blob)_objects bits,
they are all set to true with "--objects" or they are cleared
without. There is no codepath that flips these bits to set some
but not all of them to be set.
* But if we look at the the hits from
$ git grep -C2 -E -e '([.]|->)(tag|tree|blob)_objects' \*.[ch]
it is clear that the code is (at least trying to be) prepared for
them to be set independently. The .tree_objects member is often
checked without checking others to mark the tree objects on the
edge of the range uninteresting, for example.
It of course is unknown how well the code is actually prepared
for these three bits to be set independently, as nobody can set
these bits independently.
* Which makes a naïve reader to wonder if it would be sufficient
to have a silly option, like this:
} else if (!strcmp(arg, "--filter=object:type=tree")) {
revs->tag_objects = 0;
revs->tree_objects = 1;
revs->blob_objects = 0;
in the same if/else if/... cascade as the above (and other types
as well), in order to do the same thing as this series.
And, the above led to the question---the patches in your series
apparently do a lot more (even if we discount the option parsing
part), and I was wondering if that is because the independence
between these three bits the existing code aspires to maintain is
broken.
> Anyway, this is assuming that I'm not misinterpreting what you mean by
> your above comment. So please let me know if I misunderstood.
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-14 21:07 ` Junio C Hamano
@ 2021-04-15 9:57 ` Jeff King
2021-04-15 17:53 ` Junio C Hamano
0 siblings, 1 reply; 97+ messages in thread
From: Jeff King @ 2021-04-15 9:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Patrick Steinhardt, git, Christian Couder, Taylor Blau, Philip Oakley
On Wed, Apr 14, 2021 at 02:07:07PM -0700, Junio C Hamano wrote:
> * But if we look at the the hits from
>
> $ git grep -C2 -E -e '([.]|->)(tag|tree|blob)_objects' \*.[ch]
>
> it is clear that the code is (at least trying to be) prepared for
> them to be set independently. The .tree_objects member is often
> checked without checking others to mark the tree objects on the
> edge of the range uninteresting, for example.
>
> It of course is unknown how well the code is actually prepared
> for these three bits to be set independently, as nobody can set
> these bits independently.
Yeah, as somebody who has added or touched a lot of those paths, I've
often wondered this myself: what would break if you asked for blobs but
not trees? I think it does not work like you'd expect, because
list-objects.c:process_tree() will not recurse the trees at all (and
hence you'd never see any blob).
> * Which makes a naïve reader to wonder if it would be sufficient
> to have a silly option, like this:
>
> } else if (!strcmp(arg, "--filter=object:type=tree")) {
> revs->tag_objects = 0;
> revs->tree_objects = 1;
> revs->blob_objects = 0;
>
> in the same if/else if/... cascade as the above (and other types
> as well), in order to do the same thing as this series.
>
> And, the above led to the question---the patches in your series
> apparently do a lot more (even if we discount the option parsing
> part), and I was wondering if that is because the independence
> between these three bits the existing code aspires to maintain is
> broken.
I don't think the code Patrick added is that much more complex. Most if
it was cleaning up rough edges in the filtering system, and making sure
that bitmaps supported this style of filtering. I _think_ the existing
bitmap code would Just Work with the example you showed above. It uses
list-objects.c to do any fill-in traversal, and then
traverse_bitmap_commit_list() uses the type-bitmaps to filter the
results.
But it would be likewise broken for the case of "no trees, just
blobs", because of the problem in list-objects.c that I mentioned (but
worse, it would only be _half_ broken; we might even produce the right
answer if we don't have to do any fill-in traversal!).
Anyway. I do think this all could be done using the existing bits in
rev_info. But there is value in making it all part of the modular
filtering system.
-Peff
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-15 9:57 ` Jeff King
@ 2021-04-15 17:53 ` Junio C Hamano
2021-04-15 17:57 ` Junio C Hamano
0 siblings, 1 reply; 97+ messages in thread
From: Junio C Hamano @ 2021-04-15 17:53 UTC (permalink / raw)
To: Jeff King
Cc: Patrick Steinhardt, git, Christian Couder, Taylor Blau, Philip Oakley
Jeff King <peff@peff.net> writes:
> Yeah, as somebody who has added or touched a lot of those paths, I've
> often wondered this myself: what would break if you asked for blobs but
> not trees? I think it does not work like you'd expect, because
> list-objects.c:process_tree() will not recurse the trees at all (and
> hence you'd never see any blob).
Ah, OK, that much I figured. I was wondering if it is worth
"fixing", IOW, if we see blobs being asked without trees being
asked, perhaps process_tree() should be taught to descend---even if
it won't show the trees themselves, blobs contained within would.
>> And, the above led to the question---the patches in your series
>> apparently do a lot more (even if we discount the option parsing
>> part), and I was wondering if that is because the independence
>> between these three bits the existing code aspires to maintain is
>> broken.
> ...
> Anyway. I do think this all could be done using the existing bits in
> rev_info. But there is value in making it all part of the modular
> filtering system.
Yes, I do not have a problem with the approach to add new features
as part of the "modular filtering system". But that leads me to
wonder into a different direction---coalesce (tag|tree|blob)_objects
members into a single bit, say all_objects, have "--objects" and
friends set that single bit, and update places like these to check
that single bit.
The patch is for illustration purposes; I didn't even aim to cover
the entirety of these two files.
builtin/rev-list.c | 9 ++++-----
pack-bitmap.c | 4 +---
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git c/builtin/rev-list.c w/builtin/rev-list.c
index 7677b1af5a..b6f0d7e9a3 100644
--- c/builtin/rev-list.c
+++ w/builtin/rev-list.c
@@ -424,8 +424,7 @@ static int try_bitmap_count(struct rev_info *revs,
* commits to traverse, since we don't know which objects go with which
* commit.
*/
- if (revs->max_count >= 0 &&
- (revs->tag_objects || revs->tree_objects || revs->blob_objects))
+ if (revs->max_count >= 0 && revs->all_objects)
return -1;
/*
@@ -439,9 +438,9 @@ static int try_bitmap_count(struct rev_info *revs,
return -1;
count_bitmap_commit_list(bitmap_git, &commit_count,
- revs->tree_objects ? &tree_count : NULL,
- revs->blob_objects ? &blob_count : NULL,
- revs->tag_objects ? &tag_count : NULL);
+ revs->all_objects ? &tree_count : NULL,
+ revs->all_objects ? &blob_count : NULL,
+ revs->all_objects ? &tag_count : NULL);
if (max_count >= 0 && max_count < commit_count)
commit_count = max_count;
diff --git c/pack-bitmap.c w/pack-bitmap.c
index d7e9f14f92..918c80a391 100644
--- c/pack-bitmap.c
+++ w/pack-bitmap.c
@@ -801,9 +801,7 @@ static void show_extended_objects(struct bitmap_index *bitmap_git,
continue;
obj = eindex->objects[i];
- if ((obj->type == OBJ_BLOB && !revs->blob_objects) ||
- (obj->type == OBJ_TREE && !revs->tree_objects) ||
- (obj->type == OBJ_TAG && !revs->tag_objects))
+ if (!revs->all_objects && obj->type != OBJ_COMMIT)
continue;
show_reach(&obj->oid, obj->type, 0, eindex->hashes[i], NULL, 0);
^ permalink raw reply related [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-15 17:53 ` Junio C Hamano
@ 2021-04-15 17:57 ` Junio C Hamano
2021-04-17 8:58 ` Jeff King
0 siblings, 1 reply; 97+ messages in thread
From: Junio C Hamano @ 2021-04-15 17:57 UTC (permalink / raw)
To: Jeff King
Cc: Patrick Steinhardt, git, Christian Couder, Taylor Blau, Philip Oakley
Junio C Hamano <gitster@pobox.com> writes:
> ... But that leads me to
> wonder into a different direction---coalesce (tag|tree|blob)_objects
> members into a single bit, say all_objects, have "--objects" and
> friends set that single bit, and update places like these to check
> that single bit.
Just to avoid misunderstanding, I am not saying this topic needs to
address any of this unifying of three bits.
It is just an idea for those interested to think about, when they
have nothing better to do, when the codebase is quiescent.
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v4 0/8] rev-list: implement object type filter
2021-04-15 17:57 ` Junio C Hamano
@ 2021-04-17 8:58 ` Jeff King
0 siblings, 0 replies; 97+ messages in thread
From: Jeff King @ 2021-04-17 8:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: Patrick Steinhardt, git, Christian Couder, Taylor Blau, Philip Oakley
On Thu, Apr 15, 2021 at 10:57:51AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > ... But that leads me to
> > wonder into a different direction---coalesce (tag|tree|blob)_objects
> > members into a single bit, say all_objects, have "--objects" and
> > friends set that single bit, and update places like these to check
> > that single bit.
>
> Just to avoid misunderstanding, I am not saying this topic needs to
> address any of this unifying of three bits.
>
> It is just an idea for those interested to think about, when they
> have nothing better to do, when the codebase is quiescent.
It does feel like going "backwards" in a sense. We have the three flags
mostly split, and we'd lose that distinction. On the other hand, if the
current split is imperfect, it may be leading people down a confusing
path (I _think_ this "trees must be set in order to see blobs" thing is
the only real gotcha, but there could be others).
There's some other discussion in this old thread:
https://lore.kernel.org/git/06a84f8c77924b275606384ead8bb2fd7d75f7b6.1487984670.git.jonathantanmy@google.com/
(I didn't remember it, but my spider sense tingling caused me to dig in
the archive a bit).
-Peff
^ permalink raw reply [flat|nested] 97+ messages in thread
* [PATCH v5 0/8] rev-list: implement object type filter
2021-04-12 13:37 ` [PATCH v4 0/8] rev-list: " Patrick Steinhardt
` (8 preceding siblings ...)
2021-04-13 7:45 ` [PATCH v4 0/8] rev-list: implement object type filter Jeff King
@ 2021-04-19 11:46 ` Patrick Steinhardt
2021-04-19 11:46 ` [PATCH v5 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow` Patrick Steinhardt
` (8 more replies)
9 siblings, 9 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-19 11:46 UTC (permalink / raw)
To: git
Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
[-- Attachment #1: Type: text/plain, Size: 5559 bytes --]
Hi,
this is the fifth version of my patch series which implements a new
`object:type` filter for git-rev-list(1) and git-upload-pack(1) and
extends support for bitmap indices to work with combined filters.
Changes compared to v4:
- I'm now explicitly passing `strlen(v0)` to
`type_from_string_gently()` to be prepared for the future
semantics change here.
- The error message printed in case the user passes in invalid
object type to `--filter=object:type=` now prints the wrong
value passed by the user.
- Fixed missing header in list-objects-filter-options.h.
- Tests now use test_create_repo and test_commit.
- Removed a needless subshell in the tests.
I hope that this catches all feedback and that I didn't misunderstand or
miss something. If I did, please give me a shout!
Patrick
Patrick Steinhardt (8):
uploadpack.txt: document implication of `uploadpackfilter.allow`
revision: mark commit parents as NOT_USER_GIVEN
list-objects: move tag processing into its own function
list-objects: support filtering by tag and commit
list-objects: implement object type filter
pack-bitmap: implement object type filter
pack-bitmap: implement combined filter
rev-list: allow filtering of provided items
Documentation/config/uploadpack.txt | 9 ++-
Documentation/rev-list-options.txt | 8 ++
builtin/pack-objects.c | 2 +-
builtin/rev-list.c | 36 ++++++---
list-objects-filter-options.c | 15 ++++
list-objects-filter-options.h | 3 +
list-objects-filter.c | 116 ++++++++++++++++++++++++++++
list-objects-filter.h | 2 +
list-objects.c | 30 ++++++-
pack-bitmap.c | 45 +++++++++--
pack-bitmap.h | 3 +-
reachable.c | 2 +-
revision.c | 4 +-
revision.h | 3 -
t/t6112-rev-list-filters-objects.sh | 72 +++++++++++++++++
t/t6113-rev-list-bitmap-filters.sh | 68 +++++++++++++++-
16 files changed, 388 insertions(+), 30 deletions(-)
Range-diff against v4:
1: f80b9570d4 = 1: bcc81336b1 uploadpack.txt: document implication of `uploadpackfilter.allow`
2: 46c1952405 = 2: ffe8e2bf73 revision: mark commit parents as NOT_USER_GIVEN
3: 3d792f6339 = 3: 21d7f06d0a list-objects: move tag processing into its own function
4: 674da0f9ac = 4: b5d6ab6b4a list-objects: support filtering by tag and commit
5: d22a5fd37d ! 5: f462745290 list-objects: implement object type filter
@@ list-objects-filter-options.c: static int gently_parse_list_objects_filter(
return 1;
+ } else if (skip_prefix(arg, "object:type=", &v0)) {
-+ int type = type_from_string_gently(v0, -1, 1);
++ int type = type_from_string_gently(v0, strlen(v0), 1);
+ if (type < 0) {
-+ strbuf_addstr(errbuf, _("expected 'object:type=<type>'"));
++ strbuf_addf(errbuf, _("'%s' for 'object:type=<type>' is"
++ "not a valid object type"), v0);
+ return 1;
+ }
+
@@ list-objects-filter-options.c: static int gently_parse_list_objects_filter(
## list-objects-filter-options.h ##
+@@
+ #ifndef LIST_OBJECTS_FILTER_OPTIONS_H
+ #define LIST_OBJECTS_FILTER_OPTIONS_H
+
++#include "cache.h"
+ #include "parse-options.h"
+ #include "string-list.h"
+
@@ list-objects-filter-options.h: enum list_objects_filter_choice {
LOFC_BLOB_LIMIT,
LOFC_TREE_DEPTH,
@@ t/t6112-rev-list-filters-objects.sh: test_expect_success 'verify blob:limit=1m'
+# Test object:type=<type> filter.
+
+test_expect_success 'setup object-type' '
-+ git init object-type &&
-+ echo contents >object-type/blob &&
-+ git -C object-type add blob &&
-+ git -C object-type commit -m commit-message &&
++ test_create_repo object-type &&
++ test_commit --no-tag -C object-type message blob &&
+ git -C object-type tag tag -m tag-message
+'
+
@@ t/t6112-rev-list-filters-objects.sh: test_expect_success 'verify blob:limit=1m'
+'
+
+test_expect_success 'verify object:type=blob prints blob and commit' '
-+ (
-+ git -C object-type rev-parse HEAD &&
-+ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob)
-+ ) >expected &&
++ git -C object-type rev-parse HEAD >expected &&
++ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob) >>expected &&
+ git -C object-type rev-list --objects --filter=object:type=blob HEAD >actual &&
+ test_cmp expected actual
+'
6: 17c9f66bbc = 6: 90c80c1efa pack-bitmap: implement object type filter
7: 759ac54bb2 = 7: 9726e69861 pack-bitmap: implement combined filter
8: c779d222cf ! 8: fce3551e48 rev-list: allow filtering of provided items
@@ pack-bitmap.c: struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
## pack-bitmap.h ##
@@ pack-bitmap.h: void traverse_bitmap_commit_list(struct bitmap_index *,
- show_reachable_fn show_reachable);
void test_bitmap_walk(struct rev_info *revs);
+ int test_bitmap_commits(struct repository *r);
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter);
+ struct list_objects_filter_options *filter,
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 97+ messages in thread
* [PATCH v5 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow`
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
@ 2021-04-19 11:46 ` Patrick Steinhardt
2021-04-19 11:46 ` [PATCH v5 2/8] revision: mark commit parents as NOT_USER_GIVEN Patrick Steinhardt
` (7 subsequent siblings)
8 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-19 11:46 UTC (permalink / raw)
To: git
Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]
When `uploadpackfilter.allow` is set to `true`, it means that filters
are enabled by default except in the case where a filter is explicitly
disabled via `uploadpackilter.<filter>.allow`. This option will not only
enable the currently supported set of filters, but also any filters
which get added in the future. As such, an admin which wants to have
tight control over which filters are allowed and which aren't probably
shouldn't ever set `uploadpackfilter.allow=true`.
Amend the documentation to make the ramifications more explicit so that
admins are aware of this.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/config/uploadpack.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
index b0d761282c..6729a072ea 100644
--- a/Documentation/config/uploadpack.txt
+++ b/Documentation/config/uploadpack.txt
@@ -59,7 +59,8 @@ uploadpack.allowFilter::
uploadpackfilter.allow::
Provides a default value for unspecified object filters (see: the
- below configuration variable).
+ below configuration variable). If set to `true`, this will also
+ enable all filters which get added in the future.
Defaults to `true`.
uploadpackfilter.<filter>.allow::
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v5 2/8] revision: mark commit parents as NOT_USER_GIVEN
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
2021-04-19 11:46 ` [PATCH v5 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow` Patrick Steinhardt
@ 2021-04-19 11:46 ` Patrick Steinhardt
2021-04-19 11:46 ` [PATCH v5 3/8] list-objects: move tag processing into its own function Patrick Steinhardt
` (6 subsequent siblings)
8 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-19 11:46 UTC (permalink / raw)
To: git
Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
[-- Attachment #1: Type: text/plain, Size: 2338 bytes --]
The NOT_USER_GIVEN flag of an object marks whether a flag was explicitly
provided by the user or not. The most important use case for this is
when filtering objects: only objects that were not explicitly requested
will get filtered.
The flag is currently only set for blobs and trees, which has been fine
given that there are no filters for tags or commits currently. We're
about to extend filtering capabilities to add object type filter though,
which requires us to set up the NOT_USER_GIVEN flag correctly -- if it's
not set, the object wouldn't get filtered at all.
Mark unseen commit parents as NOT_USER_GIVEN when processing parents.
Like this, explicitly provided parents stay user-given and thus
unfiltered, while parents which get loaded as part of the graph walk
can be filtered.
This commit shouldn't have any user-visible impact yet as there is no
logic to filter commits yet.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
revision.c | 4 ++--
revision.h | 3 ---
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/revision.c b/revision.c
index 553c0faa9b..fd34c75e23 100644
--- a/revision.c
+++ b/revision.c
@@ -1123,7 +1123,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
mark_parents_uninteresting(p);
if (p->object.flags & SEEN)
continue;
- p->object.flags |= SEEN;
+ p->object.flags |= (SEEN | NOT_USER_GIVEN);
if (list)
commit_list_insert_by_date(p, list);
if (queue)
@@ -1165,7 +1165,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
}
p->object.flags |= left_flag;
if (!(p->object.flags & SEEN)) {
- p->object.flags |= SEEN;
+ p->object.flags |= (SEEN | NOT_USER_GIVEN);
if (list)
commit_list_insert_by_date(p, list);
if (queue)
diff --git a/revision.h b/revision.h
index a24f72dcd1..93aa012f51 100644
--- a/revision.h
+++ b/revision.h
@@ -44,9 +44,6 @@
/*
* Indicates object was reached by traversal. i.e. not given by user on
* command-line or stdin.
- * NEEDSWORK: NOT_USER_GIVEN doesn't apply to commits because we only support
- * filtering trees and blobs, but it may be useful to support filtering commits
- * in the future.
*/
#define NOT_USER_GIVEN (1u<<25)
#define TRACK_LINEAR (1u<<26)
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v5 3/8] list-objects: move tag processing into its own function
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
2021-04-19 11:46 ` [PATCH v5 1/8] uploadpack.txt: document implication of `uploadpackfilter.allow` Patrick Steinhardt
2021-04-19 11:46 ` [PATCH v5 2/8] revision: mark commit parents as NOT_USER_GIVEN Patrick Steinhardt
@ 2021-04-19 11:46 ` Patrick Steinhardt
2021-04-19 11:46 ` [PATCH v5 4/8] list-objects: support filtering by tag and commit Patrick Steinhardt
` (5 subsequent siblings)
8 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-19 11:46 UTC (permalink / raw)
To: git
Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]
Move processing of tags into its own function to make the logic easier
to extend when we're going to implement filtering for tags. No change in
behaviour is expected from this commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
list-objects.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/list-objects.c b/list-objects.c
index e19589baa0..a5a60301cb 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -213,6 +213,14 @@ static void process_tree(struct traversal_context *ctx,
free_tree_buffer(tree);
}
+static void process_tag(struct traversal_context *ctx,
+ struct tag *tag,
+ const char *name)
+{
+ tag->object.flags |= SEEN;
+ ctx->show_object(&tag->object, name, ctx->show_data);
+}
+
static void mark_edge_parents_uninteresting(struct commit *commit,
struct rev_info *revs,
show_edge_fn show_edge)
@@ -334,8 +342,7 @@ static void traverse_trees_and_blobs(struct traversal_context *ctx,
if (obj->flags & (UNINTERESTING | SEEN))
continue;
if (obj->type == OBJ_TAG) {
- obj->flags |= SEEN;
- ctx->show_object(obj, name, ctx->show_data);
+ process_tag(ctx, (struct tag *)obj, name);
continue;
}
if (!path)
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v5 4/8] list-objects: support filtering by tag and commit
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
` (2 preceding siblings ...)
2021-04-19 11:46 ` [PATCH v5 3/8] list-objects: move tag processing into its own function Patrick Steinhardt
@ 2021-04-19 11:46 ` Patrick Steinhardt
2021-04-19 11:46 ` [PATCH v5 5/8] list-objects: implement object type filter Patrick Steinhardt
` (4 subsequent siblings)
8 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-19 11:46 UTC (permalink / raw)
To: git
Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
[-- Attachment #1: Type: text/plain, Size: 4809 bytes --]
Object filters currently only support filtering blobs or trees based on
some criteria. This commit lays the foundation to also allow filtering
of tags and commits.
No change in behaviour is expected from this commit given that there are
no filters yet for those object types.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
list-objects-filter.c | 40 ++++++++++++++++++++++++++++++++++++++++
list-objects-filter.h | 2 ++
list-objects.c | 23 ++++++++++++++++++++---
3 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 39e2f15333..0ebfa52966 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -82,6 +82,16 @@ static enum list_objects_filter_result filter_blobs_none(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
/* always include all tree objects */
@@ -173,6 +183,16 @@ static enum list_objects_filter_result filter_trees_depth(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_END_TREE:
assert(obj->type == OBJ_TREE);
filter_data->current_depth--;
@@ -267,6 +287,16 @@ static enum list_objects_filter_result filter_blobs_limit(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
/* always include all tree objects */
@@ -371,6 +401,16 @@ static enum list_objects_filter_result filter_sparse(
default:
BUG("unknown filter_situation: %d", filter_situation);
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ /* always include all tag objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ /* always include all commit objects */
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
dtype = DT_DIR;
diff --git a/list-objects-filter.h b/list-objects-filter.h
index cfd784e203..9e98814111 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -55,6 +55,8 @@ enum list_objects_filter_result {
};
enum list_objects_filter_situation {
+ LOFS_COMMIT,
+ LOFS_TAG,
LOFS_BEGIN_TREE,
LOFS_END_TREE,
LOFS_BLOB
diff --git a/list-objects.c b/list-objects.c
index a5a60301cb..7f404677d5 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -217,8 +217,15 @@ static void process_tag(struct traversal_context *ctx,
struct tag *tag,
const char *name)
{
- tag->object.flags |= SEEN;
- ctx->show_object(&tag->object, name, ctx->show_data);
+ enum list_objects_filter_result r;
+
+ r = list_objects_filter__filter_object(ctx->revs->repo, LOFS_TAG,
+ &tag->object, NULL, NULL,
+ ctx->filter);
+ if (r & LOFR_MARK_SEEN)
+ tag->object.flags |= SEEN;
+ if (r & LOFR_DO_SHOW)
+ ctx->show_object(&tag->object, name, ctx->show_data);
}
static void mark_edge_parents_uninteresting(struct commit *commit,
@@ -368,6 +375,12 @@ static void do_traverse(struct traversal_context *ctx)
strbuf_init(&csp, PATH_MAX);
while ((commit = get_revision(ctx->revs)) != NULL) {
+ enum list_objects_filter_result r;
+
+ r = list_objects_filter__filter_object(ctx->revs->repo,
+ LOFS_COMMIT, &commit->object,
+ NULL, NULL, ctx->filter);
+
/*
* an uninteresting boundary commit may not have its tree
* parsed yet, but we are not going to show them anyway
@@ -382,7 +395,11 @@ static void do_traverse(struct traversal_context *ctx)
die(_("unable to load root tree for commit %s"),
oid_to_hex(&commit->object.oid));
}
- ctx->show_commit(commit, ctx->show_data);
+
+ if (r & LOFR_MARK_SEEN)
+ commit->object.flags |= SEEN;
+ if (r & LOFR_DO_SHOW)
+ ctx->show_commit(commit, ctx->show_data);
if (ctx->revs->tree_blobs_in_commit_order)
/*
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v5 5/8] list-objects: implement object type filter
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
` (3 preceding siblings ...)
2021-04-19 11:46 ` [PATCH v5 4/8] list-objects: support filtering by tag and commit Patrick Steinhardt
@ 2021-04-19 11:46 ` Patrick Steinhardt
2021-04-19 11:46 ` [PATCH v5 6/8] pack-bitmap: " Patrick Steinhardt
` (3 subsequent siblings)
8 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-19 11:46 UTC (permalink / raw)
To: git
Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
[-- Attachment #1: Type: text/plain, Size: 9674 bytes --]
While it already is possible to filter objects by some criteria in
git-rev-list(1), it is not yet possible to filter out only a specific
type of objects. This makes some filters less useful. The `blob:limit`
filter for example filters blobs such that only those which are smaller
than the given limit are returned. But it is unfit to ask only for these
smallish blobs, given that git-rev-list(1) will continue to print tags,
commits and trees.
Now that we have the infrastructure in place to also filter tags and
commits, we can improve this situation by implementing a new filter
which selects objects based on their type. Above query can thus
trivially be implemented with the following command:
$ git rev-list --objects --filter=object:type=blob \
--filter=blob:limit=200
Furthermore, this filter allows to optimize for certain other cases: if
for example only tags or commits have been selected, there is no need to
walk down trees.
The new filter is not yet supported in bitmaps. This is going to be
implemented in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/config/uploadpack.txt | 6 +--
Documentation/rev-list-options.txt | 3 ++
list-objects-filter-options.c | 15 ++++++
list-objects-filter-options.h | 3 ++
list-objects-filter.c | 76 +++++++++++++++++++++++++++++
t/t6112-rev-list-filters-objects.sh | 44 +++++++++++++++++
6 files changed, 144 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
index 6729a072ea..32fad5bbe8 100644
--- a/Documentation/config/uploadpack.txt
+++ b/Documentation/config/uploadpack.txt
@@ -66,9 +66,9 @@ uploadpackfilter.allow::
uploadpackfilter.<filter>.allow::
Explicitly allow or ban the object filter corresponding to
`<filter>`, where `<filter>` may be one of: `blob:none`,
- `blob:limit`, `tree`, `sparse:oid`, or `combine`. If using
- combined filters, both `combine` and all of the nested filter
- kinds must be allowed. Defaults to `uploadpackfilter.allow`.
+ `blob:limit`, `object:type`, `tree`, `sparse:oid`, or `combine`.
+ If using combined filters, both `combine` and all of the nested
+ filter kinds must be allowed. Defaults to `uploadpackfilter.allow`.
uploadpackfilter.tree.maxDepth::
Only allow `--filter=tree:<n>` when `<n>` is no more than the value of
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index b1c8f86c6e..3afa8fffbd 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -892,6 +892,9 @@ or units. n may be zero. The suffixes k, m, and g can be used to name
units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same
as 'blob:limit=1024'.
+
+The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects
+which are not of the requested type.
++
The form '--filter=sparse:oid=<blob-ish>' uses a sparse-checkout
specification contained in the blob (or blob-expression) '<blob-ish>'
to omit blobs that would not be not required for a sparse checkout on
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index d2d1c81caf..96a605c8ad 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -29,6 +29,8 @@ const char *list_object_filter_config_name(enum list_objects_filter_choice c)
return "tree";
case LOFC_SPARSE_OID:
return "sparse:oid";
+ case LOFC_OBJECT_TYPE:
+ return "object:type";
case LOFC_COMBINE:
return "combine";
case LOFC__COUNT:
@@ -97,6 +99,19 @@ static int gently_parse_list_objects_filter(
}
return 1;
+ } else if (skip_prefix(arg, "object:type=", &v0)) {
+ int type = type_from_string_gently(v0, strlen(v0), 1);
+ if (type < 0) {
+ strbuf_addf(errbuf, _("'%s' for 'object:type=<type>' is"
+ "not a valid object type"), v0);
+ return 1;
+ }
+
+ filter_options->object_type = type;
+ filter_options->choice = LOFC_OBJECT_TYPE;
+
+ return 0;
+
} else if (skip_prefix(arg, "combine:", &v0)) {
return parse_combine_filter(filter_options, v0, errbuf);
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index 01767c3c96..da5b6737e2 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -1,6 +1,7 @@
#ifndef LIST_OBJECTS_FILTER_OPTIONS_H
#define LIST_OBJECTS_FILTER_OPTIONS_H
+#include "cache.h"
#include "parse-options.h"
#include "string-list.h"
@@ -13,6 +14,7 @@ enum list_objects_filter_choice {
LOFC_BLOB_LIMIT,
LOFC_TREE_DEPTH,
LOFC_SPARSE_OID,
+ LOFC_OBJECT_TYPE,
LOFC_COMBINE,
LOFC__COUNT /* must be last */
};
@@ -54,6 +56,7 @@ struct list_objects_filter_options {
char *sparse_oid_name;
unsigned long blob_limit_value;
unsigned long tree_exclude_depth;
+ enum object_type object_type;
/* LOFC_COMBINE values */
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 0ebfa52966..1c1ee3d1bb 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -545,6 +545,81 @@ static void filter_sparse_oid__init(
filter->free_fn = filter_sparse_free;
}
+/*
+ * A filter for list-objects to omit large blobs.
+ * And to OPTIONALLY collect a list of the omitted OIDs.
+ */
+struct filter_object_type_data {
+ enum object_type object_type;
+};
+
+static enum list_objects_filter_result filter_object_type(
+ struct repository *r,
+ enum list_objects_filter_situation filter_situation,
+ struct object *obj,
+ const char *pathname,
+ const char *filename,
+ struct oidset *omits,
+ void *filter_data_)
+{
+ struct filter_object_type_data *filter_data = filter_data_;
+
+ switch (filter_situation) {
+ default:
+ BUG("unknown filter_situation: %d", filter_situation);
+
+ case LOFS_TAG:
+ assert(obj->type == OBJ_TAG);
+ if (filter_data->object_type == OBJ_TAG)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+ return LOFR_MARK_SEEN;
+
+ case LOFS_COMMIT:
+ assert(obj->type == OBJ_COMMIT);
+ if (filter_data->object_type == OBJ_COMMIT)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+ return LOFR_MARK_SEEN;
+
+ case LOFS_BEGIN_TREE:
+ assert(obj->type == OBJ_TREE);
+
+ /*
+ * If we only want to show commits or tags, then there is no
+ * need to walk down trees.
+ */
+ if (filter_data->object_type == OBJ_COMMIT ||
+ filter_data->object_type == OBJ_TAG)
+ return LOFR_SKIP_TREE;
+
+ if (filter_data->object_type == OBJ_TREE)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+
+ return LOFR_MARK_SEEN;
+
+ case LOFS_BLOB:
+ assert(obj->type == OBJ_BLOB);
+
+ if (filter_data->object_type == OBJ_BLOB)
+ return LOFR_MARK_SEEN | LOFR_DO_SHOW;
+ return LOFR_MARK_SEEN;
+
+ case LOFS_END_TREE:
+ return LOFR_ZERO;
+ }
+}
+
+static void filter_object_type__init(
+ struct list_objects_filter_options *filter_options,
+ struct filter *filter)
+{
+ struct filter_object_type_data *d = xcalloc(1, sizeof(*d));
+ d->object_type = filter_options->object_type;
+
+ filter->filter_data = d;
+ filter->filter_object_fn = filter_object_type;
+ filter->free_fn = free;
+}
+
/* A filter which only shows objects shown by all sub-filters. */
struct combine_filter_data {
struct subfilter *sub;
@@ -691,6 +766,7 @@ static filter_init_fn s_filters[] = {
filter_blobs_limit__init,
filter_trees_depth__init,
filter_sparse_oid__init,
+ filter_object_type__init,
filter_combine__init,
};
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index 31457d13b9..c810a62807 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -159,6 +159,50 @@ test_expect_success 'verify blob:limit=1m' '
test_must_be_empty observed
'
+# Test object:type=<type> filter.
+
+test_expect_success 'setup object-type' '
+ test_create_repo object-type &&
+ test_commit --no-tag -C object-type message blob &&
+ git -C object-type tag tag -m tag-message
+'
+
+test_expect_success 'verify object:type= fails with invalid type' '
+ test_must_fail git -C object-type rev-list --objects --filter=object:type= HEAD &&
+ test_must_fail git -C object-type rev-list --objects --filter=object:type=invalid HEAD
+'
+
+test_expect_success 'verify object:type=blob prints blob and commit' '
+ git -C object-type rev-parse HEAD >expected &&
+ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob) >>expected &&
+ git -C object-type rev-list --objects --filter=object:type=blob HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tree prints tree and commit' '
+ (
+ git -C object-type rev-parse HEAD &&
+ printf "%s \n" $(git -C object-type rev-parse HEAD^{tree})
+ ) >expected &&
+ git -C object-type rev-list --objects --filter=object:type=tree HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=commit prints commit' '
+ git -C object-type rev-parse HEAD >expected &&
+ git -C object-type rev-list --objects --filter=object:type=commit HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tag prints tag' '
+ (
+ git -C object-type rev-parse HEAD &&
+ printf "%s tag\n" $(git -C object-type rev-parse tag)
+ ) >expected &&
+ git -C object-type rev-list --objects --filter=object:type=tag tag >actual &&
+ test_cmp expected actual
+'
+
# Test sparse:path=<path> filter.
# !!!!
# NOTE: sparse:path filter support has been dropped for security reasons,
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v5 6/8] pack-bitmap: implement object type filter
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
` (4 preceding siblings ...)
2021-04-19 11:46 ` [PATCH v5 5/8] list-objects: implement object type filter Patrick Steinhardt
@ 2021-04-19 11:46 ` Patrick Steinhardt
2021-04-19 11:47 ` [PATCH v5 7/8] pack-bitmap: implement combined filter Patrick Steinhardt
` (2 subsequent siblings)
8 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-19 11:46 UTC (permalink / raw)
To: git
Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
[-- Attachment #1: Type: text/plain, Size: 3827 bytes --]
The preceding commit has added a new object filter for git-rev-list(1)
which allows to filter objects by type. Implement the equivalent filter
for packfile bitmaps so that we can answer these queries fast.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 29 ++++++++++++++++++++++++++---
t/t6113-rev-list-bitmap-filters.sh | 25 ++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 3ed15431cd..e419f4206b 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -780,9 +780,6 @@ static void filter_bitmap_exclude_type(struct bitmap_index *bitmap_git,
eword_t mask;
uint32_t i;
- if (type != OBJ_BLOB && type != OBJ_TREE)
- BUG("filter_bitmap_exclude_type: unsupported type '%d'", type);
-
/*
* The non-bitmap version of this filter never removes
* objects which the other side specifically asked for,
@@ -912,6 +909,24 @@ static void filter_bitmap_tree_depth(struct bitmap_index *bitmap_git,
OBJ_BLOB);
}
+static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
+ struct object_list *tip_objects,
+ struct bitmap *to_filter,
+ enum object_type object_type)
+{
+ if (object_type < OBJ_COMMIT || object_type > OBJ_TAG)
+ BUG("filter_bitmap_object_type given invalid object");
+
+ if (object_type != OBJ_TAG)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_TAG);
+ if (object_type != OBJ_COMMIT)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_COMMIT);
+ if (object_type != OBJ_TREE)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_TREE);
+ if (object_type != OBJ_BLOB)
+ filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_BLOB);
+}
+
static int filter_bitmap(struct bitmap_index *bitmap_git,
struct object_list *tip_objects,
struct bitmap *to_filter,
@@ -944,6 +959,14 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return 0;
}
+ if (filter->choice == LOFC_OBJECT_TYPE) {
+ if (bitmap_git)
+ filter_bitmap_object_type(bitmap_git, tip_objects,
+ to_filter,
+ filter->object_type);
+ return 0;
+ }
+
/* filter choice not handled */
return -1;
}
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index 3f889949ca..fb66735ac8 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -10,7 +10,8 @@ test_expect_success 'set up bitmapped repo' '
test_commit much-larger-blob-one &&
git repack -adb &&
test_commit two &&
- test_commit much-larger-blob-two
+ test_commit much-larger-blob-two &&
+ git tag tag
'
test_expect_success 'filters fallback to non-bitmap traversal' '
@@ -75,4 +76,26 @@ test_expect_success 'tree:1 filter' '
test_cmp expect actual
'
+test_expect_success 'object:type filter' '
+ git rev-list --objects --filter=object:type=tag tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=tag tag >actual &&
+ test_cmp expect actual &&
+
+ git rev-list --objects --filter=object:type=commit tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=commit tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter=object:type=tree tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=tree tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v5 7/8] pack-bitmap: implement combined filter
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
` (5 preceding siblings ...)
2021-04-19 11:46 ` [PATCH v5 6/8] pack-bitmap: " Patrick Steinhardt
@ 2021-04-19 11:47 ` Patrick Steinhardt
2021-04-19 11:47 ` [PATCH v5 8/8] rev-list: allow filtering of provided items Patrick Steinhardt
2021-04-19 23:16 ` [PATCH v5 0/8] rev-list: implement object type filter Junio C Hamano
8 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-19 11:47 UTC (permalink / raw)
To: git
Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]
When the user has multiple objects filters specified, then this is
internally represented by having a "combined" filter. These combined
filters aren't yet supported by bitmap indices and can thus not be
accelerated.
Fix this by implementing support for these combined filters. The
implementation is quite trivial: when there's a combined filter, we
simply recurse into `filter_bitmap()` for all of the sub-filters.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 10 ++++++++++
t/t6113-rev-list-bitmap-filters.sh | 7 +++++++
2 files changed, 17 insertions(+)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index e419f4206b..53f35d41c6 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -967,6 +967,16 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return 0;
}
+ if (filter->choice == LOFC_COMBINE) {
+ int i;
+ for (i = 0; i < filter->sub_nr; i++) {
+ if (filter_bitmap(bitmap_git, tip_objects, to_filter,
+ &filter->sub[i]) < 0)
+ return -1;
+ }
+ return 0;
+ }
+
/* filter choice not handled */
return -1;
}
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index fb66735ac8..cb9db7df6f 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -98,4 +98,11 @@ test_expect_success 'object:type filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'combine filter' '
+ git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* [PATCH v5 8/8] rev-list: allow filtering of provided items
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
` (6 preceding siblings ...)
2021-04-19 11:47 ` [PATCH v5 7/8] pack-bitmap: implement combined filter Patrick Steinhardt
@ 2021-04-19 11:47 ` Patrick Steinhardt
2021-04-19 23:16 ` [PATCH v5 0/8] rev-list: implement object type filter Junio C Hamano
8 siblings, 0 replies; 97+ messages in thread
From: Patrick Steinhardt @ 2021-04-19 11:47 UTC (permalink / raw)
To: git
Cc: Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
[-- Attachment #1: Type: text/plain, Size: 12176 bytes --]
When providing an object filter, it is currently impossible to also
filter provided items. E.g. when executing `git rev-list HEAD` , the
commit this reference points to will be treated as user-provided and is
thus excluded from the filtering mechanism. This makes it harder than
necessary to properly use the new `--filter=object:type` filter given
that even if the user wants to only see blobs, he'll still see commits
of provided references.
Improve this by introducing a new `--filter-provided-objects` option
to the git-rev-parse(1) command. If given, then all user-provided
references will be subject to filtering.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/rev-list-options.txt | 5 ++++
builtin/pack-objects.c | 2 +-
builtin/rev-list.c | 36 +++++++++++++++++++++--------
pack-bitmap.c | 6 +++--
pack-bitmap.h | 3 ++-
reachable.c | 2 +-
t/t6112-rev-list-filters-objects.sh | 28 ++++++++++++++++++++++
t/t6113-rev-list-bitmap-filters.sh | 36 +++++++++++++++++++++++++++++
8 files changed, 104 insertions(+), 14 deletions(-)
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 3afa8fffbd..5bf2a85f69 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -933,6 +933,11 @@ equivalent.
--no-filter::
Turn off any previous `--filter=` argument.
+--filter-provided-objects::
+ Filter the list of explicitly provided objects, which would otherwise
+ always be printed even if they did not match any of the filters. Only
+ useful with `--filter=`.
+
--filter-print-omitted::
Only useful with `--filter=`; prints a list of the objects omitted
by the filter. Object IDs are prefixed with a ``~'' character.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index a1e33d7507..c89e996f0d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3516,7 +3516,7 @@ static int pack_options_allow_reuse(void)
static int get_object_list_from_bitmap(struct rev_info *revs)
{
- if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options)))
+ if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options, 0)))
return -1;
if (pack_options_allow_reuse() &&
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index b4d8ea0a35..7677b1af5a 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -398,7 +398,8 @@ static inline int parse_missing_action_value(const char *value)
}
static int try_bitmap_count(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects)
{
uint32_t commit_count = 0,
tag_count = 0,
@@ -433,7 +434,7 @@ static int try_bitmap_count(struct rev_info *revs,
*/
max_count = revs->max_count;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
if (!bitmap_git)
return -1;
@@ -450,7 +451,8 @@ static int try_bitmap_count(struct rev_info *revs,
}
static int try_bitmap_traversal(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects)
{
struct bitmap_index *bitmap_git;
@@ -461,7 +463,7 @@ static int try_bitmap_traversal(struct rev_info *revs,
if (revs->max_count >= 0)
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
if (!bitmap_git)
return -1;
@@ -471,14 +473,15 @@ static int try_bitmap_traversal(struct rev_info *revs,
}
static int try_bitmap_disk_usage(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects)
{
struct bitmap_index *bitmap_git;
if (!show_disk_usage)
return -1;
- bitmap_git = prepare_bitmap_walk(revs, filter);
+ bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
if (!bitmap_git)
return -1;
@@ -499,6 +502,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
int bisect_show_vars = 0;
int bisect_find_all = 0;
int use_bitmap_index = 0;
+ int filter_provided_objects = 0;
const char *show_progress = NULL;
if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -599,6 +603,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
list_objects_filter_set_no_filter(&filter_options);
continue;
}
+ if (!strcmp(arg, "--filter-provided-objects")) {
+ filter_provided_objects = 1;
+ continue;
+ }
if (!strcmp(arg, "--filter-print-omitted")) {
arg_print_omitted = 1;
continue;
@@ -665,11 +673,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
progress = start_delayed_progress(show_progress, 0);
if (use_bitmap_index) {
- if (!try_bitmap_count(&revs, &filter_options))
+ if (!try_bitmap_count(&revs, &filter_options, filter_provided_objects))
return 0;
- if (!try_bitmap_disk_usage(&revs, &filter_options))
+ if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_objects))
return 0;
- if (!try_bitmap_traversal(&revs, &filter_options))
+ if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_objects))
return 0;
}
@@ -694,6 +702,16 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
return show_bisect_vars(&info, reaches, all);
}
+ if (filter_provided_objects) {
+ struct commit_list *c;
+ for (i = 0; i < revs.pending.nr; i++) {
+ struct object_array_entry *pending = revs.pending.objects + i;
+ pending->item->flags |= NOT_USER_GIVEN;
+ }
+ for (c = revs.commits; c; c = c->next)
+ c->item->object.flags |= NOT_USER_GIVEN;
+ }
+
if (arg_print_omitted)
oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);
if (arg_missing_action == MA_PRINT)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 53f35d41c6..6efea70ae7 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -987,7 +987,8 @@ static int can_filter_bitmap(struct list_objects_filter_options *filter)
}
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter)
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects)
{
unsigned int i;
@@ -1082,7 +1083,8 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
if (haves_bitmap)
bitmap_and_not(wants_bitmap, haves_bitmap);
- filter_bitmap(bitmap_git, wants, wants_bitmap, filter);
+ filter_bitmap(bitmap_git, (filter && filter_provided_objects) ? NULL : wants,
+ wants_bitmap, filter);
bitmap_git->result = wants_bitmap;
bitmap_git->haves = haves_bitmap;
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 78f2b3ff79..99d733eb26 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -52,7 +52,8 @@ void traverse_bitmap_commit_list(struct bitmap_index *,
void test_bitmap_walk(struct rev_info *revs);
int test_bitmap_commits(struct repository *r);
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
- struct list_objects_filter_options *filter);
+ struct list_objects_filter_options *filter,
+ int filter_provided_objects);
int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
struct packed_git **packfile,
uint32_t *entries,
diff --git a/reachable.c b/reachable.c
index 77a60c70a5..fc833cae43 100644
--- a/reachable.c
+++ b/reachable.c
@@ -223,7 +223,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
cp.progress = progress;
cp.count = 0;
- bitmap_git = prepare_bitmap_walk(revs, NULL);
+ bitmap_git = prepare_bitmap_walk(revs, NULL, 0);
if (bitmap_git) {
traverse_bitmap_commit_list(bitmap_git, revs, mark_object_seen);
free_bitmap_index(bitmap_git);
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index c810a62807..4ade105db3 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -203,6 +203,34 @@ test_expect_success 'verify object:type=tag prints tag' '
test_cmp expected actual
'
+test_expect_success 'verify object:type=blob prints only blob with --filter-provided-objects' '
+ printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=blob --filter-provided-objects HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tree prints only tree with --filter-provided-objects' '
+ printf "%s \n" $(git -C object-type rev-parse HEAD^{tree}) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=tree HEAD --filter-provided-objects >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=commit prints only commit with --filter-provided-objects' '
+ git -C object-type rev-parse HEAD >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=commit --filter-provided-objects HEAD >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'verify object:type=tag prints only tag with --filter-provided-objects' '
+ printf "%s tag\n" $(git -C object-type rev-parse tag) >expected &&
+ git -C object-type rev-list --objects \
+ --filter=object:type=tag --filter-provided-objects tag >actual &&
+ test_cmp expected actual
+'
+
# Test sparse:path=<path> filter.
# !!!!
# NOTE: sparse:path filter support has been dropped for security reasons,
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index cb9db7df6f..4d8e09167e 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -98,6 +98,28 @@ test_expect_success 'object:type filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'object:type filter with --filter-provided-objects' '
+ git rev-list --objects --filter-provided-objects --filter=object:type=tag tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=object:type=tag tag >actual &&
+ test_cmp expect actual &&
+
+ git rev-list --objects --filter-provided-objects --filter=object:type=commit tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=object:type=commit tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter-provided-objects --filter=object:type=tree tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=object:type=tree tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git rev-list --objects --filter-provided-objects --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual
+'
+
test_expect_success 'combine filter' '
git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
git rev-list --use-bitmap-index \
@@ -105,4 +127,18 @@ test_expect_success 'combine filter' '
test_bitmap_traversal expect actual
'
+test_expect_success 'combine filter with --filter-provided-objects' '
+ git rev-list --objects --filter-provided-objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
+ git rev-list --use-bitmap-index \
+ --objects --filter-provided-objects --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
+ test_bitmap_traversal expect actual &&
+
+ git cat-file --batch-check="%(objecttype) %(objectsize)" <actual >objects &&
+ while read objecttype objectsize
+ do
+ test "$objecttype" = blob || return 1
+ test "$objectsize" -le 1000 || return 1
+ done <objects
+'
+
test_done
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 97+ messages in thread
* Re: [PATCH v5 0/8] rev-list: implement object type filter
2021-04-19 11:46 ` [PATCH v5 " Patrick Steinhardt
` (7 preceding siblings ...)
2021-04-19 11:47 ` [PATCH v5 8/8] rev-list: allow filtering of provided items Patrick Steinhardt
@ 2021-04-19 23:16 ` Junio C Hamano
2021-04-23 9:13 ` Jeff King
8 siblings, 1 reply; 97+ messages in thread
From: Junio C Hamano @ 2021-04-19 23:16 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Jeff King, Christian Couder, Taylor Blau, Philip Oakley,
Ramsay Jones, Ævar Arnfjörð Bjarmason
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this is the fifth version of my patch series which implements a new
> `object:type` filter for git-rev-list(1) and git-upload-pack(1) and
> extends support for bitmap indices to work with combined filters.
>
> Changes compared to v4:
>
> - I'm now explicitly passing `strlen(v0)` to
> `type_from_string_gently()` to be prepared for the future
> semantics change here.
>
> - The error message printed in case the user passes in invalid
> object type to `--filter=object:type=` now prints the wrong
> value passed by the user.
>
> - Fixed missing header in list-objects-filter-options.h.
>
> - Tests now use test_create_repo and test_commit.
>
> - Removed a needless subshell in the tests.
>
> I hope that this catches all feedback and that I didn't misunderstand or
> miss something. If I did, please give me a shout!
Thanks. Will queue and wait.
I am expecting to go offline for about a week starting 21st or so;
hopefully the topyc is ready to merge it to 'next' by the time I
come back online.
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v5 0/8] rev-list: implement object type filter
2021-04-19 23:16 ` [PATCH v5 0/8] rev-list: implement object type filter Junio C Hamano
@ 2021-04-23 9:13 ` Jeff King
2021-04-28 2:18 ` Junio C Hamano
0 siblings, 1 reply; 97+ messages in thread
From: Jeff King @ 2021-04-23 9:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: Patrick Steinhardt, git, Christian Couder, Taylor Blau,
Philip Oakley, Ramsay Jones,
Ævar Arnfjörð Bjarmason
On Mon, Apr 19, 2021 at 04:16:58PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Hi,
> >
> > this is the fifth version of my patch series which implements a new
> > `object:type` filter for git-rev-list(1) and git-upload-pack(1) and
> > extends support for bitmap indices to work with combined filters.
> >
> > Changes compared to v4:
> >
> > - I'm now explicitly passing `strlen(v0)` to
> > `type_from_string_gently()` to be prepared for the future
> > semantics change here.
> >
> > - The error message printed in case the user passes in invalid
> > object type to `--filter=object:type=` now prints the wrong
> > value passed by the user.
> >
> > - Fixed missing header in list-objects-filter-options.h.
> >
> > - Tests now use test_create_repo and test_commit.
> >
> > - Removed a needless subshell in the tests.
> >
> > I hope that this catches all feedback and that I didn't misunderstand or
> > miss something. If I did, please give me a shout!
>
> Thanks. Will queue and wait.
>
> I am expecting to go offline for about a week starting 21st or so;
> hopefully the topyc is ready to merge it to 'next' by the time I
> come back online.
This iteration looks good to me.
-Peff
^ permalink raw reply [flat|nested] 97+ messages in thread
* Re: [PATCH v5 0/8] rev-list: implement object type filter
2021-04-23 9:13 ` Jeff King
@ 2021-04-28 2:18 ` Junio C Hamano
0 siblings, 0 replies; 97+ messages in thread
From: Junio C Hamano @ 2021-04-28 2:18 UTC (permalink / raw)
To: Jeff King
Cc: Patrick Steinhardt, git, Christian Couder, Taylor Blau,
Philip Oakley, Ramsay Jones,
Ævar Arnfjörð Bjarmason
Jeff King <peff@peff.net> writes:
>> Thanks. Will queue and wait.
>>
>> I am expecting to go offline for about a week starting 21st or so;
>> hopefully the topyc is ready to merge it to 'next' by the time I
>> come back online.
>
> This iteration looks good to me.
Thanks, both.
^ permalink raw reply [flat|nested] 97+ messages in thread