filter: support for excluding all trees and blobs
PATCH v9 5/8 revision: mark non-user-given objects instead
In reply to Matthew DeVoreCurrently, list-objects.c incorrectly treats all root trees of commits as USER_GIVEN. Also, it would be easier to mark objects that are non-user-given instead of user-given, since the places in the code where we access an object through a reference are more obvious than the places where we access an object that was given by the user.
Resolve these two problems by introducing a flag NOT_USER_GIVEN that marks blobs and trees that are non-user-given, replacing USER_GIVEN. (Only blobs and trees are marked because this mark is only used when filtering objects, and filtering of other types of objects is not supported yet.)
This fixes a bug in that git rev-list behaved differently from git pack-objects. pack-objects would *not* filter objects given explicitly on the command line and rev-list would filter. This was because the two commands used a different function to add objects to the rev_info struct. This seems to have been an oversight, and pack-objects has the correct behavior, so I added a test to make sure that rev-list now behaves properly.
--- list-objects.c | 31 +++++++++++++++++------------ revision.c | 1 - revision.h | 11 ++++++++-- t/t6112-rev-list-filters-objects.sh | 12 +++++++++++ 4 files changed, 39 insertions(+), 16 deletions(-)
list-objects.c +18 -13
@@ -53,7 +53,7 @@ static void process_blob(struct traversal_context *ctx, pathlen = path->len;strbuf_addstr (path, name); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) r = ctx->filter_fn (LOFS_BLOB , obj, path->buf, &path->buf[pathlen], ctx->filter_data);
@@ -120,17 +120,19 @@ static void process_tree_contents(struct traversal_context *ctx,continue ; } -if (S_ISDIR (entry.mode)) -process_tree (ctx, -lookup_tree (the_repository, entry.oid), - base, entry.path); +if (S_ISDIR (entry.mode)) { +struct tree *t =lookup_tree (the_repository, entry.oid); + t->object.flags |=NOT_USER_GIVEN ; +process_tree (ctx, t, base, entry.path); + }else if (S_ISGITLINK (entry.mode))process_gitlink (ctx, entry.oid->hash, base, entry.path); -else -process_blob (ctx, -lookup_blob (the_repository, entry.oid), - base, entry.path); +else { +struct blob *b =lookup_blob (the_repository, entry.oid); + b->object.flags |=NOT_USER_GIVEN ; +process_blob (ctx, b, base, entry.path); + } } }
@@ -171,7 +173,7 @@ static void process_tree(struct traversal_context *ctx, }strbuf_addstr (base, name); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) r = ctx->filter_fn (LOFS_BEGIN_TREE , obj, base->buf, &base->buf[baselen], ctx->filter_data);
@@ -185,7 +187,7 @@ static void process_tree(struct traversal_context *ctx,if (!failed_parse)process_tree_contents (ctx, tree, base); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn (LOFS_END_TREE , obj, base->buf, &base->buf[baselen], ctx->filter_data);
@@ -301,8 +303,11 @@ static void do_traverse(struct traversal_context *ctx)* an uninteresting boundary commit may not have its tree * parsed yet, but we are not going to show them anyway */ -if (get_commit_tree (commit)) -add_pending_tree (ctx->revs,get_commit_tree (commit)); +if (get_commit_tree (commit)) { +struct tree *tree =get_commit_tree (commit); + tree->object.flags |=NOT_USER_GIVEN ; +add_pending_tree (ctx->revs, tree); + } ctx->show_commit (commit, ctx->show_data);if (ctx->revs->tree_blobs_in_commit_order)
revision.c +0 -1
@@ -175,7 +175,6 @@ static void add_pending_object_with_path(struct rev_info *revs,strbuf_release (&buf);return ;/* do not add the commit itself */ } - obj->flags |=USER_GIVEN ;add_object_array_with_path (obj, name, &revs->pending, mode, path); }
revision.h +9 -2
@@ -21,9 +21,16 @@#define SYMMETRIC_LEFT (1u<<8)#define PATCHSAME (1u<<9)#define BOTTOM (1u<<10) -#define USER_GIVEN (1u<<25)/* given directly by the user */ +/* +* 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) -#define ALL_REV_FLAGS (((1u<<11)-1) | USER_GIVEN | TRACK_LINEAR) +#define ALL_REV_FLAGS (((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR)#define DECORATE_SHORT_REFS 1#define DECORATE_FULL_REFS 2
t/t6112-rev-list-filters-objects.sh +12 -0
@@ -30,6 +30,18 @@ test_expect_success 'verify blob:none omits all 5 blobs' ' test_cmp observed expected ' +test_expect_success 'specify blob explicitly prevents filtering' ' + file_3=$(git -C r1 ls-files -s file.3 | + awk -f print_2.awk) && + + file_4=$(git -C r1 ls-files -s file.4 | + awk -f print_2.awk) && + + git -C r1 rev-list --objects --filter=blob:none HEAD $file_3 >observed && + grep -q "$file_3" observed && + test_must_fail grep -q "$file_4" observed +' + test_expect_success 'verify emitted+omitted == all' ' git -C r1 rev-list HEAD --objects \ | awk -f print_1.awk \
Signature · 1 line
2.19.0.444.g18242da7ef-goog
PATCH v9 6/8 list-objects-filter: use BUG rather than die
In reply to Matthew DeVoreIn some cases in this file, BUG makes more sense than die. In such cases, a we get there from a coding error rather than a user error.
'return' has been removed following some instances of BUG since BUG does not return.
--- list-objects-filter.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
list-objects-filter.c +4 -7
@@ -44,8 +44,7 @@ static enum list_objects_filter_result filter_blobs_none(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -102,8 +101,7 @@ static enum list_objects_filter_result filter_blobs_limit(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -208,8 +206,7 @@ static enum list_objects_filter_result filter_sparse(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -389,7 +386,7 @@ void *list_objects_filter__init(assert ((sizeof (s_filters) /sizeof (s_filters[0 ])) ==LOFC__COUNT );if (filter_options->choice >=LOFC__COUNT ) -die ("invalid list-objects filter choice: %d" , +BUG ("invalid list-objects filter choice: %d" , filter_options->choice); init_fn = s_filters[filter_options->choice];
Signature · 1 line
2.19.0.444.g18242da7ef-goog
PATCH v9 7/8 list-objects-filter-options: do not over-strbuf_init
In reply to Matthew DeVoreThe function gently_parse_list_objects_filter is either called with errbuf=STRBUF_INIT or errbuf=NULL, but that function calls strbuf_init when errbuf is not NULL. strbuf_init is only necessary if errbuf contains garbage, and risks a memory leak if errbuf already has a non-STRBUF_INIT state. It should be the caller's responsibility to make sure errbuf is not garbage, since garbage content is easily avoidable with STRBUF_INIT.
--- list-objects-filter-options.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
list-objects-filter-options.c +2 -4
@@ -30,7 +30,6 @@ static int gently_parse_list_objects_filter(if (filter_options->choice) {if (errbuf) { -strbuf_init (errbuf,0 );strbuf_addstr ( errbuf,_ ("multiple filter-specs cannot be combined" ));
@@ -71,10 +70,9 @@ static int gently_parse_list_objects_filter(return 0 ; } -if (errbuf) { -strbuf_init (errbuf,0 ); +if (errbuf)strbuf_addf (errbuf,"invalid filter-spec '%s'" , arg); - } +memset (filter_options,0 ,sizeof (*filter_options));return 1 ; }
Signature · 1 line
2.19.0.444.g18242da7ef-goog
PATCH v9 8/8 list-objects-filter: implement filter tree:0
In reply to Matthew DeVoreTeach list-objects the "tree:0" filter which allows for filtering out all tree and blob objects (unless other objects are explicitly specified by the user). The purpose of this patch is to allow smaller partial clones.
The name of this filter - tree:0 - does not explicitly specify that it also filters out all blobs, but this should not cause much confusion because blobs are not at all useful without the trees that refer to them.
I also considered only:commits as a name, but this is inaccurate because it suggests that annotated tags are omitted, but actually they are included.
The name "tree:0" allows later filtering based on depth, i.e. "tree:1" would filter out all but the root tree and blobs. In order to avoid confusion between 0 and capital O, the documentation was worded in a somewhat round-about way that also hints at this future improvement to the feature.
--- Documentation/rev-list-options.txt | 5 +++ list-objects-filter-options.c | 13 +++++++ list-objects-filter-options.h | 1 + list-objects-filter.c | 49 ++++++++++++++++++++++++++ t/t5317-pack-objects-filter-objects.sh | 28 +++++++++++++++ t/t5616-partial-clone.sh | 41 +++++++++++++++++++++ t/t6112-rev-list-filters-objects.sh | 13 +++++++ 7 files changed, 150 insertions(+)
Documentation/rev-list-options.txt +5 -0
@@ -731,6 +731,11 @@ the requested refs. + The form '--filter=sparse:path=<path>' similarly uses a sparse-checkout specification contained in <path>. ++ +The form '--filter=tree:<depth>' omits all blobs and trees whose depth +from the root tree is >= <depth> (minimum depth if an object is located +at multiple depths in the commits traversed). Currently, only <depth>=0 +is supported, which omits all blobs and trees. --no-filter:: Turn off any previous `--filter=` argument.
list-objects-filter-options.c +13 -0
@@ -49,6 +49,19 @@ static int gently_parse_list_objects_filter(return 0 ; } + }else if (skip_prefix (arg,"tree:" , &v0)) { +unsigned long depth; +if (!git_parse_ulong (v0, &depth) || depth !=0 ) { +if (errbuf) { +strbuf_addstr ( + errbuf, +_ ("only 'tree:0' is supported" )); + } +return 1 ; + } + filter_options->choice =LOFC_TREE_NONE ; +return 0 ; + }else if (skip_prefix (arg,"sparse:oid=" , &v0)) {struct object_context oc;struct object_id sparse_oid;
list-objects-filter-options.h +1 -0
@@ -10,6 +10,7 @@ enum list_objects_filter_choice {LOFC_DISABLED =0 ,LOFC_BLOB_NONE ,LOFC_BLOB_LIMIT , +LOFC_TREE_NONE ,LOFC_SPARSE_OID ,LOFC_SPARSE_PATH ,LOFC__COUNT /* must be last */
list-objects-filter.c +49 -0
61 lines not on this page · raw message
t/t5317-pack-objects-filter-objects.sh +28 -0
@@ -72,6 +72,34 @@ test_expect_success 'get an error for missing tree object' ' grep -q "bad tree object" bad_tree ' +test_expect_success 'setup for tests of tree:0' ' + mkdir r1/subtree && + echo "This is a file in a subtree" >r1/subtree/file && + git -C r1 add subtree/file && + git -C r1 commit -m subtree +' + +test_expect_success 'verify tree:0 packfile has no blobs or trees' ' + git -C r1 pack-objects --rev --stdout --filter=tree:0 >commitsonly.pack <<-EOF && + HEAD + EOF + git -C r1 index-pack ../commitsonly.pack && + git -C r1 verify-pack -v ../commitsonly.pack >objs && + ! grep -E "tree|blob" objs +' + +test_expect_success 'grab tree directly when using tree:0' ' + # We should get the tree specified directly but not its blobs or subtrees. + git -C r1 pack-objects --rev --stdout --filter=tree:0 >commitsonly.pack <<-EOF && + HEAD: + EOF + git -C r1 index-pack ../commitsonly.pack && + git -C r1 verify-pack -v ../commitsonly.pack >objs && + awk "/tree|blob/{print \$1}" objs >trees_and_blobs && + git -C r1 rev-parse HEAD: >expected && + test_cmp expected trees_and_blobs +' + # Test blob:limit=<n>[kmg] filter. # We boundary test around the size parameter. The filter is strictly less than # the value, so size 500 and 1000 should have the same results, but 1001 should
t/t5616-partial-clone.sh +41 -0
47 lines not on this page · raw message
t/t6112-rev-list-filters-objects.sh +13 -0
@@ -224,6 +224,19 @@ test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for tre test_must_be_empty rev_list_err ' +# Test tree:0 filter. + +test_expect_success 'verify tree:0 includes trees in "filtered" output' ' + git -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=tree:0 | + awk -f print_1.awk | + sed s/~// | + xargs -n1 git -C r3 cat-file -t | + sort -u >filtered_types && + + printf "blob\ntree\n" >expected && + test_cmp expected filtered_types +' + # Delete some loose objects and use rev-list, but WITHOUT any filtering. # This models previously omitted objects that we did not receive.
Signature · 1 line
2.19.0.444.g18242da7ef-goog
v10 cover filter: support for excluding all trees and blobs
In reply to Matthew DeVoreThis is a minor change to the previous rollup. It moves positional arguments to the end of git-rev-list invocations. Here is an interdiff from v9:
t/t5616-partial-clone.sh +3 -2
@@ -168,7 +168,8 @@ test_expect_success 'use fsck before and after manually fetching a missing subtr git -C dst fsck && # Make sure we only have commits, and all trees and blobs are missing. - git -C dst rev-list master --missing=allow-any --objects >fetched_objects && + git -C dst rev-list --missing=allow-any --objects master \ + >fetched_objects && awk -f print_1.awk fetched_objects | xargs -n1 git -C dst cat-file -t >fetched_types &&
@@ -184,7 +185,7 @@ test_expect_success 'use fsck before and after manually fetching a missing subtr git -C dst fsck && # Auto-fetch all remaining trees and blobs with --missing=error - git -C dst rev-list master --missing=error --objects >fetched_objects && + git -C dst rev-list --missing=error --objects master >fetched_objects && test_line_count = 70 fetched_objects && awk -f print_1.awk fetched_objects |
t/t6112-rev-list-filters-objects.sh +2 -1
@@ -227,7 +227,8 @@ test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for tre # Test tree:0 filter. test_expect_success 'verify tree:0 includes trees in "filtered" output' ' - git -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=tree:0 | + git -C r3 rev-list --quiet --objects --filter-print-omitted \ + --filter=tree:0 HEAD | awk -f print_1.awk | sed s/~// | xargs -n1 git -C r3 cat-file -t |
Thank you,
Matthew DeVore (8):
list-objects: store common func args in struct
list-objects: refactor to process_tree_contents
list-objects: always parse trees gently
rev-list: handle missing tree objects properly
revision: mark non-user-given objects instead
list-objects-filter: use BUG rather than die list-objects-filter-options: do not over-strbuf_init
list-objects-filter: implement filter tree:0
Documentation/rev-list-options.txt | 5 + builtin/rev-list.c | 11 +- list-objects-filter-options.c | 19 +- list-objects-filter-options.h | 1 + list-objects-filter.c | 60 ++++++- list-objects.c | 232 +++++++++++++------------ revision.c | 1 - revision.h | 26 ++- t/t0410-partial-clone.sh | 45 +++++ t/t5317-pack-objects-filter-objects.sh | 41 +++++ t/t5616-partial-clone.sh | 42 +++++ t/t6112-rev-list-filters-objects.sh | 43 +++++ 12 files changed, 398 insertions(+), 128 deletions(-)
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v10 1/8 list-objects: store common func args in struct
This will make utility functions easier to create, as done by the next patch.
--- list-objects.c | 158 +++++++++++++++++++++++-------------------------- 1 file changed, 74 insertions(+), 84 deletions(-)
list-objects.c +74 -84
@@ -12,20 +12,25 @@#include "packfile.h" #include "object-store.h" -static void process_blob (struct rev_info *revs, +struct traversal_context { +struct rev_info *revs; +show_object_fn show_object; +show_commit_fn show_commit; +void *show_data; +filter_object_fn filter_fn; +void *filter_data; +}; + +static void process_blob (struct traversal_context *ctx,struct blob *blob, -show_object_fn show,struct strbuf *path, -const char *name, -void *cb_data, -filter_object_fn filter_fn, -void *filter_data) +const char *name) {struct object *obj = &blob->object;size_t pathlen;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; -if (!revs->blob_objects) +if (!ctx->revs->blob_objects)return ;if (!obj)die ("bad blob object" );
@@ -41,21 +46,21 @@ static void process_blob(struct rev_info *revs,* may cause the actual filter to report an incomplete list * of missing objects. */ -if (revs->exclude_promisor_objects && +if (ctx->revs->exclude_promisor_objects && !has_object_file (&obj->oid) &&is_promisor_object (&obj->oid))return ; pathlen = path->len;strbuf_addstr (path, name); -if (!(obj->flags &USER_GIVEN ) && filter_fn) - r =filter_fn (LOFS_BLOB , obj, - path->buf, &path->buf[pathlen], - filter_data); +if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) + r = ctx->filter_fn (LOFS_BLOB , obj, + path->buf, &path->buf[pathlen], + ctx->filter_data);if (r &LOFR_MARK_SEEN ) obj->flags |=SEEN ;if (r &LOFR_DO_SHOW ) -show (obj, path->buf, cb_data); + ctx->show_object (obj, path->buf, ctx->show_data);strbuf_setlen (path, pathlen); }
@@ -81,26 +86,21 @@ static void process_blob(struct rev_info *revs,* the link, and how to do it. Whether it necessarily makes * any sense what-so-ever to ever do that is another issue. */ -static void process_gitlink (struct rev_info *revs, +static void process_gitlink (struct traversal_context *ctx,const unsigned char *sha1, -show_object_fn show,struct strbuf *path, -const char *name, -void *cb_data) +const char *name) {/* Nothing to do */ } -static void process_tree (struct rev_info *revs, +static void process_tree (struct traversal_context *ctx,struct tree *tree, -show_object_fn show,struct strbuf *base, -const char *name, -void *cb_data, -filter_object_fn filter_fn, -void *filter_data) +const char *name) {struct object *obj = &tree->object; +struct rev_info *revs = ctx->revs;struct tree_desc desc;struct name_entry entry; enum interesting match = revs->diffopt.pathspec.nr == 0 ?
@@ -133,14 +133,14 @@ static void process_tree(struct rev_info *revs, }strbuf_addstr (base, name); -if (!(obj->flags &USER_GIVEN ) && filter_fn) - r =filter_fn (LOFS_BEGIN_TREE , obj, - base->buf, &base->buf[baselen], - filter_data); +if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) + r = ctx->filter_fn (LOFS_BEGIN_TREE , obj, + base->buf, &base->buf[baselen], + ctx->filter_data);if (r &LOFR_MARK_SEEN ) obj->flags |=SEEN ;if (r &LOFR_DO_SHOW ) -show (obj, base->buf, cb_data); + ctx->show_object (obj, base->buf, ctx->show_data);if (base->len)strbuf_addch (base,'/' );
@@ -157,29 +157,25 @@ static void process_tree(struct rev_info *revs, }if (S_ISDIR (entry.mode)) -process_tree (revs, +process_tree (ctx,lookup_tree (the_repository, entry.oid), - show, base, entry.path, - cb_data, filter_fn, filter_data); + base, entry.path);else if (S_ISGITLINK (entry.mode)) -process_gitlink (revs, entry.oid->hash, - show, base, entry.path, - cb_data); +process_gitlink (ctx, entry.oid->hash, base, entry.path);else -process_blob (revs, +process_blob (ctx,lookup_blob (the_repository, entry.oid), - show, base, entry.path, - cb_data, filter_fn, filter_data); + base, entry.path); } -if (!(obj->flags &USER_GIVEN ) && filter_fn) { - r =filter_fn (LOFS_END_TREE , obj, - base->buf, &base->buf[baselen], - filter_data); +if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { + r = ctx->filter_fn (LOFS_END_TREE , obj, + base->buf, &base->buf[baselen], + ctx->filter_data);if (r &LOFR_MARK_SEEN ) obj->flags |=SEEN ;if (r &LOFR_DO_SHOW ) -show (obj, base->buf, cb_data); + ctx->show_object (obj, base->buf, ctx->show_data); }strbuf_setlen (base, baselen);
@@ -242,19 +238,15 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)add_pending_object (revs, &tree->object,"" ); } -static void traverse_trees_and_blobs (struct rev_info *revs, -struct strbuf *base, -show_object_fn show_object, -void *show_data, -filter_object_fn filter_fn, -void *filter_data) +static void traverse_trees_and_blobs (struct traversal_context *ctx, +struct strbuf *base) {int i;assert (base->len ==0 ); -for (i =0 ; i < revs->pending.nr; i++) { -struct object_array_entry *pending = revs->pending.objects + i; +for (i =0 ; i < ctx->revs->pending.nr; i++) { +struct object_array_entry *pending = ctx->revs->pending.objects + i;struct object *obj = pending->item;const char *name = pending->name;const char *path = pending->path;
@@ -262,62 +254,49 @@ static void traverse_trees_and_blobs(struct rev_info *revs,continue ;if (obj->type ==OBJ_TAG ) { obj->flags |=SEEN ; -show_object (obj, name, show_data); + ctx->show_object (obj, name, ctx->show_data);continue ; }if (!path) path ="" ;if (obj->type ==OBJ_TREE ) { -process_tree (revs, (struct tree *)obj, show_object, - base, path, show_data, - filter_fn, filter_data); +process_tree (ctx, (struct tree *)obj, base, path);continue ; }if (obj->type ==OBJ_BLOB ) { -process_blob (revs, (struct blob *)obj, show_object, - base, path, show_data, - filter_fn, filter_data); +process_blob (ctx, (struct blob *)obj, base, path);continue ; }die ("unknown pending object %s (%s)" ,oid_to_hex (&obj->oid), name); } -object_array_clear (&revs->pending); +object_array_clear (&ctx->revs->pending); } -static void do_traverse (struct rev_info *revs, -show_commit_fn show_commit, -show_object_fn show_object, -void *show_data, -filter_object_fn filter_fn, -void *filter_data) +static void do_traverse (struct traversal_context *ctx) {struct commit *commit;struct strbuf csp;/* callee's scratch pad */ strbuf_init (&csp,PATH_MAX ); -while ((commit =get_revision (revs)) !=NULL ) { +while ((commit =get_revision (ctx->revs)) !=NULL ) {/* * an uninteresting boundary commit may not have its tree * parsed yet, but we are not going to show them anyway */ if (get_commit_tree (commit)) -add_pending_tree (revs,get_commit_tree (commit)); -show_commit (commit, show_data); +add_pending_tree (ctx->revs,get_commit_tree (commit)); + ctx->show_commit (commit, ctx->show_data); -if (revs->tree_blobs_in_commit_order) +if (ctx->revs->tree_blobs_in_commit_order)/* * NEEDSWORK: Adding the tree and then flushing it here * needs a reallocation for each commit. Can we pass the * tree directory without allocation churn? */ -traverse_trees_and_blobs (revs, &csp, - show_object, show_data, - filter_fn, filter_data); +traverse_trees_and_blobs (ctx, &csp); } -traverse_trees_and_blobs (revs, &csp, - show_object, show_data, - filter_fn, filter_data); +traverse_trees_and_blobs (ctx, &csp);strbuf_release (&csp); }
@@ -326,7 +305,14 @@ void traverse_commit_list(struct rev_info *revs,show_object_fn show_object, void *show_data) { -do_traverse (revs ,show_commit ,show_object ,show_data ,NULL ,NULL ); + struct traversal_context ctx; + ctx.revs = revs; + ctx.show_commit = show_commit; + ctx.show_object = show_object; + ctx.show_data = show_data; + ctx.filter_fn =NULL ; + ctx.filter_data =NULL ; +do_traverse (&ctx); } void traverse_commit_list_filtered(
@@ -337,14 +323,18 @@ void traverse_commit_list_filtered(void *show_data, struct oidset *omitted) { - filter_object_fn filter_fn = NULL; + struct traversal_context ctx;filter_free_fn filter_free_fn =NULL ; -void *filter_data =NULL ; - - filter_data =list_objects_filter__init (omitted, filter_options, - &filter_fn, &filter_free_fn); -do_traverse (revs, show_commit, show_object, show_data, - filter_fn, filter_data); -if (filter_data && filter_free_fn) -filter_free_fn (filter_data); + + ctx.revs = revs; + ctx.show_object = show_object; + ctx.show_commit = show_commit; + ctx.show_data = show_data; + ctx.filter_fn =NULL ; + + ctx.filter_data =list_objects_filter__init (omitted, filter_options, + &ctx.filter_fn, &filter_free_fn); +do_traverse (&ctx); +if (ctx.filter_data && filter_free_fn) +filter_free_fn (ctx.filter_data); }
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v10 2/8 list-objects: refactor to process_tree_contents
In reply to Matthew DeVoreThis will be used in a follow-up patch to reduce indentation needed when invoking the logic conditionally. i.e. rather than:
if (foo) {
while (...) {
/* this is very indented */
}
}we will have:
if (foo)
process_tree_contents(...);
--- list-objects.c | 68 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 27 deletions(-)
list-objects.c +41 -27
@@ -94,6 +94,46 @@ static void process_gitlink(struct traversal_context *ctx,/* Nothing to do */ } +static void process_tree (struct traversal_context *ctx, +struct tree *tree, +struct strbuf *base, +const char *name); + +static void process_tree_contents (struct traversal_context *ctx, +struct tree *tree, +struct strbuf *base) +{ +struct tree_desc desc; +struct name_entry entry; +enum interesting match = ctx->revs->diffopt.pathspec.nr ==0 ? + all_entries_interesting : entry_not_interesting; + +init_tree_desc (&desc, tree->buffer, tree->size); + +while (tree_entry (&desc, &entry)) { +if (match != all_entries_interesting) { + match =tree_entry_interesting (&entry, base,0 , + &ctx->revs->diffopt.pathspec); +if (match == all_entries_not_interesting) +break ; +if (match == entry_not_interesting) +continue ; + } + +if (S_ISDIR (entry.mode)) +process_tree (ctx, +lookup_tree (the_repository, entry.oid), + base, entry.path); +else if (S_ISGITLINK (entry.mode)) +process_gitlink (ctx, entry.oid->hash, + base, entry.path); +else +process_blob (ctx, +lookup_blob (the_repository, entry.oid), + base, entry.path); + } +} + static void process_tree(struct traversal_context *ctx, struct tree *tree, struct strbuf *base,
@@ -101,10 +141,6 @@ static void process_tree(struct traversal_context *ctx, {struct object *obj = &tree->object;struct rev_info *revs = ctx->revs; -struct tree_desc desc; -struct name_entry entry; -enum interesting match = revs->diffopt.pathspec.nr ==0 ? - all_entries_interesting: entry_not_interesting;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; int gently = revs->ignore_missing_links ||
@@ -144,29 +180,7 @@ static void process_tree(struct traversal_context *ctx,if (base->len)strbuf_addch (base,'/' ); -init_tree_desc (&desc, tree->buffer, tree->size); - -while (tree_entry (&desc, &entry)) { -if (match != all_entries_interesting) { - match =tree_entry_interesting (&entry, base,0 , - &revs->diffopt.pathspec); -if (match == all_entries_not_interesting) -break ; -if (match == entry_not_interesting) -continue ; - } - -if (S_ISDIR (entry.mode)) -process_tree (ctx, -lookup_tree (the_repository, entry.oid), - base, entry.path); -else if (S_ISGITLINK (entry.mode)) -process_gitlink (ctx, entry.oid->hash, base, entry.path); -else -process_blob (ctx, -lookup_blob (the_repository, entry.oid), - base, entry.path); - } +process_tree_contents (ctx, tree, base);if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn(LOFS_END_TREE, obj,
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v10 3/8 list-objects: always parse trees gently
In reply to Matthew DeVoreIf parsing fails when revs->ignore_missing_links and revs->exclude_promisor_objects are both false, we print the OID anyway in the die("bad tree object...") call, so any message printed by parse_tree_gently() is superfluous.
--- list-objects.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
list-objects.c +1 -3
@@ -143,8 +143,6 @@ static void process_tree(struct traversal_context *ctx,struct rev_info *revs = ctx->revs;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; -int gently = revs->ignore_missing_links || - revs->exclude_promisor_objects;if (!revs->tree_objects)return ;
@@ -152,7 +150,7 @@ static void process_tree(struct traversal_context *ctx,die ("bad tree object" );if (obj->flags & (UNINTERESTING |SEEN ))return ; -if (parse_tree_gently (tree, gently) <0 ) { +if (parse_tree_gently (tree,1 ) <0 ) {if (revs->ignore_missing_links)return ;
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v10 4/8 rev-list: handle missing tree objects properly
In reply to Matthew DeVorePreviously, we assumed only blob objects could be missing. This patch makes rev-list handle missing trees like missing blobs. The --missing=* and --exclude-promisor-objects flags now work for trees as they already do for blobs. This is demonstrated in t6112.
--- builtin/rev-list.c | 11 ++++--- list-objects.c | 11 +++++-- revision.h | 15 +++++++++ t/t0410-partial-clone.sh | 45 ++++++++++++++++++++++++++ t/t5317-pack-objects-filter-objects.sh | 13 ++++++++ t/t6112-rev-list-filters-objects.sh | 17 ++++++++++ 6 files changed, 105 insertions(+), 7 deletions(-)
builtin/rev-list.c +7 -4
@@ -6,6 +6,7 @@#include "list-objects.h" #include "list-objects-filter.h" #include "list-objects-filter-options.h" +#include "object.h" #include "object-store.h" #include "pack.h" #include "pack-bitmap.h"
@@ -209,7 +210,8 @@ static inline void finish_object__ma(struct object *obj)*/ switch (arg_missing_action) {case MA_ERROR : -die ("missing blob object '%s'" ,oid_to_hex (&obj->oid)); +die ("missing %s object '%s'" , +type_name (obj->type),oid_to_hex (&obj->oid));return ;case MA_ALLOW_ANY :
@@ -222,8 +224,8 @@ static inline void finish_object__ma(struct object *obj)case MA_ALLOW_PROMISOR :if (is_promisor_object (&obj->oid))return ; -die ("unexpected missing blob object '%s'" , -oid_to_hex (&obj->oid)); +die ("unexpected missing %s object '%s'" , +type_name (obj->type),oid_to_hex (&obj->oid));return ;default :
@@ -235,7 +237,7 @@ static inline void finish_object__ma(struct object *obj)static int finish_object (struct object *obj,const char *name,void *cb_data) {struct rev_list_info *info = cb_data; -if (obj->type ==OBJ_BLOB && !has_object_file (&obj->oid)) { +if (!has_object_file (&obj->oid)) {finish_object__ma (obj);return 1 ; }
@@ -373,6 +375,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)init_revisions (&revs, prefix); revs.abbrev =DEFAULT_ABBREV ; revs.commit_format =CMIT_FMT_UNSPECIFIED ; + revs.do_not_die_on_missing_tree =1 ;/* * Scan the argument list before invoking setup_revisions(), so that we
list-objects.c +8 -3
@@ -143,6 +143,7 @@ static void process_tree(struct traversal_context *ctx,struct rev_info *revs = ctx->revs;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; +int failed_parse;if (!revs->tree_objects)return ;
@@ -150,7 +151,9 @@ static void process_tree(struct traversal_context *ctx,die ("bad tree object" );if (obj->flags & (UNINTERESTING |SEEN ))return ; -if (parse_tree_gently (tree,1 ) <0 ) { + + failed_parse =parse_tree_gently (tree,1 ); +if (failed_parse) {if (revs->ignore_missing_links)return ;
@@ -163,7 +166,8 @@ static void process_tree(struct traversal_context *ctx, is_promisor_object(&obj->oid))return ; -die ("bad tree object %s" ,oid_to_hex (&obj->oid)); +if (!revs->do_not_die_on_missing_tree) +die ("bad tree object %s" ,oid_to_hex (&obj->oid)); }strbuf_addstr (base, name);
@@ -178,7 +182,8 @@ static void process_tree(struct traversal_context *ctx,if (base->len)strbuf_addch (base,'/' ); -process_tree_contents (ctx, tree, base); +if (!failed_parse) +process_tree_contents (ctx, tree, base);if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn(LOFS_END_TREE, obj,
revision.h +15 -0
@@ -126,6 +126,21 @@ struct rev_info { line_level_traverse:1, tree_blobs_in_commit_order:1, +/* +* Blobs are shown without regard for their existence. +* But not so for trees: unless exclude_promisor_objects +* is set and the tree in question is a promisor object; +* OR ignore_missing_links is set, the revision walker +* dies with a "bad tree object HASH" message when +* encountering a missing tree. For callers that can +* handle missing trees and want them to be filterable +* and showable, set this to true. The revision walker +* will filter and show such a missing tree as usual, +* but will not attempt to recurse into this tree +* object. +*/ + do_not_die_on_missing_tree:1, + /* for internal use only */ exclude_promisor_objects:1;
t/t0410-partial-clone.sh +45 -0
51 lines not on this page · raw message
t/t5317-pack-objects-filter-objects.sh +13 -0
@@ -59,6 +59,19 @@ test_expect_success 'verify normal and blob:none packfiles have same commits/tre test_cmp observed expected ' +test_expect_success 'get an error for missing tree object' ' + git init r5 && + echo foo >r5/foo && + git -C r5 add foo && + git -C r5 commit -m "foo" && + del=$(git -C r5 rev-parse HEAD^{tree} | sed "s|..|&/|") && + rm r5/.git/objects/$del && + test_must_fail git -C r5 pack-objects --rev --stdout 2>bad_tree <<-EOF && + HEAD + EOF + grep -q "bad tree object" bad_tree +' + # Test blob:limit=<n>[kmg] filter. # We boundary test around the size parameter. The filter is strictly less than # the value, so size 500 and 1000 should have the same results, but 1001 should
t/t6112-rev-list-filters-objects.sh +17 -0
@@ -195,6 +195,23 @@ test_expect_success 'verify sparse:oid=oid-ish omits top-level files' ' test_cmp observed expected ' +test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for trees' ' + TREE=$(git -C r3 rev-parse HEAD:dir1) && + + rm r3/.git/objects/$(echo $TREE | sed "s|^..|&/|") && + + git -C r3 rev-list --quiet --missing=print --objects HEAD >missing_objs 2>rev_list_err && + echo "?$TREE" >expected && + test_cmp expected missing_objs && + + # do not complain when a missing tree cannot be parsed + test_must_be_empty rev_list_err && + + git -C r3 rev-list --missing=allow-any --objects HEAD >objs 2>rev_list_err && + ! grep $TREE objs && + test_must_be_empty rev_list_err +' + # Delete some loose objects and use rev-list, but WITHOUT any filtering. # This models previously omitted objects that we did not receive.
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v10 5/8 revision: mark non-user-given objects instead
In reply to Matthew DeVoreCurrently, list-objects.c incorrectly treats all root trees of commits as USER_GIVEN. Also, it would be easier to mark objects that are non-user-given instead of user-given, since the places in the code where we access an object through a reference are more obvious than the places where we access an object that was given by the user.
Resolve these two problems by introducing a flag NOT_USER_GIVEN that marks blobs and trees that are non-user-given, replacing USER_GIVEN. (Only blobs and trees are marked because this mark is only used when filtering objects, and filtering of other types of objects is not supported yet.)
This fixes a bug in that git rev-list behaved differently from git pack-objects. pack-objects would *not* filter objects given explicitly on the command line and rev-list would filter. This was because the two commands used a different function to add objects to the rev_info struct. This seems to have been an oversight, and pack-objects has the correct behavior, so I added a test to make sure that rev-list now behaves properly.
--- list-objects.c | 31 +++++++++++++++++------------ revision.c | 1 - revision.h | 11 ++++++++-- t/t6112-rev-list-filters-objects.sh | 12 +++++++++++ 4 files changed, 39 insertions(+), 16 deletions(-)
list-objects.c +18 -13
@@ -53,7 +53,7 @@ static void process_blob(struct traversal_context *ctx, pathlen = path->len;strbuf_addstr (path, name); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) r = ctx->filter_fn (LOFS_BLOB , obj, path->buf, &path->buf[pathlen], ctx->filter_data);
@@ -120,17 +120,19 @@ static void process_tree_contents(struct traversal_context *ctx,continue ; } -if (S_ISDIR (entry.mode)) -process_tree (ctx, -lookup_tree (the_repository, entry.oid), - base, entry.path); +if (S_ISDIR (entry.mode)) { +struct tree *t =lookup_tree (the_repository, entry.oid); + t->object.flags |=NOT_USER_GIVEN ; +process_tree (ctx, t, base, entry.path); + }else if (S_ISGITLINK (entry.mode))process_gitlink (ctx, entry.oid->hash, base, entry.path); -else -process_blob (ctx, -lookup_blob (the_repository, entry.oid), - base, entry.path); +else { +struct blob *b =lookup_blob (the_repository, entry.oid); + b->object.flags |=NOT_USER_GIVEN ; +process_blob (ctx, b, base, entry.path); + } } }
@@ -171,7 +173,7 @@ static void process_tree(struct traversal_context *ctx, }strbuf_addstr (base, name); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) r = ctx->filter_fn (LOFS_BEGIN_TREE , obj, base->buf, &base->buf[baselen], ctx->filter_data);
@@ -185,7 +187,7 @@ static void process_tree(struct traversal_context *ctx,if (!failed_parse)process_tree_contents (ctx, tree, base); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn (LOFS_END_TREE , obj, base->buf, &base->buf[baselen], ctx->filter_data);
@@ -301,8 +303,11 @@ static void do_traverse(struct traversal_context *ctx)* an uninteresting boundary commit may not have its tree * parsed yet, but we are not going to show them anyway */ -if (get_commit_tree (commit)) -add_pending_tree (ctx->revs,get_commit_tree (commit)); +if (get_commit_tree (commit)) { +struct tree *tree =get_commit_tree (commit); + tree->object.flags |=NOT_USER_GIVEN ; +add_pending_tree (ctx->revs, tree); + } ctx->show_commit (commit, ctx->show_data);if (ctx->revs->tree_blobs_in_commit_order)
revision.c +0 -1
@@ -175,7 +175,6 @@ static void add_pending_object_with_path(struct rev_info *revs,strbuf_release (&buf);return ;/* do not add the commit itself */ } - obj->flags |=USER_GIVEN ;add_object_array_with_path (obj, name, &revs->pending, mode, path); }
revision.h +9 -2
@@ -21,9 +21,16 @@#define SYMMETRIC_LEFT (1u<<8)#define PATCHSAME (1u<<9)#define BOTTOM (1u<<10) -#define USER_GIVEN (1u<<25)/* given directly by the user */ +/* +* 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) -#define ALL_REV_FLAGS (((1u<<11)-1) | USER_GIVEN | TRACK_LINEAR) +#define ALL_REV_FLAGS (((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR)#define DECORATE_SHORT_REFS 1#define DECORATE_FULL_REFS 2
t/t6112-rev-list-filters-objects.sh +12 -0
@@ -30,6 +30,18 @@ test_expect_success 'verify blob:none omits all 5 blobs' ' test_cmp observed expected ' +test_expect_success 'specify blob explicitly prevents filtering' ' + file_3=$(git -C r1 ls-files -s file.3 | + awk -f print_2.awk) && + + file_4=$(git -C r1 ls-files -s file.4 | + awk -f print_2.awk) && + + git -C r1 rev-list --objects --filter=blob:none HEAD $file_3 >observed && + grep -q "$file_3" observed && + test_must_fail grep -q "$file_4" observed +' + test_expect_success 'verify emitted+omitted == all' ' git -C r1 rev-list HEAD --objects \ | awk -f print_1.awk \
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v10 6/8 list-objects-filter: use BUG rather than die
In reply to Matthew DeVoreIn some cases in this file, BUG makes more sense than die. In such cases, a we get there from a coding error rather than a user error.
'return' has been removed following some instances of BUG since BUG does not return.
--- list-objects-filter.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
list-objects-filter.c +4 -7
@@ -44,8 +44,7 @@ static enum list_objects_filter_result filter_blobs_none(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -102,8 +101,7 @@ static enum list_objects_filter_result filter_blobs_limit(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -208,8 +206,7 @@ static enum list_objects_filter_result filter_sparse(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -389,7 +386,7 @@ void *list_objects_filter__init(assert ((sizeof (s_filters) /sizeof (s_filters[0 ])) ==LOFC__COUNT );if (filter_options->choice >=LOFC__COUNT ) -die ("invalid list-objects filter choice: %d" , +BUG ("invalid list-objects filter choice: %d" , filter_options->choice); init_fn = s_filters[filter_options->choice];
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v10 7/8 list-objects-filter-options: do not over-strbuf_init
In reply to Matthew DeVoreThe function gently_parse_list_objects_filter is either called with errbuf=STRBUF_INIT or errbuf=NULL, but that function calls strbuf_init when errbuf is not NULL. strbuf_init is only necessary if errbuf contains garbage, and risks a memory leak if errbuf already has a non-STRBUF_INIT state. It should be the caller's responsibility to make sure errbuf is not garbage, since garbage content is easily avoidable with STRBUF_INIT.
--- list-objects-filter-options.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
list-objects-filter-options.c +2 -4
@@ -30,7 +30,6 @@ static int gently_parse_list_objects_filter(if (filter_options->choice) {if (errbuf) { -strbuf_init (errbuf,0 );strbuf_addstr ( errbuf,_ ("multiple filter-specs cannot be combined" ));
@@ -71,10 +70,9 @@ static int gently_parse_list_objects_filter(return 0 ; } -if (errbuf) { -strbuf_init (errbuf,0 ); +if (errbuf)strbuf_addf (errbuf,"invalid filter-spec '%s'" , arg); - } +memset (filter_options,0 ,sizeof (*filter_options));return 1 ; }
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v10 8/8 list-objects-filter: implement filter tree:0
In reply to Matthew DeVoreTeach list-objects the "tree:0" filter which allows for filtering out all tree and blob objects (unless other objects are explicitly specified by the user). The purpose of this patch is to allow smaller partial clones.
The name of this filter - tree:0 - does not explicitly specify that it also filters out all blobs, but this should not cause much confusion because blobs are not at all useful without the trees that refer to them.
I also considered only:commits as a name, but this is inaccurate because it suggests that annotated tags are omitted, but actually they are included.
The name "tree:0" allows later filtering based on depth, i.e. "tree:1" would filter out all but the root tree and blobs. In order to avoid confusion between 0 and capital O, the documentation was worded in a somewhat round-about way that also hints at this future improvement to the feature.
--- Documentation/rev-list-options.txt | 5 +++ list-objects-filter-options.c | 13 +++++++ list-objects-filter-options.h | 1 + list-objects-filter.c | 49 ++++++++++++++++++++++++++ t/t5317-pack-objects-filter-objects.sh | 28 +++++++++++++++ t/t5616-partial-clone.sh | 42 ++++++++++++++++++++++ t/t6112-rev-list-filters-objects.sh | 14 ++++++++ 7 files changed, 152 insertions(+)
Documentation/rev-list-options.txt +5 -0
@@ -731,6 +731,11 @@ the requested refs. + The form '--filter=sparse:path=<path>' similarly uses a sparse-checkout specification contained in <path>. ++ +The form '--filter=tree:<depth>' omits all blobs and trees whose depth +from the root tree is >= <depth> (minimum depth if an object is located +at multiple depths in the commits traversed). Currently, only <depth>=0 +is supported, which omits all blobs and trees. --no-filter:: Turn off any previous `--filter=` argument.
list-objects-filter-options.c +13 -0
@@ -49,6 +49,19 @@ static int gently_parse_list_objects_filter(return 0 ; } + }else if (skip_prefix (arg,"tree:" , &v0)) { +unsigned long depth; +if (!git_parse_ulong (v0, &depth) || depth !=0 ) { +if (errbuf) { +strbuf_addstr ( + errbuf, +_ ("only 'tree:0' is supported" )); + } +return 1 ; + } + filter_options->choice =LOFC_TREE_NONE ; +return 0 ; + }else if (skip_prefix (arg,"sparse:oid=" , &v0)) {struct object_context oc;struct object_id sparse_oid;
list-objects-filter-options.h +1 -0
@@ -10,6 +10,7 @@ enum list_objects_filter_choice {LOFC_DISABLED =0 ,LOFC_BLOB_NONE ,LOFC_BLOB_LIMIT , +LOFC_TREE_NONE ,LOFC_SPARSE_OID ,LOFC_SPARSE_PATH ,LOFC__COUNT /* must be last */
list-objects-filter.c +49 -0
61 lines not on this page · raw message
t/t5317-pack-objects-filter-objects.sh +28 -0
@@ -72,6 +72,34 @@ test_expect_success 'get an error for missing tree object' ' grep -q "bad tree object" bad_tree ' +test_expect_success 'setup for tests of tree:0' ' + mkdir r1/subtree && + echo "This is a file in a subtree" >r1/subtree/file && + git -C r1 add subtree/file && + git -C r1 commit -m subtree +' + +test_expect_success 'verify tree:0 packfile has no blobs or trees' ' + git -C r1 pack-objects --rev --stdout --filter=tree:0 >commitsonly.pack <<-EOF && + HEAD + EOF + git -C r1 index-pack ../commitsonly.pack && + git -C r1 verify-pack -v ../commitsonly.pack >objs && + ! grep -E "tree|blob" objs +' + +test_expect_success 'grab tree directly when using tree:0' ' + # We should get the tree specified directly but not its blobs or subtrees. + git -C r1 pack-objects --rev --stdout --filter=tree:0 >commitsonly.pack <<-EOF && + HEAD: + EOF + git -C r1 index-pack ../commitsonly.pack && + git -C r1 verify-pack -v ../commitsonly.pack >objs && + awk "/tree|blob/{print \$1}" objs >trees_and_blobs && + git -C r1 rev-parse HEAD: >expected && + test_cmp expected trees_and_blobs +' + # Test blob:limit=<n>[kmg] filter. # We boundary test around the size parameter. The filter is strictly less than # the value, so size 500 and 1000 should have the same results, but 1001 should
t/t5616-partial-clone.sh +42 -0
48 lines not on this page · raw message
t/t6112-rev-list-filters-objects.sh +14 -0
@@ -224,6 +224,20 @@ test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for tre test_must_be_empty rev_list_err ' +# Test tree:0 filter. + +test_expect_success 'verify tree:0 includes trees in "filtered" output' ' + git -C r3 rev-list --quiet --objects --filter-print-omitted \ + --filter=tree:0 HEAD | + awk -f print_1.awk | + sed s/~// | + xargs -n1 git -C r3 cat-file -t | + sort -u >filtered_types && + + printf "blob\ntree\n" >expected && + test_cmp expected filtered_types +' + # Delete some loose objects and use rev-list, but WITHOUT any filtering. # This models previously omitted objects that we did not receive.
Signature · 1 line
2.19.0.605.g01d371f741-goog
Quoted 4 lines · Matthew DeVore
This is a minor change to the previous rollup. It moves positional arguments to the end of git-rev-list invocations. Here is an interdiff from v9:
...
There is another problem with this patchset related to dropped exit codes and pipelines. In t6112, we run "git cat-file -t" on an object with was rm'd without being promised. It was printing an error and going undetected because it was upstream in a pipeline. The file was removed in the previous test.
So I fixed the previous test to clone the repository before manipulating it, and I fixed the latter test to not mask Git exit codes :) (This is a really insidious pattern and I should have taken it more seriously.) Below is an interdiff. The two tests are added in different commits, so each commit had to be fixed up.
I'll send a re-roll in two days or so if there are no more comments.
(quoted diff) +0 -0
b/t/t6112-rev-list-filters-objects.sh
index 5a61614b1..c8e3d87c4 100755
t/t6112-rev-list-filters-objects.sh +0 -0
@@ -210,16 +210,21 @@ test_expect_success 'verify sparse:oid=oid-ish
omits top-level files' '
test_expect_success 'rev-list W/ --missing=print and
--missing=allow-any for trees' '
TREE=$(git -C r3 rev-parse HEAD:dir1) &&
- rm r3/.git/objects/$(echo $TREE | sed "s|^..|&/|") &&
+ # Create a spare repo because we will be deleting objects
from this one.
+ git clone r3 r3.b &&
- git -C r3 rev-list --quiet --missing=print --objects HEAD
Quoted 1 line
missing_objs2>rev_list_err &&
+ rm r3.b/.git/objects/$(echo $TREE | sed "s|^..|&/|") &&
+
+ git -C r3.b rev-list --quiet --missing=print --objects HEAD \
+ >missing_objs 2>rev_list_err &&
echo "?$TREE" >expected &&
test_cmp expected missing_objs &&
# do not complain when a missing tree cannot be parsed
test_must_be_empty rev_list_err &&- git -C r3 rev-list --missing=allow-any --objects HEAD >objs
2>rev_list_err &&
+ git -C r3.b rev-list --missing=allow-any --objects HEAD \
+ >objs 2>rev_list_err &&
! grep $TREE objs &&
test_must_be_empty rev_list_err '
@@ -228,12 +233,13 @@ test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for tre
test_expect_success 'verify tree:0 includes trees in "filtered" output' '
git -C r3 rev-list --quiet --objects --filter-print-omitted \
- --filter=tree:0 HEAD |
- awk -f print_1.awk |
+ --filter=tree:0 HEAD >revs &&
+
+ awk -f print_1.awk revs |
sed s/~// | - xargs -n1 git -C r3 cat-file -t | - sort -u >filtered_types && + xargs -n1 git -C r3 cat-file -t >unsorted_filtered_types &&
+ sort -u unsorted_filtered_types >filtered_types &&
printf "blob\ntree\n" >expected &&
test_cmp expected filtered_types
'
v11 cover filter: support for excluding all trees and blobs
In reply to Matthew DeVoreHere is a clean re-rollup fixing the issue I found earlier. It fixes a problem stemming from a discarded exit code, which masked a crash in Git. The crash was not a bug because an earlier test deleted a loose object file. The fix was to make that test manipulate a clone rather than the original repo.
An interdiff from v10 is below.
Thank you,
Matt
t/t6112-rev-list-filters-objects.sh +13 -7
@@ -210,16 +210,21 @@ test_expect_success 'verify sparse:oid=oid-ish omits top-level files' ' test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for trees' ' TREE=$(git -C r3 rev-parse HEAD:dir1) && - rm r3/.git/objects/$(echo $TREE | sed "s|^..|&/|") && + # Create a spare repo because we will be deleting objects from this one. + git clone r3 r3.b && - git -C r3 rev-list --quiet --missing=print --objects HEAD >missing_objs 2>rev_list_err && + rm r3.b/.git/objects/$(echo $TREE | sed "s|^..|&/|") && + + git -C r3.b rev-list --quiet --missing=print --objects HEAD \ + >missing_objs 2>rev_list_err && echo "?$TREE" >expected && test_cmp expected missing_objs && # do not complain when a missing tree cannot be parsed test_must_be_empty rev_list_err && - git -C r3 rev-list --missing=allow-any --objects HEAD >objs 2>rev_list_err && + git -C r3.b rev-list --missing=allow-any --objects HEAD \ + >objs 2>rev_list_err && ! grep $TREE objs && test_must_be_empty rev_list_err '
@@ -228,12 +233,13 @@ test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for tre test_expect_success 'verify tree:0 includes trees in "filtered" output' ' git -C r3 rev-list --quiet --objects --filter-print-omitted \ - --filter=tree:0 HEAD | - awk -f print_1.awk | + --filter=tree:0 HEAD >revs && + + awk -f print_1.awk revs | sed s/~// | - xargs -n1 git -C r3 cat-file -t | - sort -u >filtered_types && + xargs -n1 git -C r3 cat-file -t >unsorted_filtered_types && + sort -u unsorted_filtered_types >filtered_types && printf "blob\ntree\n" >expected && test_cmp expected filtered_types '
Matthew DeVore (8):
list-objects: store common func args in struct
list-objects: refactor to process_tree_contents
list-objects: always parse trees gently
rev-list: handle missing tree objects properly
revision: mark non-user-given objects instead
list-objects-filter: use BUG rather than die list-objects-filter-options: do not over-strbuf_init
list-objects-filter: implement filter tree:0
Documentation/rev-list-options.txt | 5 + builtin/rev-list.c | 11 +- list-objects-filter-options.c | 19 +- list-objects-filter-options.h | 1 + list-objects-filter.c | 60 ++++++- list-objects.c | 232 +++++++++++++------------ revision.c | 1 - revision.h | 26 ++- t/t0410-partial-clone.sh | 45 +++++ t/t5317-pack-objects-filter-objects.sh | 41 +++++ t/t5616-partial-clone.sh | 42 +++++ t/t6112-rev-list-filters-objects.sh | 49 ++++++ 12 files changed, 404 insertions(+), 128 deletions(-)
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v11 1/8 list-objects: store common func args in struct
This will make utility functions easier to create, as done by the next patch.
--- list-objects.c | 158 +++++++++++++++++++++++-------------------------- 1 file changed, 74 insertions(+), 84 deletions(-)
list-objects.c +74 -84
@@ -12,20 +12,25 @@#include "packfile.h" #include "object-store.h" -static void process_blob (struct rev_info *revs, +struct traversal_context { +struct rev_info *revs; +show_object_fn show_object; +show_commit_fn show_commit; +void *show_data; +filter_object_fn filter_fn; +void *filter_data; +}; + +static void process_blob (struct traversal_context *ctx,struct blob *blob, -show_object_fn show,struct strbuf *path, -const char *name, -void *cb_data, -filter_object_fn filter_fn, -void *filter_data) +const char *name) {struct object *obj = &blob->object;size_t pathlen;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; -if (!revs->blob_objects) +if (!ctx->revs->blob_objects)return ;if (!obj)die ("bad blob object" );
@@ -41,21 +46,21 @@ static void process_blob(struct rev_info *revs,* may cause the actual filter to report an incomplete list * of missing objects. */ -if (revs->exclude_promisor_objects && +if (ctx->revs->exclude_promisor_objects && !has_object_file (&obj->oid) &&is_promisor_object (&obj->oid))return ; pathlen = path->len;strbuf_addstr (path, name); -if (!(obj->flags &USER_GIVEN ) && filter_fn) - r =filter_fn (LOFS_BLOB , obj, - path->buf, &path->buf[pathlen], - filter_data); +if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) + r = ctx->filter_fn (LOFS_BLOB , obj, + path->buf, &path->buf[pathlen], + ctx->filter_data);if (r &LOFR_MARK_SEEN ) obj->flags |=SEEN ;if (r &LOFR_DO_SHOW ) -show (obj, path->buf, cb_data); + ctx->show_object (obj, path->buf, ctx->show_data);strbuf_setlen (path, pathlen); }
@@ -81,26 +86,21 @@ static void process_blob(struct rev_info *revs,* the link, and how to do it. Whether it necessarily makes * any sense what-so-ever to ever do that is another issue. */ -static void process_gitlink (struct rev_info *revs, +static void process_gitlink (struct traversal_context *ctx,const unsigned char *sha1, -show_object_fn show,struct strbuf *path, -const char *name, -void *cb_data) +const char *name) {/* Nothing to do */ } -static void process_tree (struct rev_info *revs, +static void process_tree (struct traversal_context *ctx,struct tree *tree, -show_object_fn show,struct strbuf *base, -const char *name, -void *cb_data, -filter_object_fn filter_fn, -void *filter_data) +const char *name) {struct object *obj = &tree->object; +struct rev_info *revs = ctx->revs;struct tree_desc desc;struct name_entry entry; enum interesting match = revs->diffopt.pathspec.nr == 0 ?
@@ -133,14 +133,14 @@ static void process_tree(struct rev_info *revs, }strbuf_addstr (base, name); -if (!(obj->flags &USER_GIVEN ) && filter_fn) - r =filter_fn (LOFS_BEGIN_TREE , obj, - base->buf, &base->buf[baselen], - filter_data); +if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) + r = ctx->filter_fn (LOFS_BEGIN_TREE , obj, + base->buf, &base->buf[baselen], + ctx->filter_data);if (r &LOFR_MARK_SEEN ) obj->flags |=SEEN ;if (r &LOFR_DO_SHOW ) -show (obj, base->buf, cb_data); + ctx->show_object (obj, base->buf, ctx->show_data);if (base->len)strbuf_addch (base,'/' );
@@ -157,29 +157,25 @@ static void process_tree(struct rev_info *revs, }if (S_ISDIR (entry.mode)) -process_tree (revs, +process_tree (ctx,lookup_tree (the_repository, entry.oid), - show, base, entry.path, - cb_data, filter_fn, filter_data); + base, entry.path);else if (S_ISGITLINK (entry.mode)) -process_gitlink (revs, entry.oid->hash, - show, base, entry.path, - cb_data); +process_gitlink (ctx, entry.oid->hash, base, entry.path);else -process_blob (revs, +process_blob (ctx,lookup_blob (the_repository, entry.oid), - show, base, entry.path, - cb_data, filter_fn, filter_data); + base, entry.path); } -if (!(obj->flags &USER_GIVEN ) && filter_fn) { - r =filter_fn (LOFS_END_TREE , obj, - base->buf, &base->buf[baselen], - filter_data); +if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { + r = ctx->filter_fn (LOFS_END_TREE , obj, + base->buf, &base->buf[baselen], + ctx->filter_data);if (r &LOFR_MARK_SEEN ) obj->flags |=SEEN ;if (r &LOFR_DO_SHOW ) -show (obj, base->buf, cb_data); + ctx->show_object (obj, base->buf, ctx->show_data); }strbuf_setlen (base, baselen);
@@ -242,19 +238,15 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)add_pending_object (revs, &tree->object,"" ); } -static void traverse_trees_and_blobs (struct rev_info *revs, -struct strbuf *base, -show_object_fn show_object, -void *show_data, -filter_object_fn filter_fn, -void *filter_data) +static void traverse_trees_and_blobs (struct traversal_context *ctx, +struct strbuf *base) {int i;assert (base->len ==0 ); -for (i =0 ; i < revs->pending.nr; i++) { -struct object_array_entry *pending = revs->pending.objects + i; +for (i =0 ; i < ctx->revs->pending.nr; i++) { +struct object_array_entry *pending = ctx->revs->pending.objects + i;struct object *obj = pending->item;const char *name = pending->name;const char *path = pending->path;
@@ -262,62 +254,49 @@ static void traverse_trees_and_blobs(struct rev_info *revs,continue ;if (obj->type ==OBJ_TAG ) { obj->flags |=SEEN ; -show_object (obj, name, show_data); + ctx->show_object (obj, name, ctx->show_data);continue ; }if (!path) path ="" ;if (obj->type ==OBJ_TREE ) { -process_tree (revs, (struct tree *)obj, show_object, - base, path, show_data, - filter_fn, filter_data); +process_tree (ctx, (struct tree *)obj, base, path);continue ; }if (obj->type ==OBJ_BLOB ) { -process_blob (revs, (struct blob *)obj, show_object, - base, path, show_data, - filter_fn, filter_data); +process_blob (ctx, (struct blob *)obj, base, path);continue ; }die ("unknown pending object %s (%s)" ,oid_to_hex (&obj->oid), name); } -object_array_clear (&revs->pending); +object_array_clear (&ctx->revs->pending); } -static void do_traverse (struct rev_info *revs, -show_commit_fn show_commit, -show_object_fn show_object, -void *show_data, -filter_object_fn filter_fn, -void *filter_data) +static void do_traverse (struct traversal_context *ctx) {struct commit *commit;struct strbuf csp;/* callee's scratch pad */ strbuf_init (&csp,PATH_MAX ); -while ((commit =get_revision (revs)) !=NULL ) { +while ((commit =get_revision (ctx->revs)) !=NULL ) {/* * an uninteresting boundary commit may not have its tree * parsed yet, but we are not going to show them anyway */ if (get_commit_tree (commit)) -add_pending_tree (revs,get_commit_tree (commit)); -show_commit (commit, show_data); +add_pending_tree (ctx->revs,get_commit_tree (commit)); + ctx->show_commit (commit, ctx->show_data); -if (revs->tree_blobs_in_commit_order) +if (ctx->revs->tree_blobs_in_commit_order)/* * NEEDSWORK: Adding the tree and then flushing it here * needs a reallocation for each commit. Can we pass the * tree directory without allocation churn? */ -traverse_trees_and_blobs (revs, &csp, - show_object, show_data, - filter_fn, filter_data); +traverse_trees_and_blobs (ctx, &csp); } -traverse_trees_and_blobs (revs, &csp, - show_object, show_data, - filter_fn, filter_data); +traverse_trees_and_blobs (ctx, &csp);strbuf_release (&csp); }
@@ -326,7 +305,14 @@ void traverse_commit_list(struct rev_info *revs,show_object_fn show_object, void *show_data) { -do_traverse (revs ,show_commit ,show_object ,show_data ,NULL ,NULL ); + struct traversal_context ctx; + ctx.revs = revs; + ctx.show_commit = show_commit; + ctx.show_object = show_object; + ctx.show_data = show_data; + ctx.filter_fn =NULL ; + ctx.filter_data =NULL ; +do_traverse (&ctx); } void traverse_commit_list_filtered(
@@ -337,14 +323,18 @@ void traverse_commit_list_filtered(void *show_data, struct oidset *omitted) { - filter_object_fn filter_fn = NULL; + struct traversal_context ctx;filter_free_fn filter_free_fn =NULL ; -void *filter_data =NULL ; - - filter_data =list_objects_filter__init (omitted, filter_options, - &filter_fn, &filter_free_fn); -do_traverse (revs, show_commit, show_object, show_data, - filter_fn, filter_data); -if (filter_data && filter_free_fn) -filter_free_fn (filter_data); + + ctx.revs = revs; + ctx.show_object = show_object; + ctx.show_commit = show_commit; + ctx.show_data = show_data; + ctx.filter_fn =NULL ; + + ctx.filter_data =list_objects_filter__init (omitted, filter_options, + &ctx.filter_fn, &filter_free_fn); +do_traverse (&ctx); +if (ctx.filter_data && filter_free_fn) +filter_free_fn (ctx.filter_data); }
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v11 2/8 list-objects: refactor to process_tree_contents
In reply to Matthew DeVoreThis will be used in a follow-up patch to reduce indentation needed when invoking the logic conditionally. i.e. rather than:
if (foo) {
while (...) {
/* this is very indented */
}
}we will have:
if (foo)
process_tree_contents(...);
--- list-objects.c | 68 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 27 deletions(-)
list-objects.c +41 -27
@@ -94,6 +94,46 @@ static void process_gitlink(struct traversal_context *ctx,/* Nothing to do */ } +static void process_tree (struct traversal_context *ctx, +struct tree *tree, +struct strbuf *base, +const char *name); + +static void process_tree_contents (struct traversal_context *ctx, +struct tree *tree, +struct strbuf *base) +{ +struct tree_desc desc; +struct name_entry entry; +enum interesting match = ctx->revs->diffopt.pathspec.nr ==0 ? + all_entries_interesting : entry_not_interesting; + +init_tree_desc (&desc, tree->buffer, tree->size); + +while (tree_entry (&desc, &entry)) { +if (match != all_entries_interesting) { + match =tree_entry_interesting (&entry, base,0 , + &ctx->revs->diffopt.pathspec); +if (match == all_entries_not_interesting) +break ; +if (match == entry_not_interesting) +continue ; + } + +if (S_ISDIR (entry.mode)) +process_tree (ctx, +lookup_tree (the_repository, entry.oid), + base, entry.path); +else if (S_ISGITLINK (entry.mode)) +process_gitlink (ctx, entry.oid->hash, + base, entry.path); +else +process_blob (ctx, +lookup_blob (the_repository, entry.oid), + base, entry.path); + } +} + static void process_tree(struct traversal_context *ctx, struct tree *tree, struct strbuf *base,
@@ -101,10 +141,6 @@ static void process_tree(struct traversal_context *ctx, {struct object *obj = &tree->object;struct rev_info *revs = ctx->revs; -struct tree_desc desc; -struct name_entry entry; -enum interesting match = revs->diffopt.pathspec.nr ==0 ? - all_entries_interesting: entry_not_interesting;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; int gently = revs->ignore_missing_links ||
@@ -144,29 +180,7 @@ static void process_tree(struct traversal_context *ctx,if (base->len)strbuf_addch (base,'/' ); -init_tree_desc (&desc, tree->buffer, tree->size); - -while (tree_entry (&desc, &entry)) { -if (match != all_entries_interesting) { - match =tree_entry_interesting (&entry, base,0 , - &revs->diffopt.pathspec); -if (match == all_entries_not_interesting) -break ; -if (match == entry_not_interesting) -continue ; - } - -if (S_ISDIR (entry.mode)) -process_tree (ctx, -lookup_tree (the_repository, entry.oid), - base, entry.path); -else if (S_ISGITLINK (entry.mode)) -process_gitlink (ctx, entry.oid->hash, base, entry.path); -else -process_blob (ctx, -lookup_blob (the_repository, entry.oid), - base, entry.path); - } +process_tree_contents (ctx, tree, base);if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn(LOFS_END_TREE, obj,
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v11 3/8 list-objects: always parse trees gently
In reply to Matthew DeVoreIf parsing fails when revs->ignore_missing_links and revs->exclude_promisor_objects are both false, we print the OID anyway in the die("bad tree object...") call, so any message printed by parse_tree_gently() is superfluous.
--- list-objects.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
list-objects.c +1 -3
@@ -143,8 +143,6 @@ static void process_tree(struct traversal_context *ctx,struct rev_info *revs = ctx->revs;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; -int gently = revs->ignore_missing_links || - revs->exclude_promisor_objects;if (!revs->tree_objects)return ;
@@ -152,7 +150,7 @@ static void process_tree(struct traversal_context *ctx,die ("bad tree object" );if (obj->flags & (UNINTERESTING |SEEN ))return ; -if (parse_tree_gently (tree, gently) <0 ) { +if (parse_tree_gently (tree,1 ) <0 ) {if (revs->ignore_missing_links)return ;
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v11 4/8 rev-list: handle missing tree objects properly
In reply to Matthew DeVorePreviously, we assumed only blob objects could be missing. This patch makes rev-list handle missing trees like missing blobs. The --missing=* and --exclude-promisor-objects flags now work for trees as they already do for blobs. This is demonstrated in t6112.
--- builtin/rev-list.c | 11 ++++--- list-objects.c | 11 +++++-- revision.h | 15 +++++++++ t/t0410-partial-clone.sh | 45 ++++++++++++++++++++++++++ t/t5317-pack-objects-filter-objects.sh | 13 ++++++++ t/t6112-rev-list-filters-objects.sh | 22 +++++++++++++ 6 files changed, 110 insertions(+), 7 deletions(-)
builtin/rev-list.c +7 -4
@@ -6,6 +6,7 @@#include "list-objects.h" #include "list-objects-filter.h" #include "list-objects-filter-options.h" +#include "object.h" #include "object-store.h" #include "pack.h" #include "pack-bitmap.h"
@@ -209,7 +210,8 @@ static inline void finish_object__ma(struct object *obj)*/ switch (arg_missing_action) {case MA_ERROR : -die ("missing blob object '%s'" ,oid_to_hex (&obj->oid)); +die ("missing %s object '%s'" , +type_name (obj->type),oid_to_hex (&obj->oid));return ;case MA_ALLOW_ANY :
@@ -222,8 +224,8 @@ static inline void finish_object__ma(struct object *obj)case MA_ALLOW_PROMISOR :if (is_promisor_object (&obj->oid))return ; -die ("unexpected missing blob object '%s'" , -oid_to_hex (&obj->oid)); +die ("unexpected missing %s object '%s'" , +type_name (obj->type),oid_to_hex (&obj->oid));return ;default :
@@ -235,7 +237,7 @@ static inline void finish_object__ma(struct object *obj)static int finish_object (struct object *obj,const char *name,void *cb_data) {struct rev_list_info *info = cb_data; -if (obj->type ==OBJ_BLOB && !has_object_file (&obj->oid)) { +if (!has_object_file (&obj->oid)) {finish_object__ma (obj);return 1 ; }
@@ -373,6 +375,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)init_revisions (&revs, prefix); revs.abbrev =DEFAULT_ABBREV ; revs.commit_format =CMIT_FMT_UNSPECIFIED ; + revs.do_not_die_on_missing_tree =1 ;/* * Scan the argument list before invoking setup_revisions(), so that we
list-objects.c +8 -3
@@ -143,6 +143,7 @@ static void process_tree(struct traversal_context *ctx,struct rev_info *revs = ctx->revs;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; +int failed_parse;if (!revs->tree_objects)return ;
@@ -150,7 +151,9 @@ static void process_tree(struct traversal_context *ctx,die ("bad tree object" );if (obj->flags & (UNINTERESTING |SEEN ))return ; -if (parse_tree_gently (tree,1 ) <0 ) { + + failed_parse =parse_tree_gently (tree,1 ); +if (failed_parse) {if (revs->ignore_missing_links)return ;
@@ -163,7 +166,8 @@ static void process_tree(struct traversal_context *ctx, is_promisor_object(&obj->oid))return ; -die ("bad tree object %s" ,oid_to_hex (&obj->oid)); +if (!revs->do_not_die_on_missing_tree) +die ("bad tree object %s" ,oid_to_hex (&obj->oid)); }strbuf_addstr (base, name);
@@ -178,7 +182,8 @@ static void process_tree(struct traversal_context *ctx,if (base->len)strbuf_addch (base,'/' ); -process_tree_contents (ctx, tree, base); +if (!failed_parse) +process_tree_contents (ctx, tree, base);if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn(LOFS_END_TREE, obj,
revision.h +15 -0
@@ -126,6 +126,21 @@ struct rev_info { line_level_traverse:1, tree_blobs_in_commit_order:1, +/* +* Blobs are shown without regard for their existence. +* But not so for trees: unless exclude_promisor_objects +* is set and the tree in question is a promisor object; +* OR ignore_missing_links is set, the revision walker +* dies with a "bad tree object HASH" message when +* encountering a missing tree. For callers that can +* handle missing trees and want them to be filterable +* and showable, set this to true. The revision walker +* will filter and show such a missing tree as usual, +* but will not attempt to recurse into this tree +* object. +*/ + do_not_die_on_missing_tree:1, + /* for internal use only */ exclude_promisor_objects:1;
t/t0410-partial-clone.sh +45 -0
51 lines not on this page · raw message
t/t5317-pack-objects-filter-objects.sh +13 -0
@@ -59,6 +59,19 @@ test_expect_success 'verify normal and blob:none packfiles have same commits/tre test_cmp observed expected ' +test_expect_success 'get an error for missing tree object' ' + git init r5 && + echo foo >r5/foo && + git -C r5 add foo && + git -C r5 commit -m "foo" && + del=$(git -C r5 rev-parse HEAD^{tree} | sed "s|..|&/|") && + rm r5/.git/objects/$del && + test_must_fail git -C r5 pack-objects --rev --stdout 2>bad_tree <<-EOF && + HEAD + EOF + grep -q "bad tree object" bad_tree +' + # Test blob:limit=<n>[kmg] filter. # We boundary test around the size parameter. The filter is strictly less than # the value, so size 500 and 1000 should have the same results, but 1001 should
t/t6112-rev-list-filters-objects.sh +22 -0
@@ -195,6 +195,28 @@ test_expect_success 'verify sparse:oid=oid-ish omits top-level files' ' test_cmp observed expected ' +test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for trees' ' + TREE=$(git -C r3 rev-parse HEAD:dir1) && + + # Create a spare repo because we will be deleting objects from this one. + git clone r3 r3.b && + + rm r3.b/.git/objects/$(echo $TREE | sed "s|^..|&/|") && + + git -C r3.b rev-list --quiet --missing=print --objects HEAD \ + >missing_objs 2>rev_list_err && + echo "?$TREE" >expected && + test_cmp expected missing_objs && + + # do not complain when a missing tree cannot be parsed + test_must_be_empty rev_list_err && + + git -C r3.b rev-list --missing=allow-any --objects HEAD \ + >objs 2>rev_list_err && + ! grep $TREE objs && + test_must_be_empty rev_list_err +' + # Delete some loose objects and use rev-list, but WITHOUT any filtering. # This models previously omitted objects that we did not receive.
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v11 5/8 revision: mark non-user-given objects instead
In reply to Matthew DeVoreCurrently, list-objects.c incorrectly treats all root trees of commits as USER_GIVEN. Also, it would be easier to mark objects that are non-user-given instead of user-given, since the places in the code where we access an object through a reference are more obvious than the places where we access an object that was given by the user.
Resolve these two problems by introducing a flag NOT_USER_GIVEN that marks blobs and trees that are non-user-given, replacing USER_GIVEN. (Only blobs and trees are marked because this mark is only used when filtering objects, and filtering of other types of objects is not supported yet.)
This fixes a bug in that git rev-list behaved differently from git pack-objects. pack-objects would *not* filter objects given explicitly on the command line and rev-list would filter. This was because the two commands used a different function to add objects to the rev_info struct. This seems to have been an oversight, and pack-objects has the correct behavior, so I added a test to make sure that rev-list now behaves properly.
--- list-objects.c | 31 +++++++++++++++++------------ revision.c | 1 - revision.h | 11 ++++++++-- t/t6112-rev-list-filters-objects.sh | 12 +++++++++++ 4 files changed, 39 insertions(+), 16 deletions(-)
list-objects.c +18 -13
@@ -53,7 +53,7 @@ static void process_blob(struct traversal_context *ctx, pathlen = path->len;strbuf_addstr (path, name); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) r = ctx->filter_fn (LOFS_BLOB , obj, path->buf, &path->buf[pathlen], ctx->filter_data);
@@ -120,17 +120,19 @@ static void process_tree_contents(struct traversal_context *ctx,continue ; } -if (S_ISDIR (entry.mode)) -process_tree (ctx, -lookup_tree (the_repository, entry.oid), - base, entry.path); +if (S_ISDIR (entry.mode)) { +struct tree *t =lookup_tree (the_repository, entry.oid); + t->object.flags |=NOT_USER_GIVEN ; +process_tree (ctx, t, base, entry.path); + }else if (S_ISGITLINK (entry.mode))process_gitlink (ctx, entry.oid->hash, base, entry.path); -else -process_blob (ctx, -lookup_blob (the_repository, entry.oid), - base, entry.path); +else { +struct blob *b =lookup_blob (the_repository, entry.oid); + b->object.flags |=NOT_USER_GIVEN ; +process_blob (ctx, b, base, entry.path); + } } }
@@ -171,7 +173,7 @@ static void process_tree(struct traversal_context *ctx, }strbuf_addstr (base, name); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) r = ctx->filter_fn (LOFS_BEGIN_TREE , obj, base->buf, &base->buf[baselen], ctx->filter_data);
@@ -185,7 +187,7 @@ static void process_tree(struct traversal_context *ctx,if (!failed_parse)process_tree_contents (ctx, tree, base); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn (LOFS_END_TREE , obj, base->buf, &base->buf[baselen], ctx->filter_data);
@@ -301,8 +303,11 @@ static void do_traverse(struct traversal_context *ctx)* an uninteresting boundary commit may not have its tree * parsed yet, but we are not going to show them anyway */ -if (get_commit_tree (commit)) -add_pending_tree (ctx->revs,get_commit_tree (commit)); +if (get_commit_tree (commit)) { +struct tree *tree =get_commit_tree (commit); + tree->object.flags |=NOT_USER_GIVEN ; +add_pending_tree (ctx->revs, tree); + } ctx->show_commit (commit, ctx->show_data);if (ctx->revs->tree_blobs_in_commit_order)
revision.c +0 -1
@@ -175,7 +175,6 @@ static void add_pending_object_with_path(struct rev_info *revs,strbuf_release (&buf);return ;/* do not add the commit itself */ } - obj->flags |=USER_GIVEN ;add_object_array_with_path (obj, name, &revs->pending, mode, path); }
revision.h +9 -2
@@ -21,9 +21,16 @@#define SYMMETRIC_LEFT (1u<<8)#define PATCHSAME (1u<<9)#define BOTTOM (1u<<10) -#define USER_GIVEN (1u<<25)/* given directly by the user */ +/* +* 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) -#define ALL_REV_FLAGS (((1u<<11)-1) | USER_GIVEN | TRACK_LINEAR) +#define ALL_REV_FLAGS (((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR)#define DECORATE_SHORT_REFS 1#define DECORATE_FULL_REFS 2
t/t6112-rev-list-filters-objects.sh +12 -0
@@ -30,6 +30,18 @@ test_expect_success 'verify blob:none omits all 5 blobs' ' test_cmp observed expected ' +test_expect_success 'specify blob explicitly prevents filtering' ' + file_3=$(git -C r1 ls-files -s file.3 | + awk -f print_2.awk) && + + file_4=$(git -C r1 ls-files -s file.4 | + awk -f print_2.awk) && + + git -C r1 rev-list --objects --filter=blob:none HEAD $file_3 >observed && + grep -q "$file_3" observed && + test_must_fail grep -q "$file_4" observed +' + test_expect_success 'verify emitted+omitted == all' ' git -C r1 rev-list HEAD --objects \ | awk -f print_1.awk \
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v11 6/8 list-objects-filter: use BUG rather than die
In reply to Matthew DeVoreIn some cases in this file, BUG makes more sense than die. In such cases, a we get there from a coding error rather than a user error.
'return' has been removed following some instances of BUG since BUG does not return.
--- list-objects-filter.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
list-objects-filter.c +4 -7
@@ -44,8 +44,7 @@ static enum list_objects_filter_result filter_blobs_none(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -102,8 +101,7 @@ static enum list_objects_filter_result filter_blobs_limit(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -208,8 +206,7 @@ static enum list_objects_filter_result filter_sparse(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -389,7 +386,7 @@ void *list_objects_filter__init(assert ((sizeof (s_filters) /sizeof (s_filters[0 ])) ==LOFC__COUNT );if (filter_options->choice >=LOFC__COUNT ) -die ("invalid list-objects filter choice: %d" , +BUG ("invalid list-objects filter choice: %d" , filter_options->choice); init_fn = s_filters[filter_options->choice];
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v11 7/8 list-objects-filter-options: do not over-strbuf_init
In reply to Matthew DeVoreThe function gently_parse_list_objects_filter is either called with errbuf=STRBUF_INIT or errbuf=NULL, but that function calls strbuf_init when errbuf is not NULL. strbuf_init is only necessary if errbuf contains garbage, and risks a memory leak if errbuf already has a non-STRBUF_INIT state. It should be the caller's responsibility to make sure errbuf is not garbage, since garbage content is easily avoidable with STRBUF_INIT.
--- list-objects-filter-options.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
list-objects-filter-options.c +2 -4
@@ -30,7 +30,6 @@ static int gently_parse_list_objects_filter(if (filter_options->choice) {if (errbuf) { -strbuf_init (errbuf,0 );strbuf_addstr ( errbuf,_ ("multiple filter-specs cannot be combined" ));
@@ -71,10 +70,9 @@ static int gently_parse_list_objects_filter(return 0 ; } -if (errbuf) { -strbuf_init (errbuf,0 ); +if (errbuf)strbuf_addf (errbuf,"invalid filter-spec '%s'" , arg); - } +memset (filter_options,0 ,sizeof (*filter_options));return 1 ; }
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH v11 8/8 list-objects-filter: implement filter tree:0
In reply to Matthew DeVoreTeach list-objects the "tree:0" filter which allows for filtering out all tree and blob objects (unless other objects are explicitly specified by the user). The purpose of this patch is to allow smaller partial clones.
The name of this filter - tree:0 - does not explicitly specify that it also filters out all blobs, but this should not cause much confusion because blobs are not at all useful without the trees that refer to them.
I also considered only:commits as a name, but this is inaccurate because it suggests that annotated tags are omitted, but actually they are included.
The name "tree:0" allows later filtering based on depth, i.e. "tree:1" would filter out all but the root tree and blobs. In order to avoid confusion between 0 and capital O, the documentation was worded in a somewhat round-about way that also hints at this future improvement to the feature.
--- Documentation/rev-list-options.txt | 5 +++ list-objects-filter-options.c | 13 +++++++ list-objects-filter-options.h | 1 + list-objects-filter.c | 49 ++++++++++++++++++++++++++ t/t5317-pack-objects-filter-objects.sh | 28 +++++++++++++++ t/t5616-partial-clone.sh | 42 ++++++++++++++++++++++ t/t6112-rev-list-filters-objects.sh | 15 ++++++++ 7 files changed, 153 insertions(+)
Documentation/rev-list-options.txt +5 -0
@@ -731,6 +731,11 @@ the requested refs. + The form '--filter=sparse:path=<path>' similarly uses a sparse-checkout specification contained in <path>. ++ +The form '--filter=tree:<depth>' omits all blobs and trees whose depth +from the root tree is >= <depth> (minimum depth if an object is located +at multiple depths in the commits traversed). Currently, only <depth>=0 +is supported, which omits all blobs and trees. --no-filter:: Turn off any previous `--filter=` argument.
list-objects-filter-options.c +13 -0
@@ -49,6 +49,19 @@ static int gently_parse_list_objects_filter(return 0 ; } + }else if (skip_prefix (arg,"tree:" , &v0)) { +unsigned long depth; +if (!git_parse_ulong (v0, &depth) || depth !=0 ) { +if (errbuf) { +strbuf_addstr ( + errbuf, +_ ("only 'tree:0' is supported" )); + } +return 1 ; + } + filter_options->choice =LOFC_TREE_NONE ; +return 0 ; + }else if (skip_prefix (arg,"sparse:oid=" , &v0)) {struct object_context oc;struct object_id sparse_oid;
list-objects-filter-options.h +1 -0
@@ -10,6 +10,7 @@ enum list_objects_filter_choice {LOFC_DISABLED =0 ,LOFC_BLOB_NONE ,LOFC_BLOB_LIMIT , +LOFC_TREE_NONE ,LOFC_SPARSE_OID ,LOFC_SPARSE_PATH ,LOFC__COUNT /* must be last */
list-objects-filter.c +49 -0
61 lines not on this page · raw message
t/t5317-pack-objects-filter-objects.sh +28 -0
@@ -72,6 +72,34 @@ test_expect_success 'get an error for missing tree object' ' grep -q "bad tree object" bad_tree ' +test_expect_success 'setup for tests of tree:0' ' + mkdir r1/subtree && + echo "This is a file in a subtree" >r1/subtree/file && + git -C r1 add subtree/file && + git -C r1 commit -m subtree +' + +test_expect_success 'verify tree:0 packfile has no blobs or trees' ' + git -C r1 pack-objects --rev --stdout --filter=tree:0 >commitsonly.pack <<-EOF && + HEAD + EOF + git -C r1 index-pack ../commitsonly.pack && + git -C r1 verify-pack -v ../commitsonly.pack >objs && + ! grep -E "tree|blob" objs +' + +test_expect_success 'grab tree directly when using tree:0' ' + # We should get the tree specified directly but not its blobs or subtrees. + git -C r1 pack-objects --rev --stdout --filter=tree:0 >commitsonly.pack <<-EOF && + HEAD: + EOF + git -C r1 index-pack ../commitsonly.pack && + git -C r1 verify-pack -v ../commitsonly.pack >objs && + awk "/tree|blob/{print \$1}" objs >trees_and_blobs && + git -C r1 rev-parse HEAD: >expected && + test_cmp expected trees_and_blobs +' + # Test blob:limit=<n>[kmg] filter. # We boundary test around the size parameter. The filter is strictly less than # the value, so size 500 and 1000 should have the same results, but 1001 should
t/t5616-partial-clone.sh +42 -0
48 lines not on this page · raw message
t/t6112-rev-list-filters-objects.sh +15 -0
@@ -229,6 +229,21 @@ test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for tre test_must_be_empty rev_list_err ' +# Test tree:0 filter. + +test_expect_success 'verify tree:0 includes trees in "filtered" output' ' + git -C r3 rev-list --quiet --objects --filter-print-omitted \ + --filter=tree:0 HEAD >revs && + + awk -f print_1.awk revs | + sed s/~// | + xargs -n1 git -C r3 cat-file -t >unsorted_filtered_types && + + sort -u unsorted_filtered_types >filtered_types && + printf "blob\ntree\n" >expected && + test_cmp expected filtered_types +' + # Delete some loose objects and use rev-list, but WITHOUT any filtering. # This models previously omitted objects that we did not receive.
Signature · 1 line
2.19.0.605.g01d371f741-goog
Re: [PATCH v11 8/8] list-objects-filter: implement filter tree:0
Quoted 7 lines · Matthew DeVore
The name "tree:0" allows later filtering based on depth, i.e. "tree:1" would filter out all but the root tree and blobs. In order to avoid confusion between 0 and capital O, the documentation was worded in a somewhat round-about way that also hints at this future improvement to the feature.
Signed-off-by Matthew DeVore <[email protected]>
Thanks.
Documentation/rev-list-options.txt the requested refs. · line 738
Quoted 13 lines
Documentation/rev-list-options.txt +5
-0@@ -731,6 +731,11 @@ the requested refs. + The form '--filter=sparse:path=<path>' similarly uses a sparse-checkout specification contained in <path>. ++ +The form '--filter=tree:<depth>' omits all blobs and trees whose depth +from the root tree is >= <depth> (minimum depth if an object is located +at multiple depths in the commits traversed). Currently, only <depth>=0 +is supported, which omits all blobs and trees.
OK.
t/t5317-pack-objects-filter-objects.sh test_expect_success 'get an error for missing tree object' ' · line 73
Quoted 7 lines
t/t5317-pack-objects-filter-objects.sh +0
-0@@ -72,6 +72,34 @@ test_expect_success 'get an error for missing tree object' ' grep -q "bad tree object" bad_tree '
As output made inside test_expect_{succcess,failure} are discarded by default and shown while debugging tests, there is no strong reason to use "grep -q" in our tests. I saw a few instances of "grep -q" added in this series including this one
test_must_fail grep -q "$file_4" observed
that should probably be
! grep "$file_4" observed
t/t5317-pack-objects-filter-objects.sh test_expect_success 'get an error for missing tree object' '
Quoted 3 lines
t/t5317-pack-objects-filter-objects.sh +2
-0+ printf "blob\ncommit\ntree\n" >unique_types.expected && + printf "blob\ntree\n" >expected &&
Using test_write_lines is probably easier to read.
Other than these two minor classes of nits, the series look quite well cooked to me.
Thanks.
Re: [PATCH v11 8/8] list-objects-filter: implement filter tree:0
Quoted 11 lines · Junio C Hamano
As output made inside test_expect_{succcess,failure} are discarded by default and shown while debugging tests, there is no strong reason to use "grep -q" in our tests. I saw a few instances of "grep -q" added in this series including this one
test_must_failgrep -q "$file_4" observedthat should probably be
! grep "$file_4" observed
Yeah, I remember I read in the testing guidelines that you should just use ! for non-Git commands since it's not our job to make sure these tools are not crashing. Thank you for pointing this out.
Quoted 6 lines
Review
Quoted 4 lines
(quoted diff) +2
-0+ printf "blob\ncommit\ntree\n" >unique_types.expected && + printf "blob\ntree\n" >expected &&Using
test_write_linesis probably easier to read.
Done. Below is an interdiff. Let me know if you want a reroll soon. Otherwise, I will send one later this week.
- Matt
(quoted diff) +0 -0
b/t/t5317-pack-objects-filter-objects.sh
index 510d3537f..d9dccf4d4 100755
t/t5317-pack-objects-filter-objects.sh +0 -0
@@ -69,7 +69,7 @@ test_expect_success 'get an error for missing tree object' ' test_must_fail git -C r5 pack-objects --rev --stdout
2>bad_tree <<-EOF &&
HEAD
EOF
- grep -q "bad tree object" bad_tree
+ grep "bad tree object" bad_tree
'
test_expect_success 'setup for tests of tree:0' '
t/t5616-partial-clone.sh +0 -0
@@ -192,7 +192,7 @@ test_expect_success 'use fsck before and after
manually fetching a missing subtr
xargs -n1 git -C dst cat-file -t >fetched_types &&
sort -u fetched_types >unique_types.observed &&- printf "blob\ncommit\ntree\n" >unique_types.expected &&
+ test_write_lines blob commit tree >unique_types.expected &&
test_cmp unique_types.expected unique_types.observed
'
(quoted diff) +0 -0
b/t/t6112-rev-list-filters-objects.sh
index c8e3d87c4..08e0c7db6 100755
t/t6112-rev-list-filters-objects.sh +0 -0
@@ -38,8 +38,8 @@ test_expect_success 'specify blob explicitly
prevents filtering' '
awk -f print_2.awk) &&
git -C r1 rev-list --objects --filter=blob:none HEAD $file_3Quoted 1 line
observed &&
- grep -q "$file_3" observed && - test_must_fail grep -q "$file_4" observed + grep "$file_3" observed && + ! grep "$file_4" observed
'
test_expect_success 'verify emitted+omitted == all' '
@@ -240,7 +240,7 @@ test_expect_success 'verify tree:0 includes trees in "filtered" output' '
xargs -n1 git -C r3 cat-file -t >unsorted_filtered_types &&
sort -u unsorted_filtered_types >filtered_types &&- printf "blob\ntree\n" >expected &&
+ test_write_lines blob tree >expected &&
test_cmp expected filtered_types
'
v12 cover filter: support for excluding all trees and blobs
In reply to Matthew DeVoreHere is a re-roll-up since I haven't received any additional corrections for almost a week. The changes are very slight and just for clean-up so it is ready to be promoted to master.
This is the interdiff from last time:
t/t5317-pack-objects-filter-objects.sh +1 -1
@@ -69,7 +69,7 @@ test_expect_success 'get an error for missing tree object' ' test_must_fail git -C r5 pack-objects --rev --stdout 2>bad_tree <<-EOF && HEAD EOF - grep -q "bad tree object" bad_tree + grep "bad tree object" bad_tree ' test_expect_success 'setup for tests of tree:0' '
t/t5616-partial-clone.sh +1 -1
@@ -192,7 +192,7 @@ test_expect_success 'use fsck before and after manually fetching a missing subtr xargs -n1 git -C dst cat-file -t >fetched_types && sort -u fetched_types >unique_types.observed && - printf "blob\ncommit\ntree\n" >unique_types.expected && + test_write_lines blob commit tree >unique_types.expected && test_cmp unique_types.expected unique_types.observed '
t/t6112-rev-list-filters-objects.sh +3 -3
@@ -38,8 +38,8 @@ test_expect_success 'specify blob explicitly prevents filtering' ' awk -f print_2.awk) && git -C r1 rev-list --objects --filter=blob:none HEAD $file_3 >observed && - grep -q "$file_3" observed && - test_must_fail grep -q "$file_4" observed + grep "$file_3" observed && + ! grep "$file_4" observed ' test_expect_success 'verify emitted+omitted == all' '
@@ -240,7 +240,7 @@ test_expect_success 'verify tree:0 includes trees in "filtered" output' ' xargs -n1 git -C r3 cat-file -t >unsorted_filtered_types && sort -u unsorted_filtered_types >filtered_types && - printf "blob\ntree\n" >expected && + test_write_lines blob tree >expected && test_cmp expected filtered_types '
Thanks,
Matthew DeVore (8):
list-objects: store common func args in struct
list-objects: refactor to process_tree_contents
list-objects: always parse trees gently
rev-list: handle missing tree objects properly
revision: mark non-user-given objects instead
list-objects-filter: use BUG rather than die list-objects-filter-options: do not over-strbuf_init
list-objects-filter: implement filter tree:0
Documentation/rev-list-options.txt | 5 + builtin/rev-list.c | 11 +- list-objects-filter-options.c | 19 +- list-objects-filter-options.h | 1 + list-objects-filter.c | 60 ++++++- list-objects.c | 232 +++++++++++++------------ revision.c | 1 - revision.h | 26 ++- t/t0410-partial-clone.sh | 45 +++++ t/t5317-pack-objects-filter-objects.sh | 41 +++++ t/t5616-partial-clone.sh | 42 +++++ t/t6112-rev-list-filters-objects.sh | 49 ++++++ 12 files changed, 404 insertions(+), 128 deletions(-)
Signature · 1 line
2.19.1.331.ge82ca0e54c-goog
PATCH v12 1/8 list-objects: store common func args in struct
This will make utility functions easier to create, as done by the next patch.
--- list-objects.c | 158 +++++++++++++++++++++++-------------------------- 1 file changed, 74 insertions(+), 84 deletions(-)
list-objects.c +74 -84
@@ -12,20 +12,25 @@#include "packfile.h" #include "object-store.h" -static void process_blob (struct rev_info *revs, +struct traversal_context { +struct rev_info *revs; +show_object_fn show_object; +show_commit_fn show_commit; +void *show_data; +filter_object_fn filter_fn; +void *filter_data; +}; + +static void process_blob (struct traversal_context *ctx,struct blob *blob, -show_object_fn show,struct strbuf *path, -const char *name, -void *cb_data, -filter_object_fn filter_fn, -void *filter_data) +const char *name) {struct object *obj = &blob->object;size_t pathlen;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; -if (!revs->blob_objects) +if (!ctx->revs->blob_objects)return ;if (!obj)die ("bad blob object" );
@@ -41,21 +46,21 @@ static void process_blob(struct rev_info *revs,* may cause the actual filter to report an incomplete list * of missing objects. */ -if (revs->exclude_promisor_objects && +if (ctx->revs->exclude_promisor_objects && !has_object_file (&obj->oid) &&is_promisor_object (&obj->oid))return ; pathlen = path->len;strbuf_addstr (path, name); -if (!(obj->flags &USER_GIVEN ) && filter_fn) - r =filter_fn (LOFS_BLOB , obj, - path->buf, &path->buf[pathlen], - filter_data); +if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) + r = ctx->filter_fn (LOFS_BLOB , obj, + path->buf, &path->buf[pathlen], + ctx->filter_data);if (r &LOFR_MARK_SEEN ) obj->flags |=SEEN ;if (r &LOFR_DO_SHOW ) -show (obj, path->buf, cb_data); + ctx->show_object (obj, path->buf, ctx->show_data);strbuf_setlen (path, pathlen); }
@@ -81,26 +86,21 @@ static void process_blob(struct rev_info *revs,* the link, and how to do it. Whether it necessarily makes * any sense what-so-ever to ever do that is another issue. */ -static void process_gitlink (struct rev_info *revs, +static void process_gitlink (struct traversal_context *ctx,const unsigned char *sha1, -show_object_fn show,struct strbuf *path, -const char *name, -void *cb_data) +const char *name) {/* Nothing to do */ } -static void process_tree (struct rev_info *revs, +static void process_tree (struct traversal_context *ctx,struct tree *tree, -show_object_fn show,struct strbuf *base, -const char *name, -void *cb_data, -filter_object_fn filter_fn, -void *filter_data) +const char *name) {struct object *obj = &tree->object; +struct rev_info *revs = ctx->revs;struct tree_desc desc;struct name_entry entry; enum interesting match = revs->diffopt.pathspec.nr == 0 ?
@@ -133,14 +133,14 @@ static void process_tree(struct rev_info *revs, }strbuf_addstr (base, name); -if (!(obj->flags &USER_GIVEN ) && filter_fn) - r =filter_fn (LOFS_BEGIN_TREE , obj, - base->buf, &base->buf[baselen], - filter_data); +if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) + r = ctx->filter_fn (LOFS_BEGIN_TREE , obj, + base->buf, &base->buf[baselen], + ctx->filter_data);if (r &LOFR_MARK_SEEN ) obj->flags |=SEEN ;if (r &LOFR_DO_SHOW ) -show (obj, base->buf, cb_data); + ctx->show_object (obj, base->buf, ctx->show_data);if (base->len)strbuf_addch (base,'/' );
@@ -157,29 +157,25 @@ static void process_tree(struct rev_info *revs, }if (S_ISDIR (entry.mode)) -process_tree (revs, +process_tree (ctx,lookup_tree (the_repository, entry.oid), - show, base, entry.path, - cb_data, filter_fn, filter_data); + base, entry.path);else if (S_ISGITLINK (entry.mode)) -process_gitlink (revs, entry.oid->hash, - show, base, entry.path, - cb_data); +process_gitlink (ctx, entry.oid->hash, base, entry.path);else -process_blob (revs, +process_blob (ctx,lookup_blob (the_repository, entry.oid), - show, base, entry.path, - cb_data, filter_fn, filter_data); + base, entry.path); } -if (!(obj->flags &USER_GIVEN ) && filter_fn) { - r =filter_fn (LOFS_END_TREE , obj, - base->buf, &base->buf[baselen], - filter_data); +if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { + r = ctx->filter_fn (LOFS_END_TREE , obj, + base->buf, &base->buf[baselen], + ctx->filter_data);if (r &LOFR_MARK_SEEN ) obj->flags |=SEEN ;if (r &LOFR_DO_SHOW ) -show (obj, base->buf, cb_data); + ctx->show_object (obj, base->buf, ctx->show_data); }strbuf_setlen (base, baselen);
@@ -242,19 +238,15 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)add_pending_object (revs, &tree->object,"" ); } -static void traverse_trees_and_blobs (struct rev_info *revs, -struct strbuf *base, -show_object_fn show_object, -void *show_data, -filter_object_fn filter_fn, -void *filter_data) +static void traverse_trees_and_blobs (struct traversal_context *ctx, +struct strbuf *base) {int i;assert (base->len ==0 ); -for (i =0 ; i < revs->pending.nr; i++) { -struct object_array_entry *pending = revs->pending.objects + i; +for (i =0 ; i < ctx->revs->pending.nr; i++) { +struct object_array_entry *pending = ctx->revs->pending.objects + i;struct object *obj = pending->item;const char *name = pending->name;const char *path = pending->path;
@@ -262,62 +254,49 @@ static void traverse_trees_and_blobs(struct rev_info *revs,continue ;if (obj->type ==OBJ_TAG ) { obj->flags |=SEEN ; -show_object (obj, name, show_data); + ctx->show_object (obj, name, ctx->show_data);continue ; }if (!path) path ="" ;if (obj->type ==OBJ_TREE ) { -process_tree (revs, (struct tree *)obj, show_object, - base, path, show_data, - filter_fn, filter_data); +process_tree (ctx, (struct tree *)obj, base, path);continue ; }if (obj->type ==OBJ_BLOB ) { -process_blob (revs, (struct blob *)obj, show_object, - base, path, show_data, - filter_fn, filter_data); +process_blob (ctx, (struct blob *)obj, base, path);continue ; }die ("unknown pending object %s (%s)" ,oid_to_hex (&obj->oid), name); } -object_array_clear (&revs->pending); +object_array_clear (&ctx->revs->pending); } -static void do_traverse (struct rev_info *revs, -show_commit_fn show_commit, -show_object_fn show_object, -void *show_data, -filter_object_fn filter_fn, -void *filter_data) +static void do_traverse (struct traversal_context *ctx) {struct commit *commit;struct strbuf csp;/* callee's scratch pad */ strbuf_init (&csp,PATH_MAX ); -while ((commit =get_revision (revs)) !=NULL ) { +while ((commit =get_revision (ctx->revs)) !=NULL ) {/* * an uninteresting boundary commit may not have its tree * parsed yet, but we are not going to show them anyway */ if (get_commit_tree (commit)) -add_pending_tree (revs,get_commit_tree (commit)); -show_commit (commit, show_data); +add_pending_tree (ctx->revs,get_commit_tree (commit)); + ctx->show_commit (commit, ctx->show_data); -if (revs->tree_blobs_in_commit_order) +if (ctx->revs->tree_blobs_in_commit_order)/* * NEEDSWORK: Adding the tree and then flushing it here * needs a reallocation for each commit. Can we pass the * tree directory without allocation churn? */ -traverse_trees_and_blobs (revs, &csp, - show_object, show_data, - filter_fn, filter_data); +traverse_trees_and_blobs (ctx, &csp); } -traverse_trees_and_blobs (revs, &csp, - show_object, show_data, - filter_fn, filter_data); +traverse_trees_and_blobs (ctx, &csp);strbuf_release (&csp); }
@@ -326,7 +305,14 @@ void traverse_commit_list(struct rev_info *revs,show_object_fn show_object, void *show_data) { -do_traverse (revs ,show_commit ,show_object ,show_data ,NULL ,NULL ); + struct traversal_context ctx; + ctx.revs = revs; + ctx.show_commit = show_commit; + ctx.show_object = show_object; + ctx.show_data = show_data; + ctx.filter_fn =NULL ; + ctx.filter_data =NULL ; +do_traverse (&ctx); } void traverse_commit_list_filtered(
@@ -337,14 +323,18 @@ void traverse_commit_list_filtered(void *show_data, struct oidset *omitted) { - filter_object_fn filter_fn = NULL; + struct traversal_context ctx;filter_free_fn filter_free_fn =NULL ; -void *filter_data =NULL ; - - filter_data =list_objects_filter__init (omitted, filter_options, - &filter_fn, &filter_free_fn); -do_traverse (revs, show_commit, show_object, show_data, - filter_fn, filter_data); -if (filter_data && filter_free_fn) -filter_free_fn (filter_data); + + ctx.revs = revs; + ctx.show_object = show_object; + ctx.show_commit = show_commit; + ctx.show_data = show_data; + ctx.filter_fn =NULL ; + + ctx.filter_data =list_objects_filter__init (omitted, filter_options, + &ctx.filter_fn, &filter_free_fn); +do_traverse (&ctx); +if (ctx.filter_data && filter_free_fn) +filter_free_fn (ctx.filter_data); }
Signature · 1 line
2.19.1.331.ge82ca0e54c-goog
PATCH v12 2/8 list-objects: refactor to process_tree_contents
In reply to Matthew DeVoreThis will be used in a follow-up patch to reduce indentation needed when invoking the logic conditionally. i.e. rather than:
if (foo) {
while (...) {
/* this is very indented */
}
}we will have:
if (foo)
process_tree_contents(...);
--- list-objects.c | 68 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 27 deletions(-)
list-objects.c +41 -27
@@ -94,6 +94,46 @@ static void process_gitlink(struct traversal_context *ctx,/* Nothing to do */ } +static void process_tree (struct traversal_context *ctx, +struct tree *tree, +struct strbuf *base, +const char *name); + +static void process_tree_contents (struct traversal_context *ctx, +struct tree *tree, +struct strbuf *base) +{ +struct tree_desc desc; +struct name_entry entry; +enum interesting match = ctx->revs->diffopt.pathspec.nr ==0 ? + all_entries_interesting : entry_not_interesting; + +init_tree_desc (&desc, tree->buffer, tree->size); + +while (tree_entry (&desc, &entry)) { +if (match != all_entries_interesting) { + match =tree_entry_interesting (&entry, base,0 , + &ctx->revs->diffopt.pathspec); +if (match == all_entries_not_interesting) +break ; +if (match == entry_not_interesting) +continue ; + } + +if (S_ISDIR (entry.mode)) +process_tree (ctx, +lookup_tree (the_repository, entry.oid), + base, entry.path); +else if (S_ISGITLINK (entry.mode)) +process_gitlink (ctx, entry.oid->hash, + base, entry.path); +else +process_blob (ctx, +lookup_blob (the_repository, entry.oid), + base, entry.path); + } +} + static void process_tree(struct traversal_context *ctx, struct tree *tree, struct strbuf *base,
@@ -101,10 +141,6 @@ static void process_tree(struct traversal_context *ctx, {struct object *obj = &tree->object;struct rev_info *revs = ctx->revs; -struct tree_desc desc; -struct name_entry entry; -enum interesting match = revs->diffopt.pathspec.nr ==0 ? - all_entries_interesting: entry_not_interesting;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; int gently = revs->ignore_missing_links ||
@@ -144,29 +180,7 @@ static void process_tree(struct traversal_context *ctx,if (base->len)strbuf_addch (base,'/' ); -init_tree_desc (&desc, tree->buffer, tree->size); - -while (tree_entry (&desc, &entry)) { -if (match != all_entries_interesting) { - match =tree_entry_interesting (&entry, base,0 , - &revs->diffopt.pathspec); -if (match == all_entries_not_interesting) -break ; -if (match == entry_not_interesting) -continue ; - } - -if (S_ISDIR (entry.mode)) -process_tree (ctx, -lookup_tree (the_repository, entry.oid), - base, entry.path); -else if (S_ISGITLINK (entry.mode)) -process_gitlink (ctx, entry.oid->hash, base, entry.path); -else -process_blob (ctx, -lookup_blob (the_repository, entry.oid), - base, entry.path); - } +process_tree_contents (ctx, tree, base);if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn(LOFS_END_TREE, obj,
Signature · 1 line
2.19.1.331.ge82ca0e54c-goog
PATCH v12 3/8 list-objects: always parse trees gently
In reply to Matthew DeVoreIf parsing fails when revs->ignore_missing_links and revs->exclude_promisor_objects are both false, we print the OID anyway in the die("bad tree object...") call, so any message printed by parse_tree_gently() is superfluous.
--- list-objects.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
list-objects.c +1 -3
@@ -143,8 +143,6 @@ static void process_tree(struct traversal_context *ctx,struct rev_info *revs = ctx->revs;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; -int gently = revs->ignore_missing_links || - revs->exclude_promisor_objects;if (!revs->tree_objects)return ;
@@ -152,7 +150,7 @@ static void process_tree(struct traversal_context *ctx,die ("bad tree object" );if (obj->flags & (UNINTERESTING |SEEN ))return ; -if (parse_tree_gently (tree, gently) <0 ) { +if (parse_tree_gently (tree,1 ) <0 ) {if (revs->ignore_missing_links)return ;
Signature · 1 line
2.19.1.331.ge82ca0e54c-goog
PATCH v12 4/8 rev-list: handle missing tree objects properly
In reply to Matthew DeVorePreviously, we assumed only blob objects could be missing. This patch makes rev-list handle missing trees like missing blobs. The --missing=* and --exclude-promisor-objects flags now work for trees as they already do for blobs. This is demonstrated in t6112.
--- builtin/rev-list.c | 11 ++++--- list-objects.c | 11 +++++-- revision.h | 15 +++++++++ t/t0410-partial-clone.sh | 45 ++++++++++++++++++++++++++ t/t5317-pack-objects-filter-objects.sh | 13 ++++++++ t/t6112-rev-list-filters-objects.sh | 22 +++++++++++++ 6 files changed, 110 insertions(+), 7 deletions(-)
builtin/rev-list.c +7 -4
@@ -6,6 +6,7 @@#include "list-objects.h" #include "list-objects-filter.h" #include "list-objects-filter-options.h" +#include "object.h" #include "object-store.h" #include "pack.h" #include "pack-bitmap.h"
@@ -209,7 +210,8 @@ static inline void finish_object__ma(struct object *obj)*/ switch (arg_missing_action) {case MA_ERROR : -die ("missing blob object '%s'" ,oid_to_hex (&obj->oid)); +die ("missing %s object '%s'" , +type_name (obj->type),oid_to_hex (&obj->oid));return ;case MA_ALLOW_ANY :
@@ -222,8 +224,8 @@ static inline void finish_object__ma(struct object *obj)case MA_ALLOW_PROMISOR :if (is_promisor_object (&obj->oid))return ; -die ("unexpected missing blob object '%s'" , -oid_to_hex (&obj->oid)); +die ("unexpected missing %s object '%s'" , +type_name (obj->type),oid_to_hex (&obj->oid));return ;default :
@@ -235,7 +237,7 @@ static inline void finish_object__ma(struct object *obj)static int finish_object (struct object *obj,const char *name,void *cb_data) {struct rev_list_info *info = cb_data; -if (obj->type ==OBJ_BLOB && !has_object_file (&obj->oid)) { +if (!has_object_file (&obj->oid)) {finish_object__ma (obj);return 1 ; }
@@ -373,6 +375,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)init_revisions (&revs, prefix); revs.abbrev =DEFAULT_ABBREV ; revs.commit_format =CMIT_FMT_UNSPECIFIED ; + revs.do_not_die_on_missing_tree =1 ;/* * Scan the argument list before invoking setup_revisions(), so that we
list-objects.c +8 -3
@@ -143,6 +143,7 @@ static void process_tree(struct traversal_context *ctx,struct rev_info *revs = ctx->revs;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; +int failed_parse;if (!revs->tree_objects)return ;
@@ -150,7 +151,9 @@ static void process_tree(struct traversal_context *ctx,die ("bad tree object" );if (obj->flags & (UNINTERESTING |SEEN ))return ; -if (parse_tree_gently (tree,1 ) <0 ) { + + failed_parse =parse_tree_gently (tree,1 ); +if (failed_parse) {if (revs->ignore_missing_links)return ;
@@ -163,7 +166,8 @@ static void process_tree(struct traversal_context *ctx, is_promisor_object(&obj->oid))return ; -die ("bad tree object %s" ,oid_to_hex (&obj->oid)); +if (!revs->do_not_die_on_missing_tree) +die ("bad tree object %s" ,oid_to_hex (&obj->oid)); }strbuf_addstr (base, name);
@@ -178,7 +182,8 @@ static void process_tree(struct traversal_context *ctx,if (base->len)strbuf_addch (base,'/' ); -process_tree_contents (ctx, tree, base); +if (!failed_parse) +process_tree_contents (ctx, tree, base);if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn(LOFS_END_TREE, obj,
revision.h +15 -0
@@ -126,6 +126,21 @@ struct rev_info { line_level_traverse:1, tree_blobs_in_commit_order:1, +/* +* Blobs are shown without regard for their existence. +* But not so for trees: unless exclude_promisor_objects +* is set and the tree in question is a promisor object; +* OR ignore_missing_links is set, the revision walker +* dies with a "bad tree object HASH" message when +* encountering a missing tree. For callers that can +* handle missing trees and want them to be filterable +* and showable, set this to true. The revision walker +* will filter and show such a missing tree as usual, +* but will not attempt to recurse into this tree +* object. +*/ + do_not_die_on_missing_tree:1, + /* for internal use only */ exclude_promisor_objects:1;
t/t0410-partial-clone.sh +45 -0
51 lines not on this page · raw message
t/t5317-pack-objects-filter-objects.sh +13 -0
@@ -59,6 +59,19 @@ test_expect_success 'verify normal and blob:none packfiles have same commits/tre test_cmp observed expected ' +test_expect_success 'get an error for missing tree object' ' + git init r5 && + echo foo >r5/foo && + git -C r5 add foo && + git -C r5 commit -m "foo" && + del=$(git -C r5 rev-parse HEAD^{tree} | sed "s|..|&/|") && + rm r5/.git/objects/$del && + test_must_fail git -C r5 pack-objects --rev --stdout 2>bad_tree <<-EOF && + HEAD + EOF + grep "bad tree object" bad_tree +' + # Test blob:limit=<n>[kmg] filter. # We boundary test around the size parameter. The filter is strictly less than # the value, so size 500 and 1000 should have the same results, but 1001 should
t/t6112-rev-list-filters-objects.sh +22 -0
@@ -195,6 +195,28 @@ test_expect_success 'verify sparse:oid=oid-ish omits top-level files' ' test_cmp observed expected ' +test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for trees' ' + TREE=$(git -C r3 rev-parse HEAD:dir1) && + + # Create a spare repo because we will be deleting objects from this one. + git clone r3 r3.b && + + rm r3.b/.git/objects/$(echo $TREE | sed "s|^..|&/|") && + + git -C r3.b rev-list --quiet --missing=print --objects HEAD \ + >missing_objs 2>rev_list_err && + echo "?$TREE" >expected && + test_cmp expected missing_objs && + + # do not complain when a missing tree cannot be parsed + test_must_be_empty rev_list_err && + + git -C r3.b rev-list --missing=allow-any --objects HEAD \ + >objs 2>rev_list_err && + ! grep $TREE objs && + test_must_be_empty rev_list_err +' + # Delete some loose objects and use rev-list, but WITHOUT any filtering. # This models previously omitted objects that we did not receive.
Signature · 1 line
2.19.1.331.ge82ca0e54c-goog
PATCH v12 5/8 revision: mark non-user-given objects instead
In reply to Matthew DeVoreCurrently, list-objects.c incorrectly treats all root trees of commits as USER_GIVEN. Also, it would be easier to mark objects that are non-user-given instead of user-given, since the places in the code where we access an object through a reference are more obvious than the places where we access an object that was given by the user.
Resolve these two problems by introducing a flag NOT_USER_GIVEN that marks blobs and trees that are non-user-given, replacing USER_GIVEN. (Only blobs and trees are marked because this mark is only used when filtering objects, and filtering of other types of objects is not supported yet.)
This fixes a bug in that git rev-list behaved differently from git pack-objects. pack-objects would *not* filter objects given explicitly on the command line and rev-list would filter. This was because the two commands used a different function to add objects to the rev_info struct. This seems to have been an oversight, and pack-objects has the correct behavior, so I added a test to make sure that rev-list now behaves properly.
--- list-objects.c | 31 +++++++++++++++++------------ revision.c | 1 - revision.h | 11 ++++++++-- t/t6112-rev-list-filters-objects.sh | 12 +++++++++++ 4 files changed, 39 insertions(+), 16 deletions(-)
list-objects.c +18 -13
@@ -53,7 +53,7 @@ static void process_blob(struct traversal_context *ctx, pathlen = path->len;strbuf_addstr (path, name); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) r = ctx->filter_fn (LOFS_BLOB , obj, path->buf, &path->buf[pathlen], ctx->filter_data);
@@ -120,17 +120,19 @@ static void process_tree_contents(struct traversal_context *ctx,continue ; } -if (S_ISDIR (entry.mode)) -process_tree (ctx, -lookup_tree (the_repository, entry.oid), - base, entry.path); +if (S_ISDIR (entry.mode)) { +struct tree *t =lookup_tree (the_repository, entry.oid); + t->object.flags |=NOT_USER_GIVEN ; +process_tree (ctx, t, base, entry.path); + }else if (S_ISGITLINK (entry.mode))process_gitlink (ctx, entry.oid->hash, base, entry.path); -else -process_blob (ctx, -lookup_blob (the_repository, entry.oid), - base, entry.path); +else { +struct blob *b =lookup_blob (the_repository, entry.oid); + b->object.flags |=NOT_USER_GIVEN ; +process_blob (ctx, b, base, entry.path); + } } }
@@ -171,7 +173,7 @@ static void process_tree(struct traversal_context *ctx, }strbuf_addstr (base, name); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) r = ctx->filter_fn (LOFS_BEGIN_TREE , obj, base->buf, &base->buf[baselen], ctx->filter_data);
@@ -185,7 +187,7 @@ static void process_tree(struct traversal_context *ctx,if (!failed_parse)process_tree_contents (ctx, tree, base); -if (!(obj->flags &USER_GIVEN ) && ctx->filter_fn) { +if ((obj->flags &NOT_USER_GIVEN ) && ctx->filter_fn) { r = ctx->filter_fn (LOFS_END_TREE , obj, base->buf, &base->buf[baselen], ctx->filter_data);
@@ -301,8 +303,11 @@ static void do_traverse(struct traversal_context *ctx)* an uninteresting boundary commit may not have its tree * parsed yet, but we are not going to show them anyway */ -if (get_commit_tree (commit)) -add_pending_tree (ctx->revs,get_commit_tree (commit)); +if (get_commit_tree (commit)) { +struct tree *tree =get_commit_tree (commit); + tree->object.flags |=NOT_USER_GIVEN ; +add_pending_tree (ctx->revs, tree); + } ctx->show_commit (commit, ctx->show_data);if (ctx->revs->tree_blobs_in_commit_order)
revision.c +0 -1
@@ -175,7 +175,6 @@ static void add_pending_object_with_path(struct rev_info *revs,strbuf_release (&buf);return ;/* do not add the commit itself */ } - obj->flags |=USER_GIVEN ;add_object_array_with_path (obj, name, &revs->pending, mode, path); }
revision.h +9 -2
@@ -21,9 +21,16 @@#define SYMMETRIC_LEFT (1u<<8)#define PATCHSAME (1u<<9)#define BOTTOM (1u<<10) -#define USER_GIVEN (1u<<25)/* given directly by the user */ +/* +* 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) -#define ALL_REV_FLAGS (((1u<<11)-1) | USER_GIVEN | TRACK_LINEAR) +#define ALL_REV_FLAGS (((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR)#define DECORATE_SHORT_REFS 1#define DECORATE_FULL_REFS 2
t/t6112-rev-list-filters-objects.sh +12 -0
@@ -30,6 +30,18 @@ test_expect_success 'verify blob:none omits all 5 blobs' ' test_cmp observed expected ' +test_expect_success 'specify blob explicitly prevents filtering' ' + file_3=$(git -C r1 ls-files -s file.3 | + awk -f print_2.awk) && + + file_4=$(git -C r1 ls-files -s file.4 | + awk -f print_2.awk) && + + git -C r1 rev-list --objects --filter=blob:none HEAD $file_3 >observed && + grep "$file_3" observed && + ! grep "$file_4" observed +' + test_expect_success 'verify emitted+omitted == all' ' git -C r1 rev-list HEAD --objects \ | awk -f print_1.awk \
Signature · 1 line
2.19.1.331.ge82ca0e54c-goog
PATCH v12 6/8 list-objects-filter: use BUG rather than die
In reply to Matthew DeVoreIn some cases in this file, BUG makes more sense than die. In such cases, a we get there from a coding error rather than a user error.
'return' has been removed following some instances of BUG since BUG does not return.
--- list-objects-filter.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
list-objects-filter.c +4 -7
@@ -44,8 +44,7 @@ static enum list_objects_filter_result filter_blobs_none(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -102,8 +101,7 @@ static enum list_objects_filter_result filter_blobs_limit(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -208,8 +206,7 @@ static enum list_objects_filter_result filter_sparse(switch (filter_situation) {default : -die ("unknown filter_situation" ); -return LOFR_ZERO ; +BUG ("unknown filter_situation: %d" , filter_situation);case LOFS_BEGIN_TREE :assert (obj->type ==OBJ_TREE );
@@ -389,7 +386,7 @@ void *list_objects_filter__init(assert ((sizeof (s_filters) /sizeof (s_filters[0 ])) ==LOFC__COUNT );if (filter_options->choice >=LOFC__COUNT ) -die ("invalid list-objects filter choice: %d" , +BUG ("invalid list-objects filter choice: %d" , filter_options->choice); init_fn = s_filters[filter_options->choice];
Signature · 1 line
2.19.1.331.ge82ca0e54c-goog
PATCH v12 7/8 list-objects-filter-options: do not over-strbuf_init
In reply to Matthew DeVoreThe function gently_parse_list_objects_filter is either called with errbuf=STRBUF_INIT or errbuf=NULL, but that function calls strbuf_init when errbuf is not NULL. strbuf_init is only necessary if errbuf contains garbage, and risks a memory leak if errbuf already has a non-STRBUF_INIT state. It should be the caller's responsibility to make sure errbuf is not garbage, since garbage content is easily avoidable with STRBUF_INIT.
--- list-objects-filter-options.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
list-objects-filter-options.c +2 -4
@@ -30,7 +30,6 @@ static int gently_parse_list_objects_filter(if (filter_options->choice) {if (errbuf) { -strbuf_init (errbuf,0 );strbuf_addstr ( errbuf,_ ("multiple filter-specs cannot be combined" ));
@@ -71,10 +70,9 @@ static int gently_parse_list_objects_filter(return 0 ; } -if (errbuf) { -strbuf_init (errbuf,0 ); +if (errbuf)strbuf_addf (errbuf,"invalid filter-spec '%s'" , arg); - } +memset (filter_options,0 ,sizeof (*filter_options));return 1 ; }
Signature · 1 line
2.19.1.331.ge82ca0e54c-goog
PATCH v12 8/8 list-objects-filter: implement filter tree:0
In reply to Matthew DeVoreTeach list-objects the "tree:0" filter which allows for filtering out all tree and blob objects (unless other objects are explicitly specified by the user). The purpose of this patch is to allow smaller partial clones.
The name of this filter - tree:0 - does not explicitly specify that it also filters out all blobs, but this should not cause much confusion because blobs are not at all useful without the trees that refer to them.
I also considered only:commits as a name, but this is inaccurate because it suggests that annotated tags are omitted, but actually they are included.
The name "tree:0" allows later filtering based on depth, i.e. "tree:1" would filter out all but the root tree and blobs. In order to avoid confusion between 0 and capital O, the documentation was worded in a somewhat round-about way that also hints at this future improvement to the feature.
--- Documentation/rev-list-options.txt | 5 +++ list-objects-filter-options.c | 13 +++++++ list-objects-filter-options.h | 1 + list-objects-filter.c | 49 ++++++++++++++++++++++++++ t/t5317-pack-objects-filter-objects.sh | 28 +++++++++++++++ t/t5616-partial-clone.sh | 42 ++++++++++++++++++++++ t/t6112-rev-list-filters-objects.sh | 15 ++++++++ 7 files changed, 153 insertions(+)
Documentation/rev-list-options.txt +5 -0
@@ -731,6 +731,11 @@ the requested refs. + The form '--filter=sparse:path=<path>' similarly uses a sparse-checkout specification contained in <path>. ++ +The form '--filter=tree:<depth>' omits all blobs and trees whose depth +from the root tree is >= <depth> (minimum depth if an object is located +at multiple depths in the commits traversed). Currently, only <depth>=0 +is supported, which omits all blobs and trees. --no-filter:: Turn off any previous `--filter=` argument.
list-objects-filter-options.c +13 -0
@@ -49,6 +49,19 @@ static int gently_parse_list_objects_filter(return 0 ; } + }else if (skip_prefix (arg,"tree:" , &v0)) { +unsigned long depth; +if (!git_parse_ulong (v0, &depth) || depth !=0 ) { +if (errbuf) { +strbuf_addstr ( + errbuf, +_ ("only 'tree:0' is supported" )); + } +return 1 ; + } + filter_options->choice =LOFC_TREE_NONE ; +return 0 ; + }else if (skip_prefix (arg,"sparse:oid=" , &v0)) {struct object_context oc;struct object_id sparse_oid;
list-objects-filter-options.h +1 -0
@@ -10,6 +10,7 @@ enum list_objects_filter_choice {LOFC_DISABLED =0 ,LOFC_BLOB_NONE ,LOFC_BLOB_LIMIT , +LOFC_TREE_NONE ,LOFC_SPARSE_OID ,LOFC_SPARSE_PATH ,LOFC__COUNT /* must be last */
list-objects-filter.c +49 -0
61 lines not on this page · raw message
t/t5317-pack-objects-filter-objects.sh +28 -0
@@ -72,6 +72,34 @@ test_expect_success 'get an error for missing tree object' ' grep "bad tree object" bad_tree ' +test_expect_success 'setup for tests of tree:0' ' + mkdir r1/subtree && + echo "This is a file in a subtree" >r1/subtree/file && + git -C r1 add subtree/file && + git -C r1 commit -m subtree +' + +test_expect_success 'verify tree:0 packfile has no blobs or trees' ' + git -C r1 pack-objects --rev --stdout --filter=tree:0 >commitsonly.pack <<-EOF && + HEAD + EOF + git -C r1 index-pack ../commitsonly.pack && + git -C r1 verify-pack -v ../commitsonly.pack >objs && + ! grep -E "tree|blob" objs +' + +test_expect_success 'grab tree directly when using tree:0' ' + # We should get the tree specified directly but not its blobs or subtrees. + git -C r1 pack-objects --rev --stdout --filter=tree:0 >commitsonly.pack <<-EOF && + HEAD: + EOF + git -C r1 index-pack ../commitsonly.pack && + git -C r1 verify-pack -v ../commitsonly.pack >objs && + awk "/tree|blob/{print \$1}" objs >trees_and_blobs && + git -C r1 rev-parse HEAD: >expected && + test_cmp expected trees_and_blobs +' + # Test blob:limit=<n>[kmg] filter. # We boundary test around the size parameter. The filter is strictly less than # the value, so size 500 and 1000 should have the same results, but 1001 should
t/t5616-partial-clone.sh +42 -0
48 lines not on this page · raw message
t/t6112-rev-list-filters-objects.sh +15 -0
@@ -229,6 +229,21 @@ test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for tre test_must_be_empty rev_list_err ' +# Test tree:0 filter. + +test_expect_success 'verify tree:0 includes trees in "filtered" output' ' + git -C r3 rev-list --quiet --objects --filter-print-omitted \ + --filter=tree:0 HEAD >revs && + + awk -f print_1.awk revs | + sed s/~// | + xargs -n1 git -C r3 cat-file -t >unsorted_filtered_types && + + sort -u unsorted_filtered_types >filtered_types && + test_write_lines blob tree >expected && + test_cmp expected filtered_types +' + # Delete some loose objects and use rev-list, but WITHOUT any filtering. # This models previously omitted objects that we did not receive.
Signature · 1 line
2.19.1.331.ge82ca0e54c-goog
Quoted 2 lines · Matthew DeVore
Here is a re-roll-up since I haven't received any additional corrections for almost a week.
Sorry, but doesn't this topic already in 'next'? If so, please make these small clean-ups as incremental patches.
Thansk.
Quoted 7 lines · Junio C Hamano
Quoted 2 lines · Matthew DeVore
Here is a re-roll-up since I haven't received any additional corrections for almost a week.
Sorry, but doesn't this topic already sit in 'next'? If so, please make these small clean-ups as incremental patches.
Here is what I'd queue for now, with forged s-o-by from you ;-).
Thanks.
-- >8 --
From: Matthew DeVore <[email protected]>
Date: Fri, 12 Oct 2018 13:01:41 -0700
Subject: [PATCH] filter-trees: code clean-up of tests
A few trivial updates to test to match the current best practices.
- avoid "grep -q" that strips potentially useful output from running tests under "-v".
- use test_write_lines to prepare multi-line expected output file
- reserve use of test_must_fail to "git" commands.
--- t/t5317-pack-objects-filter-objects.sh | 2 +- t/t5616-partial-clone.sh | 2 +- t/t6112-rev-list-filters-objects.sh | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-)
t/t5317-pack-objects-filter-objects.sh +1 -1
@@ -69,7 +69,7 @@ test_expect_success 'get an error for missing tree object' ' test_must_fail git -C r5 pack-objects --rev --stdout 2>bad_tree <<-EOF && HEAD EOF - grep -q "bad tree object" bad_tree + grep "bad tree object" bad_tree ' test_expect_success 'setup for tests of tree:0' '
t/t5616-partial-clone.sh +1 -1
@@ -192,7 +192,7 @@ test_expect_success 'use fsck before and after manually fetching a missing subtr xargs -n1 git -C dst cat-file -t >fetched_types && sort -u fetched_types >unique_types.observed && - printf "blob\ncommit\ntree\n" >unique_types.expected && + test_write_lines blob commit tree >unique_types.expected && test_cmp unique_types.expected unique_types.observed '
t/t6112-rev-list-filters-objects.sh +3 -3
@@ -38,8 +38,8 @@ test_expect_success 'specify blob explicitly prevents filtering' ' awk -f print_2.awk) && git -C r1 rev-list --objects --filter=blob:none HEAD $file_3 >observed && - grep -q "$file_3" observed && - test_must_fail grep -q "$file_4" observed + grep "$file_3" observed && + ! grep "$file_4" observed ' test_expect_success 'verify emitted+omitted == all' '
@@ -241,7 +241,7 @@ test_expect_success 'verify tree:0 includes trees in "filtered" output' ' xargs -n1 git -C r3 cat-file -t >unsorted_filtered_types && sort -u unsorted_filtered_types >filtered_types && - printf "blob\ntree\n" >expected && + test_write_lines blob tree >expected && test_cmp expected filtered_types '
Signature · 1 line
2.19.1-328-g5a0cc8aca7
Quoted 5 lines · Junio C Hamano
Quoted 2 lines · Junio C Hamano
Matthew DeVore <[email protected]> writes:
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Here is a re-roll-up since I haven't received any additional corrections for almost a week.
Sorry, but doesn't this topic already sit in 'next'? If so, please make these small clean-ups as incremental patches.
Here is what I'd queue for now, with forged s-o-by from you ;-).
Yes, this is fine, thank you! I've reapplied the patch in my own repo on top of "next" in case I need to fix it and re-send, but please queue what you have as-is.