{"thread":{"id":"47223","subject":"[PATCH v4 1/6] dir: allow exclusions from blob in addition to file","startedAt":"2017-11-16T18:08:14Z","lastAt":"2017-11-17T22:19:44Z","messageCount":16,"participants":["Jeff Hostetler","Jonathan Tan","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":6},"messages":[{"id":"332683","messageId":"20171116180743.61353-2-git@jeffhostetler.com","threadId":"47223","inReplyTo":"20171116180743.61353-1-git@jeffhostetler.com","subject":"[PATCH v4 1/6] dir: allow exclusions from blob in addition to file","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:07:38Z","receivedAt":"2017-11-16T18:08:14Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nRefactor add_excludes() to separate the reading of the\nexclude file into a buffer and the parsing of the buffer\ninto exclude_list items.\n\nAdd add_excludes_from_blob_to_list() to allow an exclude\nfile be specified with an OID without assuming a local\nworktree or index exists.\n\nRefactor read_skip_worktree_file_from_index() and add\ndo_read_blob() to eliminate duplication of preliminary\nprocessing of blob contents.\n\nSigned-off-by: Jeff Hostetler \n---\n dir.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++----------------\n dir.h | 3 ++\n 2 files changed, 104 insertions(+), 31 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 1d17b80..1962374 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -220,6 +220,57 @@ int within_depth(const char *name, int namelen,\n \treturn 1;\n }\n \n+/*\n+ * Read the contents of the blob with the given OID into a buffer.\n+ * Append a trailing LF to the end if the last line doesn't have one.\n+ *\n+ * Returns:\n+ * -1 when the OID is invalid or unknown or does not refer to a blob.\n+ * 0 when the blob is empty.\n+ * 1 along with { data, size } of the (possibly augmented) buffer\n+ * when successful.\n+ *\n+ * Optionally updates the given sha1_stat with the given OID (when valid).\n+ */\n+static int do_read_blob(const struct object_id *oid,\n+\t\t\tstruct sha1_stat *sha1_stat,\n+\t\t\tsize_t *size_out,\n+\t\t\tchar **data_out)\n+{\n+\tenum object_type type;\n+\tunsigned long sz;\n+\tchar *data;\n+\n+\t*size_out = 0;\n+\t*data_out = NULL;\n+\n+\tdata = read_sha1_file(oid->hash, &type, &sz);\n+\tif (!data || type != OBJ_BLOB) {\n+\t\tfree(data);\n+\t\treturn -1;\n+\t}\n+\n+\tif (sha1_stat) {\n+\t\tmemset(&sha1_stat->stat, 0, sizeof(sha1_stat->stat));\n+\t\thashcpy(sha1_stat->sha1, oid->hash);\n+\t}\n+\n+\tif (sz == 0) {\n+\t\tfree(data);\n+\t\treturn 0;\n+\t}\n+\n+\tif (data[sz - 1] != '\\n') {\n+\t\tdata = xrealloc(data, st_add(sz, 1));\n+\t\tdata[sz++] = '\\n';\n+\t}\n+\n+\t*size_out = xsize_t(sz);\n+\t*data_out = data;\n+\n+\treturn 1;\n+}\n+\n #define DO_MATCH_EXCLUDE (1<<0)\n #define DO_MATCH_DIRECTORY (1<<1)\n #define DO_MATCH_SUBMODULE (1<<2)\n@@ -600,32 +651,22 @@ void add_exclude(const char *string, const char *base,\n \tx->el = el;\n }\n \n-static void *read_skip_worktree_file_from_index(const struct index_state *istate,\n-\t\t\t\t\t\tconst char *path, size_t *size,\n-\t\t\t\t\t\tstruct sha1_stat *sha1_stat)\n+static int read_skip_worktree_file_from_index(const struct index_state *istate,\n+\t\t\t\t\t const char *path,\n+\t\t\t\t\t size_t *size_out,\n+\t\t\t\t\t char **data_out,\n+\t\t\t\t\t struct sha1_stat *sha1_stat)\n {\n \tint pos, len;\n-\tunsigned long sz;\n-\tenum object_type type;\n-\tvoid *data;\n \n \tlen = strlen(path);\n \tpos = index_name_pos(istate, path, len);\n \tif (pos < 0)\n-\t\treturn NULL;\n+\t\treturn -1;\n \tif (!ce_skip_worktree(istate->cache[pos]))\n-\t\treturn NULL;\n-\tdata = read_sha1_file(istate->cache[pos]->oid.hash, &type, &sz);\n-\tif (!data || type != OBJ_BLOB) {\n-\t\tfree(data);\n-\t\treturn NULL;\n-\t}\n-\t*size = xsize_t(sz);\n-\tif (sha1_stat) {\n-\t\tmemset(&sha1_stat->stat, 0, sizeof(sha1_stat->stat));\n-\t\thashcpy(sha1_stat->sha1, istate->cache[pos]->oid.hash);\n-\t}\n-\treturn data;\n+\t\treturn -1;\n+\n+\treturn do_read_blob(&istate->cache[pos]->oid, sha1_stat, size_out, data_out);\n }\n \n /*\n@@ -739,6 +780,10 @@ static void invalidate_directory(struct untracked_cache *uc,\n \t\tdir->dirs[i]->recurse = 0;\n }\n \n+static int add_excludes_from_buffer(char *buf, size_t size,\n+\t\t\t\t const char *base, int baselen,\n+\t\t\t\t struct exclude_list *el);\n+\n /*\n * Given a file with name \"fname\", read it (either from disk, or from\n * an index if 'istate' is non-null), parse it and store the\n@@ -754,9 +799,10 @@ static int add_excludes(const char *fname, const char *base, int baselen,\n \t\t\tstruct sha1_stat *sha1_stat)\n {\n \tstruct stat st;\n-\tint fd, i, lineno = 1;\n+\tint r;\n+\tint fd;\n \tsize_t size = 0;\n-\tchar *buf, *entry;\n+\tchar *buf;\n \n \tfd = open(fname, O_RDONLY);\n \tif (fd < 0 || fstat(fd, &st) < 0) {\n@@ -764,17 +810,13 @@ static int add_excludes(const char *fname, const char *base, int baselen,\n \t\t\twarn_on_fopen_errors(fname);\n \t\telse\n \t\t\tclose(fd);\n-\t\tif (!istate ||\n-\t\t (buf = read_skip_worktree_file_from_index(istate, fname, &size, sha1_stat)) == NULL)\n+\t\tif (!istate)\n \t\t\treturn -1;\n-\t\tif (size == 0) {\n-\t\t\tfree(buf);\n-\t\t\treturn 0;\n-\t\t}\n-\t\tif (buf[size-1] != '\\n') {\n-\t\t\tbuf = xrealloc(buf, st_add(size, 1));\n-\t\t\tbuf[size++] = '\\n';\n-\t\t}\n+\t\tr = read_skip_worktree_file_from_index(istate, fname,\n+\t\t\t\t\t\t &size, &buf,\n+\t\t\t\t\t\t sha1_stat);\n+\t\tif (r != 1)\n+\t\t\treturn r;\n \t} else {\n \t\tsize = xsize_t(st.st_size);\n \t\tif (size == 0) {\n@@ -813,6 +855,17 @@ static int add_excludes(const char *fname, const char *base, int baselen,\n \t\t}\n \t}\n \n+\tadd_excludes_from_buffer(buf, size, base, baselen, el);\n+\treturn 0;\n+}\n+\n+static int add_excludes_from_buffer(char *buf, size_t size,\n+\t\t\t\t const char *base, int baselen,\n+\t\t\t\t struct exclude_list *el)\n+{\n+\tint i, lineno = 1;\n+\tchar *entry;\n+\n \tel->filebuf = buf;\n \n \tif (skip_utf8_bom(&buf, size))\n@@ -841,6 +894,23 @@ int add_excludes_from_file_to_list(const char *fname, const char *base,\n \treturn add_excludes(fname, base, baselen, el, istate, NULL);\n }\n \n+int add_excludes_from_blob_to_list(\n+\tstruct object_id *oid,\n+\tconst char *base, int baselen,\n+\tstruct exclude_list *el)\n+{\n+\tchar *buf;\n+\tsize_t size;\n+\tint r;\n+\n+\tr = do_read_blob(oid, NULL, &size, &buf);\n+\tif (r != 1)\n+\t\treturn r;\n+\n+\tadd_excludes_from_buffer(buf, size, base, baselen, el);\n+\treturn 0;\n+}\n+\n struct exclude_list *add_exclude_list(struct dir_struct *dir,\n \t\t\t\t int group_type, const char *src)\n {\ndiff --git a/dir.h b/dir.h\nindex e371705..1bcf391 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -256,6 +256,9 @@ extern struct exclude_list *add_exclude_list(struct dir_struct *dir,\n extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,\n \t\t\t\t\t struct exclude_list *el, struct index_state *istate);\n extern void add_excludes_from_file(struct dir_struct *, const char *fname);\n+extern int add_excludes_from_blob_to_list(struct object_id *oid,\n+\t\t\t\t\t const char *base, int baselen,\n+\t\t\t\t\t struct exclude_list *el);\n extern void parse_exclude_pattern(const char **string, int *patternlen, unsigned *flags, int *nowildcardlen);\n extern void add_exclude(const char *string, const char *base,\n \t\t\tint baselen, struct exclude_list *el, int srcpos);\n-- \n2.9.3\n\n"},{"id":"332684","messageId":"20171116180743.61353-4-git@jeffhostetler.com","threadId":"47223","inReplyTo":"20171116180743.61353-1-git@jeffhostetler.com","subject":"[PATCH v4 3/6] oidset: add iterator methods to oidset","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:07:40Z","receivedAt":"2017-11-16T18:08:16Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nAdd the usual iterator methods to oidset.\nAdd oidset_remove().\n\nSigned-off-by: Jeff Hostetler \n---\n oidset.c | 10 ++++++++++\n oidset.h | 36 ++++++++++++++++++++++++++++++++++++\n 2 files changed, 46 insertions(+)\n\ndiff --git a/oidset.c b/oidset.c\nindex f1f874a..454c54f 100644\n--- a/oidset.c\n+++ b/oidset.c\n@@ -24,6 +24,16 @@ int oidset_insert(struct oidset *set, const struct object_id *oid)\n \treturn 0;\n }\n \n+int oidset_remove(struct oidset *set, const struct object_id *oid)\n+{\n+\tstruct oidmap_entry *entry;\n+\n+\tentry = oidmap_remove(&set->map, oid);\n+\tfree(entry);\n+\n+\treturn (entry != NULL);\n+}\n+\n void oidset_clear(struct oidset *set)\n {\n \toidmap_free(&set->map, 1);\ndiff --git a/oidset.h b/oidset.h\nindex f4c9e0f..783abce 100644\n--- a/oidset.h\n+++ b/oidset.h\n@@ -24,6 +24,12 @@ struct oidset {\n \n #define OIDSET_INIT { OIDMAP_INIT }\n \n+\n+static inline void oidset_init(struct oidset *set, size_t initial_size)\n+{\n+\treturn oidmap_init(&set->map, initial_size);\n+}\n+\n /**\n * Returns true iff `set` contains `oid`.\n */\n@@ -39,9 +45,39 @@ int oidset_contains(const struct oidset *set, const struct object_id *oid);\n int oidset_insert(struct oidset *set, const struct object_id *oid);\n \n /**\n+ * Remove the oid from the set.\n+ *\n+ * Returns 1 if the oid was present in the set, 0 otherwise.\n+ */\n+int oidset_remove(struct oidset *set, const struct object_id *oid);\n+\n+/**\n * Remove all entries from the oidset, freeing any resources associated with\n * it.\n */\n void oidset_clear(struct oidset *set);\n \n+struct oidset_iter {\n+\tstruct oidmap_iter m_iter;\n+};\n+\n+static inline void oidset_iter_init(struct oidset *set,\n+\t\t\t\t struct oidset_iter *iter)\n+{\n+\toidmap_iter_init(&set->map, &iter->m_iter);\n+}\n+\n+static inline struct object_id *oidset_iter_next(struct oidset_iter *iter)\n+{\n+\tstruct oidmap_entry *e = oidmap_iter_next(&iter->m_iter);\n+\treturn e ? &e->oid : NULL;\n+}\n+\n+static inline struct object_id *oidset_iter_first(struct oidset *set,\n+\t\t\t\t\t\t struct oidset_iter *iter)\n+{\n+\toidset_iter_init(set, iter);\n+\treturn oidset_iter_next(iter);\n+}\n+\n #endif /* OIDSET_H */\n-- \n2.9.3\n\n"},{"id":"332685","messageId":"20171116180743.61353-3-git@jeffhostetler.com","threadId":"47223","inReplyTo":"20171116180743.61353-1-git@jeffhostetler.com","subject":"[PATCH v4 2/6] oidmap: add oidmap iterator methods","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:07:39Z","receivedAt":"2017-11-16T18:08:17Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nAdd the usual map iterator functions to oidmap.\n\nSigned-off-by: Jeff Hostetler \n---\n oidmap.h | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n\ndiff --git a/oidmap.h b/oidmap.h\nindex 18f54cd..d3cd2bb 100644\n--- a/oidmap.h\n+++ b/oidmap.h\n@@ -65,4 +65,26 @@ extern void *oidmap_put(struct oidmap *map, void *entry);\n */\n extern void *oidmap_remove(struct oidmap *map, const struct object_id *key);\n \n+\n+struct oidmap_iter {\n+\tstruct hashmap_iter h_iter;\n+};\n+\n+static inline void oidmap_iter_init(struct oidmap *map, struct oidmap_iter *iter)\n+{\n+\thashmap_iter_init(&map->map, &iter->h_iter);\n+}\n+\n+static inline void *oidmap_iter_next(struct oidmap_iter *iter)\n+{\n+\treturn hashmap_iter_next(&iter->h_iter);\n+}\n+\n+static inline void *oidmap_iter_first(struct oidmap *map,\n+\t\t\t\t struct oidmap_iter *iter)\n+{\n+\toidmap_iter_init(map, iter);\n+\treturn oidmap_iter_next(iter);\n+}\n+\n #endif\n-- \n2.9.3\n\n"},{"id":"332686","messageId":"20171116180743.61353-1-git@jeffhostetler.com","threadId":"47223","inReplyTo":null,"subject":"[PATCH v4 0/6] Partial clone part 1: object filtering","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:07:37Z","receivedAt":"2017-11-16T18:08:20Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nHere is V4 of the list-object filtering, rev-list, and pack-objects.\n\nThis version addresses comments on the V3 version series.\n\nThis version replaces the code to scan and reject the filter-spec\nfor injection characters with a new hex-encoding technique. The\npurpose of this is only to guard against injection attacks containing\ncharacters like semicolon, quotes, spaces, and etc. when a filter-spec\nis handed to a subordinate command. It does not eliminate the need\nfor the recipient to validate the contents.\n\nThis version also combines the various command line flags for\nhandling missing objects into a single --missing={error,print,allow-any}\nflag.\n\n\nJeff Hostetler (6):\n dir: allow exclusions from blob in addition to file\n oidmap: add oidmap iterator methods\n oidset: add iterator methods to oidset\n list-objects: filter objects in traverse_commit_list\n rev-list: add list-objects filtering support\n pack-objects: add list-objects filtering\n\n Documentation/git-pack-objects.txt | 12 +-\n Documentation/git-rev-list.txt | 4 +-\n Documentation/rev-list-options.txt | 37 +++\n Makefile | 2 +\n builtin/pack-objects.c | 64 +++++-\n builtin/rev-list.c | 108 ++++++++-\n dir.c | 132 ++++++++---\n dir.h | 3 +\n list-objects-filter-options.c | 149 ++++++++++++\n list-objects-filter-options.h | 57 +++++\n list-objects-filter.c | 401 +++++++++++++++++++++++++++++++++\n list-objects-filter.h | 77 +++++++\n list-objects.c | 95 ++++++--\n list-objects.h | 13 +-\n object.h | 1 +\n oidmap.h | 22 ++\n oidset.c | 10 +\n oidset.h | 36 +++\n t/t5317-pack-objects-filter-objects.sh | 375 ++++++++++++++++++++++++++++++\n t/t6112-rev-list-filters-objects.sh | 225 ++++++++++++++++++\n 20 files changed, 1770 insertions(+), 53 deletions(-)\n create mode 100644 list-objects-filter-options.c\n create mode 100644 list-objects-filter-options.h\n create mode 100644 list-objects-filter.c\n create mode 100644 list-objects-filter.h\n create mode 100755 t/t5317-pack-objects-filter-objects.sh\n create mode 100755 t/t6112-rev-list-filters-objects.sh\n\n-- \n2.9.3\n\n"},{"id":"332687","messageId":"20171116180743.61353-6-git@jeffhostetler.com","threadId":"47223","inReplyTo":"20171116180743.61353-1-git@jeffhostetler.com","subject":"[PATCH v4 5/6] rev-list: add list-objects filtering support","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:07:42Z","receivedAt":"2017-11-16T18:08:27Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nTeach rev-list to use the filtering provided by the\ntraverse_commit_list_filtered() interface to omit\nunwanted objects from the result. This feature is\nintended to help with partial clone.\n\nObject filtering is only allowed when one of the \"--objects*\"\noptions are used.\n\nWhen the \"--filter-print-omitted\" option is used, the omitted\nobjects are printed at the end. These are marked with a \"~\".\nThis option can be combined with \"--quiet\" to get a list of\njust the omitted objects.\n\nAdded \"--missing=(error|print|omit)\" argument to specify how\nrev-list should behave when it encounters a missing object\n(presumably from a prior partial clone).\n\nWhen \"--missing=print\" is used, rev-list will print a list of\nany missing objects that should have been included in the output.\nThese are marked with a \"?\".\n\nAdd t6112 test.\n\nSigned-off-by: Jeff Hostetler \n---\n Documentation/git-rev-list.txt | 4 +-\n Documentation/rev-list-options.txt | 37 ++++++\n builtin/rev-list.c | 108 ++++++++++++++++-\n t/t6112-rev-list-filters-objects.sh | 225 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 371 insertions(+), 3 deletions(-)\n create mode 100755 t/t6112-rev-list-filters-objects.sh\n\ndiff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\nindex ef22f17..397a0dd 100644\n--- a/Documentation/git-rev-list.txt\n+++ b/Documentation/git-rev-list.txt\n@@ -47,7 +47,9 @@ SYNOPSIS\n \t [ --fixed-strings | -F ]\n \t [ --date=]\n \t [ [ --objects | --objects-edge | --objects-edge-aggressive ]\n-\t [ --unpacked ] ]\n+\t [ --unpacked ]\n+\t [ --filter= [ --filter-print-omitted ] ] ]\n+\t [ --missing=(error|allow-any|print) ]\n \t [ --pretty | --header ]\n \t [ --bisect ]\n \t [ --bisect-vars ]\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 13501e1..c84e465 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -706,6 +706,43 @@ ifdef::git-rev-list[]\n --unpacked::\n \tOnly useful with `--objects`; print the object IDs that are not\n \tin packs.\n+\n+--filter=::\n+\tOnly useful with one of the `--objects*`; omits objects (usually\n+\tblobs) from the list of printed objects. The ''\n+\tmay be one of the following:\n++\n+The form '--filter=blob:none' omits all blobs.\n++\n+The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes\n+or units. The value may be zero. Special files matching '.git*' are\n+alwayse included, regardless of size.\n++\n+The form '--filter=sparse:oid=' uses a sparse-checkout\n+specification contained in the object (or the object that the expression\n+evaluates to) to omit blobs not required by the corresponding sparse\n+checkout.\n++\n+The form '--filter=sparse:path=' similarly uses a sparse-checkout\n+specification contained in .\n+\n+--filter-print-omitted::\n+\tOnly useful with `--filter=`; prints a list of the omitted objects.\n+\tObject IDs are prefixed with a ``~'' character.\n+\n+--missing=(error|allow-any|print)::\n+\tSpecifies how missing objects are handled. The repository may\n+\thave missing objects after a partial clone, for example.\n++\n+The value 'error' requests that rev-list stop with an error if a missing\n+object is encountered. This is the default action.\n++\n+The value 'allow-any' will allow object traversal to continue if a\n+missing object is encountered. Missing objects will silently be omitted\n+from the results.\n++\n+The value 'print' is like 'allow-any', but will also print a list of the\n+missing objects. Object IDs are prefixed with a ``?'' character.\n endif::git-rev-list[]\n \n --no-walk[=(sorted|unsorted)]::\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex c1c74d4..da4a39b 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -4,6 +4,8 @@\n #include \"diff.h\"\n #include \"revision.h\"\n #include \"list-objects.h\"\n+#include \"list-objects-filter.h\"\n+#include \"list-objects-filter-options.h\"\n #include \"pack.h\"\n #include \"pack-bitmap.h\"\n #include \"builtin.h\"\n@@ -12,6 +14,7 @@\n #include \"bisect.h\"\n #include \"progress.h\"\n #include \"reflog-walk.h\"\n+#include \"oidset.h\"\n \n static const char rev_list_usage[] =\n \"git rev-list [OPTION] ... [ -- paths... ]\\n\"\n@@ -55,6 +58,20 @@ static const char rev_list_usage[] =\n static struct progress *progress;\n static unsigned progress_counter;\n \n+static struct list_objects_filter_options filter_options;\n+static struct oidset omitted_objects;\n+static int arg_print_omitted; /* print objects omitted by filter */\n+\n+static struct oidset missing_objects;\n+enum missing_action {\n+\tMA_ERROR = 0, /* fail if any missing objects are encountered */\n+\tMA_ALLOW_ANY, /* silently allow ALL missing objects */\n+\tMA_PRINT, /* print ALL missing objects in special section */\n+};\n+static enum missing_action arg_missing_action;\n+\n+#define DEFAULT_OIDSET_SIZE (16*1024)\n+\n static void finish_commit(struct commit *commit, void *data);\n static void show_commit(struct commit *commit, void *data)\n {\n@@ -178,11 +195,31 @@ static void finish_commit(struct commit *commit, void *data)\n \tfree_commit_buffer(commit);\n }\n \n+static inline void finish_object__ma(struct object *obj)\n+{\n+\tswitch (arg_missing_action) {\n+\tcase MA_ERROR:\n+\t\tdie(\"missing blob object '%s'\", oid_to_hex(&obj->oid));\n+\t\treturn;\n+\n+\tcase MA_ALLOW_ANY:\n+\t\treturn;\n+\n+\tcase MA_PRINT:\n+\t\toidset_insert(&missing_objects, &obj->oid);\n+\t\treturn;\n+\n+\tdefault:\n+\t\tBUG(\"unhandled missing_action\");\n+\t\treturn;\n+\t}\n+}\n+\n static void finish_object(struct object *obj, const char *name, void *cb_data)\n {\n \tstruct rev_list_info *info = cb_data;\n \tif (obj->type == OBJ_BLOB && !has_object_file(&obj->oid))\n-\t\tdie(\"missing blob object '%s'\", oid_to_hex(&obj->oid));\n+\t\tfinish_object__ma(obj);\n \tif (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n \t\tparse_object(&obj->oid);\n }\n@@ -269,6 +306,26 @@ static int show_object_fast(\n \treturn 1;\n }\n \n+static inline int parse_missing_action_value(const char *value)\n+{\n+\tif (!strcmp(value, \"error\")) {\n+\t\targ_missing_action = MA_ERROR;\n+\t\treturn 1;\n+\t}\n+\n+\tif (!strcmp(value, \"allow-any\")) {\n+\t\targ_missing_action = MA_ALLOW_ANY;\n+\t\treturn 1;\n+\t}\n+\n+\tif (!strcmp(value, \"print\")) {\n+\t\targ_missing_action = MA_PRINT;\n+\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n int cmd_rev_list(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n@@ -335,6 +392,26 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t\tshow_progress = arg;\n \t\t\tcontinue;\n \t\t}\n+\n+\t\tif (skip_prefix(arg, (\"--\" CL_ARG__FILTER \"=\"), &arg)) {\n+\t\t\tparse_list_objects_filter(&filter_options, arg);\n+\t\t\tif (filter_options.choice && !revs.blob_objects)\n+\t\t\t\tdie(_(\"object filtering requires --objects\"));\n+\t\t\tif (filter_options.choice == LOFC_SPARSE_OID &&\n+\t\t\t !filter_options.sparse_oid_value)\n+\t\t\t\tdie(_(\"invalid sparse value '%s'\"),\n+\t\t\t\t filter_options.raw_value);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"--filter-print-omitted\")) {\n+\t\t\targ_print_omitted = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (skip_prefix(arg, \"--missing=\", &arg) &&\n+\t\t parse_missing_action_value(arg))\n+\t\t\tcontinue;\n+\t\t\n \t\tusage(rev_list_usage);\n \n \t}\n@@ -360,6 +437,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tif (revs.show_notes)\n \t\tdie(_(\"rev-list does not support display of notes\"));\n \n+\tif (filter_options.choice && use_bitmap_index)\n+\t\tdie(_(\"cannot combine --use-bitmap-index with object filtering\"));\n+\n \tsave_commit_buffer = (revs.verbose_header ||\n \t\t\t revs.grep_filter.pattern_list ||\n \t\t\t revs.grep_filter.header_list);\n@@ -404,7 +484,31 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\n \t}\n \n-\ttraverse_commit_list(&revs, show_commit, show_object, &info);\n+\tif (arg_print_omitted)\n+\t\toidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);\n+\tif (arg_missing_action == MA_PRINT)\n+\t\toidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n+\n+\ttraverse_commit_list_filtered(\n+\t\t&filter_options, &revs, show_commit, show_object, &info,\n+\t\t(arg_print_omitted ? &omitted_objects : NULL));\n+\n+\tif (arg_print_omitted) {\n+\t\tstruct oidset_iter iter;\n+\t\tstruct object_id *oid;\n+\t\toidset_iter_init(&omitted_objects, &iter);\n+\t\twhile ((oid = oidset_iter_next(&iter)))\n+\t\t\tprintf(\"~%s\\n\", oid_to_hex(oid));\n+\t\toidset_clear(&omitted_objects);\n+\t}\n+\tif (arg_missing_action == MA_PRINT) {\n+\t\tstruct oidset_iter iter;\n+\t\tstruct object_id *oid;\n+\t\toidset_iter_init(&missing_objects, &iter);\n+\t\twhile ((oid = oidset_iter_next(&iter)))\n+\t\t\tprintf(\"?%s\\n\", oid_to_hex(oid));\n+\t\toidset_clear(&missing_objects);\n+\t}\n \n \tstop_progress(&progress);\n \ndiff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh\nnew file mode 100755\nindex 0000000..73166db\n--- /dev/null\n+++ b/t/t6112-rev-list-filters-objects.sh\n@@ -0,0 +1,225 @@\n+#!/bin/sh\n+\n+test_description='git rev-list with object filtering for partial clone'\n+\n+. ./test-lib.sh\n+\n+# Test the blob:none filter.\n+\n+test_expect_success 'setup r1' '\n+\techo \"{print \\$1}\" >print_1.awk &&\n+\techo \"{print \\$2}\" >print_2.awk &&\n+\n+\tgit init r1 &&\n+\tfor n in 1 2 3 4 5\n+\tdo\n+\t\techo \"This is file: $n\" > r1/file.$n\n+\t\tgit -C r1 add file.$n\n+\t\tgit -C r1 commit -m \"$n\"\n+\tdone\n+'\n+\n+test_expect_success 'verify blob:none omits all 5 blobs' '\n+\tgit -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r1 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:none \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify emitted+omitted == all' '\n+\tgit -C r1 rev-list HEAD --objects \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r1 rev-list HEAD --objects --filter-print-omitted --filter=blob:none \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+\n+# Test blob:limit=[kmg] filter.\n+# We boundary test around the size parameter. The filter is strictly less than\n+# the value, so size 500 and 1000 should have the same results, but 1001 should\n+# filter more.\n+\n+test_expect_success 'setup r2' '\n+\tgit init r2 &&\n+\tfor n in 1000 10000\n+\tdo\n+\t\tprintf \"%\"$n\"s\" X > r2/large.$n\n+\t\tgit -C r2 add large.$n\n+\t\tgit -C r2 commit -m \"$n\"\n+\tdone\n+'\n+\n+test_expect_success 'verify blob:limit=500 omits all blobs' '\n+\tgit -C r2 ls-files -s large.1000 large.10000 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=500 \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify emitted+omitted == all' '\n+\tgit -C r2 rev-list HEAD --objects \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 rev-list HEAD --objects --filter-print-omitted --filter=blob:limit=500 \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify blob:limit=1000' '\n+\tgit -C r2 ls-files -s large.1000 large.10000 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=1000 \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify blob:limit=1001' '\n+\tgit -C r2 ls-files -s large.10000 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=1001 \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify blob:limit=1k' '\n+\tgit -C r2 ls-files -s large.10000 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=1k \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify blob:limit=1m' '\n+\tcat expected &&\n+\tgit -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=1m \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+# Test sparse:path= filter.\n+# Use a local file containing a sparse-checkout specification to filter\n+# out blobs not required for the corresponding sparse-checkout. We do not\n+# require sparse-checkout to actually be enabled.\n+\n+test_expect_success 'setup r3' '\n+\tgit init r3 &&\n+\tmkdir r3/dir1 &&\n+\tfor n in sparse1 sparse2\n+\tdo\n+\t\techo \"This is file: $n\" > r3/$n\n+\t\tgit -C r3 add $n\n+\t\techo \"This is file: dir1/$n\" > r3/dir1/$n\n+\t\tgit -C r3 add dir1/$n\n+\tdone &&\n+\tgit -C r3 commit -m \"sparse\" &&\n+\techo dir1/ >pattern1 &&\n+\techo sparse1 >pattern2\n+'\n+\n+test_expect_success 'verify sparse:path=pattern1 omits top-level files' '\n+\tgit -C r3 ls-files -s sparse1 sparse2 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=sparse:path=../pattern1 \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify sparse:path=pattern2 omits both sparse2 files' '\n+\tgit -C r3 ls-files -s sparse2 dir1/sparse2 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=sparse:path=../pattern2 \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+# Test sparse:oid= filter.\n+# Like sparse:path, but we get the sparse-checkout specification from\n+# a blob rather than a file on disk.\n+\n+test_expect_success 'setup r3 part 2' '\n+\techo dir1/ >r3/pattern &&\n+\tgit -C r3 add pattern &&\n+\tgit -C r3 commit -m \"pattern\"\n+'\n+\n+test_expect_success 'verify sparse:oid=OID omits top-level files' '\n+\tgit -C r3 ls-files -s pattern sparse1 sparse2 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\toid=$(git -C r3 ls-files -s pattern | awk -f print_2.awk) &&\n+\tgit -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=sparse:oid=$oid \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify sparse:oid=oid-ish omits top-level files' '\n+\tgit -C r3 ls-files -s pattern sparse1 sparse2 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=sparse:oid=master:pattern \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/~//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+# Delete some loose objects and use rev-list, but WITHOUT any filtering.\n+# This models previously omitted objects that we did not receive.\n+\n+test_expect_success 'rev-list W/ --missing=print' '\n+\tgit -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tfor id in `cat expected | sed \"s|..|&/|\"`\n+\tdo\n+\t\trm r1/.git/objects/$id\n+\tdone &&\n+\tgit -C r1 rev-list --quiet HEAD --missing=print --objects \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sed \"s/?//\" \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'rev-list W/O --missing fails' '\n+\ttest_must_fail git -C r1 rev-list --quiet --objects HEAD\n+'\n+\n+test_expect_success 'rev-list W/ missing=allow-any' '\n+\tgit -C r1 rev-list --quiet --missing=allow-any --objects HEAD\n+'\n+\n+test_done\n-- \n2.9.3\n\n"},{"id":"332688","messageId":"20171116180743.61353-7-git@jeffhostetler.com","threadId":"47223","inReplyTo":"20171116180743.61353-1-git@jeffhostetler.com","subject":"[PATCH v4 6/6] pack-objects: add list-objects filtering","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:07:43Z","receivedAt":"2017-11-16T18:08:30Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nTeach pack-objects to use the filtering provided by the\ntraverse_commit_list_filtered() interface to omit unwanted\nobjects from the resulting packfile.\n\nThis feature is intended for partial clone/fetch.\n\nFiltering requires the use of the \"--stdout\" option.\n\nAdd t5317 test.\n\nSigned-off-by: Jeff Hostetler \n---\n Documentation/git-pack-objects.txt | 12 +-\n builtin/pack-objects.c | 64 +++++-\n t/t5317-pack-objects-filter-objects.sh | 375 +++++++++++++++++++++++++++++++++\n 3 files changed, 449 insertions(+), 2 deletions(-)\n create mode 100755 t/t5317-pack-objects-filter-objects.sh\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 473a161..5fad696 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -12,7 +12,8 @@ SYNOPSIS\n 'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied]\n \t[--no-reuse-delta] [--delta-base-offset] [--non-empty]\n \t[--local] [--incremental] [--window=] [--depth=]\n-\t[--revs [--unpacked | --all]] [--stdout | base-name]\n+\t[--revs [--unpacked | --all]]\n+\t[--stdout [--filter=] | base-name]\n \t[--shallow] [--keep-true-parents] < object-list\n \n \n@@ -236,6 +237,15 @@ So does `git bundle` (see linkgit:git-bundle[1]) when it creates a bundle.\n \tWith this option, parents that are hidden by grafts are packed\n \tnevertheless.\n \n+--filter=::\n+\tRequires `--stdout`. Omits certain objects (usually blobs) from\n+\tthe resulting packfile. See linkgit:git-rev-list[1] for valid\n+\t`` forms.\n+\n+--missing=(error|allow-any):\n+\tSpecifies how missing objects are handled. This is useful, for\n+\texample, when there are missing objects from a prior partial clone.\n+\n SEE ALSO\n --------\n linkgit:git-rev-list[1]\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 6e77dfd..45ad35d 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -15,6 +15,8 @@\n #include \"diff.h\"\n #include \"revision.h\"\n #include \"list-objects.h\"\n+#include \"list-objects-filter.h\"\n+#include \"list-objects-filter-options.h\"\n #include \"pack-objects.h\"\n #include \"progress.h\"\n #include \"refs.h\"\n@@ -79,6 +81,15 @@ static unsigned long cache_max_small_delta_size = 1000;\n \n static unsigned long window_memory_limit = 0;\n \n+static struct list_objects_filter_options filter_options;\n+\n+enum missing_action {\n+\tMA_ERROR = 0, /* fail if any missing objects are encountered */\n+\tMA_ALLOW_ANY, /* silently allow ALL missing objects */\n+};\n+static enum missing_action arg_missing_action;\n+static show_object_fn fn_show_object;\n+\n /*\n * stats\n */\n@@ -2552,6 +2563,42 @@ static void show_object(struct object *obj, const char *name, void *data)\n \tobj->flags |= OBJECT_ADDED;\n }\n \n+static void show_object__ma_allow_any(struct object *obj, const char *name, void *data)\n+{\n+\tassert(arg_missing_action == MA_ALLOW_ANY);\n+\n+\t/*\n+\t * Quietly ignore ALL missing objects. This avoids problems with\n+\t * staging them now and getting an odd error later.\n+\t */\n+\tif (!has_object_file(&obj->oid))\n+\t\treturn;\n+\n+\tshow_object(obj, name, data);\n+}\n+\n+static int option_parse_missing_action(const struct option *opt,\n+\t\t\t\t const char *arg, int unset)\n+{\n+\tassert(arg);\n+\tassert(!unset);\n+\n+\tif (!strcmp(arg, \"error\")) {\n+\t\targ_missing_action = MA_ERROR;\n+\t\tfn_show_object = show_object;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(arg, \"allow-any\")) {\n+\t\targ_missing_action = MA_ALLOW_ANY;\n+\t\tfn_show_object = show_object__ma_allow_any;\n+\t\treturn 0;\n+\t}\n+\n+\tdie(_(\"invalid value for --missing\"));\n+\treturn 0;\n+}\n+\n static void show_edge(struct commit *commit)\n {\n \tadd_preferred_base(commit->object.oid.hash);\n@@ -2816,7 +2863,12 @@ static void get_object_list(int ac, const char **av)\n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\n \tmark_edges_uninteresting(&revs, show_edge);\n-\ttraverse_commit_list(&revs, show_commit, show_object, NULL);\n+\n+\tif (!fn_show_object)\n+\t\tfn_show_object = show_object;\n+\ttraverse_commit_list_filtered(&filter_options, &revs,\n+\t\t\t\t show_commit, fn_show_object, NULL,\n+\t\t\t\t NULL);\n \n \tif (unpack_unreachable_expiration) {\n \t\trevs.ignore_missing_links = 1;\n@@ -2952,6 +3004,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"use a bitmap index if available to speed up counting objects\")),\n \t\tOPT_BOOL(0, \"write-bitmap-index\", &write_bitmap_index,\n \t\t\t N_(\"write a bitmap index together with the pack index\")),\n+\t\tOPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),\n+\t\t{ OPTION_CALLBACK, 0, \"missing\", NULL, N_(\"action\"),\n+\t\t N_(\"handling for missing objects\"), PARSE_OPT_NONEG,\n+\t\t option_parse_missing_action },\n \t\tOPT_END(),\n \t};\n \n@@ -3028,6 +3084,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (!rev_list_all || !rev_list_reflog || !rev_list_index)\n \t\tunpack_unreachable_expiration = 0;\n \n+\tif (filter_options.choice) {\n+\t\tif (!pack_to_stdout)\n+\t\t\tdie(\"cannot use --filter without --stdout.\");\n+\t\tuse_bitmap_index = 0;\n+\t}\n+\n \t/*\n \t * \"soft\" reasons not to use bitmaps - for on-disk repack by default we want\n \t *\ndiff --git a/t/t5317-pack-objects-filter-objects.sh b/t/t5317-pack-objects-filter-objects.sh\nnew file mode 100755\nindex 0000000..ee8f204\n--- /dev/null\n+++ b/t/t5317-pack-objects-filter-objects.sh\n@@ -0,0 +1,375 @@\n+#!/bin/sh\n+\n+test_description='git pack-objects with object filtering for partial clone'\n+\n+. ./test-lib.sh\n+\n+# Test blob:none filter.\n+\n+test_expect_success 'setup r1' '\n+\techo \"{print \\$1}\" >print_1.awk &&\n+\techo \"{print \\$2}\" >print_2.awk &&\n+\n+\tgit init r1 &&\n+\tfor n in 1 2 3 4 5\n+\tdo\n+\t\techo \"This is file: $n\" > r1/file.$n\n+\t\tgit -C r1 add file.$n\n+\t\tgit -C r1 commit -m \"$n\"\n+\tdone\n+'\n+\n+test_expect_success 'verify blob count in normal packfile' '\n+\tgit -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r1 pack-objects --rev --stdout >all.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r1 index-pack ../all.pack &&\n+\tgit -C r1 verify-pack -v ../all.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify blob:none packfile has no blobs' '\n+\tgit -C r1 pack-objects --rev --stdout --filter=blob:none >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r1 index-pack ../filter.pack &&\n+\tgit -C r1 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\tnr=$(wc -l expected &&\n+\tgit -C r1 verify-pack -v ../filter.pack \\\n+\t\t| grep -E \"commit|tree\" \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+# Test blob:limit=[kmg] filter.\n+# We boundary test around the size parameter. The filter is strictly less than\n+# the value, so size 500 and 1000 should have the same results, but 1001 should\n+# filter more.\n+\n+test_expect_success 'setup r2' '\n+\tgit init r2 &&\n+\tfor n in 1000 10000\n+\tdo\n+\t\tprintf \"%\"$n\"s\" X > r2/large.$n\n+\t\tgit -C r2 add large.$n\n+\t\tgit -C r2 commit -m \"$n\"\n+\tdone\n+'\n+\n+test_expect_success 'verify blob count in normal packfile' '\n+\tgit -C r2 ls-files -s large.1000 large.10000 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 pack-objects --rev --stdout >all.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r2 index-pack ../all.pack &&\n+\tgit -C r2 verify-pack -v ../all.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify blob:limit=500 omits all blobs' '\n+\tgit -C r2 pack-objects --rev --stdout --filter=blob:limit=500 >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r2 index-pack ../filter.pack &&\n+\tgit -C r2 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\tnr=$(wc -l filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r2 index-pack ../filter.pack &&\n+\tgit -C r2 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\tnr=$(wc -l expected &&\n+\tgit -C r2 pack-objects --rev --stdout --filter=blob:limit=1001 >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r2 index-pack ../filter.pack &&\n+\tgit -C r2 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify blob:limit=10001' '\n+\tgit -C r2 ls-files -s large.1000 large.10000 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 pack-objects --rev --stdout --filter=blob:limit=10001 >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r2 index-pack ../filter.pack &&\n+\tgit -C r2 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify blob:limit=1k' '\n+\tgit -C r2 ls-files -s large.1000 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 pack-objects --rev --stdout --filter=blob:limit=1k >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r2 index-pack ../filter.pack &&\n+\tgit -C r2 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify blob:limit=1m' '\n+\tgit -C r2 ls-files -s large.1000 large.10000 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 pack-objects --rev --stdout --filter=blob:limit=1m >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r2 index-pack ../filter.pack &&\n+\tgit -C r2 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify normal and blob:limit packfiles have same commits/trees' '\n+\tgit -C r2 verify-pack -v ../all.pack \\\n+\t\t| grep -E \"commit|tree\" \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r2 verify-pack -v ../filter.pack \\\n+\t\t| grep -E \"commit|tree\" \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+# Test sparse:path= filter.\n+# Use a local file containing a sparse-checkout specification to filter\n+# out blobs not required for the corresponding sparse-checkout. We do not\n+# require sparse-checkout to actually be enabled.\n+\n+test_expect_success 'setup r3' '\n+\tgit init r3 &&\n+\tmkdir r3/dir1 &&\n+\tfor n in sparse1 sparse2\n+\tdo\n+\t\techo \"This is file: $n\" > r3/$n\n+\t\tgit -C r3 add $n\n+\t\techo \"This is file: dir1/$n\" > r3/dir1/$n\n+\t\tgit -C r3 add dir1/$n\n+\tdone &&\n+\tgit -C r3 commit -m \"sparse\" &&\n+\techo dir1/ >pattern1 &&\n+\techo sparse1 >pattern2\n+'\n+\n+test_expect_success 'verify blob count in normal packfile' '\n+\tgit -C r3 ls-files -s sparse1 sparse2 dir1/sparse1 dir1/sparse2 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r3 pack-objects --rev --stdout >all.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r3 index-pack ../all.pack &&\n+\tgit -C r3 verify-pack -v ../all.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify sparse:path=pattern1' '\n+\tgit -C r3 ls-files -s dir1/sparse1 dir1/sparse2 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r3 pack-objects --rev --stdout --filter=sparse:path=../pattern1 >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r3 index-pack ../filter.pack &&\n+\tgit -C r3 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify normal and sparse:path=pattern1 packfiles have same commits/trees' '\n+\tgit -C r3 verify-pack -v ../all.pack \\\n+\t\t| grep -E \"commit|tree\" \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r3 verify-pack -v ../filter.pack \\\n+\t\t| grep -E \"commit|tree\" \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify sparse:path=pattern2' '\n+\tgit -C r3 ls-files -s sparse1 dir1/sparse1 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r3 pack-objects --rev --stdout --filter=sparse:path=../pattern2 >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r3 index-pack ../filter.pack &&\n+\tgit -C r3 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify normal and sparse:path=pattern2 packfiles have same commits/trees' '\n+\tgit -C r3 verify-pack -v ../all.pack \\\n+\t\t| grep -E \"commit|tree\" \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r3 verify-pack -v ../filter.pack \\\n+\t\t| grep -E \"commit|tree\" \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+# Test sparse:oid= filter.\n+# Like sparse:path, but we get the sparse-checkout specification from\n+# a blob rather than a file on disk.\n+\n+test_expect_success 'setup r4' '\n+\tgit init r4 &&\n+\tmkdir r4/dir1 &&\n+\tfor n in sparse1 sparse2\n+\tdo\n+\t\techo \"This is file: $n\" > r4/$n\n+\t\tgit -C r4 add $n\n+\t\techo \"This is file: dir1/$n\" > r4/dir1/$n\n+\t\tgit -C r4 add dir1/$n\n+\tdone &&\n+\techo dir1/ >r4/pattern &&\n+\tgit -C r4 add pattern &&\n+\tgit -C r4 commit -m \"pattern\"\n+'\n+\n+test_expect_success 'verify blob count in normal packfile' '\n+\tgit -C r4 ls-files -s pattern sparse1 sparse2 dir1/sparse1 dir1/sparse2 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r4 pack-objects --rev --stdout >all.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r4 index-pack ../all.pack &&\n+\tgit -C r4 verify-pack -v ../all.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify sparse:oid=OID' '\n+\tgit -C r4 ls-files -s dir1/sparse1 dir1/sparse2 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\toid=$(git -C r4 ls-files -s pattern | awk -f print_2.awk) &&\n+\tgit -C r4 pack-objects --rev --stdout --filter=sparse:oid=$oid >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r4 index-pack ../filter.pack &&\n+\tgit -C r4 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+test_expect_success 'verify sparse:oid=oid-ish' '\n+\tgit -C r4 ls-files -s dir1/sparse1 dir1/sparse2 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tgit -C r4 pack-objects --rev --stdout --filter=sparse:oid=master:pattern >filter.pack <<-EOF &&\n+\tHEAD\n+\tEOF\n+\tgit -C r4 index-pack ../filter.pack &&\n+\tgit -C r4 verify-pack -v ../filter.pack \\\n+\t\t| grep blob \\\n+\t\t| awk -f print_1.awk \\\n+\t\t| sort >observed &&\n+\ttest_cmp observed expected\n+'\n+\n+# Delete some loose objects and use pack-objects, but WITHOUT any filtering.\n+# This models previously omitted objects that we did not receive.\n+\n+test_expect_success 'setup r1 - delete loose blobs' '\n+\tgit -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 \\\n+\t\t| awk -f print_2.awk \\\n+\t\t| sort >expected &&\n+\tfor id in `cat expected | sed \"s|..|&/|\"`\n+\tdo\n+\t\trm r1/.git/objects/$id\n+\tdone\n+'\n+\n+test_expect_success 'verify pack-objects fails w/ missing objects' '\n+\ttest_must_fail git -C r1 pack-objects --rev --stdout >miss.pack <<-EOF\n+\tHEAD\n+\tEOF\n+'\n+\n+test_expect_success 'verify pack-objects fails w/ --missing=error' '\n+\ttest_must_fail git -C r1 pack-objects --rev --stdout --missing=error >miss.pack <<-EOF\n+\tHEAD\n+\tEOF\n+'\n+\n+test_expect_success 'verify pack-objects w/ --missing=allow-any' '\n+\tgit -C r1 pack-objects --rev --stdout --missing=allow-any >miss.pack <<-EOF\n+\tHEAD\n+\tEOF\n+'\n+\n+test_done\n-- \n2.9.3\n\n"},{"id":"332689","messageId":"20171116180743.61353-5-git@jeffhostetler.com","threadId":"47223","inReplyTo":"20171116180743.61353-1-git@jeffhostetler.com","subject":"[PATCH v4 4/6] list-objects: filter objects in traverse_commit_list","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:07:41Z","receivedAt":"2017-11-16T18:08:37Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nCreate traverse_commit_list_filtered() and add filtering\ninterface to allow certain objects to be omitted from the\ntraversal.\n\nUpdate traverse_commit_list() to be a wrapper for the above\nwith a null filter to minimize the number of callers that\nneeded to be changed.\n\nObject filtering will be used in a future commit by rev-list\nand pack-objects for partial clone and fetch to omit unwanted\nobjects from the result.\n\ntraverse_bitmap_commit_list() does not work with filtering.\nIf a packfile bitmap is present, it will not be used. It\nshould be possible to extend such support in the future (at\nleast to simple filters that do not require object pathnames),\nbut that is beyond the scope of this patch series.\n\nSigned-off-by: Jeff Hostetler \n---\n Makefile | 2 +\n list-objects-filter-options.c | 149 ++++++++++++++++\n list-objects-filter-options.h | 57 ++++++\n list-objects-filter.c | 401 ++++++++++++++++++++++++++++++++++++++++++\n list-objects-filter.h | 77 ++++++++\n list-objects.c | 95 ++++++++--\n list-objects.h | 13 +-\n object.h | 1 +\n 8 files changed, 778 insertions(+), 17 deletions(-)\n create mode 100644 list-objects-filter-options.c\n create mode 100644 list-objects-filter-options.h\n create mode 100644 list-objects-filter.c\n create mode 100644 list-objects-filter.h\n\ndiff --git a/Makefile b/Makefile\nindex cd75985..ca378a4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -807,6 +807,8 @@ LIB_OBJS += levenshtein.o\n LIB_OBJS += line-log.o\n LIB_OBJS += line-range.o\n LIB_OBJS += list-objects.o\n+LIB_OBJS += list-objects-filter.o\n+LIB_OBJS += list-objects-filter-options.o\n LIB_OBJS += ll-merge.o\n LIB_OBJS += lockfile.o\n LIB_OBJS += log-tree.o\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nnew file mode 100644\nindex 0000000..a9298fd\n--- /dev/null\n+++ b/list-objects-filter-options.c\n@@ -0,0 +1,149 @@\n+#include \"cache.h\"\n+#include \"commit.h\"\n+#include \"config.h\"\n+#include \"revision.h\"\n+#include \"argv-array.h\"\n+#include \"list-objects.h\"\n+#include \"list-objects-filter.h\"\n+#include \"list-objects-filter-options.h\"\n+\n+/*\n+ * Return 1 if the given string needs armoring because of \"special\"\n+ * characters that may cause injection problems when a command passes\n+ * the argument to a subordinate command (such as when upload-pack\n+ * launches pack-objects).\n+ *\n+ * The usual alphanumeric and key punctuation do not trigger it.\n+ */ \n+static int arg_needs_armor(const char *arg)\n+{\n+\tconst unsigned char *p;\n+\n+\tfor (p = (const unsigned char *)arg; *p; p++) {\n+\t\tif (*p >= 'a' && *p <= 'z')\n+\t\t\tcontinue;\n+\t\tif (*p >= 'A' && *p <= 'Z')\n+\t\t\tcontinue;\n+\t\tif (*p >= '0' && *p <= '9')\n+\t\t\tcontinue;\n+\t\tif (*p == '-' || *p == '_' || *p == '.' || *p == '/')\n+\t\t\tcontinue;\n+\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+void armor_encode_arg(struct strbuf *buf, const char *arg)\n+{\n+\tstatic const char hex[] = \"0123456789abcdef\";\n+\tconst unsigned char *p;\n+\n+\tfor (p = (const unsigned char *)arg; *p; p++) {\n+\t\tunsigned int val = *p;\n+\t\tstrbuf_addch(buf, hex[val >> 4]);\n+\t\tstrbuf_addch(buf, hex[val & 0xf]);\n+\t}\n+}\n+\n+int armor_decode_arg(struct strbuf *buf, const char *arg)\n+{\n+\tconst char *p;\n+\n+\tfor (p = arg; *p; p += 2) {\n+\t\tint val = hex2chr(p);\n+\t\tunsigned char ch;\n+\t\tif (val < 0)\n+\t\t\treturn -1;\n+\t\tch = val;\n+\t\tstrbuf_addch(buf, ch);\n+\t}\n+\treturn 0;\n+}\n+\n+/*\n+ * Parse value of the argument to the \"filter\" keword.\n+ * On the command line this looks like:\n+ * --filter=\n+ * and in the pack protocol as:\n+ * \"filter\" SP \n+ *\n+ * The filter keyword will be used by many commands.\n+ * See Documentation/rev-list-options.txt for allowed values for .\n+ *\n+ * Capture the given arg as the \"raw_value\". This can be forwarded to\n+ * subordinate commands when necessary. We also \"intern\" the arg for\n+ * the convenience of the current command.\n+ */\n+int parse_list_objects_filter(struct list_objects_filter_options *filter_options,\n+\t\t\t const char *arg)\n+{\n+\tconst char *v0;\n+\n+\tif (filter_options->choice)\n+\t\tdie(_(\"multiple object filter types cannot be combined\"));\n+\n+\tfilter_options->raw_value = strdup(arg);\n+\n+\tif (!strcmp(arg, \"blob:none\")) {\n+\t\tfilter_options->choice = LOFC_BLOB_NONE;\n+\t\treturn 0;\n+\t}\n+\n+\tif (skip_prefix(arg, \"blob:limit=\", &v0)) {\n+\t\tif (!git_parse_ulong(v0, &filter_options->blob_limit_value))\n+\t\t\tdie(_(\"invalid filter-spec expression '%s'\"), arg);\n+\t\tfilter_options->choice = LOFC_BLOB_LIMIT;\n+\t\treturn 0;\n+\t}\n+\n+\tif (skip_prefix(arg, \"sparse:oid=\", &v0)) {\n+\t\tstruct object_context oc;\n+\t\tstruct object_id sparse_oid;\n+\n+\t\t/*\n+\t\t * Try to parse into an OID for the current\n+\t\t * command, but DO NOT complain if we don't have the blob or\n+\t\t * ref locally.\n+\t\t */\n+\t\tif (!get_oid_with_context(v0, GET_OID_BLOB,\n+\t\t\t\t\t &sparse_oid, &oc))\n+\t\t\tfilter_options->sparse_oid_value = oiddup(&sparse_oid);\n+\t\tfilter_options->choice = LOFC_SPARSE_OID;\n+\t\tif (arg_needs_armor(v0))\n+\t\t\tfilter_options->requires_armor = v0 - arg;\n+\t\treturn 0;\n+\t}\n+\n+\tif (skip_prefix(arg, \"sparse:path=\", &v0)) {\n+\t\tfilter_options->choice = LOFC_SPARSE_PATH;\n+\t\tfilter_options->sparse_path_value = strdup(v0);\n+\t\tif (arg_needs_armor(v0))\n+\t\t\tfilter_options->requires_armor = v0 - arg;\n+\t\treturn 0;\n+\t}\n+\n+\tif (skip_prefix(arg, \"x:\", &v0)) {\n+\t\tint r;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tif (armor_decode_arg(&buf, v0) < 0)\n+\t\t\tdie(_(\"invalid filter-spec expression '%s'\"), arg);\n+\t\tr = parse_list_objects_filter(filter_options, buf.buf);\n+\t\tstrbuf_release(&buf);\n+\t\treturn r;\n+\t}\n+\n+\tdie(_(\"invalid filter-spec expression '%s'\"), arg);\n+\treturn 0;\n+}\n+\n+int opt_parse_list_objects_filter(const struct option *opt,\n+\t\t\t\t const char *arg, int unset)\n+{\n+\tstruct list_objects_filter_options *filter_options = opt->value;\n+\n+\tassert(arg);\n+\tassert(!unset);\n+\n+\treturn parse_list_objects_filter(filter_options, arg);\n+}\ndiff --git a/list-objects-filter-options.h b/list-objects-filter-options.h\nnew file mode 100644\nindex 0000000..797bd3a\n--- /dev/null\n+++ b/list-objects-filter-options.h\n@@ -0,0 +1,57 @@\n+#ifndef LIST_OBJECTS_FILTER_OPTIONS_H\n+#define LIST_OBJECTS_FILTER_OPTIONS_H\n+\n+#include \"parse-options.h\"\n+\n+/*\n+ * The list of defined filters for list-objects.\n+ */\n+enum list_objects_filter_choice {\n+\tLOFC_DISABLED = 0,\n+\tLOFC_BLOB_NONE,\n+\tLOFC_BLOB_LIMIT,\n+\tLOFC_SPARSE_OID,\n+\tLOFC_SPARSE_PATH,\n+\tLOFC__COUNT /* must be last */\n+};\n+\n+struct list_objects_filter_options {\n+\t/*\n+\t * The raw argument value given on the command line or\n+\t * protocol request. (The part after the \"--keyword=\".)\n+\t */\n+\tchar *raw_value;\n+\n+\t/*\n+\t * Positive if raw_value contains special characters that may\n+\t * cause injection or quoting issues when raw_value is passed\n+\t * to a subordinate command or script.\n+\t */\n+\toff_t requires_armor;\n+\n+\t/*\n+\t * Parsed values. Only 1 will be set depending on the flags below.\n+\t */\n+\tstruct object_id *sparse_oid_value;\n+\tchar *sparse_path_value;\n+\tunsigned long blob_limit_value;\n+\n+\tenum list_objects_filter_choice choice;\n+};\n+\n+/* Normalized command line arguments */\n+#define CL_ARG__FILTER \"filter\"\n+\n+int parse_list_objects_filter(\n+\tstruct list_objects_filter_options *filter_options,\n+\tconst char *arg);\n+\n+int opt_parse_list_objects_filter(const struct option *opt,\n+\t\t\t\t const char *arg, int unset);\n+\n+#define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \\\n+\t{ OPTION_CALLBACK, 0, CL_ARG__FILTER, fo, N_(\"args\"), \\\n+\t N_(\"object filtering\"), PARSE_OPT_NONEG, \\\n+\t opt_parse_list_objects_filter }\n+\n+#endif /* LIST_OBJECTS_FILTER_OPTIONS_H */\ndiff --git a/list-objects-filter.c b/list-objects-filter.c\nnew file mode 100644\nindex 0000000..4356c45\n--- /dev/null\n+++ b/list-objects-filter.c\n@@ -0,0 +1,401 @@\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"tag.h\"\n+#include \"commit.h\"\n+#include \"tree.h\"\n+#include \"blob.h\"\n+#include \"diff.h\"\n+#include \"tree-walk.h\"\n+#include \"revision.h\"\n+#include \"list-objects.h\"\n+#include \"list-objects-filter.h\"\n+#include \"list-objects-filter-options.h\"\n+#include \"oidset.h\"\n+\n+/* Remember to update object flag allocation in object.h */\n+/*\n+ * FILTER_SHOWN_BUT_REVISIT -- we set this bit on tree objects\n+ * that have been shown, but should be revisited if they appear\n+ * in the traversal (until we mark it SEEN). This is a way to\n+ * let us silently de-dup calls to show() in the caller. This\n+ * is subtly different from the \"revision.h:SHOWN\" and the\n+ * \"sha1_name.c:ONELINE_SEEN\" bits. And also different from\n+ * the non-de-dup usage in pack-bitmap.c\n+ */\n+#define FILTER_SHOWN_BUT_REVISIT (1<<21)\n+\n+/*\n+ * A filter for list-objects to omit ALL blobs from the traversal.\n+ * And to OPTIONALLY collect a list of the omitted OIDs.\n+ */\n+struct filter_blobs_none_data {\n+\tstruct oidset *omits;\n+};\n+\n+static enum list_objects_filter_result filter_blobs_none(\n+\tenum list_objects_filter_situation filter_situation,\n+\tstruct object *obj,\n+\tconst char *pathname,\n+\tconst char *filename,\n+\tvoid *filter_data_)\n+{\n+\tstruct filter_blobs_none_data *filter_data = filter_data_;\n+\n+\tswitch (filter_situation) {\n+\tdefault:\n+\t\tdie(\"unknown filter_situation\");\n+\t\treturn LOFR_ZERO;\n+\n+\tcase LOFS_BEGIN_TREE:\n+\t\tassert(obj->type == OBJ_TREE);\n+\t\t/* always include all tree objects */\n+\t\treturn LOFR_MARK_SEEN | LOFR_DO_SHOW;\n+\n+\tcase LOFS_END_TREE:\n+\t\tassert(obj->type == OBJ_TREE);\n+\t\treturn LOFR_ZERO;\n+\n+\tcase LOFS_BLOB:\n+\t\tassert(obj->type == OBJ_BLOB);\n+\t\tassert((obj->flags & SEEN) == 0);\n+\n+\t\tif (filter_data->omits)\n+\t\t\toidset_insert(filter_data->omits, &obj->oid);\n+\t\treturn LOFR_MARK_SEEN; /* but not LOFR_DO_SHOW (hard omit) */\n+\t}\n+}\n+\n+static void *filter_blobs_none__init(\n+\tstruct oidset *omitted,\n+\tstruct list_objects_filter_options *filter_options,\n+\tfilter_object_fn *filter_fn,\n+\tfilter_free_fn *filter_free_fn)\n+{\n+\tstruct filter_blobs_none_data *d = xcalloc(1, sizeof(*d));\n+\td->omits = omitted;\n+\n+\t*filter_fn = filter_blobs_none;\n+\t*filter_free_fn = free;\n+\treturn d;\n+}\n+\n+/*\n+ * A filter for list-objects to omit large blobs.\n+ * And to OPTIONALLY collect a list of the omitted OIDs.\n+ */\n+struct filter_blobs_limit_data {\n+\tstruct oidset *omits;\n+\tunsigned long max_bytes;\n+};\n+\n+static enum list_objects_filter_result filter_blobs_limit(\n+\tenum list_objects_filter_situation filter_situation,\n+\tstruct object *obj,\n+\tconst char *pathname,\n+\tconst char *filename,\n+\tvoid *filter_data_)\n+{\n+\tstruct filter_blobs_limit_data *filter_data = filter_data_;\n+\tunsigned long object_length;\n+\tenum object_type t;\n+\n+\tswitch (filter_situation) {\n+\tdefault:\n+\t\tdie(\"unknown filter_situation\");\n+\t\treturn LOFR_ZERO;\n+\n+\tcase LOFS_BEGIN_TREE:\n+\t\tassert(obj->type == OBJ_TREE);\n+\t\t/* always include all tree objects */\n+\t\treturn LOFR_MARK_SEEN | LOFR_DO_SHOW;\n+\n+\tcase LOFS_END_TREE:\n+\t\tassert(obj->type == OBJ_TREE);\n+\t\treturn LOFR_ZERO;\n+\n+\tcase LOFS_BLOB:\n+\t\tassert(obj->type == OBJ_BLOB);\n+\t\tassert((obj->flags & SEEN) == 0);\n+\n+\t\tt = sha1_object_info(obj->oid.hash, &object_length);\n+\t\tif (t != OBJ_BLOB) { /* probably OBJ_NONE */\n+\t\t\t/*\n+\t\t\t * We DO NOT have the blob locally, so we cannot\n+\t\t\t * apply the size filter criteria. Be conservative\n+\t\t\t * and force show it (and let the caller deal with\n+\t\t\t * the ambiguity).\n+\t\t\t */\n+\t\t\tgoto include_it;\n+\t\t}\n+\n+\t\tif (object_length < filter_data->max_bytes)\n+\t\t\tgoto include_it;\n+\n+\t\tif (filter_data->omits)\n+\t\t\toidset_insert(filter_data->omits, &obj->oid);\n+\t\treturn LOFR_MARK_SEEN; /* but not LOFR_DO_SHOW (hard omit) */\n+\t}\n+\n+include_it:\n+\tif (filter_data->omits)\n+\t\toidset_remove(filter_data->omits, &obj->oid);\n+\treturn LOFR_MARK_SEEN | LOFR_DO_SHOW;\n+}\n+\n+static void *filter_blobs_limit__init(\n+\tstruct oidset *omitted,\n+\tstruct list_objects_filter_options *filter_options,\n+\tfilter_object_fn *filter_fn,\n+\tfilter_free_fn *filter_free_fn)\n+{\n+\tstruct filter_blobs_limit_data *d = xcalloc(1, sizeof(*d));\n+\td->omits = omitted;\n+\td->max_bytes = filter_options->blob_limit_value;\n+\n+\t*filter_fn = filter_blobs_limit;\n+\t*filter_free_fn = free;\n+\treturn d;\n+}\n+\n+/*\n+ * A filter driven by a sparse-checkout specification to only\n+ * include blobs that a sparse checkout would populate.\n+ *\n+ * The sparse-checkout spec can be loaded from a blob with the\n+ * given OID or from a local pathname. We allow an OID because\n+ * the repo may be bare or we may be doing the filtering on the\n+ * server.\n+ */\n+struct frame {\n+\t/*\n+\t * defval is the usual default include/exclude value that\n+\t * should be inherited as we recurse into directories based\n+\t * upon pattern matching of the directory itself or of a\n+\t * containing directory.\n+\t */\n+\tint defval;\n+\n+\t/*\n+\t * 1 if the directory (recursively) contains any provisionally\n+\t * omitted objects.\n+\t *\n+\t * 0 if everything (recursively) contained in this directory\n+\t * has been explicitly included (SHOWN) in the result and\n+\t * the directory may be short-cut later in the traversal.\n+\t */\n+\tunsigned child_prov_omit : 1;\n+};\n+\n+struct filter_sparse_data {\n+\tstruct oidset *omits;\n+\tstruct exclude_list el;\n+\n+\tsize_t nr, alloc;\n+\tstruct frame *array_frame;\n+};\n+\n+static enum list_objects_filter_result filter_sparse(\n+\tenum list_objects_filter_situation filter_situation,\n+\tstruct object *obj,\n+\tconst char *pathname,\n+\tconst char *filename,\n+\tvoid *filter_data_)\n+{\n+\tstruct filter_sparse_data *filter_data = filter_data_;\n+\tint val, dtype;\n+\tstruct frame *frame;\n+\n+\tswitch (filter_situation) {\n+\tdefault:\n+\t\tdie(\"unknown filter_situation\");\n+\t\treturn LOFR_ZERO;\n+\n+\tcase LOFS_BEGIN_TREE:\n+\t\tassert(obj->type == OBJ_TREE);\n+\t\tdtype = DT_DIR;\n+\t\tval = is_excluded_from_list(pathname, strlen(pathname),\n+\t\t\t\t\t filename, &dtype, &filter_data->el,\n+\t\t\t\t\t &the_index);\n+\t\tif (val < 0)\n+\t\t\tval = filter_data->array_frame[filter_data->nr].defval;\n+\n+\t\tALLOC_GROW(filter_data->array_frame, filter_data->nr + 1,\n+\t\t\t filter_data->alloc);\n+\t\tfilter_data->nr++;\n+\t\tfilter_data->array_frame[filter_data->nr].defval = val;\n+\t\tfilter_data->array_frame[filter_data->nr].child_prov_omit = 0;\n+\n+\t\t/*\n+\t\t * A directory with this tree OID may appear in multiple\n+\t\t * places in the tree. (Think of a directory move or copy,\n+\t\t * with no other changes, so the OID is the same, but the\n+\t\t * full pathnames of objects within this directory are new\n+\t\t * and may match is_excluded() patterns differently.)\n+\t\t * So we cannot mark this directory as SEEN (yet), since\n+\t\t * that will prevent process_tree() from revisiting this\n+\t\t * tree object with other pathname prefixes.\n+\t\t *\n+\t\t * Only _DO_SHOW the tree object the first time we visit\n+\t\t * this tree object.\n+\t\t *\n+\t\t * We always show all tree objects. A future optimization\n+\t\t * may want to attempt to narrow this.\n+\t\t */\n+\t\tif (obj->flags & FILTER_SHOWN_BUT_REVISIT)\n+\t\t\treturn LOFR_ZERO;\n+\t\tobj->flags |= FILTER_SHOWN_BUT_REVISIT;\n+\t\treturn LOFR_DO_SHOW;\n+\n+\tcase LOFS_END_TREE:\n+\t\tassert(obj->type == OBJ_TREE);\n+\t\tassert(filter_data->nr > 0);\n+\n+\t\tframe = &filter_data->array_frame[filter_data->nr];\n+\t\tfilter_data->nr--;\n+\n+\t\t/*\n+\t\t * Tell our parent directory if any of our children were\n+\t\t * provisionally omitted.\n+\t\t */\n+\t\tfilter_data->array_frame[filter_data->nr].child_prov_omit |=\n+\t\t\tframe->child_prov_omit;\n+\n+\t\t/*\n+\t\t * If there are NO provisionally omitted child objects (ALL child\n+\t\t * objects in this folder were INCLUDED), then we can mark the\n+\t\t * folder as SEEN (so we will not have to revisit it again).\n+\t\t */\n+\t\tif (!frame->child_prov_omit)\n+\t\t\treturn LOFR_MARK_SEEN;\n+\t\treturn LOFR_ZERO;\n+\n+\tcase LOFS_BLOB:\n+\t\tassert(obj->type == OBJ_BLOB);\n+\t\tassert((obj->flags & SEEN) == 0);\n+\n+\t\tframe = &filter_data->array_frame[filter_data->nr];\n+\n+\t\tdtype = DT_REG;\n+\t\tval = is_excluded_from_list(pathname, strlen(pathname),\n+\t\t\t\t\t filename, &dtype, &filter_data->el,\n+\t\t\t\t\t &the_index);\n+\t\tif (val < 0)\n+\t\t\tval = frame->defval;\n+\t\tif (val > 0) {\n+\t\t\tif (filter_data->omits)\n+\t\t\t\toidset_remove(filter_data->omits, &obj->oid);\n+\t\t\treturn LOFR_MARK_SEEN | LOFR_DO_SHOW;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Provisionally omit it. We've already established that\n+\t\t * this pathname is not in the sparse-checkout specification\n+\t\t * with the CURRENT pathname, so we *WANT* to omit this blob.\n+\t\t *\n+\t\t * However, a pathname elsewhere in the tree may also\n+\t\t * reference this same blob, so we cannot reject it yet.\n+\t\t * Leave the LOFR_ bits unset so that if the blob appears\n+\t\t * again in the traversal, we will be asked again.\n+\t\t */\n+\t\tif (filter_data->omits)\n+\t\t\toidset_insert(filter_data->omits, &obj->oid);\n+\n+\t\t/*\n+\t\t * Remember that at least 1 blob in this tree was\n+\t\t * provisionally omitted. This prevents us from short\n+\t\t * cutting the tree in future iterations.\n+\t\t */\n+\t\tframe->child_prov_omit = 1;\n+\t\treturn LOFR_ZERO;\n+\t}\n+}\n+\n+\n+static void filter_sparse_free(void *filter_data)\n+{\n+\tstruct filter_sparse_data *d = filter_data;\n+\t/* TODO free contents of 'd' */\n+\tfree(d);\n+}\n+\n+static void *filter_sparse_oid__init(\n+\tstruct oidset *omitted,\n+\tstruct list_objects_filter_options *filter_options,\n+\tfilter_object_fn *filter_fn,\n+\tfilter_free_fn *filter_free_fn)\n+{\n+\tstruct filter_sparse_data *d = xcalloc(1, sizeof(*d));\n+\td->omits = omitted;\n+\tif (add_excludes_from_blob_to_list(filter_options->sparse_oid_value,\n+\t\t\t\t\t NULL, 0, &d->el) < 0)\n+\t\tdie(\"could not load filter specification\");\n+\n+\tALLOC_GROW(d->array_frame, d->nr + 1, d->alloc);\n+\td->array_frame[d->nr].defval = 0; /* default to include */\n+\td->array_frame[d->nr].child_prov_omit = 0;\n+\n+\t*filter_fn = filter_sparse;\n+\t*filter_free_fn = filter_sparse_free;\n+\treturn d;\n+}\n+\n+static void *filter_sparse_path__init(\n+\tstruct oidset *omitted,\n+\tstruct list_objects_filter_options *filter_options,\n+\tfilter_object_fn *filter_fn,\n+\tfilter_free_fn *filter_free_fn)\n+{\n+\tstruct filter_sparse_data *d = xcalloc(1, sizeof(*d));\n+\td->omits = omitted;\n+\tif (add_excludes_from_file_to_list(filter_options->sparse_path_value,\n+\t\t\t\t\t NULL, 0, &d->el, NULL) < 0)\n+\t\tdie(\"could not load filter specification\");\n+\n+\tALLOC_GROW(d->array_frame, d->nr + 1, d->alloc);\n+\td->array_frame[d->nr].defval = 0; /* default to include */\n+\td->array_frame[d->nr].child_prov_omit = 0;\n+\n+\t*filter_fn = filter_sparse;\n+\t*filter_free_fn = filter_sparse_free;\n+\treturn d;\n+}\n+\n+typedef void *(*filter_init_fn)(\n+\tstruct oidset *omitted,\n+\tstruct list_objects_filter_options *filter_options,\n+\tfilter_object_fn *filter_fn,\n+\tfilter_free_fn *filter_free_fn);\n+\n+/*\n+ * Must match \"enum list_objects_filter_choice\".\n+ */\n+static filter_init_fn s_filters[] = {\n+\tNULL,\n+\tfilter_blobs_none__init,\n+\tfilter_blobs_limit__init,\n+\tfilter_sparse_oid__init,\n+\tfilter_sparse_path__init,\n+};\n+\n+void *list_objects_filter__init(\n+\tstruct oidset *omitted,\n+\tstruct list_objects_filter_options *filter_options,\n+\tfilter_object_fn *filter_fn,\n+\tfilter_free_fn *filter_free_fn)\n+{\n+\tfilter_init_fn init_fn;\n+\n+\tassert((sizeof(s_filters) / sizeof(s_filters[0])) == LOFC__COUNT);\n+\n+\tif (filter_options->choice >= LOFC__COUNT)\n+\t\tdie(\"invalid list-objects filter choice: %d\",\n+\t\t filter_options->choice);\n+\n+\tinit_fn = s_filters[filter_options->choice];\n+\tif (init_fn)\n+\t\treturn init_fn(omitted, filter_options,\n+\t\t\t filter_fn, filter_free_fn);\n+\t*filter_fn = NULL;\n+\t*filter_free_fn = NULL;\n+\treturn NULL;\n+}\ndiff --git a/list-objects-filter.h b/list-objects-filter.h\nnew file mode 100644\nindex 0000000..a963d02\n--- /dev/null\n+++ b/list-objects-filter.h\n@@ -0,0 +1,77 @@\n+#ifndef LIST_OBJECTS_FILTER_H\n+#define LIST_OBJECTS_FILTER_H\n+\n+/*\n+ * During list-object traversal we allow certain objects to be\n+ * filtered (omitted) from the result. The active filter uses\n+ * these result values to guide list-objects.\n+ *\n+ * _ZERO : Do nothing with the object at this time. It may\n+ * be revisited if it appears in another place in\n+ * the tree or in another commit during the overall\n+ * traversal.\n+ *\n+ * _MARK_SEEN : Mark this object as \"SEEN\" in the object flags.\n+ * This will prevent it from being revisited during\n+ * the remainder of the traversal. This DOES NOT\n+ * imply that it will be included in the results.\n+ *\n+ * _DO_SHOW : Show this object in the results (call show() on it).\n+ * In general, objects should only be shown once, but\n+ * this result DOES NOT imply that we mark it SEEN.\n+ *\n+ * Most of the time, you want the combination (_MARK_SEEN | _DO_SHOW)\n+ * but they can be used independently, such as when sparse-checkout\n+ * pattern matching is being applied.\n+ *\n+ * A _MARK_SEEN without _DO_SHOW can be called a hard-omit -- the\n+ * object is not shown and will never be reconsidered (unless a\n+ * previous iteration has already shown it).\n+ *\n+ * A _DO_SHOW without _MARK_SEEN can be used, for example, to\n+ * include a directory, but then revisit it to selectively include\n+ * or omit objects within it.\n+ *\n+ * A _ZERO can be called a provisional-omit -- the object is NOT shown,\n+ * but *may* be revisited (if the object appears again in the traversal).\n+ * Therefore, it will be omitted from the results *unless* a later\n+ * iteration causes it to be shown.\n+ */\n+enum list_objects_filter_result {\n+\tLOFR_ZERO = 0,\n+\tLOFR_MARK_SEEN = 1<<0,\n+\tLOFR_DO_SHOW = 1<<1,\n+};\n+\n+enum list_objects_filter_situation {\n+\tLOFS_BEGIN_TREE,\n+\tLOFS_END_TREE,\n+\tLOFS_BLOB\n+};\n+\n+typedef enum list_objects_filter_result (*filter_object_fn)(\n+\tenum list_objects_filter_situation filter_situation,\n+\tstruct object *obj,\n+\tconst char *pathname,\n+\tconst char *filename,\n+\tvoid *filter_data);\n+\n+typedef void (*filter_free_fn)(void *filter_data);\n+\n+/*\n+ * Constructor for the set of defined list-objects filters.\n+ * Returns a generic \"void *filter_data\".\n+ *\n+ * The returned \"filter_fn\" will be used by traverse_commit_list()\n+ * to filter the results.\n+ *\n+ * The returned \"filter_free_fn\" is a destructor for the\n+ * filter_data.\n+ */\n+void *list_objects_filter__init(\n+\tstruct oidset *omitted,\n+\tstruct list_objects_filter_options *filter_options,\n+\tfilter_object_fn *filter_fn,\n+\tfilter_free_fn *filter_free_fn);\n+\n+#endif /* LIST_OBJECTS_FILTER_H */\ndiff --git a/list-objects.c b/list-objects.c\nindex b3931fa..d9e83d0 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -7,16 +7,21 @@\n #include \"tree-walk.h\"\n #include \"revision.h\"\n #include \"list-objects.h\"\n+#include \"list-objects-filter.h\"\n+#include \"list-objects-filter-options.h\"\n \n static void process_blob(struct rev_info *revs,\n \t\t\t struct blob *blob,\n \t\t\t show_object_fn show,\n \t\t\t struct strbuf *path,\n \t\t\t const char *name,\n-\t\t\t void *cb_data)\n+\t\t\t void *cb_data,\n+\t\t\t filter_object_fn filter_fn,\n+\t\t\t void *filter_data)\n {\n \tstruct object *obj = &blob->object;\n \tsize_t pathlen;\n+\tenum list_objects_filter_result r = LOFR_MARK_SEEN | LOFR_DO_SHOW;\n \n \tif (!revs->blob_objects)\n \t\treturn;\n@@ -24,11 +29,17 @@ static void process_blob(struct rev_info *revs,\n \t\tdie(\"bad blob object\");\n \tif (obj->flags & (UNINTERESTING | SEEN))\n \t\treturn;\n-\tobj->flags |= SEEN;\n \n \tpathlen = path->len;\n \tstrbuf_addstr(path, name);\n-\tshow(obj, path->buf, cb_data);\n+\tif (filter_fn)\n+\t\tr = filter_fn(LOFS_BLOB, obj,\n+\t\t\t path->buf, &path->buf[pathlen],\n+\t\t\t filter_data);\n+\tif (r & LOFR_MARK_SEEN)\n+\t\tobj->flags |= SEEN;\n+\tif (r & LOFR_DO_SHOW)\n+\t\tshow(obj, path->buf, cb_data);\n \tstrbuf_setlen(path, pathlen);\n }\n \n@@ -69,7 +80,9 @@ static void process_tree(struct rev_info *revs,\n \t\t\t show_object_fn show,\n \t\t\t struct strbuf *base,\n \t\t\t const char *name,\n-\t\t\t void *cb_data)\n+\t\t\t void *cb_data,\n+\t\t\t filter_object_fn filter_fn,\n+\t\t\t void *filter_data)\n {\n \tstruct object *obj = &tree->object;\n \tstruct tree_desc desc;\n@@ -77,6 +90,7 @@ static void process_tree(struct rev_info *revs,\n \tenum interesting match = revs->diffopt.pathspec.nr == 0 ?\n \t\tall_entries_interesting: entry_not_interesting;\n \tint baselen = base->len;\n+\tenum list_objects_filter_result r = LOFR_MARK_SEEN | LOFR_DO_SHOW;\n \n \tif (!revs->tree_objects)\n \t\treturn;\n@@ -90,9 +104,15 @@ static void process_tree(struct rev_info *revs,\n \t\tdie(\"bad tree object %s\", oid_to_hex(&obj->oid));\n \t}\n \n-\tobj->flags |= SEEN;\n \tstrbuf_addstr(base, name);\n-\tshow(obj, base->buf, cb_data);\n+\tif (filter_fn)\n+\t\tr = filter_fn(LOFS_BEGIN_TREE, obj,\n+\t\t\t base->buf, &base->buf[baselen],\n+\t\t\t filter_data);\n+\tif (r & LOFR_MARK_SEEN)\n+\t\tobj->flags |= SEEN;\n+\tif (r & LOFR_DO_SHOW)\n+\t\tshow(obj, base->buf, cb_data);\n \tif (base->len)\n \t\tstrbuf_addch(base, '/');\n \n@@ -112,7 +132,7 @@ static void process_tree(struct rev_info *revs,\n \t\t\tprocess_tree(revs,\n \t\t\t\t lookup_tree(entry.oid),\n \t\t\t\t show, base, entry.path,\n-\t\t\t\t cb_data);\n+\t\t\t\t cb_data, filter_fn, filter_data);\n \t\telse if (S_ISGITLINK(entry.mode))\n \t\t\tprocess_gitlink(revs, entry.oid->hash,\n \t\t\t\t\tshow, base, entry.path,\n@@ -121,8 +141,19 @@ static void process_tree(struct rev_info *revs,\n \t\t\tprocess_blob(revs,\n \t\t\t\t lookup_blob(entry.oid),\n \t\t\t\t show, base, entry.path,\n-\t\t\t\t cb_data);\n+\t\t\t\t cb_data, filter_fn, filter_data);\n \t}\n+\n+\tif (filter_fn) {\n+\t\tr = filter_fn(LOFS_END_TREE, obj,\n+\t\t\t base->buf, &base->buf[baselen],\n+\t\t\t filter_data);\n+\t\tif (r & LOFR_MARK_SEEN)\n+\t\t\tobj->flags |= SEEN;\n+\t\tif (r & LOFR_DO_SHOW)\n+\t\t\tshow(obj, base->buf, cb_data);\n+\t}\n+\n \tstrbuf_setlen(base, baselen);\n \tfree_tree_buffer(tree);\n }\n@@ -183,10 +214,12 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n \tadd_pending_object(revs, &tree->object, \"\");\n }\n \n-void traverse_commit_list(struct rev_info *revs,\n-\t\t\t show_commit_fn show_commit,\n-\t\t\t show_object_fn show_object,\n-\t\t\t void *data)\n+static void do_traverse(struct rev_info *revs,\n+\t\t\tshow_commit_fn show_commit,\n+\t\t\tshow_object_fn show_object,\n+\t\t\tvoid *show_data,\n+\t\t\tfilter_object_fn filter_fn,\n+\t\t\tvoid *filter_data)\n {\n \tint i;\n \tstruct commit *commit;\n@@ -200,7 +233,7 @@ void traverse_commit_list(struct rev_info *revs,\n \t\t */\n \t\tif (commit->tree)\n \t\t\tadd_pending_tree(revs, commit->tree);\n-\t\tshow_commit(commit, data);\n+\t\tshow_commit(commit, show_data);\n \t}\n \tfor (i = 0; i < revs->pending.nr; i++) {\n \t\tstruct object_array_entry *pending = revs->pending.objects + i;\n@@ -211,19 +244,21 @@ void traverse_commit_list(struct rev_info *revs,\n \t\t\tcontinue;\n \t\tif (obj->type == OBJ_TAG) {\n \t\t\tobj->flags |= SEEN;\n-\t\t\tshow_object(obj, name, data);\n+\t\t\tshow_object(obj, name, show_data);\n \t\t\tcontinue;\n \t\t}\n \t\tif (!path)\n \t\t\tpath = \"\";\n \t\tif (obj->type == OBJ_TREE) {\n \t\t\tprocess_tree(revs, (struct tree *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t &base, path, show_data,\n+\t\t\t\t filter_fn, filter_data);\n \t\t\tcontinue;\n \t\t}\n \t\tif (obj->type == OBJ_BLOB) {\n \t\t\tprocess_blob(revs, (struct blob *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t &base, path, show_data,\n+\t\t\t\t filter_fn, filter_data);\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"unknown pending object %s (%s)\",\n@@ -232,3 +267,31 @@ void traverse_commit_list(struct rev_info *revs,\n \tobject_array_clear(&revs->pending);\n \tstrbuf_release(&base);\n }\n+\n+void traverse_commit_list(struct rev_info *revs,\n+\t\t\t show_commit_fn show_commit,\n+\t\t\t show_object_fn show_object,\n+\t\t\t void *show_data)\n+{\n+\tdo_traverse(revs, show_commit, show_object, show_data, NULL, NULL);\n+}\n+\n+void traverse_commit_list_filtered(\n+\tstruct list_objects_filter_options *filter_options,\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tvoid *show_data,\n+\tstruct oidset *omitted)\n+{\n+\tfilter_object_fn filter_fn = NULL;\n+\tfilter_free_fn filter_free_fn = NULL;\n+\tvoid *filter_data = NULL;\n+\n+\tfilter_data = list_objects_filter__init(omitted, filter_options,\n+\t\t\t\t\t\t&filter_fn, &filter_free_fn);\n+\tdo_traverse(revs, show_commit, show_object, show_data,\n+\t\t filter_fn, filter_data);\n+\tif (filter_data && filter_free_fn)\n+\t\tfilter_free_fn(filter_data);\n+}\ndiff --git a/list-objects.h b/list-objects.h\nindex 0cebf85..aa618d7 100644\n--- a/list-objects.h\n+++ b/list-objects.h\n@@ -8,4 +8,15 @@ void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, voi\n typedef void (*show_edge_fn)(struct commit *);\n void mark_edges_uninteresting(struct rev_info *, show_edge_fn);\n \n-#endif\n+struct oidset;\n+struct list_objects_filter_options;\n+\n+void traverse_commit_list_filtered(\n+\tstruct list_objects_filter_options *filter_options,\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tvoid *show_data,\n+\tstruct oidset *omitted);\n+\n+#endif /* LIST_OBJECTS_H */\ndiff --git a/object.h b/object.h\nindex df8abe9..f34461d 100644\n--- a/object.h\n+++ b/object.h\n@@ -38,6 +38,7 @@ struct object_array {\n * http-push.c: 16-----19\n * commit.c: 16-----19\n * sha1_name.c: 20\n+ * list-objects-filter.c: 21\n * builtin/fsck.c: 0--3\n */\n #define FLAG_BITS 27\n-- \n2.9.3\n\n"},{"id":"332720","messageId":"20171116122133.4cc718414579c1a5a682174b@google.com","threadId":"47223","inReplyTo":"20171116180743.61353-5-git@jeffhostetler.com","subject":"Re: [PATCH v4 4/6] list-objects: filter objects in traverse_commit_list","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-11-16T20:21:33Z","receivedAt":"2017-11-16T20:21:40Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 16 Nov 2017 18:07:41 +0000\nJeff Hostetler wrote:\n\n> +/*\n> + * Return 1 if the given string needs armoring because of \"special\"\n> + * characters that may cause injection problems when a command passes\n> + * the argument to a subordinate command (such as when upload-pack\n> + * launches pack-objects).\n> + *\n> + * The usual alphanumeric and key punctuation do not trigger it.\n> + */ \n> +static int arg_needs_armor(const char *arg)\n\nFirst of all, about the injection problem, replying to your previous e-mail\n[1]:\n\nhttps://public-inbox.org/git/61855872-221b-0e97-abaa-24a011ad899e@jeffhostetler.com/\n\n> I couldn't use quote.[ch] because it is more concerned with\n> quoting pathnames because of LF and CR characters within\n> them -- rather than semicolons and quotes and the like which\n> I was concerned about.\n\nsq_quote_buf() (or one of the other similarly-named functions) should\nsolve this problem, right? The single quotes around the argument takes\ncare of LF, CR, and semicolons, and things like backslashes and quotes\nare taken care of as documented.\n\nI don't think we need to invent another encoding to solve this.\n\n> +{\n> +\tconst unsigned char *p;\n> +\n> +\tfor (p = (const unsigned char *)arg; *p; p++) {\n> +\t\tif (*p >= 'a' && *p <= 'z')\n> +\t\t\tcontinue;\n> +\t\tif (*p >= 'A' && *p <= 'Z')\n> +\t\t\tcontinue;\n> +\t\tif (*p >= '0' && *p <= '9')\n> +\t\t\tcontinue;\n> +\t\tif (*p == '-' || *p == '_' || *p == '.' || *p == '/')\n> +\t\t\tcontinue;\n\nIf we do take this approach, can ':' also be included?\n\n> +\tif (skip_prefix(arg, \"sparse:oid=\", &v0)) {\n> +\t\tstruct object_context oc;\n> +\t\tstruct object_id sparse_oid;\n> +\n> +\t\t/*\n> +\t\t * Try to parse into an OID for the current\n> +\t\t * command, but DO NOT complain if we don't have the blob or\n> +\t\t * ref locally.\n> +\t\t */\n> +\t\tif (!get_oid_with_context(v0, GET_OID_BLOB,\n> +\t\t\t\t\t &sparse_oid, &oc))\n> +\t\t\tfilter_options->sparse_oid_value = oiddup(&sparse_oid);\n> +\t\tfilter_options->choice = LOFC_SPARSE_OID;\n> +\t\tif (arg_needs_armor(v0))\n> +\t\t\tfilter_options->requires_armor = v0 - arg;\n> +\t\treturn 0;\n> +\t}\n\nIn your previous e-mail, you mentioned:\n\n> yes. I always pass filter_options.raw_value over the wire.\n> The code above tries to parse it and put it in an OID for\n> private use by the current process -- just like the size limit\n> value in the blob:limit filter.\n\nSo I think this function should complain if you don't have the blob or\nref locally. (I envision that if a filter string is to be directly sent\nto a server, it should be stored as a string, not processed by this\nfunction first.)\n"},{"id":"332722","messageId":"20171116124354.3d800cc27dbe3ea668c64519@google.com","threadId":"47223","inReplyTo":"20171116180743.61353-6-git@jeffhostetler.com","subject":"Re: [PATCH v4 5/6] rev-list: add list-objects filtering support","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-11-16T20:43:54Z","receivedAt":"2017-11-16T20:44:01Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 16 Nov 2017 18:07:42 +0000\nJeff Hostetler wrote:\n\n> From: Jeff Hostetler \n> \n> Teach rev-list to use the filtering provided by the\n> traverse_commit_list_filtered() interface to omit\n> unwanted objects from the result. This feature is\n> intended to help with partial clone.\n> \n> Object filtering is only allowed when one of the \"--objects*\"\n> options are used.\n> \n> When the \"--filter-print-omitted\" option is used, the omitted\n> objects are printed at the end. These are marked with a \"~\".\n> This option can be combined with \"--quiet\" to get a list of\n> just the omitted objects.\n> \n> Added \"--missing=(error|print|omit)\" argument to specify how\n> rev-list should behave when it encounters a missing object\n> (presumably from a prior partial clone).\n> \n> When \"--missing=print\" is used, rev-list will print a list of\n> any missing objects that should have been included in the output.\n> These are marked with a \"?\".\n> \n> Add t6112 test.\n\nThe patch itself looks good, except that I have a nagging feeling about\nthe usage of the term \"partial clone\" in the commit message,\ndocumentation, and test description. I feel that the usage here leads\none to believe that partial clones haphazardly leave repositories\nwithout random objects (and at the point that this patch is merged,\nthere will not be any patch in the main repo contradicting this\nviewpoint), contrary to the fact that we will have a tracking mechanism\nto track which missing objects are expected to be missing. (If I'm the\nonly one feeling this way, though, then I'll just let it slide.)\n\nIf it were up to me, I would remove all existing mentions of \"partial\nclone\" and explain the presence of the \"--missing\" argument as follows:\n\n In the future, we will introduce a \"partial clone\" mechanism wherein\n an object in a repo, obtained from a remote, may reference a missing\n object that can be dynamically fetched from that remote once needed.\n This \"partial clone\" mechanism will have a way, sometimes slow, of\n determining if a missing link is one of the links expected to be\n produced by this mechanism.\n\n This patch introduces handling of missing objects to help debugging\n and development of the \"partial clone\" mechanism, and once the\n mechanism is implemented, for a power user to perform operations\n that are missing-object-aware without incurring the cost of checking\n if a missing link is expected.\n"},{"id":"332725","messageId":"bdf75d90-570d-8cf0-886a-2c5c9cdba1c1@jeffhostetler.com","threadId":"47223","inReplyTo":"20171116122133.4cc718414579c1a5a682174b@google.com","subject":"Re: [PATCH v4 4/6] list-objects: filter objects in traverse_commit_list","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T21:49:08Z","receivedAt":"2017-11-16T21:49:15Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 11/16/2017 3:21 PM, Jonathan Tan wrote:\n> On Thu, 16 Nov 2017 18:07:41 +0000\n> Jeff Hostetler wrote:\n> \n>> +/*\n>> + * Return 1 if the given string needs armoring because of \"special\"\n>> + * characters that may cause injection problems when a command passes\n>> + * the argument to a subordinate command (such as when upload-pack\n>> + * launches pack-objects).\n>> + *\n>> + * The usual alphanumeric and key punctuation do not trigger it.\n>> + */\n>> +static int arg_needs_armor(const char *arg)\n> \n> First of all, about the injection problem, replying to your previous e-mail\n> [1]:\n> \n> https://public-inbox.org/git/61855872-221b-0e97-abaa-24a011ad899e@jeffhostetler.com/\n> \n>> I couldn't use quote.[ch] because it is more concerned with\n>> quoting pathnames because of LF and CR characters within\n>> them -- rather than semicolons and quotes and the like which\n>> I was concerned about.\n> \n> sq_quote_buf() (or one of the other similarly-named functions) should\n> solve this problem, right? The single quotes around the argument takes\n> care of LF, CR, and semicolons, and things like backslashes and quotes\n> are taken care of as documented.\n> \n> I don't think we need to invent another encoding to solve this.\n\nI'll take another look, sq_quote_buf() looks like it might work.\nI was looking at quote_c_style() and that didn't seem right for\nmy needs. Thanks.\n\n\n> \n>> +{\n>> +\tconst unsigned char *p;\n>> +\n>> +\tfor (p = (const unsigned char *)arg; *p; p++) {\n>> +\t\tif (*p >= 'a' && *p <= 'z')\n>> +\t\t\tcontinue;\n>> +\t\tif (*p >= 'A' && *p <= 'Z')\n>> +\t\t\tcontinue;\n>> +\t\tif (*p >= '0' && *p <= '9')\n>> +\t\t\tcontinue;\n>> +\t\tif (*p == '-' || *p == '_' || *p == '.' || *p == '/')\n>> +\t\t\tcontinue;\n> \n> If we do take this approach, can ':' also be included?\n\nSure. I just picked the common ones.\n\n> \n>> +\tif (skip_prefix(arg, \"sparse:oid=\", &v0)) {\n>> +\t\tstruct object_context oc;\n>> +\t\tstruct object_id sparse_oid;\n>> +\n>> +\t\t/*\n>> +\t\t * Try to parse into an OID for the current\n>> +\t\t * command, but DO NOT complain if we don't have the blob or\n>> +\t\t * ref locally.\n>> +\t\t */\n>> +\t\tif (!get_oid_with_context(v0, GET_OID_BLOB,\n>> +\t\t\t\t\t &sparse_oid, &oc))\n>> +\t\t\tfilter_options->sparse_oid_value = oiddup(&sparse_oid);\n>> +\t\tfilter_options->choice = LOFC_SPARSE_OID;\n>> +\t\tif (arg_needs_armor(v0))\n>> +\t\t\tfilter_options->requires_armor = v0 - arg;\n>> +\t\treturn 0;\n>> +\t}\n> \n> In your previous e-mail, you mentioned:\n> \n>> yes. I always pass filter_options.raw_value over the wire.\n>> The code above tries to parse it and put it in an OID for\n>> private use by the current process -- just like the size limit\n>> value in the blob:limit filter.\n> \n> So I think this function should complain if you don't have the blob or\n> ref locally. (I envision that if a filter string is to be directly sent\n> to a server, it should be stored as a string, not processed by this\n> function first.)\n\nThe whole point was for clone to be able to ask the server to use\na known sparse-checkout spec, based on a branch name and a pathname\nwithin the tree. That's more usable. My expectation was that users\ncould keep one or more sparse-checkout specs in the tree based upon\nthe area/subset of the tree they want to work on. Then do a partial\nclone based upon the desired subset. Granted, we make them publish\nOIDs (outside of git) and then make the user request the partial\nclone by OID, but that's awkward.\n\nThere are 2 uses of the \"--filter=...\" parser -- one to actually use\nthe data within the current process (rev-list and pack-objects) and\none to capture the value and pass it on to another process (fetch-pack\nand upload-pack). The former want the fields fully decoded into\nactual variables and the latter want the raw string to pass on.\nThe parser routine provides both values for its callers. (In an\nearlier draft, the parser would replace the raw_value of a sparse\nspec with the actual OID when it had it locally, but I took that\nout based on your earlier comments. Now, the raw_value is always\npassed by the second type of usages.)\n\nJeff\n\n"},{"id":"332727","messageId":"20171116215725.k44e3alk2lq6bbwu@sigill.intra.peff.net","threadId":"47223","inReplyTo":"bdf75d90-570d-8cf0-886a-2c5c9cdba1c1@jeffhostetler.com","subject":"Re: [PATCH v4 4/6] list-objects: filter objects in traverse_commit_list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-16T21:57:26Z","receivedAt":"2017-11-16T21:57:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 16, 2017 at 04:49:08PM -0500, Jeff Hostetler wrote:\n\n> > First of all, about the injection problem, replying to your previous e-mail\n> > [1]:\n> > \n> > https://public-inbox.org/git/61855872-221b-0e97-abaa-24a011ad899e@jeffhostetler.com/\n> > \n> > > I couldn't use quote.[ch] because it is more concerned with\n> > > quoting pathnames because of LF and CR characters within\n> > > them -- rather than semicolons and quotes and the like which\n> > > I was concerned about.\n> > \n> > sq_quote_buf() (or one of the other similarly-named functions) should\n> > solve this problem, right? The single quotes around the argument takes\n> > care of LF, CR, and semicolons, and things like backslashes and quotes\n> > are taken care of as documented.\n> > \n> > I don't think we need to invent another encoding to solve this.\n> \n> I'll take another look, sq_quote_buf() looks like it might work.\n> I was looking at quote_c_style() and that didn't seem right for\n> my needs. Thanks.\n\nI admit I haven't been following this thread closely, but I couldn't\nseem to find any indication of exactly which interfaces need quoting, or\nwho is expected to unquote (here or in the previous iterations).\n\nIt sounds like you're worried about shell injection, but shouldn't we\nworry about that the actual shell boundary? Likewise, if these values\nare being passed over the git protocol, shouldn't that part of the\nprotocol be designed to encode arbitrary bytes?\n\nThose encodings don't necessarily need to be the same, because they're\nabout transport. Inside each process we'd have the raw bytes, and encode\nthem as appropriate to whatever sub-program we're going to pass to (or\nnot at all if we skip the shell for sub-processes, which is usually a\ngood idea).\n\nI have the feeling I'm missing something.\n\n-Peff\n"},{"id":"332737","messageId":"xmqqh8tttzwq.fsf@gitster.mtv.corp.google.com","threadId":"47223","inReplyTo":"20171116215725.k44e3alk2lq6bbwu@sigill.intra.peff.net","subject":"Re: [PATCH v4 4/6] list-objects: filter objects in traverse_commit_list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-17T02:14:13Z","receivedAt":"2017-11-17T02:14:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Those encodings don't necessarily need to be the same, because they're\n> about transport. Inside each process we'd have the raw bytes, and encode\n> them as appropriate to whatever sub-program we're going to pass to (or\n> not at all if we skip the shell for sub-processes, which is usually a\n> good idea).\n\nYes, I share the same feeling. It does not help that the series\ndefines its own notion of arg_needs_armor() and uses it to set a\nfield called requires_armor that is not yet used, the definition of\n\"armor\"ing being each byte getting encoded as two hexadecimal digits\nwithout any sign (which makes me wonder what a receiver of\n\"deadbeef\" would do---did it receive an armored string or a plain\none???). I do not understand why these strings are not passed as\nopaque sequences of bytes and instead converted at this low a layer.\n\n\n\n\n\n"},{"id":"332738","messageId":"xmqqd14htzw5.fsf@gitster.mtv.corp.google.com","threadId":"47223","inReplyTo":"20171116124354.3d800cc27dbe3ea668c64519@google.com","subject":"Re: [PATCH v4 5/6] rev-list: add list-objects filtering support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-17T02:14:34Z","receivedAt":"2017-11-17T02:14:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> If it were up to me, I would remove all existing mentions of \"partial\n> clone\" and explain the presence of the \"--missing\" argument as follows:\n>\n> In the future, we will introduce a \"partial clone\" mechanism wherein\n> an object in a repo, obtained from a remote, may reference a missing\n> object that can be dynamically fetched from that remote once needed.\n> This \"partial clone\" mechanism will have a way, sometimes slow, of\n> determining if a missing link is one of the links expected to be\n> produced by this mechanism.\n>\n> This patch introduces handling of missing objects to help debugging\n> and development of the \"partial clone\" mechanism, and once the\n> mechanism is implemented, for a power user to perform operations\n> that are missing-object-aware without incurring the cost of checking\n> if a missing link is expected.\n\nThat sounds quite sensible.\n"},{"id":"332759","messageId":"6f433987-f91b-d5b5-242e-3a241b7442c7@jeffhostetler.com","threadId":"47223","inReplyTo":"xmqqh8tttzwq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/6] list-objects: filter objects in traverse_commit_list","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-17T15:42:52Z","receivedAt":"2017-11-17T15:42:59Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 11/16/2017 9:14 PM, Junio C Hamano wrote:\n> Jeff King writes:\n> \n>> Those encodings don't necessarily need to be the same, because they're\n>> about transport. Inside each process we'd have the raw bytes, and encode\n>> them as appropriate to whatever sub-program we're going to pass to (or\n>> not at all if we skip the shell for sub-processes, which is usually a\n>> good idea).\n> \n> Yes, I share the same feeling. It does not help that the series\n> defines its own notion of arg_needs_armor() and uses it to set a\n> field called requires_armor that is not yet used, the definition of\n> \"armor\"ing being each byte getting encoded as two hexadecimal digits\n> without any sign (which makes me wonder what a receiver of\n> \"deadbeef\" would do---did it receive an armored string or a plain\n> one???). I do not understand why these strings are not passed as\n> opaque sequences of bytes and instead converted at this low a layer.\n\nI'm probably being too paranoid. My fear is that a client could pass\nan expression to clone/fetch/fetch-pack that would be sent to the\nserver and evaluated by the interface between upload-pack and pack-objects.\nI'm not worried about the pack-protocol transport. I'm mainly concerned\nin how upload-pack passes that *client-expression* to pack-objects and are\nthere ways for that to go south on the server with a carefully crafted\nexpression.\n\nEven if we assume that upload-pack on the server directly invokes\npack-objects (rather than a shell), there still might be issues.\nFor platforms like Linux which have a native execve() and can pass\nargs in an argv (and which the sub-process also receives in an argv\nin their main()), my paranoia is probably overkill.\n\nBut on Windows, where the native interface takes a command-line string\nrather than an argv, I was concerned. Yes, there is code in compat/mingw.c\nto quote args when building a command line from an argv (and I'm *not*\nsaying there are bugs in that), but again maybe I am being paranoid.\n\nI'll take another look and the existing quoting mechanisms and re-eval.\n\nThanks,\nJeff\n\n\n"},{"id":"332763","messageId":"f59d4b4a-3eec-6cc2-d822-f65d0d8fd99e@jeffhostetler.com","threadId":"47223","inReplyTo":"xmqqd14htzw5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 5/6] rev-list: add list-objects filtering support","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-17T17:36:07Z","receivedAt":"2017-11-17T17:36:13Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 11/16/2017 9:14 PM, Junio C Hamano wrote:\n> Jonathan Tan writes:\n> \n>> If it were up to me, I would remove all existing mentions of \"partial\n>> clone\" and explain the presence of the \"--missing\" argument as follows:\n>>\n>> In the future, we will introduce a \"partial clone\" mechanism wherein\n>> an object in a repo, obtained from a remote, may reference a missing\n>> object that can be dynamically fetched from that remote once needed.\n>> This \"partial clone\" mechanism will have a way, sometimes slow, of\n>> determining if a missing link is one of the links expected to be\n>> produced by this mechanism.\n>>\n>> This patch introduces handling of missing objects to help debugging\n>> and development of the \"partial clone\" mechanism, and once the\n>> mechanism is implemented, for a power user to perform operations\n>> that are missing-object-aware without incurring the cost of checking\n>> if a missing link is expected.\n> \n> That sounds quite sensible.\n> \n\nwill do. thanks.\nJeff\n"},{"id":"332780","messageId":"20171117221936.zsokhvmslnf6jete@sigill.intra.peff.net","threadId":"47223","inReplyTo":"6f433987-f91b-d5b5-242e-3a241b7442c7@jeffhostetler.com","subject":"Re: [PATCH v4 4/6] list-objects: filter objects in traverse_commit_list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-17T22:19:36Z","receivedAt":"2017-11-17T22:19:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 17, 2017 at 10:42:52AM -0500, Jeff Hostetler wrote:\n\n> > Yes, I share the same feeling. It does not help that the series\n> > defines its own notion of arg_needs_armor() and uses it to set a\n> > field called requires_armor that is not yet used, the definition of\n> > \"armor\"ing being each byte getting encoded as two hexadecimal digits\n> > without any sign (which makes me wonder what a receiver of\n> > \"deadbeef\" would do---did it receive an armored string or a plain\n> > one???). I do not understand why these strings are not passed as\n> > opaque sequences of bytes and instead converted at this low a layer.\n> \n> I'm probably being too paranoid. My fear is that a client could pass\n> an expression to clone/fetch/fetch-pack that would be sent to the\n> server and evaluated by the interface between upload-pack and pack-objects.\n> I'm not worried about the pack-protocol transport. I'm mainly concerned\n> in how upload-pack passes that *client-expression* to pack-objects and are\n> there ways for that to go south on the server with a carefully crafted\n> expression.\n\nI think you have to trust that those interfaces are capable of passing\nraw bytes, whether directly via execve() or because we got the quoting\nright. If there's a bug there, it's going to be a bigger problem than\njust this code path (and the fix needs to be there, not second-guessing\nit in the callers).\n\nSo I'd say that yeah, you are being too paranoid.\n\nAs an aside, though, I wonder if these client expressions should be fed\nover stdin to pack-objects. That removes any argv limits we might run\ninto on the server side. It also removes shell injections as a\npossibility, though of course we'd need quoting in that layer to avoid\nargument-injection to pack-objects.\n\n-Peff\n"}]}