Partial clone part 1: object filtering
Subsystems (2)
v3 cover Partial clone part 1: object filtering
From: Jeff Hostetler <[email protected]>
Here is V3 of the list-object filtering. This addresses the comments on the mailing list for the V2 series as well as the various TODO items I left in the code. I also documented some of the bit flags and fields that I added.
In the blob size filter, I removed the ".git*" pattern matching for special files. I don't think we need it any more and it simplifies the code. This patch series does not support traverse_bitmap_commit_list() and the --use-bitmap-index feature in rev-list, but by removing the ".git*" pattern matching now we should be able allow filtering and bitmaps to be used together in a future effort. (That is beyond the scope of the current partial-clone effort.)
With this patch series, I think part 1 is complete unless there are further comments or questions.
Jeff Hostetler (6):
dir: allow exclusions from blob in addition to file
oidmap: add oidmap iterator methods
oidset: add iterator methods to oidset
list-objects: filter objects in traverse_commit_list
rev-list: add list-objects filtering support
pack-objects: add list-objects filtering
Documentation/git-pack-objects.txt | 12 +- Documentation/git-rev-list.txt | 6 +- Documentation/rev-list-options.txt | 34 +++ Makefile | 2 + builtin/pack-objects.c | 28 ++- builtin/rev-list.c | 75 +++++- dir.c | 132 ++++++++--- dir.h | 3 + list-objects-filter-options.c | 148 ++++++++++++ list-objects-filter-options.h | 50 ++++ list-objects-filter.c | 401 +++++++++++++++++++++++++++++++++ list-objects-filter.h | 77 +++++++ list-objects.c | 95 ++++++-- list-objects.h | 13 +- object.h | 1 + oidmap.h | 22 ++ oidset.c | 10 + oidset.h | 36 +++ t/t5317-pack-objects-filter-objects.sh | 369 ++++++++++++++++++++++++++++++ t/t6112-rev-list-filters-objects.sh | 225 ++++++++++++++++++ 20 files changed, 1686 insertions(+), 53 deletions(-) create mode 100644 list-objects-filter-options.c create mode 100644 list-objects-filter-options.h create mode 100644 list-objects-filter.c create mode 100644 list-objects-filter.h create mode 100755 t/t5317-pack-objects-filter-objects.sh create mode 100755 t/t6112-rev-list-filters-objects.sh
Signature · 1 line
2.9.3
PATCH v3 1/6 dir: allow exclusions from blob in addition to file
From: Jeff Hostetler <[email protected]>
Refactor add_excludes() to separate the reading of the exclude file into a buffer and the parsing of the buffer into exclude_list items.
Add add_excludes_from_blob_to_list() to allow an exclude file be specified with an OID without assuming a local worktree or index exists.
Refactor read_skip_worktree_file_from_index() and add do_read_blob() to eliminate duplication of preliminary processing of blob contents.
--- dir.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++---------------- dir.h | 3 ++ 2 files changed, 104 insertions(+), 31 deletions(-)
dir.c +101 -31
@@ -220,6 +220,57 @@ int within_depth(const char *name, int namelen,return 1 ; } +/* +* Read the contents of the blob with the given OID into a buffer. +* Append a trailing LF to the end if the last line doesn't have one. +* +* Returns: +* -1 when the OID is invalid or unknown or does not refer to a blob. +* 0 when the blob is empty. +* 1 along with { data, size } of the (possibly augmented) buffer +* when successful. +* +* Optionally updates the given sha1_stat with the given OID (when valid). +*/ +static int do_read_blob (const struct object_id *oid, +struct sha1_stat *sha1_stat, +size_t *size_out, +char **data_out) +{ +enum object_type type; +unsigned long sz; +char *data; + + *size_out =0 ; + *data_out =NULL ; + + data =read_sha1_file (oid->hash, &type, &sz); +if (!data || type !=OBJ_BLOB ) { +free (data); +return -1 ; + } + +if (sha1_stat) { +memset (&sha1_stat->stat,0 ,sizeof (sha1_stat->stat)); +hashcpy (sha1_stat->sha1, oid->hash); + } + +if (sz ==0 ) { +free (data); +return 0 ; + } + +if (data[sz -1 ] !='\n' ) { + data =xrealloc (data,st_add (sz,1 )); + data[sz++] ='\n' ; + } + + *size_out =xsize_t (sz); + *data_out = data; + +return 1 ; +} +#define DO_MATCH_EXCLUDE (1<<0)#define DO_MATCH_DIRECTORY (1<<1)#define DO_MATCH_SUBMODULE (1<<2)
@@ -600,32 +651,22 @@ void add_exclude(const char *string, const char *base, x->el = el; } -static void *read_skip_worktree_file_from_index (const struct index_state *istate, -const char *path,size_t *size, -struct sha1_stat *sha1_stat) +static int read_skip_worktree_file_from_index (const struct index_state *istate, +const char *path, +size_t *size_out, +char **data_out, +struct sha1_stat *sha1_stat) {int pos, len; -unsigned long sz; -enum object_type type; -void *data; len =strlen (path); pos =index_name_pos (istate, path, len);if (pos <0 ) -return NULL ; +return -1 ;if (!ce_skip_worktree (istate->cache[pos])) -return NULL ; - data =read_sha1_file (istate->cache[pos]->oid.hash, &type, &sz); -if (!data || type !=OBJ_BLOB ) { -free (data); -return NULL ; - } - *size =xsize_t (sz); -if (sha1_stat) { -memset (&sha1_stat->stat,0 ,sizeof (sha1_stat->stat)); -hashcpy (sha1_stat->sha1, istate->cache[pos]->oid.hash); - } -return data; +return -1 ; + +return do_read_blob (&istate->cache[pos]->oid, sha1_stat, size_out, data_out); }/*
@@ -739,6 +780,10 @@ static void invalidate_directory(struct untracked_cache *uc, dir->dirs[i]->recurse =0 ; } +static int add_excludes_from_buffer (char *buf,size_t size, +const char *base,int baselen, +struct exclude_list *el); +/* * Given a file with name "fname", read it (either from disk, or from * an index if 'istate' is non-null), parse it and store the
@@ -754,9 +799,10 @@ static int add_excludes(const char *fname, const char *base, int baselen, struct sha1_stat *sha1_stat) {struct stat st; -int fd, i, lineno =1 ; +int r; +int fd;size_t size =0 ; -char *buf, *entry; +char *buf; fd =open (fname,O_RDONLY );if (fd <0 ||fstat (fd, &st) <0 ) {
@@ -764,17 +810,13 @@ static int add_excludes(const char *fname, const char *base, int baselen,warn_on_fopen_errors (fname); elseclose (fd ); -if (!istate || - (buf =read_skip_worktree_file_from_index (istate, fname, &size, sha1_stat)) ==NULL ) +if (!istate)return -1 ; -if (size ==0 ) { -free (buf); -return 0 ; - } -if (buf[size-1 ] !='\n' ) { - buf =xrealloc (buf,st_add (size,1 )); - buf[size++] ='\n' ; - } + r =read_skip_worktree_file_from_index (istate, fname, + &size, &buf, + sha1_stat); +if (r !=1 ) +return r; }else { size =xsize_t (st.st_size);if (size ==0 ) {
@@ -813,6 +855,17 @@ static int add_excludes(const char *fname, const char *base, int baselen, } } +add_excludes_from_buffer (buf, size, base, baselen, el); +return 0 ; +} + +static int add_excludes_from_buffer (char *buf,size_t size, +const char *base,int baselen, +struct exclude_list *el) +{ +int i, lineno =1 ; +char *entry; + el->filebuf = buf;if (skip_utf8_bom (&buf, size))
@@ -841,6 +894,23 @@ int add_excludes_from_file_to_list(const char *fname, const char *base,return add_excludes (fname, base, baselen, el, istate,NULL ); } +int add_excludes_from_blob_to_list ( +struct object_id *oid, +const char *base,int baselen, +struct exclude_list *el) +{ +char *buf; +size_t size; +int r; + + r =do_read_blob (oid,NULL , &size, &buf); +if (r !=1 ) +return r; + +add_excludes_from_buffer (buf, size, base, baselen, el); +return 0 ; +} +struct exclude_list *add_exclude_list (struct dir_struct *dir,int group_type,const char *src) {
dir.h +3 -0
@@ -256,6 +256,9 @@ extern struct exclude_list *add_exclude_list(struct dir_struct *dir,extern int add_excludes_from_file_to_list (const char *fname,const char *base,int baselen,struct exclude_list *el,struct index_state *istate);extern void add_excludes_from_file (struct dir_struct *,const char *fname); +extern int add_excludes_from_blob_to_list (struct object_id *oid, +const char *base,int baselen, +struct exclude_list *el);extern void parse_exclude_pattern (const char **string,int *patternlen,unsigned *flags,int *nowildcardlen);extern void add_exclude (const char *string,const char *base,int baselen,struct exclude_list *el,int srcpos);
Signature · 1 line
2.9.3
PATCH v3 2/6 oidmap: add oidmap iterator methods
In reply to Jeff HostetlerFrom: Jeff Hostetler <[email protected]>
Add the usual map iterator functions to oidmap.
--- oidmap.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
oidmap.h +22 -0
@@ -65,4 +65,26 @@ extern void *oidmap_put(struct oidmap *map, void *entry);*/ extern void *oidmap_remove (struct oidmap *map,const struct object_id *key); + +struct oidmap_iter { +struct hashmap_iter h_iter; +}; + +static inline void oidmap_iter_init (struct oidmap *map,struct oidmap_iter *iter) +{ +hashmap_iter_init (&map->map, &iter->h_iter); +} + +static inline void *oidmap_iter_next (struct oidmap_iter *iter) +{ +return hashmap_iter_next (&iter->h_iter); +} + +static inline void *oidmap_iter_first (struct oidmap *map, +struct oidmap_iter *iter) +{ +oidmap_iter_init (map, iter); +return oidmap_iter_next (iter); +} +#endif
Signature · 1 line
2.9.3
PATCH v3 3/6 oidset: add iterator methods to oidset
In reply to Jeff HostetlerFrom: Jeff Hostetler <[email protected]>
Add the usual iterator methods to oidset.
Add oidset_remove().
--- oidset.c | 10 ++++++++++ oidset.h | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+)
oidset.c +10 -0
@@ -24,6 +24,16 @@ int oidset_insert(struct oidset *set, const struct object_id *oid)return 0 ; } +int oidset_remove (struct oidset *set,const struct object_id *oid) +{ +struct oidmap_entry *entry; + + entry =oidmap_remove (&set->map, oid); +free (entry); + +return (entry !=NULL ); +} +void oidset_clear (struct oidset *set) {oidmap_free (&set->map,1 );
oidset.h +36 -0
@@ -24,6 +24,12 @@ struct oidset {#define OIDSET_INIT { OIDMAP_INIT } + +static inline void oidset_init (struct oidset *set,size_t initial_size) +{ +return oidmap_init (&set->map, initial_size); +} +/** * Returns true iff `set` contains `oid`. */
@@ -39,9 +45,39 @@ int oidset_contains(const struct oidset *set, const struct object_id *oid);int oidset_insert (struct oidset *set,const struct object_id *oid);/** +* Remove the oid from the set. +* +* Returns 1 if the oid was present in the set, 0 otherwise. +*/ +int oidset_remove (struct oidset *set,const struct object_id *oid); + +/** * Remove all entries from the oidset, freeing any resources associated with * it. */ void oidset_clear (struct oidset *set); +struct oidset_iter { +struct oidmap_iter m_iter; +}; + +static inline void oidset_iter_init (struct oidset *set, +struct oidset_iter *iter) +{ +oidmap_iter_init (&set->map, &iter->m_iter); +} + +static inline struct object_id *oidset_iter_next (struct oidset_iter *iter) +{ +struct oidmap_entry *e =oidmap_iter_next (&iter->m_iter); +return e ? &e->oid :NULL ; +} + +static inline struct object_id *oidset_iter_first (struct oidset *set, +struct oidset_iter *iter) +{ +oidset_iter_init (set, iter); +return oidset_iter_next (iter); +} +#endif /* OIDSET_H */
Signature · 1 line
2.9.3
PATCH v3 4/6 list-objects: filter objects in traverse_commit_list
In reply to Jeff HostetlerFrom: Jeff Hostetler <[email protected]>
Create traverse_commit_list_filtered() and add filtering interface to allow certain objects to be omitted from the traversal.
Update traverse_commit_list() to be a wrapper for the above with a null filter to minimize the number of callers that needed to be changed.
Object filtering will be used in a future commit by rev-list and pack-objects for partial clone and fetch to omit unwanted objects from the result.
traverse_bitmap_commit_list() does not work with filtering. If a packfile bitmap is present, it will not be used. It should be possible to extend such support in the future (at least to simple filters that do not require object pathnames), but that is beyond the scope of this patch series.
--- Makefile | 2 + list-objects-filter-options.c | 148 ++++++++++++++++ list-objects-filter-options.h | 50 ++++++ list-objects-filter.c | 401 ++++++++++++++++++++++++++++++++++++++++++ list-objects-filter.h | 77 ++++++++ list-objects.c | 95 ++++++++-- list-objects.h | 13 +- object.h | 1 + 8 files changed, 770 insertions(+), 17 deletions(-) create mode 100644 list-objects-filter-options.c create mode 100644 list-objects-filter-options.h create mode 100644 list-objects-filter.c create mode 100644 list-objects-filter.h
Makefile +2 -0
@@ -807,6 +807,8 @@ LIB_OBJS += levenshtein.o LIB_OBJS += line-log.o LIB_OBJS += line-range.o LIB_OBJS += list-objects.o +LIB_OBJS += list-objects-filter.o +LIB_OBJS += list-objects-filter-options.o LIB_OBJS += ll-merge.o LIB_OBJS += lockfile.o LIB_OBJS += log-tree.o
list-objects-filter-options.c new file +148 -0
148 lines not on this page · raw message
list-objects-filter-options.h new file +50 -0
50 lines not on this page · raw message
list-objects-filter.c new file +401 -0
401 lines not on this page · raw message
list-objects-filter.h new file +77 -0
77 lines not on this page · raw message
list-objects.c +79 -16
182 lines not on this page · raw message
list-objects.h +12 -1
@@ -8,4 +8,15 @@ void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, voitypedef void (*show_edge_fn )(struct commit *);void mark_edges_uninteresting (struct rev_info *,show_edge_fn ); -#endif +struct oidset ; +struct list_objects_filter_options ; + +void traverse_commit_list_filtered ( +struct list_objects_filter_options *filter_options, +struct rev_info *revs, +show_commit_fn show_commit, +show_object_fn show_object, +void *show_data, +struct oidset *omitted); + +#endif /* LIST_OBJECTS_H */
object.h +1 -0
@@ -38,6 +38,7 @@ struct object_array {* http-push.c: 16-----19 * commit.c: 16-----19 * sha1_name.c: 20 +* list-objects-filter.c: 21 * builtin/fsck.c: 0--3 */ #define FLAG_BITS 27
Signature · 1 line
2.9.3
Re: [PATCH v3 4/6] list-objects: filter objects in traverse_commit_list
Quoted 6 lines · On Tue, 7 Nov 2017 19:35:44 +0000 Jeff Hostetler <[email protected]> wrote:
list-objects-filter-options.c new file +6
-0+/* +* Reject the arg if it contains any characters that might +* require quoting or escaping when handing to a sub-command. +*/ +static int reject_injection_chars (const char *arg) +{
[snip]
Quoted 1 line
+}
Someone pointed me to quote.{c,h}, which is probably sufficient to ensure shell safety if we do invoke subcommands through the shell. If that is so, we probably don't need a blacklist.
Having said that, though, it might be safer to still introduce one, and relax it later if necessary - it is much easier to relax a constraint than to increase one.
list-objects-filter-options.c line 126
Quoted 12 lines
list-objects-filter-options.c new file +12
-0+ }else if (skip_prefix (arg,"sparse:" , &v0)) { + +if (skip_prefix (v0,"oid=" , &v1)) { +struct object_context oc; +struct object_id sparse_oid; + filter_options->choice =LOFC_SPARSE_OID ; +if (!get_oid_with_context (v1,GET_OID_BLOB , + &sparse_oid, &oc)) + filter_options->sparse_oid_value = +oiddup (&sparse_oid); +return 0 ; + }
In your recent e-mail [1], you said that you will change it to always pass the original expression - is that still the plan?
Quoted 1 line
+/* Remember to update object flag allocation in
object.h*/
You probably can delete this line.
Quoted 5 lines
+/*
+ *FILTER_SHOWN_BUT_REVISIT-- we set this bit on tree objects
+ * that have been shown, but should be revisited if they appear
+ * in the traversal (until we mark it SEEN). This is a way to
+ * let us silently de-dup calls toshow()in the caller.
This is unclear to me at first reading. Maybe something like:
FILTER_SHOWN_BUT_REVISIT -- we set this bit on tree objects that have been shown, but should not be skipped over if they reappear in the traversal. This ensures that the tree's descendants are re-processed if the tree reappears subsequently, and that the tree is not shown twice.
Quoted 5 lines
+ * This
+ * is subtly different from the "revision.h:SHOWN" and the
+ * "sha1_name.c:ONELINE_SEEN" bits. And also different from
+ * the non-de-dup usage inpack-bitmap.c
+ */
Optional: I'm not sure if this comparison is useful. (Maybe it is useful to others, though.)
Quoted 7 lines
list-objects-filter.c new file +7
-0+/* +* A filter driven by a sparse-checkout specification to only +* include blobs that a sparse checkout would populate. +* +* The sparse-checkout spec can be loaded from a blob with the +* given OID or from a local pathname. We allow an OID because +* the repo may be bare or we may be doing the filtering on the
list-objects-filter.c line 176
Quoted 10 lines
list-objects-filter.c new file +10
-0+* server. +*/ +struct frame { +/* +* defval is the usual default include/exclude value that +* should be inherited as we recurse into directories based +* upon pattern matching of the directory itself or of a +* containing directory. +*/ +int defval;
Can this be an "unsigned defval : 1" as well? In the function below, I see that you assign to an "int val" first (which can take -1, 0, and 1) before assigning to this, so that is fine.
Also, maybe a better name would be "exclude", with the documentation:
1 if the directory is excluded, 0 otherwise. Excluded directories will still be recursed through, because an "include" rule for an object might override an "exclude" rule for one of its ancestors.
Re: [PATCH v3 4/6] list-objects: filter objects in traverse_commit_list
Quoted 3 lines · Jonathan Tan
Having said that, though, it might be safer to still introduce one, and relax it later if necessary - it is much easier to relax a constraint than to increase one.
It would also be more error prone to have such a long switch () statement, each of whose case arm needs to be carefully looked at.
While protection against attacks over the wire against the process that receives the request is necessary and doing the quoting right at this layer is one valuable component of it, we would need to be careful about what features we allow the other side to request.
For example, an innocent-looking use of get_oid_with_context() can trigger an expensive operation, e.g. "master^{/sekritCodeName}", may not just waste resources but also may reveal the presence of an object that we might not want to leak to a stranger. Limiting such an abuse must sit at a lot higher layer than a byte-by-byte check over the request like the code does.
Re: [PATCH v3 4/6] list-objects: filter objects in traverse_commit_list
Quoted 14 lines · Junio C Hamano
Quoted 3 lines · Jonathan Tan
Having said that, though, it might be safer to still introduce one, and relax it later if necessary - it is much easier to relax a constraint than to increase one.
It would also be more error prone to have such a long switch () statement, each of whose case arm needs to be carefully looked at.
While protection against attacks over the wire against the process that receives the request is necessary and doing the quoting right at this layer is one valuable component of it, we would need to be careful about what features we allow the other side to request.
Quoted 7 lines
For example, an innocent-looking use of
get_oid_with_context()can trigger an expensive operation, e.g. "master^{/sekritCodeName}", may not just waste resources but also may reveal the presence of an object that we might not want to leak to a stranger. Limiting such an abuse must sit at a lot higher layer than a byte-by-byte check over the request like the code does.
Right. I could see adding another server-side variable in the spirit of the existing "uploadpack.allow*" variables.
My main concern at this point has been avoiding injections.
Jeff
Re: [PATCH v3 4/6] list-objects: filter objects in traverse_commit_list
In reply to Jonathan TanQuoted 12 lines · Jonathan Tan
Quoted 6 lines · On Tue, 7 Nov 2017 19:35:44 +0000 Jeff Hostetler <[email protected]> wrote:
list-objects-filter-options.c new file +6
-0+/* +* Reject the arg if it contains any characters that might +* require quoting or escaping when handing to a sub-command. +*/ +static int reject_injection_chars (const char *arg) +{[snip]
Quoted 1 line
+}
Quoted 7 lines
Someone pointed me to quote.{c,h}, which is probably sufficient to ensure shell safety if we do invoke subcommands through the shell. If that is so, we probably don't need a blacklist.
Having said that, though, it might be safer to still introduce one, and relax it later if necessary - it is much easier to relax a constraint than to increase one.
I couldn't use quote.[ch] because it is more concerned with quoting pathnames because of LF and CR characters within them -- rather than semicolons and quotes and the like which I was concerned about.
Anyway, in my next patch series I've replaced all of the injection code from my last series with something a little stronger and not restricting.
Quoted 17 lines
list-objects-filter-options.c line 126
Quoted 13 lines
list-objects-filter-options.c new file +12
-0+ }else if (skip_prefix (arg,"sparse:" , &v0)) { + +if (skip_prefix (v0,"oid=" , &v1)) { +struct object_context oc; +struct object_id sparse_oid; + filter_options->choice =LOFC_SPARSE_OID ; +if (!get_oid_with_context (v1,GET_OID_BLOB , + &sparse_oid, &oc)) + filter_options->sparse_oid_value = +oiddup (&sparse_oid); +return 0 ; + }In your recent e-mail [1], you said that you will change it to always pass the original expression - is that still the plan?
Quoted 1 line
yes. I always pass filter_options.raw_value over the wire. The code above tries to parse it and put it in an OID for private use by the current process -- just like the size limit value in the blob:limit filter.
Quoted 3 lines
Quoted 1 line
+/* Remember to update object flag allocation in
object.h*/You probably can delete this line.
Every other place that defined flag bits included this comment, so I did too. (It really made it easier to find the other random places that define bits, actually.)
Quoted 15 lines
Quoted 6 lines
+/*
+ *FILTER_SHOWN_BUT_REVISIT-- we set this bit on tree objects
+ * that have been shown, but should be revisited if they appear
+ * in the traversal (until we mark it SEEN). This is a way to
+ * let us silently de-dup calls toshow()in the caller.This is unclear to me at first reading. Maybe something like:
FILTER_SHOWN_BUT_REVISIT-- we set this bit on tree objects that have been shown, but should not be skipped over if they reappear in the traversal. This ensures that the tree's descendants are re-processed if the tree reappears subsequently, and that the tree is not shown twice.
Quoted 8 lines
Quoted 5 lines
+ * This
+ * is subtly different from the "revision.h:SHOWN" and the
+ * "sha1_name.c:ONELINE_SEEN" bits. And also different from
+ * the non-de-dup usage inpack-bitmap.c
+ */Optional: I'm not sure if this comparison is useful. (Maybe it is useful to others, though.)
I was thinking the first comment about my FILTER_SHOWN field would be to ask why I wasn't just using the existing SHOWN bit. There are subtle differences between the bits and I wanted to point out that I was not just duplicating the usage of an existing bit.
Quoted 23 lines
Quoted 8 lines
list-objects-filter.c new file +7
-0+/* +* A filter driven by a sparse-checkout specification to only +* include blobs that a sparse checkout would populate. +* +* The sparse-checkout spec can be loaded from a blob with the +* given OID or from a local pathname. We allow an OID because +* the repo may be bare or we may be doing the filtering on the list-objects-filter.c line 176
Quoted 10 lines
list-objects-filter.c new file +10
-0+* server. +*/ +struct frame { +/* +* defval is the usual default include/exclude value that +* should be inherited as we recurse into directories based +* upon pattern matching of the directory itself or of a +* containing directory. +*/ +int defval;Can this be an "unsigned defval : 1" as well? In the function below, I see that you assign to an "int val" first (which can take -1, 0, and 1) before assigning to this, so that is fine.
Quoted 6 lines
Also, maybe a better name would be "exclude", with the documentation:
1 if the directory is excluded, 0 otherwise. Excluded directories will still be recursed through, because an "include" rule for an object might override an "exclude" rule for one of its ancestors.
The name "defval" is used unpack-trees.c during the clear_ce_flags() recursion while looking at the exclusion list. I was just trying to match that behavior.
Thanks
Jeff
PATCH v3 5/6 rev-list: add list-objects filtering support
In reply to Jeff HostetlerFrom: Jeff Hostetler <[email protected]>
Teach rev-list to use the filtering provided by the traverse_commit_list_filtered() interface to omit unwanted objects from the result. This feature is intended to help with partial clone.
Object filtering is only allowed when one of the "--objects*" options are used.
When the "--filter-print-omitted" option is used, the omitted objects are printed at the end. These are marked with a "~". This option can be combined with "--quiet" to get a list of just the omitted objects.
Normally, rev-list will stop with an error when there are missing objects.
When the "--filter-print-missing" option is used, rev-list will print a list of any missing objects that should have been included in the output (rather than stopping). These are marked with a "?".
When the "--filter-ignore-missing" option is used, rev-list will silently ignore any missing objects and continue without error.
Add t6112 test.
--- Documentation/git-rev-list.txt | 6 +- Documentation/rev-list-options.txt | 34 ++++++ builtin/rev-list.c | 75 +++++++++++- t/t6112-rev-list-filters-objects.sh | 225 ++++++++++++++++++++++++++++++++++++ 4 files changed, 337 insertions(+), 3 deletions(-) create mode 100755 t/t6112-rev-list-filters-objects.sh
Documentation/git-rev-list.txt +5 -1
@@ -47,7 +47,11 @@ SYNOPSIS [ --fixed-strings | -F ] [ --date=<format>] [ [ --objects | --objects-edge | --objects-edge-aggressive ] - [ --unpacked ] ] + [ --unpacked ] + [ --filter=<filter-spec> ] ] + [ --filter-print-missing ] + [ --filter-print-omitted ] + [ --filter-ignore-missing ] [ --pretty | --header ] [ --bisect ] [ --bisect-vars ]
Documentation/rev-list-options.txt +34 -0
@@ -706,6 +706,40 @@ ifdef::git-rev-list[] --unpacked:: Only useful with `--objects`; print the object IDs that are not in packs. + +--filter=<filter-spec>:: + Only useful with one of the `--objects*`; omits objects (usually + blobs) from the list of printed objects. The '<filter-spec>' + may be one of the following: ++ +The form '--filter=blob:none' omits all blobs. ++ +The form '--filter=blob:limit=<n>[kmg]' omits blobs larger than n bytes +or units. The value may be zero. Special files matching '.git*' are +alwayse included, regardless of size. ++ +The form '--filter=sparse:oid=<oid-ish>' uses a sparse-checkout +specification contained in the object (or the object that the expression +evaluates to) to omit blobs not required by the corresponding sparse +checkout. ++ +The form '--filter=sparse:path=<path>' similarly uses a sparse-checkout +specification contained in <path>. + +--filter-print-missing:: + Prints a list of the missing objects for the requested traversal. + Object IDs are prefixed with a ``?'' character. The object type + is printed after the ID. This may be used with or without any of + the above filtering options. + +--filter-ignore-missing:: + Ignores missing objects encountered during the requested traversal. + This may be used with or without any of the above filtering options. + +--filter-print-omitted:: + Only useful with one of the above `--filter*`; prints a list + of the omitted objects. Object IDs are prefixed with a ``~'' + character. endif::git-rev-list[] --no-walk[=(sorted|unsorted)]::
builtin/rev-list.c +73 -2
118 lines not on this page · raw message
t/t6112-rev-list-filters-objects.sh new file +225 -0
225 lines not on this page · raw message
Signature · 1 line
2.9.3
PATCH v3 6/6 pack-objects: add list-objects filtering
In reply to Jeff HostetlerFrom: Jeff Hostetler <[email protected]>
Teach pack-objects to use the filtering provided by the traverse_commit_list_filtered() interface to omit unwanted objects from the resulting packfile.
This feature is intended for partial clone/fetch.
Filtering requires the use of the "--stdout" option.
Add t5317 test.
--- Documentation/git-pack-objects.txt | 12 +- builtin/pack-objects.c | 28 ++- t/t5317-pack-objects-filter-objects.sh | 369 +++++++++++++++++++++++++++++++++ 3 files changed, 407 insertions(+), 2 deletions(-) create mode 100755 t/t5317-pack-objects-filter-objects.sh
Documentation/git-pack-objects.txt +11 -1
@@ -12,7 +12,8 @@ SYNOPSIS 'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied] [--no-reuse-delta] [--delta-base-offset] [--non-empty] [--local] [--incremental] [--window=<n>] [--depth=<n>] - [--revs [--unpacked | --all]] [--stdout | base-name] + [--revs [--unpacked | --all]] + [--stdout [--filter=<filter-spec>] | base-name] [--shallow] [--keep-true-parents] < object-list
@@ -236,6 +237,15 @@ So does `git bundle` (see linkgit:git-bundle[1]) when it creates a bundle. With this option, parents that are hidden by grafts are packed nevertheless. +--filter=<filter-spec>:: + Requires `--stdout`. Omits certain objects (usually blobs) from + the resulting packfile. See linkgit:git-rev-list[1] for valid + `<filter-spec>` forms. + +--filter-ignore-missing: + Ignore missing objects without error. This may be used with + or without and of the above filtering. + SEE ALSO -------- linkgit:git-rev-list[1]
builtin/pack-objects.c +27 -1
@@ -15,6 +15,8 @@#include "diff.h" #include "revision.h" #include "list-objects.h" +#include "list-objects-filter.h" +#include "list-objects-filter-options.h" #include "pack-objects.h" #include "progress.h" #include "refs.h"
@@ -79,6 +81,9 @@ static unsigned long cache_max_small_delta_size = 1000;static unsigned long window_memory_limit =0 ; +static struct list_objects_filter_options filter_options; +static int arg_ignore_missing; +/* * stats */
@@ -2547,6 +2552,15 @@ static void show_commit(struct commit *commit, void *data)static void show_object (struct object *obj,const char *name,void *data) { +/* +* Quietly ignore missing objects when they are expected. This +* avoids staging them and getting an odd error later. If we are +* not expecting them, stage it and let the normal error handling +* deal with it. +*/ +if (arg_ignore_missing && !has_object_file (&obj->oid)) +return ; +add_preferred_base_object (name);add_object_entry (obj->oid.hash, obj->type, name,0 ); obj->flags |=OBJECT_ADDED ;
@@ -2816,7 +2830,10 @@ static void get_object_list(int ac, const char **av)if (prepare_revision_walk (&revs))die ("revision walk setup failed" );mark_edges_uninteresting (&revs, show_edge); -traverse_commit_list (&revs, show_commit, show_object,NULL ); + +traverse_commit_list_filtered (&filter_options, &revs, + show_commit, show_object,NULL , +NULL );if (unpack_unreachable_expiration) { revs.ignore_missing_links =1 ;
@@ -2952,6 +2969,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) N_("use a bitmap index if available to speed up counting objects")),OPT_BOOL (0 ,"write-bitmap-index" , &write_bitmap_index,N_ ("write a bitmap index together with the pack index" )), +OPT_PARSE_LIST_OBJECTS_FILTER (&filter_options), +OPT_BOOL (0 ,"filter-ignore-missing" , &arg_ignore_missing, +N_ ("ignore and omit missing objects from packfile" )), OPT_END(), };
@@ -3028,6 +3048,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)if (!rev_list_all || !rev_list_reflog || !rev_list_index) unpack_unreachable_expiration =0 ; +if (filter_options.choice) { +if (!pack_to_stdout) +die ("cannot use filtering with an indexable pack." ); + use_bitmap_index =0 ; + } +/* * "soft" reasons not to use bitmaps - for on-disk repack by default we want *
t/t5317-pack-objects-filter-objects.sh new file +369 -0
369 lines not on this page · raw message
Signature · 1 line
2.9.3
Re: [PATCH v3 6/6] pack-objects: add list-objects filtering
Documentation/git-pack-objects.txt So does `git bundle` (see linkgit:git-bundle[1]) when it creates a bundle. · line 247
Quoted 3 lines · On Tue, 7 Nov 2017 19:35:46 +0000 Jeff Hostetler <[email protected]> wrote:
+--filter-ignore-missing:
Documentation/git-pack-objects.txt +2
-0+ Ignore missing objects without error. This may be used with + or without and of the above filtering.
There is a discussion about this parameter (and the corresponding ones in patch 5/6), to which I have just replied:
builtin/pack-objects.c int cmd_pack_objects(int argc, const char **argv, const char *prefix) · line 3053
Quoted 7 lines
builtin/pack-objects.c +3
-0@@ -3028,6 +3048,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)if (!rev_list_all || !rev_list_reflog || !rev_list_index) unpack_unreachable_expiration =0 ; +if (filter_options.choice) { +if (!pack_to_stdout) +die ("cannot use filtering with an indexable pack." );
I'm not sure what an "indexable pack" is - probably a better message is "cannot use --filter without --stdout".
Quoted 16 lines · Jeff Hostetler
From: Jeff Hostetler <[email protected]>
Here is V3 of the list-object filtering. This addresses the comments on the mailing list for the V2 series as well as the various TODO items I left in the code. I also documented some of the bit flags and fields that I added.
In the blob size filter, I removed the ".git*" pattern matching for special files. I don't think we need it any more and it simplifies the code. This patch series does not support
traverse_bitmap_commit_list()and the --use-bitmap-index feature in rev-list, but by removing the ".git*" pattern matching now we should be able allow filtering and bitmaps to be used together in a future effort. (That is beyond the scope of the current partial-clone effort.)
Quoted 2 lines
With this patch series, I think part 1 is complete unless there are further comments or questions.
Thanks.
I tried to rebase the other two on top and queued them in 'pu'.