{"thread":{"id":"46809","subject":"[PATCH 01/13] dir: refactor add_excludes()","startedAt":"2017-09-22T20:27:07Z","lastAt":"2017-09-27T20:49:42Z","messageCount":16,"participants":["Jeff Hostetler","Brandon Williams","Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"328669","messageId":"20170922202632.53714-2-git@jeffhostetler.com","threadId":"46809","inReplyTo":"20170922202632.53714-1-git@jeffhostetler.com","subject":"[PATCH 01/13] dir: refactor add_excludes()","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-22T20:26:20Z","receivedAt":"2017-09-22T20:27:07Z","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.\n\nSigned-off-by: Jeff Hostetler \n---\n dir.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--\n dir.h | 4 ++++\n 2 files changed, 55 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex ae6f5c9..3aeae06 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -739,6 +739,11 @@ static void invalidate_directory(struct untracked_cache *uc,\n \t\tdir->dirs[i]->recurse = 0;\n }\n \n+static int add_excludes_from_buffer(\n+\tchar *buf, size_t size,\n+\tconst char *base, int baselen,\n+\tstruct 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 +759,9 @@ 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 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@@ -813,6 +818,18 @@ 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(\n+\tchar *buf, size_t size,\n+\tconst char *base, int baselen,\n+\tstruct 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 +858,38 @@ 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+\tunsigned long size;\n+\tenum object_type type;\n+\n+\tbuf = read_sha1_file(oid->hash, &type, &size);\n+\tif (!buf)\n+\t\treturn -1;\n+\n+\tif (type != OBJ_BLOB) {\n+\t\tfree(buf);\n+\t\treturn -1;\n+\t}\n+\n+\tif (size == 0) {\n+\t\tfree(buf);\n+\t\treturn 0;\n+\t}\n+\n+\tif (buf[size - 1] != '\\n') {\n+\t\tbuf = xrealloc(buf, st_add(size, 1));\n+\t\tbuf[size++] = '\\n';\n+\t}\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..242de63 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -256,6 +256,10 @@ 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(\n+\tstruct object_id *oid,\n+\tconst char *base, int baselen,\n+\tstruct 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":"328670","messageId":"20170922202632.53714-3-git@jeffhostetler.com","threadId":"46809","inReplyTo":"20170922202632.53714-1-git@jeffhostetler.com","subject":"[PATCH 02/13] oidset2: create oidset subclass with object length and pathname","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-22T20:26:21Z","receivedAt":"2017-09-22T20:27:10Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nCreate subclass of oidset where each entry has a\nfield to store the length of the object's content\nand an optional pathname.\n\nThis will be used in a future commit to build a\nmanifest of omitted objects in a partial/narrow\nclone/fetch.\n\nSigned-off-by: Jeff Hostetler \n---\n Makefile | 1 +\n oidset2.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n oidset2.h | 58 +++++++++++++++++++++++++++++++++++\n 3 files changed, 163 insertions(+)\n create mode 100644 oidset2.c\n create mode 100644 oidset2.h\n\ndiff --git a/Makefile b/Makefile\nindex 461c845..4e0cc39 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -816,6 +816,7 @@ LIB_OBJS += notes-merge.o\n LIB_OBJS += notes-utils.o\n LIB_OBJS += object.o\n LIB_OBJS += oidset.o\n+LIB_OBJS += oidset2.o\n LIB_OBJS += pack-bitmap.o\n LIB_OBJS += pack-bitmap-write.o\n LIB_OBJS += pack-check.o\ndiff --git a/oidset2.c b/oidset2.c\nnew file mode 100644\nindex 0000000..43bc9ee\n--- /dev/null\n+++ b/oidset2.c\n@@ -0,0 +1,104 @@\n+#include \"cache.h\"\n+#include \"oidset2.h\"\n+\n+static int oidset2_hashcmp(const void *unused_cmp_data,\n+\t\t\t const void *va, const void *vb,\n+\t\t\t const void *vkey)\n+{\n+\tconst struct oidset2_entry *a = va, *b = vb;\n+\tconst struct object_id *key = vkey;\n+\treturn oidcmp(&a->oid, key ? key : &b->oid);\n+}\n+\n+struct oidset2_entry *oidset2_get(const struct oidset2 *set, const struct object_id *oid)\n+{\n+\tstruct hashmap_entry key;\n+\tstruct oidset2_entry *value;\n+\n+\tif (!set->map.cmpfn)\n+\t\treturn NULL;\n+\n+\thashmap_entry_init(&key, sha1hash(oid->hash));\n+\tvalue = hashmap_get(&set->map, &key, oid);\n+\n+\treturn value;\n+}\n+\n+int oidset2_contains(const struct oidset2 *set, const struct object_id *oid)\n+{\n+\treturn !!oidset2_get(set, oid);\n+}\n+\n+int oidset2_insert(struct oidset2 *set, const struct object_id *oid,\n+\t\t enum object_type type, int64_t object_length,\n+\t\t const char *pathname)\n+{\n+\tstruct oidset2_entry *entry;\n+\n+\tif (!set->map.cmpfn)\n+\t\thashmap_init(&set->map, oidset2_hashcmp, NULL, 0);\n+\n+\tif (oidset2_contains(set, oid))\n+\t\treturn 1;\n+\n+\tentry = xcalloc(1, sizeof(*entry));\n+\thashmap_entry_init(&entry->hash, sha1hash(oid->hash));\n+\toidcpy(&entry->oid, oid);\n+\n+\tentry->type = type;\n+\tentry->object_length = object_length;\n+\tif (pathname)\n+\t entry->pathname = strdup(pathname);\n+\n+\thashmap_add(&set->map, entry);\n+\treturn 0;\n+}\n+\n+void oidset2_remove(struct oidset2 *set, const struct object_id *oid)\n+{\n+\tstruct hashmap_entry key;\n+\tstruct oidset2_entry *e;\n+\n+\thashmap_entry_init(&key, sha1hash(oid->hash));\n+\te = hashmap_remove(&set->map, &key, oid);\n+\n+\tfree(e->pathname);\n+\tfree(e);\n+}\n+\n+void oidset2_clear(struct oidset2 *set)\n+{\n+\thashmap_free(&set->map, 1);\n+}\n+\n+static int oidset2_cmp(const void *a, const void *b)\n+{\n+\tconst struct oidset2_entry *ae = *((const struct oidset2_entry **)a);\n+\tconst struct oidset2_entry *be = *((const struct oidset2_entry **)b);\n+\n+\treturn oidcmp(&ae->oid, &be->oid);\n+}\n+\n+void oidset2_foreach(struct oidset2 *set, oidset2_foreach_cb cb, void *cb_data)\n+{\n+\tstruct hashmap_iter iter;\n+\tstruct oidset2_entry **array;\n+\tstruct oidset2_entry *e;\n+\tint j, k;\n+\n+\tarray = xcalloc(set->map.size, sizeof(*e));\n+\n+\thashmap_iter_init(&set->map, &iter);\n+\tk = 0;\n+\twhile ((e = hashmap_iter_next(&iter)))\n+\t\tarray[k++] = e;\n+\n+\tQSORT(array, k, oidset2_cmp);\n+\n+\tfor (j = 0; j < k; j++) {\n+\t\te = array[j];\n+\t\tcb(j, k, e, cb_data);\n+\t}\n+\n+\tfree(array);\n+}\ndiff --git a/oidset2.h b/oidset2.h\nnew file mode 100644\nindex 0000000..67d8a5a\n--- /dev/null\n+++ b/oidset2.h\n@@ -0,0 +1,58 @@\n+#ifndef OIDSET2_H\n+#define OIDSET2_H\n+\n+/**\n+ * oidset2 is a variant of oidset, but allows additional fields for each object.\n+ */\n+\n+/**\n+ * A single oidset2; should be zero-initialized (or use OIDSET2_INIT).\n+ */\n+struct oidset2 {\n+\tstruct hashmap map;\n+};\n+\n+#define OIDSET2_INIT { { NULL } }\n+\n+struct oidset2_entry {\n+\tstruct hashmap_entry hash;\n+\tstruct object_id oid;\n+\n+\tenum object_type type;\n+\tint64_t object_length;\t/* This is SIGNED. Use -1 when unknown. */\n+\tchar *pathname;\n+};\n+\n+struct oidset2_entry *oidset2_get(const struct oidset2 *set, const struct object_id *oid);\n+\n+/**\n+ * Returns true iff `set` contains `oid`.\n+ */\n+int oidset2_contains(const struct oidset2 *set, const struct object_id *oid);\n+\n+/**\n+ * Insert the oid into the set; a copy is made, so \"oid\" does not need\n+ * to persist after this function is called.\n+ *\n+ * Returns 1 if the oid was already in the set, 0 otherwise. This can be used\n+ * to perform an efficient check-and-add.\n+ */\n+int oidset2_insert(struct oidset2 *set, const struct object_id *oid,\n+\t\t enum object_type type, int64_t object_length,\n+\t\t const char *pathname);\n+\n+void oidset2_remove(struct oidset2 *set, const struct object_id *oid);\n+\n+typedef void (*oidset2_foreach_cb)(\n+\tint i, int i_limit,\n+\tstruct oidset2_entry *e, void *cb_data);\n+\n+void oidset2_foreach(struct oidset2 *set, oidset2_foreach_cb cb, void *cb_data);\n+\n+/**\n+ * Remove all entries from the oidset2, freeing any resources associated with\n+ * it.\n+ */\n+void oidset2_clear(struct oidset2 *set);\n+\n+#endif /* OIDSET2_H */\n-- \n2.9.3\n\n"},{"id":"328671","messageId":"20170922202632.53714-4-git@jeffhostetler.com","threadId":"46809","inReplyTo":"20170922202632.53714-1-git@jeffhostetler.com","subject":"[PATCH 03/13] list-objects: filter objects in traverse_commit_list","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-22T20:26:22Z","receivedAt":"2017-09-22T20:27:14Z","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 (not shown)\nduring a traversal.\n\nUpdate traverse_commit_list() to be a wrapper for the above.\n\nFiltering will be used in a future commit by rev-list and\npack-objects for narrow/partial clone/fetch to omit certain\nblobs from the output.\n\ntraverse_bitmap_commit_list() does not work with filtering.\nIf a packfile bitmap is present, it will not be used.\n\nSigned-off-by: Jeff Hostetler \n---\n list-objects.c | 66 ++++++++++++++++++++++++++++++++++++++++++++--------------\n list-objects.h | 30 ++++++++++++++++++++++++++\n 2 files changed, 80 insertions(+), 16 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex b3931fa..3e86008 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -13,10 +13,13 @@ static void process_blob(struct rev_info *revs,\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,\n+\t\t\t void *filter_data)\n {\n \tstruct object *obj = &blob->object;\n \tsize_t pathlen;\n+\tlist_objects_filter_result r = LOFR_MARK_SEEN | LOFR_SHOW;\n \n \tif (!revs->blob_objects)\n \t\treturn;\n@@ -24,11 +27,15 @@ 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)\n+\t\tr = filter(LOFT_BLOB, obj, path->buf, &path->buf[pathlen], filter_data);\n+\tif (r & LOFR_MARK_SEEN)\n+\t\tobj->flags |= SEEN;\n+\tif (r & LOFR_SHOW)\n+\t\tshow(obj, path->buf, cb_data);\n \tstrbuf_setlen(path, pathlen);\n }\n \n@@ -69,7 +76,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,\n+\t\t\t void *filter_data)\n {\n \tstruct object *obj = &tree->object;\n \tstruct tree_desc desc;\n@@ -77,6 +86,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+\tlist_objects_filter_result r = LOFR_MARK_SEEN | LOFR_SHOW;\n \n \tif (!revs->tree_objects)\n \t\treturn;\n@@ -90,9 +100,13 @@ 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)\n+\t\tr = filter(LOFT_BEGIN_TREE, obj, base->buf, &base->buf[baselen], filter_data);\n+\tif (r & LOFR_MARK_SEEN)\n+\t\tobj->flags |= SEEN;\n+\tif (r & LOFR_SHOW)\n+\t\tshow(obj, base->buf, cb_data);\n \tif (base->len)\n \t\tstrbuf_addch(base, '/');\n \n@@ -112,7 +126,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, 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 +135,17 @@ 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, filter_data);\n \t}\n+\n+\tif (filter) {\n+\t\tr = filter(LOFT_END_TREE, obj, base->buf, &base->buf[baselen], filter_data);\n+\t\tif (r & LOFR_MARK_SEEN)\n+\t\t\tobj->flags |= SEEN;\n+\t\tif (r & LOFR_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 +206,10 @@ 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+void traverse_commit_list_worker(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit, show_object_fn show_object, void *show_data,\n+\tfilter_object_fn filter, void *filter_data)\n {\n \tint i;\n \tstruct commit *commit;\n@@ -200,7 +223,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 +234,19 @@ 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, filter, 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, filter, filter_data);\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"unknown pending object %s (%s)\",\n@@ -232,3 +255,14 @@ 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+\ttraverse_commit_list_worker(\n+\t\trevs,\n+\t\tshow_commit, show_object, show_data,\n+\t\tNULL, NULL);\n+}\ndiff --git a/list-objects.h b/list-objects.h\nindex 0cebf85..39fcbb5 100644\n--- a/list-objects.h\n+++ b/list-objects.h\n@@ -8,4 +8,34 @@ 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+enum list_objects_filter_result {\n+\tLOFR_ZERO = 0,\n+\tLOFR_MARK_SEEN = 1<<0,\n+\tLOFR_SHOW = 1<<1,\n+};\n+\n+/* See object.h and revision.h */\n+#define FILTER_REVISIT (1<<25)\n+\n+enum list_objects_filter_type {\n+\tLOFT_BEGIN_TREE,\n+\tLOFT_END_TREE,\n+\tLOFT_BLOB\n+};\n+\n+typedef enum list_objects_filter_result list_objects_filter_result;\n+typedef enum list_objects_filter_type list_objects_filter_type;\n+\n+typedef list_objects_filter_result (*filter_object_fn)(\n+\tlist_objects_filter_type filter_type,\n+\tstruct object *obj,\n+\tconst char *pathname,\n+\tconst char *filename,\n+\tvoid *filter_data);\n+\n+void traverse_commit_list_worker(\n+\tstruct rev_info *,\n+\tshow_commit_fn, show_object_fn, void *show_data,\n+\tfilter_object_fn filter, void *filter_data);\n+\n #endif\n-- \n2.9.3\n\n"},{"id":"328672","messageId":"20170922202632.53714-5-git@jeffhostetler.com","threadId":"46809","inReplyTo":"20170922202632.53714-1-git@jeffhostetler.com","subject":"[PATCH 04/13] list-objects-filter-all: add filter to omit all blobs","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-22T20:26:23Z","receivedAt":"2017-09-22T20:27:17Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nCreate a simple filter for traverse_commit_list_worker() to omit\nall blobs from the result.\n\nThis filter will be used in a future commit by rev-list and pack-objects\nto create a \"commits and trees\" result. This is intended for partial\nclone and fetch support.\n\nSigned-off-by: Jeff Hostetler \n---\n Makefile | 1 +\n list-objects-filter-all.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++\n list-objects-filter-all.h | 18 ++++++++++\n 3 files changed, 104 insertions(+)\n create mode 100644 list-objects-filter-all.c\n create mode 100644 list-objects-filter-all.h\n\ndiff --git a/Makefile b/Makefile\nindex 4e0cc39..b98e3dc 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -798,6 +798,7 @@ 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-all.o\n LIB_OBJS += ll-merge.o\n LIB_OBJS += lockfile.o\n LIB_OBJS += log-tree.o\ndiff --git a/list-objects-filter-all.c b/list-objects-filter-all.c\nnew file mode 100644\nindex 0000000..2faccb3\n--- /dev/null\n+++ b/list-objects-filter-all.c\n@@ -0,0 +1,85 @@\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-all.h\"\n+\n+/*\n+ * A filter for list-objects to omit ALL blobs from the traversal.\n+ */\n+struct filter_omit_all_blobs_data {\n+\tstruct oidset2 omits;\n+};\n+\n+static list_objects_filter_result filter_omit_all_blobs(\n+\tlist_objects_filter_type filter_type,\n+\tstruct object *obj,\n+\tconst char *pathname,\n+\tconst char *filename,\n+\tvoid *filter_data_)\n+{\n+\tstruct filter_omit_all_blobs_data *filter_data = filter_data_;\n+\tint64_t object_length = -1;\n+\tunsigned long s;\n+\tenum object_type t;\n+\n+\tswitch (filter_type) {\n+\tdefault:\n+\t\tdie(\"unkown filter_type\");\n+\t\treturn LOFR_ZERO;\n+\n+\tcase LOFT_BEGIN_TREE:\n+\t\tassert(obj->type == OBJ_TREE);\n+\t\t/* always include all tree objects */\n+\t\treturn LOFR_MARK_SEEN | LOFR_SHOW;\n+\n+\tcase LOFT_END_TREE:\n+\t\tassert(obj->type == OBJ_TREE);\n+\t\treturn LOFR_ZERO;\n+\n+\tcase LOFT_BLOB:\n+\t\tassert(obj->type == OBJ_BLOB);\n+\t\tassert((obj->flags & SEEN) == 0);\n+\n+\t\t/*\n+\t\t * Since we always omit all blobs (and never provisionally omit),\n+\t\t * we should never see a blob twice.\n+\t\t */\n+\t\tassert(!oidset2_contains(&filter_data->omits, &obj->oid));\n+\n+\t\tt = sha1_object_info(obj->oid.hash, &s);\n+\t\tassert(t == OBJ_BLOB);\n+\t\tobject_length = (int64_t)((uint64_t)(s));\n+\n+\t\t/* Insert OID into the omitted list. No need for a pathname. */\n+\t\toidset2_insert(&filter_data->omits, &obj->oid, t, object_length,\n+\t\t\t NULL);\n+\t\treturn LOFR_MARK_SEEN; /* but not LOFR_SHOW (hard omit) */\n+\t}\n+}\n+\n+void traverse_commit_list_omit_all_blobs(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\toidset2_foreach_cb print_omitted_object,\n+\tvoid *ctx_data)\n+{\n+\tstruct filter_omit_all_blobs_data d;\n+\n+\tmemset(&d, 0, sizeof(d));\n+\n+\ttraverse_commit_list_worker(revs, show_commit, show_object, ctx_data,\n+\t\t\t\t filter_omit_all_blobs, &d);\n+\n+\tif (print_omitted_object)\n+\t\toidset2_foreach(&d.omits, print_omitted_object, ctx_data);\n+\n+\toidset2_clear(&d.omits);\n+}\ndiff --git a/list-objects-filter-all.h b/list-objects-filter-all.h\nnew file mode 100644\nindex 0000000..591589f\n--- /dev/null\n+++ b/list-objects-filter-all.h\n@@ -0,0 +1,18 @@\n+#ifndef LIST_OBJECTS_FILTER_ALL_H\n+#define LIST_OBJECTS_FILTER_ALL_H\n+\n+#include \"oidset2.h\"\n+\n+/*\n+ * A filter for list-objects to omit ALL blobs\n+ * from the traversal.\n+ */\n+void traverse_commit_list_omit_all_blobs(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\toidset2_foreach_cb print_omitted_object,\n+\tvoid *ctx_data);\n+\n+#endif /* LIST_OBJECTS_FILTER_ALL_H */\n+\n-- \n2.9.3\n\n"},{"id":"328673","messageId":"20170922202632.53714-1-git@jeffhostetler.com","threadId":"46809","inReplyTo":null,"subject":"[PATCH 00/13] RFC object filtering for parital clone","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-22T20:26:19Z","receivedAt":"2017-09-22T20:27:32Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\n\nThis patch series contains WIP code demonstrating object (blob) filtering\nin rev-list and pack-objects using a common filtering API in\nlist-objects and traverse-commit-list that allows both commands\nto perform the same type of filter operations. And serve as the\nbasis of partial-clone and partial-fetch.\n\nThis draft contains filters to:\n() omit all blobs\n() omit blobs larger than some size\n() omit blobs using a sparse-checkout specification\n\nIn addition to specifying the filter criteria, the rev-list command\nwas updated to include options to:\n() print a list of the omitted objects (due to the current filtering\n criteria)\n() print a list of missing objects (probably from a prior partial\n clone/fetch).\n\nThis latter print option can be used with or without a new filter\ncriteria allowing it to be used with a pre-checkout bulk pre-fetch\ncommand.\n\nFor example, if blobs were omitted during the clone or a fetch, the\nclient can do:\n\n git rev-list --quiet --objects --filter-print-missing NEWBRANCH\n\nand get a list of just the objects that are required to checkout\nNEWBRANCH.\n\nOr if a sparse-checkout is in effect, the client can specify the\nsame criteria to look for just the missing blobs needed to do the\nsparse-checkout:\n\n git rev-list --quiet --objects --filter-print-missing \\\n --filter-use-path=./git/info/sparse-checkout NEWBRANCH\n\nIt does not matter why a blob is missing; that is, what filter\ncriteria was used during the clone or fetch. All that matters\nis the blob is missing and is now needed.\n\nThese commands output a list of missing blobs that can be fed\ninto a bulk fetch object request. The goal here is to minimize\nthe need for dynamic object fetch mechanisms currently being\ndiscussed. (We cannot eliminate the need for dynamic fetching,\nbut we can use this to precompute/prefetch in bulk.)\n\nPack-objects was updated to allow the server to build incomplete\npackfiles without unwanted blobs.\n\nThis is the first step to support partial-clone and -fetch. I've\nomitted from this patch series corresponding changes to fetch-pack,\nupload-pack, index-pack, verify-pack, fsck, gc, and the git protocol.\nI can make these available if there is interest. I omit them from\nthis RFC to not distract from the basic filtering ideas.\n\nIt also does not address the promisor/promised ideas currently\nbeing discussed [2,3]. These should be considered independently.\n\nThe code in this patch series can be seen here [1].\n\n[1] https://github.com/jeffhostetler/git/pull/3\n[2] https://public-inbox.org/git/xmqq8thbqlqf.fsf@gitster.mtv.corp.google.com/t/\n[3] https://github.com/jonathantanmy/git/commits/partialclone2\n\n\nJeff Hostetler (13):\n dir: refactor add_excludes()\n oidset2: create oidset subclass with object length and pathname\n list-objects: filter objects in traverse_commit_list\n list-objects-filter-all: add filter to omit all blobs\n list-objects-filter-large: add large blob filter to list-objects\n list-objects-filter-sparse: add sparse-checkout based filter\n object-filter: common declarations for object filtering\n list-objects: add traverse_commit_list_filtered method\n rev-list: add object filtering support\n rev-list: add filtering help text\n t6112: rev-list object filtering test\n pack-objects: add object filtering support\n pack-objects: add filtering help text\n\n Documentation/git-pack-objects.txt | 17 +++\n Documentation/git-rev-list.txt | 9 +-\n Documentation/rev-list-options.txt | 32 +++++\n Makefile | 5 +\n builtin/pack-objects.c | 24 +++-\n builtin/rev-list.c | 73 +++++++++-\n dir.c | 53 ++++++-\n dir.h | 4 +\n list-objects-filter-all.c | 85 ++++++++++++\n list-objects-filter-all.h | 18 +++\n list-objects-filter-large.c | 108 +++++++++++++++\n list-objects-filter-large.h | 18 +++\n list-objects-filter-sparse.c | 221 +++++++++++++++++++++++++++++\n list-objects-filter-sparse.h | 30 ++++\n list-objects.c | 100 +++++++++++---\n list-objects.h | 41 ++++++\n object-filter.c | 269 ++++++++++++++++++++++++++++++++++++\n object-filter.h | 173 +++++++++++++++++++++++\n oidset2.c | 104 ++++++++++++++\n oidset2.h | 58 ++++++++\n t/t6112-rev-list-filters-objects.sh | 237 +++++++++++++++++++++++++++++++\n 21 files changed, 1657 insertions(+), 22 deletions(-)\n create mode 100644 list-objects-filter-all.c\n create mode 100644 list-objects-filter-all.h\n create mode 100644 list-objects-filter-large.c\n create mode 100644 list-objects-filter-large.h\n create mode 100644 list-objects-filter-sparse.c\n create mode 100644 list-objects-filter-sparse.h\n create mode 100644 object-filter.c\n create mode 100644 object-filter.h\n create mode 100644 oidset2.c\n create mode 100644 oidset2.h\n create mode 100755 t/t6112-rev-list-filters-objects.sh\n\n-- \n2.9.3\n\n"},{"id":"328683","messageId":"20170922204211.GA24036@google.com","threadId":"46809","inReplyTo":"20170922202632.53714-3-git@jeffhostetler.com","subject":"Re: [PATCH 02/13] oidset2: create oidset subclass with object length and pathname","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-09-22T20:42:11Z","receivedAt":"2017-09-22T20:42:19Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 09/22, Jeff Hostetler wrote:\n> From: Jeff Hostetler \n> \n> Create subclass of oidset where each entry has a\n> field to store the length of the object's content\n> and an optional pathname.\n> \n> This will be used in a future commit to build a\n> manifest of omitted objects in a partial/narrow\n> clone/fetch.\n> \n> Signed-off-by: Jeff Hostetler \n\nI think it may be better to create an 'oidmap' instead of an\n'oidset+some specific other bits' that way, in the future, we have a\ngeneralized data structure which we can use to provide a mapping\nbetween oids and some arbitrary data.\n\n-- \nBrandon Williams\n"},{"id":"328709","messageId":"20170922173957.f5601015cc8d6760c80293a9@google.com","threadId":"46809","inReplyTo":"20170922202632.53714-1-git@jeffhostetler.com","subject":"Re: [PATCH 00/13] RFC object filtering for parital clone","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-23T00:39:57Z","receivedAt":"2017-09-23T00:40:05Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Fri, 22 Sep 2017 20:26:19 +0000\nJeff Hostetler wrote:\n\n> This draft contains filters to:\n> () omit all blobs\n> () omit blobs larger than some size\n> () omit blobs using a sparse-checkout specification\n> \n> In addition to specifying the filter criteria, the rev-list command\n> was updated to include options to:\n> () print a list of the omitted objects (due to the current filtering\n> criteria)\n> () print a list of missing objects (probably from a prior partial\n> clone/fetch).\n\nThanks, this last point seems useful.\n\nI tried applying your patches and it doesn't apply cleanly on master.\nCould you try rebasing? In particular, the code references\nget_sha1_with_context(), which no longer exists (it was replaced with\nget_oid_with_context()).\n"},{"id":"328947","messageId":"03a00220-f637-5931-c8f4-07833a9b3d4f@jeffhostetler.com","threadId":"46809","inReplyTo":"20170922173957.f5601015cc8d6760c80293a9@google.com","subject":"Re: [PATCH 00/13] RFC object filtering for parital clone","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-26T14:55:38Z","receivedAt":"2017-09-26T14:55:43Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/22/2017 8:39 PM, Jonathan Tan wrote:\n> On Fri, 22 Sep 2017 20:26:19 +0000\n> Jeff Hostetler wrote:\n> \n>> This draft contains filters to:\n>> () omit all blobs\n>> () omit blobs larger than some size\n>> () omit blobs using a sparse-checkout specification\n>>\n>> In addition to specifying the filter criteria, the rev-list command\n>> was updated to include options to:\n>> () print a list of the omitted objects (due to the current filtering\n>> criteria)\n>> () print a list of missing objects (probably from a prior partial\n>> clone/fetch).\n> \n> Thanks, this last point seems useful.\n> \n> I tried applying your patches and it doesn't apply cleanly on master.\n> Could you try rebasing? In particular, the code references\n> get_sha1_with_context(), which no longer exists (it was replaced with\n> get_oid_with_context()).\n> \n\nI usually build and submit patches relative to the latest\navailable tag (v2.14.1) rather than a moving target, but\nyeah I'll look thru the other comments and rebase it forward.\n\nThanks\nJeff\n"},{"id":"328958","messageId":"974a0851-816b-d913-4632-9a6114beaa0a@jeffhostetler.com","threadId":"46809","inReplyTo":"03a00220-f637-5931-c8f4-07833a9b3d4f@jeffhostetler.com","subject":"Re: [PATCH 00/13] RFC object filtering for parital clone","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-26T19:23:22Z","receivedAt":"2017-09-26T19:23:30Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/26/2017 10:55 AM, Jeff Hostetler wrote:\n> \n> \n> On 9/22/2017 8:39 PM, Jonathan Tan wrote:\n>> On Fri, 22 Sep 2017 20:26:19 +0000\n>> Jeff Hostetler wrote:\n>> ...\n>>\n>> I tried applying your patches and it doesn't apply cleanly on master.\n>> Could you try rebasing? In particular, the code references\n>> get_sha1_with_context(), which no longer exists (it was replaced with\n>> get_oid_with_context()).\n>>\n> \n> I usually build and submit patches relative to the latest\n> available tag (v2.14.1) rather than a moving target, but\n> yeah I'll look thru the other comments and rebase it forward.\n\nhttps://github.com/jeffhostetler/git/tree/core/rev_list_filtering\n\nI rebased this branch onto a current master and fixed\nthe get_sha1_with_context() problems.\n\nLet me know if you have problems with it (or would prefer that I\nsend it to the mailing list).\n\nJeff\n"},{"id":"328967","messageId":"20170926152022.67d3d6bcc93d862aa1066207@google.com","threadId":"46809","inReplyTo":"20170922202632.53714-3-git@jeffhostetler.com","subject":"Re: [PATCH 02/13] oidset2: create oidset subclass with object length and pathname","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-26T22:20:22Z","receivedAt":"2017-09-26T22:20:30Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Fri, 22 Sep 2017 20:26:21 +0000\nJeff Hostetler wrote:\n\n> From: Jeff Hostetler \n> \n> Create subclass of oidset where each entry has a\n> field to store the length of the object's content\n> and an optional pathname.\n> \n> This will be used in a future commit to build a\n> manifest of omitted objects in a partial/narrow\n> clone/fetch.\n\nAs Brandon mentioned, I think \"oidmap\" should be the new data structure\nof choice (with \"oidset\" modified to use it).\n\n> +struct oidset2_entry {\n> +\tstruct hashmap_entry hash;\n> +\tstruct object_id oid;\n> +\n> +\tenum object_type type;\n> +\tint64_t object_length;\t/* This is SIGNED. Use -1 when unknown. */\n> +\tchar *pathname;\n> +};\n\nobject_length is defined to be \"unsigned long\" in Git code, I think.\nWhen is object_length not known, and in those cases, would it be better\nto use a separate data structure to store what we need?\n"},{"id":"328969","messageId":"20170926153141.6a8d7e5024eeed5bbda838c8@google.com","threadId":"46809","inReplyTo":"20170922202632.53714-4-git@jeffhostetler.com","subject":"Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-26T22:31:41Z","receivedAt":"2017-09-26T22:31:47Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Fri, 22 Sep 2017 20:26:22 +0000\nJeff Hostetler wrote:\n\n> From: Jeff Hostetler \n> \n> Create traverse_commit_list_filtered() and add filtering\n\nYou mention _filtered() here, but this patch contains _worker().\n\n> list-objects.h | 30 ++++++++++++++++++++++++++\n> 2 files changed, 80 insertions(+), 16 deletions(-)\n> \n> diff --git a/list-objects.c b/list-objects.c\n> index b3931fa..3e86008 100644\n> --- a/list-objects.c\n> +++ b/list-objects.c\n> @@ -13,10 +13,13 @@ static void process_blob(struct rev_info *revs,\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,\n> +\t\t\t void *filter_data)\n\nI had some thoughts about modifying \"show\" (instead of adding \"filter\",\nas you have done in this patch) to indicate to the caller that the\ncorresponding object should not be marked \"seen\". That does have the\nadvantage that we don't have so many callbacks flying around, but it\nalso makes things more complicated, so I don't know which is better.\n\n> +\tif (filter) {\n> +\t\tr = filter(LOFT_END_TREE, obj, base->buf, &base->buf[baselen], filter_data);\n> +\t\tif (r & LOFR_MARK_SEEN)\n> +\t\t\tobj->flags |= SEEN;\n> +\t\tif (r & LOFR_SHOW)\n> +\t\t\tshow(obj, base->buf, cb_data);\n> +\t}\n\nThis LOFT_END_TREE was added to support the code in\nlist-objects-filter-sparse - would it be OK to completely remove the\noptimization involving the \"provisional\" omission of blobs? (I don't\nthink the exact same tree object will occur multiple times often.) If\nyes, then I think this block can be removed too and will simplify the\ncode.\n\n> +/* See object.h and revision.h */\n> +#define FILTER_REVISIT (1<<25)\n\nIf you do add this, also update object.h. But I don't think this is the\nright place to do it - it's probably better in\nlist-objects-filter-sparse.\n\n> +typedef enum list_objects_filter_result list_objects_filter_result;\n> +typedef enum list_objects_filter_type list_objects_filter_type;\n\nI don't think Git style is to typedef enums.\n\n> +void traverse_commit_list_worker(\n> +\tstruct rev_info *,\n> +\tshow_commit_fn, show_object_fn, void *show_data,\n> +\tfilter_object_fn filter, void *filter_data);\n\nHere (and throughout), as far as I can tell, the style is to indent to\nthe character after the opening parenthesis.\n"},{"id":"329038","messageId":"5f028dc4-ce57-d628-19a3-006df7aa5bea@jeffhostetler.com","threadId":"46809","inReplyTo":"20170926152022.67d3d6bcc93d862aa1066207@google.com","subject":"Re: [PATCH 02/13] oidset2: create oidset subclass with object length and pathname","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-27T14:47:13Z","receivedAt":"2017-09-27T14:47:20Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/26/2017 6:20 PM, Jonathan Tan wrote:\n> On Fri, 22 Sep 2017 20:26:21 +0000\n> Jeff Hostetler wrote:\n> \n>> From: Jeff Hostetler \n>>\n>> Create subclass of oidset where each entry has a\n>> field to store the length of the object's content\n>> and an optional pathname.\n>>\n>> This will be used in a future commit to build a\n>> manifest of omitted objects in a partial/narrow\n>> clone/fetch.\n> \n> As Brandon mentioned, I think \"oidmap\" should be the new data structure\n> of choice (with \"oidset\" modified to use it).\n\nI'll take a look at that. I'm not exactly happy with\nmy oidset2, but it works and it minimized touching other\nthings. But yes, it may clear up a few things.\n\n\n>> +struct oidset2_entry {\n>> +\tstruct hashmap_entry hash;\n>> +\tstruct object_id oid;\n>> +\n>> +\tenum object_type type;\n>> +\tint64_t object_length;\t/* This is SIGNED. Use -1 when unknown. */\n>> +\tchar *pathname;\n>> +};\n> \n> object_length is defined to be \"unsigned long\" in Git code, I think.\n> When is object_length not known, and in those cases, would it be better\n> to use a separate data structure to store what we need?\n\nYeah, I struggled with that one. Git currently treats file size as\na 32-bit unsigned value throughout the code. I assume eventually there\nwill be a round of changes to support 64-bit values, so this anticipates\nthat.\n\nI could change it to be an unknown flag, rather assuming -1, but in an\nearlier draft I was printing -1 in the rev-list output. I can change this.\n\nWRT a separate structure, the SET I create will contain entries for items\nwhere we may or may not know the size and that depends on the context.\nWhen building a list of already-missing blobs (with the --filter-print-missing)\nwe never know the size. But when building a list of to-be-omitted blobs\n(from the current set of filter options), we may or may not know. I'm\nnot sure we need 2 _entry definitions right now.\n\n"},{"id":"329042","messageId":"d5de0eda-18aa-a796-e7d0-d536d2e59605@jeffhostetler.com","threadId":"46809","inReplyTo":"20170926153141.6a8d7e5024eeed5bbda838c8@google.com","subject":"Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-27T17:04:42Z","receivedAt":"2017-09-27T17:04:49Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/26/2017 6:31 PM, Jonathan Tan wrote:\n> On Fri, 22 Sep 2017 20:26:22 +0000\n> Jeff Hostetler wrote:\n> \n>> From: Jeff Hostetler \n>>\n>> Create traverse_commit_list_filtered() and add filtering\n> \n> You mention _filtered() here, but this patch contains _worker().\n\nthanks.\n\n \n>> list-objects.h | 30 ++++++++++++++++++++++++++\n>> 2 files changed, 80 insertions(+), 16 deletions(-)\n>>\n>> diff --git a/list-objects.c b/list-objects.c\n>> index b3931fa..3e86008 100644\n>> --- a/list-objects.c\n>> +++ b/list-objects.c\n>> @@ -13,10 +13,13 @@ static void process_blob(struct rev_info *revs,\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,\n>> +\t\t\t void *filter_data)\n> \n> I had some thoughts about modifying \"show\" (instead of adding \"filter\",\n> as you have done in this patch) to indicate to the caller that the\n> corresponding object should not be marked \"seen\". That does have the\n> advantage that we don't have so many callbacks flying around, but it\n> also makes things more complicated, so I don't know which is better.\n\nI wanted the filtering function to be independent of the showing function.\nAs you can see in pack-objects.c and rev-list.c: All I needed to change\nthere was to call the new version of the traverse code and not alter the\nshow function. The filtering is completely isolated inside the traverse\ncode.\n\nIt also means that the filtering code can do higher-level filtering.\nCurrently, the show-object callback gets called with each terminal node\n(blob) and it has already lost any chance for tree-level optimizations.\nThe filtering code participates in the treewalk on the commit and can do\nsubtree elimination.\n \n> \n>> +\tif (filter) {\n>> +\t\tr = filter(LOFT_END_TREE, obj, base->buf, &base->buf[baselen], filter_data);\n>> +\t\tif (r & LOFR_MARK_SEEN)\n>> +\t\t\tobj->flags |= SEEN;\n>> +\t\tif (r & LOFR_SHOW)\n>> +\t\t\tshow(obj, base->buf, cb_data);\n>> +\t}\n> \n> This LOFT_END_TREE was added to support the code in\n> list-objects-filter-sparse - would it be OK to completely remove the\n> optimization involving the \"provisional\" omission of blobs? (I don't\n> think the exact same tree object will occur multiple times often.) If\n> yes, then I think this block can be removed too and will simplify the\n> code.\n\nThe sparse filter is looking at pathnames and using the same rules\nas sparse-checkout to decide which to *include* in the result. This\nis essentially backwards from the other filters which are looking for\nreasons to *exclude* a blob. If I see a {pathname, sha} pair and the\npathname is not wanted (by the sparse-checkout rules), I still don't\nknow anything about the object -- since the same SHA may appear later\nin the treewalk but with a different pathname that *does* match the\npatterns and *is* wanted.\n\nThe net-net is that I have to mark the blob as \"provisionally omitted\"\nuntil I've seen all the pathnames associated with it. Only then can I\nsay it should be omitted.\n\nLikewise, there are things about the tree object that we cannot\ndecide until we've seen all possible directory paths that reference it.\nFor example, if you rename a tree/directory between 2 commits, but make no\nother changes within the directory, it will/should have the same SHA in the\nsecond commit. If one of those paths is included in the sparse-checkout\nand one is not, then you need include those blobs (and the tree object)\nin the result. If the treewalk visits the excluded case first, you don't\nwant to discard the tree object (and shortcut future treewalks) because\nthe filter won't get a chance to see the included directory path case.\n\nAlso, the current code does not attempt to omit tree objects, but a\nfuture version may. And having the _BEGIN_ and _END_ events means the\nfilter can keep track of the nesting without the expense of having to\ndiscover it by parsing the pathname looking for slashes as we do elsewhere.\n\n> \n>> +/* See object.h and revision.h */\n>> +#define FILTER_REVISIT (1<<25)\n> \n> If you do add this, also update object.h. But I don't think this is the\n> right place to do it - it's probably better in\n> list-objects-filter-sparse.\n> \n>> +typedef enum list_objects_filter_result list_objects_filter_result;\n>> +typedef enum list_objects_filter_type list_objects_filter_type;\n> \n> I don't think Git style is to typedef enums.\n> \n>> +void traverse_commit_list_worker(\n>> +\tstruct rev_info *,\n>> +\tshow_commit_fn, show_object_fn, void *show_data,\n>> +\tfilter_object_fn filter, void *filter_data);\n> \n> Here (and throughout), as far as I can tell, the style is to indent to\n> the character after the opening parenthesis.\n> \n\nright. thanks.\n\n"},{"id":"329048","messageId":"20170927110007.9ee2f725c01f0c7465838c04@google.com","threadId":"46809","inReplyTo":"d5de0eda-18aa-a796-e7d0-d536d2e59605@jeffhostetler.com","subject":"Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-27T18:00:07Z","receivedAt":"2017-09-27T18:00:25Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 27 Sep 2017 13:04:42 -0400\nJeff Hostetler wrote:\n\n> The sparse filter is looking at pathnames and using the same rules\n> as sparse-checkout to decide which to *include* in the result. This\n> is essentially backwards from the other filters which are looking for\n> reasons to *exclude* a blob. If I see a {pathname, sha} pair and the\n> pathname is not wanted (by the sparse-checkout rules), I still don't\n> know anything about the object -- since the same SHA may appear later\n> in the treewalk but with a different pathname that *does* match the\n> patterns and *is* wanted.\n> \n> The net-net is that I have to mark the blob as \"provisionally omitted\"\n> until I've seen all the pathnames associated with it. Only then can I\n> say it should be omitted.\n\nHow is this different from refraining from marking the blob as\nLOFR_MARK_SEEN? When you would provisionally omit the blob, return\nLOFR_ZERO so that a future iteration will revisit the blob again, and\nwhen you would include it in the output, return\nLOFR_MARK_SEEN|LOFR_SHOW.\n\n> Likewise, there are things about the tree object that we cannot\n> decide until we've seen all possible directory paths that reference it.\n> For example, if you rename a tree/directory between 2 commits, but make no\n> other changes within the directory, it will/should have the same SHA in the\n> second commit. If one of those paths is included in the sparse-checkout\n> and one is not, then you need include those blobs (and the tree object)\n> in the result. If the treewalk visits the excluded case first, you don't\n> want to discard the tree object (and shortcut future treewalks) because\n> the filter won't get a chance to see the included directory path case.\n\nFor trees, I guess it's slightly different in that you do need an extra\nflag to keep track of whether the tree has been shown. So mark SHOWN and\nreturn LOFR_SHOW on the first time the tree is shown, and LOFR_ZERO\notherwise. And trees must never be marked as LOFR_MARK_SEEN.\n\n(This SHOWN flag might play a similar role to your FILTER_REVISIT.)\n\nUntil now, it seems to me that the _END event is not required.\n\n> Also, the current code does not attempt to omit tree objects, but a\n> future version may. And having the _BEGIN_ and _END_ events means the\n> filter can keep track of the nesting without the expense of having to\n> discover it by parsing the pathname looking for slashes as we do elsewhere.\n\nA feature that omits tree objects would need _END, true. But until\nthen, I personally don't think we should add such infrastructure until\nwe have a feature that needs it.\n"},{"id":"329052","messageId":"1577b87b-d6de-1434-930a-83fde384d4ca@jeffhostetler.com","threadId":"46809","inReplyTo":"20170927110007.9ee2f725c01f0c7465838c04@google.com","subject":"Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-27T19:09:43Z","receivedAt":"2017-09-27T19:09:50Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/27/2017 2:00 PM, Jonathan Tan wrote:\n> On Wed, 27 Sep 2017 13:04:42 -0400\n> Jeff Hostetler wrote:\n> \n>> The sparse filter is looking at pathnames and using the same rules\n>> as sparse-checkout to decide which to *include* in the result. This\n>> is essentially backwards from the other filters which are looking for\n>> reasons to *exclude* a blob. If I see a {pathname, sha} pair and the\n>> pathname is not wanted (by the sparse-checkout rules), I still don't\n>> know anything about the object -- since the same SHA may appear later\n>> in the treewalk but with a different pathname that *does* match the\n>> patterns and *is* wanted.\n>>\n>> The net-net is that I have to mark the blob as \"provisionally omitted\"\n>> until I've seen all the pathnames associated with it. Only then can I\n>> say it should be omitted.\n> \n> How is this different from refraining from marking the blob as\n> LOFR_MARK_SEEN? When you would provisionally omit the blob, return\n> LOFR_ZERO so that a future iteration will revisit the blob again, and\n> when you would include it in the output, return\n> LOFR_MARK_SEEN|LOFR_SHOW.\n\nBy adding it to the set of provisionally omitted objects, we\nhave the option to capture a little extra information with it\nand refer to that the next time we see the object in the traversal.\nFor example, in the sparse-checkout case, the first time we see the\nobject we know the pathname and know that it does not need to be\nincluded. The second time we see that object, we can see if the\nnew pathname is the same as the previous one with a simple strcmp\nand avoid the expensive is_excluded_from_list() computation. Keep\nin mind that rev-list or pack-objects could be called be on something\nlike HEAD~100000..HEAD or that there may be 50,000 tips. So a file\nthat doesn't change across that range will be visited many times\nwith the same {pathname, sha}.\n\nThen when the traversal is finished, we have a resulting set of\n(now actually) omitted objects. Which we can iterate over if the\ncaller is interested in.\n\n> \n>> Likewise, there are things about the tree object that we cannot\n>> decide until we've seen all possible directory paths that reference it.\n>> For example, if you rename a tree/directory between 2 commits, but make no\n>> other changes within the directory, it will/should have the same SHA in the\n>> second commit. If one of those paths is included in the sparse-checkout\n>> and one is not, then you need include those blobs (and the tree object)\n>> in the result. If the treewalk visits the excluded case first, you don't\n>> want to discard the tree object (and shortcut future treewalks) because\n>> the filter won't get a chance to see the included directory path case.\n> \n> For trees, I guess it's slightly different in that you do need an extra\n> flag to keep track of whether the tree has been shown. So mark SHOWN and\n> return LOFR_SHOW on the first time the tree is shown, and LOFR_ZERO\n> otherwise. And trees must never be marked as LOFR_MARK_SEEN.\n\nRight now I want to force the tree to be shown the first time it is\nvisited (because I don't want to do tree filtering yet). I don't mark\nit SEEN yet because we may want to revisit blobs within (say, after a\nfolder rename like I described previously).\n\nI do, however, mark the tree object as SEEN (in the _END event) when I\ncan verify that I've included ALL of the children.\n\nSo it might be possible that I could change the flags and not use\nFILTER_REVISIT on tree objects, I hesitate to do that right now.\n\n\nHaving the FILTER_REVISIT flag on blob objects means I can avoid\ndoing a hash/oidset lookup on subsequent visits.\n\n \n> (This SHOWN flag might play a similar role to your FILTER_REVISIT.)\n> \n> Until now, it seems to me that the _END event is not required.\n> \n>> Also, the current code does not attempt to omit tree objects, but a\n>> future version may. And having the _BEGIN_ and _END_ events means the\n>> filter can keep track of the nesting without the expense of having to\n>> discover it by parsing the pathname looking for slashes as we do elsewhere.\n> \n> A feature that omits tree objects would need _END, true. But until\n> then, I personally don't think we should add such infrastructure until\n> we have a feature that needs it.\n\nThe sparse filter is using the _END now to try to help shortcut the\ntreewalk (when all referenced objects within it are known to already\nbe included).\n\nThe _END event's role may expand with a 4th filter that also filters\ntree objects, but I don't want to eliminate it right now.\n\nThanks\nJeff\n\n\n"},{"id":"329059","messageId":"20170927134934.d40b24d08749c6d4589e8b33@google.com","threadId":"46809","inReplyTo":"1577b87b-d6de-1434-930a-83fde384d4ca@jeffhostetler.com","subject":"Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-27T20:49:34Z","receivedAt":"2017-09-27T20:49:42Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 27 Sep 2017 15:09:43 -0400\nJeff Hostetler wrote:\n\n> By adding it to the set of provisionally omitted objects, we\n> have the option to capture a little extra information with it\n> and refer to that the next time we see the object in the traversal.\n> For example, in the sparse-checkout case, the first time we see the\n> object we know the pathname and know that it does not need to be\n> included. The second time we see that object, we can see if the\n> new pathname is the same as the previous one with a simple strcmp\n> and avoid the expensive is_excluded_from_list() computation. Keep\n> in mind that rev-list or pack-objects could be called be on something\n> like HEAD~100000..HEAD or that there may be 50,000 tips. So a file\n> that doesn't change across that range will be visited many times\n> with the same {pathname, sha}.\n\nAh, capturing the extra information makes sense. I missed that detail.\n\n> Right now I want to force the tree to be shown the first time it is\n> visited (because I don't want to do tree filtering yet). I don't mark\n> it SEEN yet because we may want to revisit blobs within (say, after a\n> folder rename like I described previously).\n> \n> I do, however, mark the tree object as SEEN (in the _END event) when I\n> can verify that I've included ALL of the children.\n\nThis optimization makes sense too.\n\n> So it might be possible that I could change the flags and not use\n> FILTER_REVISIT on tree objects, I hesitate to do that right now.\n\nYou're probably right that we need some sort of flag on tree objects,\nand FILTER_REVISIT can do the job. (My suggestion SHOWN plays a similar\nrole anyway.)\n\n> Having the FILTER_REVISIT flag on blob objects means I can avoid\n> doing a hash/oidset lookup on subsequent visits.\n\nBy the hash/oidset lookup, I presume you mean the lookup on the set of\nprovisionally omitted objects? If yes, this makes sense.\n\nThanks for your clarifications - I'll take another look at the code\nhere.\n"}]}