{"thread":{"id":"47024","subject":"[PATCH 00/13] WIP Partial clone part 1: object filtering","startedAt":"2017-10-24T18:54:04Z","lastAt":"2017-10-30T22:27:34Z","messageCount":37,"participants":["Jeff Hostetler","Jonathan Tan","Eric Sunshine","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"330934","messageId":"20171024185332.57261-1-git@jeffhostetler.com","threadId":"47024","inReplyTo":null,"subject":"[PATCH 00/13] WIP Partial clone part 1: object filtering","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:19Z","receivedAt":"2017-10-24T18:54:04Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nI've been working with Jonathan Tan to combine our partial clone\nproposals. This patch series represents a first step in that effort\nand introduces an object filtering mechanism to select unwanted\nobjects.\n\n[1] traverse_commit_list and list-objects is extended to allow\n various filters.\n[2] rev-list is extended to expose filtering. This allows testing\n of the filtering options. And can be used later to predict\n missing objects before commands like checkout or merge.\n[3] pack-objects is extended to use filtering parameters and build\n packfiles that omit unwanted objects.\n\nThis patch series lays the ground work for subsequent parts which\nwill extend clone, fetch, fetch-pack, upload-pack, fsck, and etc.\n\n\nJeff Hostetler (13):\n dir: allow exclusions from blob in addition to file\n list-objects-filter-map: extend oidmap to collect omitted objects\n list-objects: filter objects in traverse_commit_list\n list-objects-filter-blobs-none: add filter to omit all blobs\n list-objects-filter-blobs-limit: add large blob filtering\n list-objects-filter-sparse: add sparse filter\n list-objects-filter-options: common argument parsing\n list-objects: add traverse_commit_list_filtered method\n extension.partialclone: introduce partial clone extension\n rev-list: add list-objects filtering support\n t6112: rev-list object filtering test\n pack-objects: add list-objects filtering\n t5317: pack-objects object filtering test\n\n Documentation/git-pack-objects.txt | 8 +-\n Documentation/git-rev-list.txt | 5 +-\n Documentation/rev-list-options.txt | 30 ++\n Documentation/technical/repository-version.txt | 22 ++\n Makefile | 6 +\n builtin/pack-objects.c | 18 +-\n builtin/rev-list.c | 84 +++++-\n cache.h | 4 +\n config.h | 3 +\n dir.c | 51 +++-\n dir.h | 3 +\n environment.c | 2 +\n list-objects-filter-blobs-limit.c | 146 ++++++++++\n list-objects-filter-blobs-limit.h | 18 ++\n list-objects-filter-blobs-none.c | 83 ++++++\n list-objects-filter-blobs-none.h | 18 ++\n list-objects-filter-map.c | 63 ++++\n list-objects-filter-map.h | 26 ++\n list-objects-filter-options.c | 101 +++++++\n list-objects-filter-options.h | 50 ++++\n list-objects-filter-sparse.c | 241 ++++++++++++++++\n list-objects-filter-sparse.h | 30 ++\n list-objects.c | 111 +++++--\n list-objects.h | 43 ++-\n partial-clone-utils.c | 99 +++++++\n partial-clone-utils.h | 34 +++\n setup.c | 15 +\n t/t5317-pack-objects-filter-objects.sh | 384 +++++++++++++++++++++++++\n t/t6112-rev-list-filters-objects.sh | 223 ++++++++++++++\n 29 files changed, 1897 insertions(+), 24 deletions(-)\n create mode 100644 list-objects-filter-blobs-limit.c\n create mode 100644 list-objects-filter-blobs-limit.h\n create mode 100644 list-objects-filter-blobs-none.c\n create mode 100644 list-objects-filter-blobs-none.h\n create mode 100644 list-objects-filter-map.c\n create mode 100644 list-objects-filter-map.h\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-sparse.c\n create mode 100644 list-objects-filter-sparse.h\n create mode 100644 partial-clone-utils.c\n create mode 100644 partial-clone-utils.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":"330935","messageId":"20171024185332.57261-3-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 02/13] list-objects-filter-map: extend oidmap to collect omitted objects","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:21Z","receivedAt":"2017-10-24T18:54:06Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nCreate helper class to extend oidmap to collect a list of\nomitted or missing objects during traversal.\n\nThis will be used in a later commit by the list-object filtering\ncode.\n\nSigned-off-by: Jeff Hostetler \n---\n Makefile | 1 +\n list-objects-filter-map.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++\n list-objects-filter-map.h | 26 +++++++++++++++++++\n 3 files changed, 90 insertions(+)\n create mode 100644 list-objects-filter-map.c\n create mode 100644 list-objects-filter-map.h\n\ndiff --git a/Makefile b/Makefile\nindex cd75985..e59f12d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -807,6 +807,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-map.o\n LIB_OBJS += ll-merge.o\n LIB_OBJS += lockfile.o\n LIB_OBJS += log-tree.o\ndiff --git a/list-objects-filter-map.c b/list-objects-filter-map.c\nnew file mode 100644\nindex 0000000..7e496b3\n--- /dev/null\n+++ b/list-objects-filter-map.c\n@@ -0,0 +1,63 @@\n+#include \"cache.h\"\n+#include \"list-objects-filter-map.h\"\n+\n+int list_objects_filter_map_insert(struct oidmap *map,\n+\t\t\t\t const struct object_id *oid,\n+\t\t\t\t const char *pathname, enum object_type type)\n+{\n+\tsize_t len, size;\n+\tstruct list_objects_filter_map_entry *e;\n+\n+\tif (oidmap_get(map, oid))\n+\t\treturn 1;\n+\n+\tlen = ((pathname && *pathname) ? strlen(pathname) : 0);\n+\tsize = (offsetof(struct list_objects_filter_map_entry, pathname) + len + 1);\n+\te = xcalloc(1, size);\n+\n+\toidcpy(&e->entry.oid, oid);\n+\te->type = type;\n+\tif (pathname && *pathname)\n+\t\tstrcpy(e->pathname, pathname);\n+\n+\toidmap_put(map, e);\n+\treturn 0;\n+}\n+\n+static int my_cmp(const void *a, const void *b)\n+{\n+\tconst struct oidmap_entry *ea, *eb;\n+\n+\tea = *(const struct oidmap_entry **)a;\n+\teb = *(const struct oidmap_entry **)b;\n+\n+\treturn oidcmp(&ea->oid, &eb->oid);\n+}\n+\n+void list_objects_filter_map_foreach(struct oidmap *map,\n+\t\t\t\t list_objects_filter_map_foreach_cb cb,\n+\t\t\t\t void *cb_data)\n+{\n+\tstruct hashmap_iter iter;\n+\tstruct list_objects_filter_map_entry **array;\n+\tstruct list_objects_filter_map_entry *e;\n+\tint k, nr;\n+\n+\tnr = hashmap_get_size(&map->map);\n+\tif (!nr)\n+\t\treturn;\n+\n+\tarray = xcalloc(nr, sizeof(*e));\n+\n+\tk = 0;\n+\thashmap_iter_init(&map->map, &iter);\n+\twhile ((e = hashmap_iter_next(&iter)))\n+\t\tarray[k++] = e;\n+\n+\tQSORT(array, nr, my_cmp);\n+\n+\tfor (k = 0; k < nr; k++)\n+\t\tcb(k, nr, array[k], cb_data);\n+\n+\tfree(array);\n+}\ndiff --git a/list-objects-filter-map.h b/list-objects-filter-map.h\nnew file mode 100644\nindex 0000000..794fc81\n--- /dev/null\n+++ b/list-objects-filter-map.h\n@@ -0,0 +1,26 @@\n+#ifndef LIST_OBJECTS_FILTER_MAP_H\n+#define LIST_OBJECTS_FILTER_MAP_H\n+\n+#include \"oidmap.h\"\n+\n+struct list_objects_filter_map_entry {\n+\tstruct oidmap_entry entry; /* must be first */\n+\tenum object_type type;\n+\tchar pathname[FLEX_ARRAY];\n+};\n+\n+extern int list_objects_filter_map_insert(\n+\tstruct oidmap *map,\n+\tconst struct object_id *oid,\n+\tconst char *pathname, enum object_type type);\n+\n+typedef void (*list_objects_filter_map_foreach_cb)(\n+\tint i, int i_limit,\n+\tstruct list_objects_filter_map_entry *e, void *cb_data);\n+\n+extern void list_objects_filter_map_foreach(\n+\tstruct oidmap *map,\n+\tlist_objects_filter_map_foreach_cb cb,\n+\tvoid *cb_data);\n+\n+#endif /* LIST_OBJECTS_FILTER_MAP_H */\n-- \n2.9.3\n\n"},{"id":"330936","messageId":"20171024185332.57261-7-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 06/13] list-objects-filter-sparse: add sparse filter","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:25Z","receivedAt":"2017-10-24T18:54:08Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nCreate a filter for traverse_commit_list_worker() to only include\nthe blobs the would be referenced by a sparse-checkout using the\ngiven specification.\n\nSigned-off-by: Jeff Hostetler \n---\n Makefile | 1 +\n list-objects-filter-sparse.c | 241 +++++++++++++++++++++++++++++++++++++++++++\n list-objects-filter-sparse.h | 30 ++++++\n 3 files changed, 272 insertions(+)\n create mode 100644 list-objects-filter-sparse.c\n create mode 100644 list-objects-filter-sparse.h\n\ndiff --git a/Makefile b/Makefile\nindex 0fdeabb..fc82664 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -810,6 +810,7 @@ LIB_OBJS += list-objects.o\n LIB_OBJS += list-objects-filter-blobs-limit.o\n LIB_OBJS += list-objects-filter-blobs-none.o\n LIB_OBJS += list-objects-filter-map.o\n+LIB_OBJS += list-objects-filter-sparse.o\n LIB_OBJS += ll-merge.o\n LIB_OBJS += lockfile.o\n LIB_OBJS += log-tree.o\ndiff --git a/list-objects-filter-sparse.c b/list-objects-filter-sparse.c\nnew file mode 100644\nindex 0000000..386b667\n--- /dev/null\n+++ b/list-objects-filter-sparse.c\n@@ -0,0 +1,241 @@\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-sparse.h\"\n+\n+#define DEFAULT_MAP_SIZE (16*1024)\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+\tint defval;\n+\tint child_prov_omit : 1;\n+};\n+\n+struct filter_use_sparse_data {\n+\tstruct oidmap *omits;\n+\tstruct exclude_list el;\n+\n+\tsize_t nr, alloc;\n+\tstruct frame *array_frame;\n+};\n+\n+static list_objects_filter_result filter_use_sparse(\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_use_sparse_data *filter_data = filter_data_;\n+\tstruct list_objects_filter_map_entry *entry_prev = NULL;\n+\tint val, dtype;\n+\tstruct frame *frame;\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\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, with\n+\t\t * no other changes.) And with a different pathname, the\n+\t\t * is_excluded...() results for this directory and items\n+\t\t * contained within it may be different. So we cannot\n+\t\t * mark it SEEN (yet), since that will prevent process_tree()\n+\t\t * from revisiting this tree object with other pathnames.\n+\t\t *\n+\t\t * Only SHOW the tree object the first time we visit this\n+\t\t * 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_REVISIT)\n+\t\t\treturn LOFR_ZERO;\n+\t\tobj->flags |= FILTER_REVISIT;\n+\t\treturn LOFR_SHOW;\n+\n+\tcase LOFT_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 LOFT_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\t/*\n+\t\t * If we are keeping a list of the omitted objects\n+\t\t * for the caller *AND* we previsously provisionally\n+\t\t * omitted this object (because the THEN pathname\n+\t\t * is excluded) *AND* it has the same pathname, we\n+\t\t * can avoid duplicating the is_excluded lookup\n+\t\t * costs and continue provisionally omitting it.\n+\t\t */\n+\t\tif (filter_data->omits) {\n+\t\t\tentry_prev = oidmap_get(\n+\t\t\t\tfilter_data->omits, &obj->oid);\n+\t\t\tif (entry_prev &&\n+\t\t\t !strcmp(pathname, entry_prev->pathname)) {\n+\t\t\t\tframe->child_prov_omit = 1;\n+\t\t\t\treturn LOFR_ZERO;\n+\t\t\t}\n+\t\t}\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 (entry_prev) {\n+\t\t\t\tentry_prev = oidmap_remove(filter_data->omits,\n+\t\t\t\t\t\t\t &obj->oid);\n+\t\t\t\tfree(entry_prev);\n+\t\t\t}\n+\t\t\treturn LOFR_MARK_SEEN | LOFR_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\t * The pathname that we associate with this omit is just\n+\t\t * the first one we saw for this blob. Other instances of\n+\t\t * this blob may have other pathnames and that is fine.\n+\t\t * We just use it for perf to do the entry_prev lookup\n+\t\t * above (because most of the time, the blob will be in\n+\t\t * the same place as we walk the commits).\n+\t\t */\n+\t\tif (filter_data->omits)\n+\t\t\tlist_objects_filter_map_insert(filter_data->omits,\n+\t\t\t\t\t\t &obj->oid, pathname,\n+\t\t\t\t\t\t obj->type);\n+\n+\t\tframe->child_prov_omit = 1;\n+\t\treturn LOFR_ZERO;\n+\t}\n+}\n+\n+static void do_sparse(\n+\tstruct filter_use_sparse_data *d,\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *ctx_data)\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+\ttraverse_commit_list_worker(revs, show_commit, show_object, ctx_data,\n+\t\t\t\t filter_use_sparse, d);\n+\n+\tif (print_omitted_object) {\n+\t\tlist_objects_filter_map_foreach(d->omits, print_omitted_object, ctx_data);\n+\t\toidmap_free(d->omits, 1);\n+\t}\n+}\n+\n+void traverse_commit_list__sparse_oid(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *ctx_data,\n+\tstruct object_id *oid)\n+{\n+\tstruct filter_use_sparse_data d;\n+\n+\tmemset(&d, 0, sizeof(d));\n+\tif (print_omitted_object) {\n+\t\td.omits = xcalloc(1, sizeof(*d.omits));\n+\t\toidmap_init(d.omits, DEFAULT_MAP_SIZE);\n+\t}\n+\tif (add_excludes_from_blob_to_list(oid, NULL, 0, &d.el) < 0)\n+\t\tdie(\"could not load filter specification\");\n+\n+\tdo_sparse(&d, revs, show_commit, show_object, print_omitted_object,\n+\t\t ctx_data);\n+}\n+\n+void traverse_commit_list__sparse_path(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *ctx_data,\n+\tconst char *path)\n+{\n+\tstruct filter_use_sparse_data d;\n+\n+\tmemset(&d, 0, sizeof(d));\n+\tif (print_omitted_object) {\n+\t\td.omits = xcalloc(1, sizeof(*d.omits));\n+\t\toidmap_init(d.omits, DEFAULT_MAP_SIZE);\n+\t}\n+\tif (add_excludes_from_file_to_list(path, NULL, 0, &d.el, NULL) < 0)\n+\t\tdie(\"could not load filter specification\");\n+\n+\tdo_sparse(&d, revs, show_commit, show_object, print_omitted_object,\n+\t\t ctx_data);\n+}\ndiff --git a/list-objects-filter-sparse.h b/list-objects-filter-sparse.h\nnew file mode 100644\nindex 0000000..6c715bf\n--- /dev/null\n+++ b/list-objects-filter-sparse.h\n@@ -0,0 +1,30 @@\n+#ifndef LIST_OBJECTS_FILTERS_SPARSE_H\n+#define LIST_OBJECTS_FILTERS_SPARSE_H\n+\n+#include \"list-objects-filter-map.h\"\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, a blob with a blob-ish path, or from a local pathname.\n+ * We allow an OID because the repo may be bare or we may be doing\n+ * the filtering on the server.\n+ */\n+void traverse_commit_list__sparse_oid(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *ctx_data,\n+\tstruct object_id *oid);\n+void traverse_commit_list__sparse_path(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *ctx_data,\n+\tconst char *path);\n+\n+#endif /* LIST_OBJECTS_FILTERS_SPARSE_H */\n-- \n2.9.3\n\n"},{"id":"330937","messageId":"20171024185332.57261-9-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 08/13] list-objects: add traverse_commit_list_filtered method","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:27Z","receivedAt":"2017-10-24T18:54:11Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nAdd traverse_commit_list_filtered() wrapper around the various\nfilter methods using common data in object_filter_options.\n\nSigned-off-by: Jeff Hostetler \n---\n list-objects.c | 45 +++++++++++++++++++++++++++++++++++++++++++++\n list-objects.h | 11 +++++++++++\n 2 files changed, 56 insertions(+)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex 3e86008..4ce2593 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -7,6 +7,9 @@\n #include \"tree-walk.h\"\n #include \"revision.h\"\n #include \"list-objects.h\"\n+#include \"list-objects-filter-blobs-none.h\"\n+#include \"list-objects-filter-blobs-limit.h\"\n+#include \"list-objects-filter-sparse.h\"\n \n static void process_blob(struct rev_info *revs,\n \t\t\t struct blob *blob,\n@@ -266,3 +269,45 @@ void traverse_commit_list(struct rev_info *revs,\n \t\tshow_commit, show_object, show_data,\n \t\tNULL, 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+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *show_data)\n+{\n+\tswitch (filter_options->choice) {\n+\tcase LOFC_DISABLED:\n+\t\ttraverse_commit_list(revs, show_commit, show_object, show_data);\n+\t\treturn;\n+\n+\tcase LOFC_BLOB_NONE:\n+\t\ttraverse_commit_list__blobs_none(\n+\t\t\trevs, show_commit, show_object, print_omitted_object,\n+\t\t\tshow_data);\n+\t\treturn;\n+\n+\tcase LOFC_BLOB_LIMIT:\n+\t\ttraverse_commit_list__blobs_limit(\n+\t\t\trevs, show_commit, show_object, print_omitted_object,\n+\t\t\tshow_data, filter_options->blob_limit_value);\n+\t\treturn;\n+\n+\tcase LOFC_SPARSE_OID:\n+\t\ttraverse_commit_list__sparse_oid(\n+\t\t\trevs, show_commit, show_object, print_omitted_object,\n+\t\t\tshow_data, filter_options->sparse_oid_value);\n+\t\treturn;\n+\n+\tcase LOFC_SPARSE_PATH:\n+\t\ttraverse_commit_list__sparse_path(\n+\t\t\trevs, show_commit, show_object, print_omitted_object,\n+\t\t\tshow_data, filter_options->sparse_path_value);\n+\t\treturn;\n+\n+\tdefault:\n+\t\tdie(\"unspecified list-objects filter\");\n+\t}\n+}\ndiff --git a/list-objects.h b/list-objects.h\nindex 43a06fb..d14b0e0 100644\n--- a/list-objects.h\n+++ b/list-objects.h\n@@ -1,6 +1,9 @@\n #ifndef LIST_OBJECTS_H\n #define LIST_OBJECTS_H\n \n+#include \"list-objects-filter-map.h\"\n+#include \"list-objects-filter-options.h\"\n+\n typedef void (*show_commit_fn)(struct commit *, void *);\n typedef void (*show_object_fn)(struct object *, const char *, void *);\n void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, void *);\n@@ -38,4 +41,12 @@ void traverse_commit_list_worker(\n \tshow_commit_fn, show_object_fn, void *show_data,\n \tfilter_object_fn filter, void *filter_data);\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+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *show_data);\n+\n #endif /* LIST_OBJECTS_H */\n-- \n2.9.3\n\n"},{"id":"330938","messageId":"20171024185332.57261-12-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 11/13] t6112: rev-list object filtering test","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:30Z","receivedAt":"2017-10-24T18:54:14Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nSigned-off-by: Jeff Hostetler \n---\n t/t6112-rev-list-filters-objects.sh | 223 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 223 insertions(+)\n create mode 100755 t/t6112-rev-list-filters-objects.sh\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..26fa12f\n--- /dev/null\n+++ b/t/t6112-rev-list-filters-objects.sh\n@@ -0,0 +1,223 @@\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/~//\" >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/~//\" >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/~//\" >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/~//\" >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/~//\" >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/~//\" >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/~//\" >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/~//\" >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/~//\" >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/~//\" >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/ print-missing' '\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 --filter-print-missing --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 print-missing fails' '\n+\ttest_must_fail git -C r1 rev-list --quiet --objects HEAD\n+'\n+\n+test_expect_success 'rev-list W/ extension.partialcloneremote set succeeds' '\n+\tgit -C r1 config --local core.repositoryformatversion 1 &&\n+\tgit -C r1 config --local extensions.partialcloneremote \"origin\" &&\n+\tgit -C r1 rev-list --quiet --objects HEAD\n+'\n+\n+test_expect_success 'rev-list W/ extension.partialclonefilter set succeeds' '\n+\tgit -C r1 config --local core.repositoryformatversion 1 &&\n+\tgit -C r1 config --local extensions.partialclonefilter \"something\" &&\n+\tgit -C r1 rev-list --quiet --objects HEAD\n+'\n+\n+test_done\n-- \n2.9.3\n\n"},{"id":"330939","messageId":"20171024185332.57261-6-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 05/13] list-objects-filter-blobs-limit: add large blob filtering","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:24Z","receivedAt":"2017-10-24T18:54:16Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nCreate a filter for traverse_commit_list_worker() to omit blobs\nlarger than a requested size from the result, but always include\n\".git*\" special files.\n\nSigned-off-by: Jeff Hostetler \n---\n Makefile | 1 +\n list-objects-filter-blobs-limit.c | 146 ++++++++++++++++++++++++++++++++++++++\n list-objects-filter-blobs-limit.h | 18 +++++\n 3 files changed, 165 insertions(+)\n create mode 100644 list-objects-filter-blobs-limit.c\n create mode 100644 list-objects-filter-blobs-limit.h\n\ndiff --git a/Makefile b/Makefile\nindex 7e9d1f4..0fdeabb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -807,6 +807,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-blobs-limit.o\n LIB_OBJS += list-objects-filter-blobs-none.o\n LIB_OBJS += list-objects-filter-map.o\n LIB_OBJS += ll-merge.o\ndiff --git a/list-objects-filter-blobs-limit.c b/list-objects-filter-blobs-limit.c\nnew file mode 100644\nindex 0000000..f68963d\n--- /dev/null\n+++ b/list-objects-filter-blobs-limit.c\n@@ -0,0 +1,146 @@\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-blobs-limit.h\"\n+\n+#define DEFAULT_MAP_SIZE (16*1024)\n+\n+/*\n+ * A filter for list-objects to omit large blobs,\n+ * but always include \".git*\" special files.\n+ * And to OPTIONALLY collect a list of the omitted OIDs.\n+ */\n+struct filter_blobs_limit_data {\n+\tstruct oidmap *omits;\n+\tunsigned long max_bytes;\n+};\n+\n+static list_objects_filter_result filter_blobs_limit(\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_blobs_limit_data *filter_data = filter_data_;\n+\tstruct list_objects_filter_data_entry *entry;\n+\tunsigned long object_length;\n+\tenum object_type t;\n+\tint is_special_filename;\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\tis_special_filename = ((strncmp(filename, \".git\", 4) == 0) &&\n+\t\t\t\t filename[4]);\n+\n+\t\t/*\n+\t\t * If we are keeping a list of the omitted objects\n+\t\t * for the caller *AND* we previously \"provisionally\"\n+\t\t * omitted this object (because of size) *AND* it now\n+\t\t * has a special filename, make it not-omitted.\n+\t\t * Otherwise, continue to provisionally omit it.\n+\t\t */\n+\t\tif (filter_data->omits &&\n+\t\t oidmap_get(filter_data->omits, &obj->oid)) {\n+\t\t\tif (!is_special_filename)\n+\t\t\t\treturn LOFR_ZERO;\n+\t\t\tentry = oidmap_remove(filter_data->omits, &obj->oid);\n+\t\t\tfree(entry);\n+\t\t\treturn LOFR_MARK_SEEN | LOFR_SHOW;\n+\t\t}\n+\n+\t\t/*\n+\t\t * If filename matches \".git*\", always include it (regardless\n+\t\t * of size). (This may include blobs that we do not have\n+\t\t * locally.)\n+\t\t */\n+\t\tif (is_special_filename)\n+\t\t\treturn LOFR_MARK_SEEN | LOFR_SHOW;\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). (This matches the behavior above\n+\t\t\t * when the special filename matches.)\n+\t\t\t */\n+\t\t\treturn LOFR_MARK_SEEN | LOFR_SHOW;\n+\t\t}\n+\n+\t\tif (object_length < filter_data->max_bytes)\n+\t\t\treturn LOFR_MARK_SEEN | LOFR_SHOW;\n+\n+\t\t/*\n+\t\t * Provisionally omit it. We've already established\n+\t\t * that this blob is too big and doesn't have a special\n+\t\t * filename, so we *WANT* to omit it. However, there\n+\t\t * may be a special file elsewhere in the tree that\n+\t\t * references this same blob, so we cannot reject it\n+\t\t * just yet. Leave the LOFR_ bits unset so that *IF*\n+\t\t * the blob appears again in the traversal, we will\n+\t\t * be asked again.\n+\t\t *\n+\t\t * If we are keeping a list of the ommitted objects,\n+\t\t * provisionally add it to the list.\n+\t\t */\n+\n+\t\tif (filter_data->omits)\n+\t\t\tlist_objects_filter_map_insert(filter_data->omits,\n+\t\t\t\t\t\t &obj->oid, pathname,\n+\t\t\t\t\t\t obj->type);\n+\n+\t\treturn LOFR_ZERO;\n+\t}\n+}\n+\n+void traverse_commit_list__blobs_limit(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *ctx_data,\n+\tunsigned long large_byte_limit)\n+{\n+\tstruct filter_blobs_limit_data d;\n+\n+\tmemset(&d, 0, sizeof(d));\n+\tif (print_omitted_object) {\n+\t\td.omits = xcalloc(1, sizeof(*d.omits));\n+\t\toidmap_init(d.omits, DEFAULT_MAP_SIZE);\n+\t}\n+\td.max_bytes = large_byte_limit;\n+\n+\ttraverse_commit_list_worker(revs, show_commit, show_object, ctx_data,\n+\t\t\t\t filter_blobs_limit, &d);\n+\n+\tif (print_omitted_object) {\n+\t\tlist_objects_filter_map_foreach(d.omits, print_omitted_object,\n+\t\t\t\t\t\tctx_data);\n+\t\toidmap_free(d.omits, 1);\n+\t}\n+}\ndiff --git a/list-objects-filter-blobs-limit.h b/list-objects-filter-blobs-limit.h\nnew file mode 100644\nindex 0000000..ea05088\n--- /dev/null\n+++ b/list-objects-filter-blobs-limit.h\n@@ -0,0 +1,18 @@\n+#ifndef LIST_OBJECTS_FILTER_BLOBS_LIMIT_H\n+#define LIST_OBJECTS_FILTER_BLOBS_LIMIT_H\n+\n+#include \"list-objects-filter-map.h\"\n+\n+/*\n+ * A filter for list-objects to omit large blobs,\n+ * but always include \".git*\" special files.\n+ */\n+void traverse_commit_list__blobs_limit(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *ctx_data,\n+\tunsigned long large_byte_limit);\n+\n+#endif /* LIST_OBJECTS_FILTER_BLOBS_LIMIT_H */\n-- \n2.9.3\n\n"},{"id":"330940","messageId":"20171024185332.57261-14-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 13/13] t5317: pack-objects object filtering test","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:32Z","receivedAt":"2017-10-24T18:54:19Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nSigned-off-by: Jeff Hostetler \n---\n t/t5317-pack-objects-filter-objects.sh | 384 +++++++++++++++++++++++++++++++++\n 1 file changed, 384 insertions(+)\n create mode 100755 t/t5317-pack-objects-filter-objects.sh\n\ndiff --git a/t/t5317-pack-objects-filter-objects.sh b/t/t5317-pack-objects-filter-objects.sh\nnew file mode 100755\nindex 0000000..ef7a8f6\n--- /dev/null\n+++ b/t/t5317-pack-objects-filter-objects.sh\n@@ -0,0 +1,384 @@\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+if ! test_have_prereq TODO; then\n+\tskip_all='TODO Allow pack-objects to work with missing objects'\n+\ttest_done\n+fi\n+\n+test_expect_success 'verify pack-objects w/ extension.partialcloneremote set succeeds' '\n+\tgit -C r1 config --local core.repositoryformatversion 1 &&\n+\tgit -C r1 config --local extensions.partialcloneremote \"origin\" &&\n+\tgit -C r1 pack-objects --rev --stdout >miss.pack <<-EOF\n+\tHEAD\n+\tEOF\n+'\n+\n+test_expect_success 'veify pack-objects w/ extension.partialclonefilter set succeeds' '\n+\tgit -C r1 config --local core.repositoryformatversion 1 &&\n+\tgit -C r1 config --local extensions.partialclonefilter \"something\" &&\n+\tgit -C r1 pack-objects --rev --stdout >miss.pack <<-EOF\n+\tHEAD\n+\tEOF\n+'\n+\n+test_done\n-- \n2.9.3\n\n"},{"id":"330941","messageId":"20171024185332.57261-13-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 12/13] pack-objects: add list-objects filtering","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:31Z","receivedAt":"2017-10-24T18:54:21Z","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\nSigned-off-by: Jeff Hostetler \n---\n Documentation/git-pack-objects.txt | 8 +++++++-\n builtin/pack-objects.c | 18 +++++++++++++++++-\n 2 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 473a161..8b4a223 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,11 @@ 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 SEE ALSO\n --------\n linkgit:git-rev-list[1]\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 6e77dfd..a251850 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -79,6 +79,8 @@ 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 /*\n * stats\n */\n@@ -2816,7 +2818,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+\tif (filter_options.choice)\n+\t\ttraverse_commit_list_filtered(&filter_options, &revs,\n+\t\t\t\t\t show_commit, show_object,\n+\t\t\t\t\t NULL, NULL);\n+\telse\n+\t\ttraverse_commit_list(&revs, show_commit, show_object, NULL);\n \n \tif (unpack_unreachable_expiration) {\n \t\trevs.ignore_missing_links = 1;\n@@ -2952,6 +2959,9 @@ 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+\n+\t\tOPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),\n+\n \t\tOPT_END(),\n \t};\n \n@@ -3028,6 +3038,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 filtering with an indexable pack.\");\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 *\n-- \n2.9.3\n\n"},{"id":"330942","messageId":"20171024185332.57261-11-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 10/13] rev-list: add list-objects filtering support","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:29Z","receivedAt":"2017-10-24T18:54:24Z","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.\n\nThis feature is only enabled when one of the \"--objects*\"\noptions are used.\n\nFurthermore, when the \"--filter-print-omitted\" option is\nused, the omitted objects are printed at the end. These\nare marked with a \"~\". This option can be combined with\n\"--quiet\" to get a list of just the omitted objects.\n\nSigned-off-by: Jeff Hostetler \n---\n Documentation/git-rev-list.txt | 5 ++-\n Documentation/rev-list-options.txt | 30 ++++++++++++++\n builtin/rev-list.c | 84 +++++++++++++++++++++++++++++++++++++-\n 3 files changed, 116 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\nindex ef22f17..6d2e60d 100644\n--- a/Documentation/git-rev-list.txt\n+++ b/Documentation/git-rev-list.txt\n@@ -47,7 +47,10 @@ 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= ] ]\n+\t [ --filter-print-missing ]\n+\t [ --filter-print-omitted ]\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 7d860bf..88f8878 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -706,6 +706,36 @@ 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-missing::\n+\tPrints a list of the missing objects for the requested traversal.\n+\tObject IDs are prefixed with a ``?'' character. The object type\n+\tis printed after the ID. This may be used with or without any of\n+\tthe above filtering options.\n+\n+--filter-print-omitted::\n+\tOnly useful with one of the above `--filter*`; prints a list\n+\tof the omitted objects. Object IDs are prefixed with a ``~''\n+\tcharacter.\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..7a0353f 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -12,6 +12,7 @@\n #include \"bisect.h\"\n #include \"progress.h\"\n #include \"reflog-walk.h\"\n+#include \"partial-clone-utils.h\"\n \n static const char rev_list_usage[] =\n \"git rev-list [OPTION] ... [ -- paths... ]\\n\"\n@@ -54,6 +55,11 @@ static const char rev_list_usage[] =\n \n static struct progress *progress;\n static unsigned progress_counter;\n+static struct list_objects_filter_options filter_options;\n+static struct oidmap missing_objects;\n+static int arg_print_missing;\n+static int arg_print_omitted;\n+#define DEFAULT_MAP_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@@ -181,8 +187,26 @@ static void finish_commit(struct commit *commit, void *data)\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+\tif (obj->type == OBJ_BLOB && !has_object_file(&obj->oid)) {\n+\t\tif (arg_print_missing) {\n+\t\t\tlist_objects_filter_map_insert(\n+\t\t\t\t&missing_objects, &obj->oid, name, obj->type);\n+\t\t\treturn;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Relax consistency checks when we expect missing\n+\t\t * objects because of partial-clone or a previous\n+\t\t * partial-fetch.\n+\t\t *\n+\t\t * Note that this is independent of any filtering that\n+\t\t * we are doing in this run.\n+\t\t */\n+\t\tif (is_partial_clone_registered())\n+\t\t\treturn;\n+\n \t\tdie(\"missing blob object '%s'\", oid_to_hex(&obj->oid));\n+\t}\n \tif (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n \t\tparse_object(&obj->oid);\n }\n@@ -202,6 +226,22 @@ static void show_edge(struct commit *commit)\n \tprintf(\"-%s\\n\", oid_to_hex(&commit->object.oid));\n }\n \n+static void print_omitted_object(int i, int i_limit, struct list_objects_filter_map_entry *e, void *cb_data)\n+{\n+\t/* struct rev_list_info *info = cb_data; */\n+\tconst char *tn = typename(e->type);\n+\n+\tprintf(\"~%s %s\\n\", oid_to_hex(&e->entry.oid), tn);\n+}\n+\n+static void print_missing_object(int i, int i_limit, struct list_objects_filter_map_entry *e, void *cb_data)\n+{\n+\t/* struct rev_list_info *info = cb_data; */\n+\tconst char *tn = typename(e->type);\n+\n+\tprintf(\"?%s %s\\n\", oid_to_hex(&e->entry.oid), tn);\n+}\n+\n static void print_var_str(const char *var, const char *val)\n {\n \tprintf(\"%s='%s'\\n\", var, val);\n@@ -335,6 +375,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-missing\")) {\n+\t\t\targ_print_missing = 1;\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+\t\t\n \t\tusage(rev_list_usage);\n \n \t}\n@@ -360,6 +420,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 +467,24 @@ 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_missing) {\n+\t\tmemset(&missing_objects, 0, sizeof(missing_objects));\n+\t\toidmap_init(&missing_objects, DEFAULT_MAP_SIZE);\n+\t}\n+\n+\tif (filter_options.choice)\n+\t\ttraverse_commit_list_filtered(&filter_options, &revs,\n+\t\t\tshow_commit, show_object,\n+\t\t\t(arg_print_omitted ? print_omitted_object : NULL),\n+\t\t\t&info);\n+\telse\n+\t\ttraverse_commit_list(&revs, show_commit, show_object, &info);\n+\n+\tif (arg_print_missing) {\n+\t\tlist_objects_filter_map_foreach(&missing_objects,\n+\t\t\t\t\t\tprint_missing_object, &info);\n+\t\toidmap_free(&missing_objects, 1);\n+\t}\n \n \tstop_progress(&progress);\n \n-- \n2.9.3\n\n"},{"id":"330943","messageId":"20171024185332.57261-5-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 04/13] list-objects-filter-blobs-none: add filter to omit all blobs","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:23Z","receivedAt":"2017-10-24T18:54:28Z","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-blobs-none.c | 83 ++++++++++++++++++++++++++++++++++++++++\n list-objects-filter-blobs-none.h | 18 +++++++++\n 3 files changed, 102 insertions(+)\n create mode 100644 list-objects-filter-blobs-none.c\n create mode 100644 list-objects-filter-blobs-none.h\n\ndiff --git a/Makefile b/Makefile\nindex e59f12d..7e9d1f4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -807,6 +807,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-blobs-none.o\n LIB_OBJS += list-objects-filter-map.o\n LIB_OBJS += ll-merge.o\n LIB_OBJS += lockfile.o\ndiff --git a/list-objects-filter-blobs-none.c b/list-objects-filter-blobs-none.c\nnew file mode 100644\nindex 0000000..1b548b9\n--- /dev/null\n+++ b/list-objects-filter-blobs-none.c\n@@ -0,0 +1,83 @@\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-blobs-none.h\"\n+\n+#define DEFAULT_MAP_SIZE (16*1024)\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 oidmap *omits;\n+};\n+\n+static list_objects_filter_result filter_blobs_none(\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_blobs_none_data *filter_data = filter_data_;\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\tif (filter_data->omits)\n+\t\t\tlist_objects_filter_map_insert(\n+\t\t\t\tfilter_data->omits, &obj->oid, pathname,\n+\t\t\t\tobj->type);\n+\n+\t\treturn LOFR_MARK_SEEN; /* but not LOFR_SHOW (hard omit) */\n+\t}\n+}\n+\n+void traverse_commit_list__blobs_none(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *ctx_data)\n+{\n+\tstruct filter_blobs_none_data d;\n+\n+\tmemset(&d, 0, sizeof(d));\n+\tif (print_omitted_object) {\n+\t\td.omits = xcalloc(1, sizeof(*d.omits));\n+\t\toidmap_init(d.omits, DEFAULT_MAP_SIZE);\n+\t}\n+\n+\ttraverse_commit_list_worker(revs, show_commit, show_object, ctx_data,\n+\t\t\t\t filter_blobs_none, &d);\n+\n+\tif (print_omitted_object) {\n+\t\tlist_objects_filter_map_foreach(d.omits,\n+\t\t\t\t\t\tprint_omitted_object,\n+\t\t\t\t\t\tctx_data);\n+\t\toidmap_free(d.omits, 1);\n+\t}\n+}\ndiff --git a/list-objects-filter-blobs-none.h b/list-objects-filter-blobs-none.h\nnew file mode 100644\nindex 0000000..363c9de\n--- /dev/null\n+++ b/list-objects-filter-blobs-none.h\n@@ -0,0 +1,18 @@\n+#ifndef LIST_OBJECTS_FILTER_BLOBS_NONE_H\n+#define LIST_OBJECTS_FILTER_BLOBS_NONE_H\n+\n+#include \"list-objects-filter-map.h\"\n+\n+/*\n+ * A filter for list-objects to omit ALL blobs\n+ * from the traversal.\n+ */\n+void traverse_commit_list__blobs_none(\n+\tstruct rev_info *revs,\n+\tshow_commit_fn show_commit,\n+\tshow_object_fn show_object,\n+\tlist_objects_filter_map_foreach_cb print_omitted_object,\n+\tvoid *ctx_data);\n+\n+#endif /* LIST_OBJECTS_FILTER_BLOBS_NONE_H */\n+\n-- \n2.9.3\n\n"},{"id":"330944","messageId":"20171024185332.57261-10-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 09/13] extension.partialclone: introduce partial clone extension","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:28Z","receivedAt":"2017-10-24T18:54:30Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nIntroduce the ability to have missing objects in a repo. This\nfunctionality is guarded by new repository extension options:\n `extensions.partialcloneremote` and\n `extensions.partialclonefilter`.\n\nSee the update to Documentation/technical/repository-version.txt\nin this patch for more information.\n\nThis patch is part of a patch originally authored by:\nJonathan Tan \n\nSigned-off-by: Jeff Hostetler \n---\n Documentation/technical/repository-version.txt | 22 ++++++\n Makefile | 1 +\n cache.h | 4 ++\n config.h | 3 +\n environment.c | 2 +\n partial-clone-utils.c | 99 ++++++++++++++++++++++++++\n partial-clone-utils.h | 34 +++++++++\n setup.c | 15 ++++\n 8 files changed, 180 insertions(+)\n create mode 100644 partial-clone-utils.c\n create mode 100644 partial-clone-utils.h\n\ndiff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt\nindex 00ad379..9d488db 100644\n--- a/Documentation/technical/repository-version.txt\n+++ b/Documentation/technical/repository-version.txt\n@@ -86,3 +86,25 @@ for testing format-1 compatibility.\n When the config key `extensions.preciousObjects` is set to `true`,\n objects in the repository MUST NOT be deleted (e.g., by `git-prune` or\n `git repack -d`).\n+\n+`partialcloneremote`\n+~~~~~~~~~~~~~~~~~~~~\n+\n+When the config key `extensions.partialcloneremote` is set, it indicates\n+that the repo was created with a partial clone (or later performed\n+a partial fetch) and that the remote may have omitted sending\n+certain unwanted objects. Such a remote is called a \"promisor remote\"\n+and it promises that all such omitted objects can be fetched from it\n+in the future.\n+\n+The value of this key is the name of the promisor remote.\n+\n+`partialclonefilter`\n+~~~~~~~~~~~~~~~~~~~~\n+\n+When the config key `extensions.partialclonefilter` is set, it gives\n+the initial filter expression used to create the partial clone.\n+This value becomed the default filter expression for subsequent\n+fetches (called \"partial fetches\") from the promisor remote. This\n+value may also be set by the first explicit partial fetch following a\n+normal clone.\ndiff --git a/Makefile b/Makefile\nindex b9ff0b4..38632fb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -841,6 +841,7 @@ LIB_OBJS += pack-write.o\n LIB_OBJS += pager.o\n LIB_OBJS += parse-options.o\n LIB_OBJS += parse-options-cb.o\n+LIB_OBJS += partial-clone-utils.o\n LIB_OBJS += patch-delta.o\n LIB_OBJS += patch-ids.o\n LIB_OBJS += path.o\ndiff --git a/cache.h b/cache.h\nindex 6440e2b..4b785c0 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -860,12 +860,16 @@ extern int grafts_replace_parents;\n #define GIT_REPO_VERSION 0\n #define GIT_REPO_VERSION_READ 1\n extern int repository_format_precious_objects;\n+extern char *repository_format_partial_clone_remote;\n+extern char *repository_format_partial_clone_filter;\n \n struct repository_format {\n \tint version;\n \tint precious_objects;\n \tint is_bare;\n \tchar *work_tree;\n+\tchar *partial_clone_remote; /* value of extensions.partialcloneremote */\n+\tchar *partial_clone_filter; /* value of extensions.partialclonefilter */\n \tstruct string_list unknown_extensions;\n };\n \ndiff --git a/config.h b/config.h\nindex a49d264..90544ef 100644\n--- a/config.h\n+++ b/config.h\n@@ -34,6 +34,9 @@ struct config_options {\n \tconst char *git_dir;\n };\n \n+#define KEY_PARTIALCLONEREMOTE \"partialcloneremote\"\n+#define KEY_PARTIALCLONEFILTER \"partialclonefilter\"\n+\n typedef int (*config_fn_t)(const char *, const char *, void *);\n extern int git_default_config(const char *, const char *, void *);\n extern int git_config_from_file(config_fn_t fn, const char *, void *);\ndiff --git a/environment.c b/environment.c\nindex 8289c25..2fcf9bb 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -27,6 +27,8 @@ int warn_ambiguous_refs = 1;\n int warn_on_object_refname_ambiguity = 1;\n int ref_paranoia = -1;\n int repository_format_precious_objects;\n+char *repository_format_partial_clone_remote;\n+char *repository_format_partial_clone_filter;\n const char *git_commit_encoding;\n const char *git_log_output_encoding;\n const char *apply_default_whitespace;\ndiff --git a/partial-clone-utils.c b/partial-clone-utils.c\nnew file mode 100644\nindex 0000000..8c925ae\n--- /dev/null\n+++ b/partial-clone-utils.c\n@@ -0,0 +1,99 @@\n+#include \"cache.h\"\n+#include \"config.h\"\n+#include \"partial-clone-utils.h\"\n+\n+int is_partial_clone_registered(void)\n+{\n+\tif (repository_format_partial_clone_remote ||\n+\t repository_format_partial_clone_filter)\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+void partial_clone_utils_register(\n+\tconst struct list_objects_filter_options *filter_options,\n+\tconst char *remote,\n+\tconst char *cmd_name)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tif (is_partial_clone_registered()) {\n+\t\t/*\n+\t\t * The original partial-clone or a previous partial-fetch\n+\t\t * already registered the partial-clone settings.\n+\t\t * If we get here, we are in a subsequent partial-* command\n+\t\t * (with explicit filter args on the command line).\n+\t\t *\n+\t\t * For now, we restrict subsequent commands to one\n+\t\t * consistent with the original request. We may relax\n+\t\t * this later after we get more experience with the\n+\t\t * partial-clone feature.\n+\t\t *\n+\t\t * [] Restrict to same remote because our dynamic\n+\t\t * object loading only knows how to fetch objects\n+\t\t * from 1 remote.\n+\t\t */\n+\t\tassert(filter_options && filter_options->choice);\n+\t\tassert(remote && *remote);\n+\n+\t\tif (strcmp(remote, repository_format_partial_clone_remote))\n+\t\t\tdie(\"%s --%s currently limited to remote '%s'\",\n+\t\t\t cmd_name, CL_ARG__FILTER,\n+\t\t\t repository_format_partial_clone_remote);\n+\n+\t\t/*\n+\t\t * Treat the (possibly new) filter-spec as transient;\n+\t\t * use it for the current command, but do not overwrite\n+\t\t * the default.\n+\t\t */\n+\t\treturn;\n+\t}\n+\n+\trepository_format_partial_clone_remote = xstrdup(remote);\n+\trepository_format_partial_clone_filter = xstrdup(filter_options->raw_value);\n+\n+\t/*\n+\t * Force repo version > 0 to enable extensions namespace.\n+\t */\n+\tgit_config_set(\"core.repositoryformatversion\", \"1\");\n+\n+\t/*\n+\t * Use the \"extensions\" namespace in the config to record\n+\t * the name of the remote used in the partial clone.\n+\t * This will help us return to that server when we need\n+\t * to backfill missing objects.\n+\t *\n+\t * It is also used to indicate that there *MAY* be\n+\t * missing objects so that subsequent commands don't\n+\t * immediately die if they hit one.\n+\t *\n+\t * Also remember the initial filter settings used by\n+\t * clone as a default for future fetches.\n+\t */\n+\tgit_config_set(\"extensions.\" KEY_PARTIALCLONEREMOTE,\n+\t\t repository_format_partial_clone_remote);\n+\tgit_config_set(\"extensions.\" KEY_PARTIALCLONEFILTER,\n+\t\t repository_format_partial_clone_filter);\n+\n+\t/*\n+\t * TODO Do we need to record both partial-clone\n+\t * parameters in the extensions namespace and in the\n+\t * section for the remote?\n+\t *\n+\t * Or should we just remember 1 in each, as in:\n+\t * \"extension.partialcloneremote=\"\n+\t * \"remote..filter=\"\n+\t * The issue is when can we set both of the\n+\t * repository_format_partial_clone_* globals\n+\t * durint subsequent startups.\n+\t * See setup.c:check_repo_format().\n+\t */\n+\tstrbuf_addf(&buf, \"remote.%s.%s\", remote, KEY_PARTIALCLONEREMOTE);\n+\tgit_config_set(buf.buf, repository_format_partial_clone_remote);\n+\n+\tstrbuf_addf(&buf, \"remote.%s.%s\", remote, KEY_PARTIALCLONEFILTER);\n+\tgit_config_set(buf.buf, repository_format_partial_clone_filter);\n+\n+\tstrbuf_release(&buf);\n+}\ndiff --git a/partial-clone-utils.h b/partial-clone-utils.h\nnew file mode 100644\nindex 0000000..b527570\n--- /dev/null\n+++ b/partial-clone-utils.h\n@@ -0,0 +1,34 @@\n+#ifndef PARTIAL_CLONE_UTILS_H\n+#define PARTIAL_CLONE_UTILS_H\n+\n+#include \"list-objects-filter-options.h\"\n+\n+/*\n+ * Register that partial-clone was used to create the repo and\n+ * update the config on disk.\n+ *\n+ * If nothing else, this indicates that the ODB may have missing\n+ * objects and that various commands should handle that gracefully.\n+ *\n+ * Record the remote used for the clone so that we know where\n+ * to get missing objects in the future.\n+ *\n+ * Also record the filter expression so that we know something\n+ * about the missing objects (e.g., size-limit vs sparse).\n+ *\n+ * May also be used by a partial-fetch following a normal clone\n+ * to turn on the above tracking.\n+ */ \n+extern void partial_clone_utils_register(\n+\tconst struct list_objects_filter_options *filter_options,\n+\tconst char *remote,\n+\tconst char *cmd_name);\n+\n+/*\n+ * Return 1 if partial-clone was used to create the repo\n+ * or a subsequent partial-fetch was used. This is an\n+ * indicator that there may be missing objects.\n+ */\n+extern int is_partial_clone_registered(void);\n+\n+#endif /* PARTIAL_CLONE_UTILS_H */\ndiff --git a/setup.c b/setup.c\nindex 03f51e0..bc4133d 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -420,6 +420,19 @@ static int check_repo_format(const char *var, const char *value, void *vdata)\n \t\t\t;\n \t\telse if (!strcmp(ext, \"preciousobjects\"))\n \t\t\tdata->precious_objects = git_config_bool(var, value);\n+\n+\t\telse if (!strcmp(ext, KEY_PARTIALCLONEREMOTE))\n+\t\t\tif (!value)\n+\t\t\t\treturn config_error_nonbool(var);\n+\t\t\telse\n+\t\t\t\tdata->partial_clone_remote = xstrdup(value);\n+\n+\t\telse if (!strcmp(ext, KEY_PARTIALCLONEFILTER))\n+\t\t\tif (!value)\n+\t\t\t\treturn config_error_nonbool(var);\n+\t\t\telse\n+\t\t\t\tdata->partial_clone_filter = xstrdup(value);\n+\n \t\telse\n \t\t\tstring_list_append(&data->unknown_extensions, ext);\n \t} else if (strcmp(var, \"core.bare\") == 0) {\n@@ -463,6 +476,8 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)\n \t}\n \n \trepository_format_precious_objects = candidate.precious_objects;\n+\trepository_format_partial_clone_remote = candidate.partial_clone_remote;\n+\trepository_format_partial_clone_filter = candidate.partial_clone_filter;\n \tstring_list_clear(&candidate.unknown_extensions, 0);\n \tif (!has_common) {\n \t\tif (candidate.is_bare != -1) {\n-- \n2.9.3\n\n"},{"id":"330945","messageId":"20171024185332.57261-2-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 01/13] dir: allow exclusions from blob in addition to file","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:20Z","receivedAt":"2017-10-24T18:54:33Z","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 | 51 +++++++++++++++++++++++++++++++++++++++++++++++++--\n dir.h | 3 +++\n 2 files changed, 52 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 1d17b80..d848f2b 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -739,6 +739,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 +758,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 +817,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 +856,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..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":"330946","messageId":"20171024185332.57261-8-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"[PATCH 07/13] list-objects-filter-options: common argument parsing","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-24T18:53:26Z","receivedAt":"2017-10-24T18:54:36Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nCreate common routines and defines for parsing\nlist-objects-filter-related command line arguments and\npack-protocol fields.\n\nSigned-off-by: Jeff Hostetler \n---\n Makefile | 1 +\n list-objects-filter-options.c | 101 ++++++++++++++++++++++++++++++++++++++++++\n list-objects-filter-options.h | 50 +++++++++++++++++++++\n 3 files changed, 152 insertions(+)\n create mode 100644 list-objects-filter-options.c\n create mode 100644 list-objects-filter-options.h\n\ndiff --git a/Makefile b/Makefile\nindex fc82664..b9ff0b4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -810,6 +810,7 @@ LIB_OBJS += list-objects.o\n LIB_OBJS += list-objects-filter-blobs-limit.o\n LIB_OBJS += list-objects-filter-blobs-none.o\n LIB_OBJS += list-objects-filter-map.o\n+LIB_OBJS += list-objects-filter-options.o\n LIB_OBJS += list-objects-filter-sparse.o\n LIB_OBJS += ll-merge.o\n LIB_OBJS += lockfile.o\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nnew file mode 100644\nindex 0000000..40f48ac\n--- /dev/null\n+++ b/list-objects-filter-options.c\n@@ -0,0 +1,101 @@\n+#include \"cache.h\"\n+#include \"commit.h\"\n+#include \"config.h\"\n+#include \"revision.h\"\n+#include \"list-objects.h\"\n+#include \"list-objects-filter-options.h\"\n+\n+/*\n+ * Parse value of the argument to the \"filter\" keword.\n+ * On the command line this looks like: --filter=\n+ * and in the pack protocol as: filter \n+ *\n+ * ::= blob:none\n+ * blob:limit:[kmg]\n+ * sparse:oid:\n+ * sparse:path:\n+ */\n+int parse_list_objects_filter(struct list_objects_filter_options *filter_options,\n+\t\t\t const char *arg)\n+{\n+\tstruct object_context oc;\n+\tstruct object_id sparse_oid;\n+\tconst char *v0;\n+\tconst char *v1;\n+\n+\tif (filter_options->choice)\n+\t\tdie(_(\"multiple object filter types cannot be combined\"));\n+\n+\t/*\n+\t * TODO consider rejecting 'arg' if it contains any\n+\t * TODO injection characters (since we might send this\n+\t * TODO to a sub-command or to the server and we don't\n+\t * TODO want to deal with legacy quoting/escaping for\n+\t * TODO a new feature).\n+\t */\n+\n+\tfilter_options->raw_value = strdup(arg);\n+\n+\tif (skip_prefix(arg, \"blob:\", &v0) || skip_prefix(arg, \"blobs:\", &v0)) {\n+\t\tif (!strcmp(v0, \"none\")) {\n+\t\t\tfilter_options->choice = LOFC_BLOB_NONE;\n+\t\t\treturn 0;\n+\t\t}\n+\n+\t\tif (skip_prefix(v0, \"limit=\", &v1) &&\n+\t\t git_parse_ulong(v1, &filter_options->blob_limit_value)) {\n+\t\t\tfilter_options->choice = LOFC_BLOB_LIMIT;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\telse if (skip_prefix(arg, \"sparse:\", &v0)) {\n+\t\tif (skip_prefix(v0, \"oid=\", &v1)) {\n+\t\t\tfilter_options->choice = LOFC_SPARSE_OID;\n+\t\t\tif (!get_oid_with_context(v1, GET_OID_BLOB,\n+\t\t\t\t\t\t &sparse_oid, &oc)) {\n+\t\t\t\t/*\n+\t\t\t\t * We successfully converted the \n+\t\t\t\t * into an actual OID. Rewrite the raw_value\n+\t\t\t\t * in canonoical form with just the OID.\n+\t\t\t\t * (If we send this request to the server, we\n+\t\t\t\t * want an absolute expression rather than a\n+\t\t\t\t * local-ref-relative expression.)\n+\t\t\t\t */\n+\t\t\t\tfree((char *)filter_options->raw_value);\n+\t\t\t\tfilter_options->raw_value =\n+\t\t\t\t\txstrfmt(\"sparse:oid=%s\",\n+\t\t\t\t\t\toid_to_hex(&sparse_oid));\n+\t\t\t\tfilter_options->sparse_oid_value =\n+\t\t\t\t\toiddup(&sparse_oid);\n+\t\t\t} else {\n+\t\t\t\t/*\n+\t\t\t\t * We could not turn the into an\n+\t\t\t\t * OID. Leave the raw_value as is in case\n+\t\t\t\t * the server can parse it. (It may refer to\n+\t\t\t\t * a branch, commit, or blob we don't have.)\n+\t\t\t\t */\n+\t\t\t}\n+\t\t\treturn 0;\n+\t\t}\n+\n+\t\tif (skip_prefix(v0, \"path=\", &v1)) {\n+\t\t\tfilter_options->choice = LOFC_SPARSE_PATH;\n+\t\t\tfilter_options->sparse_path_value = strdup(v1);\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n+\tdie(_(\"invalid filter 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..23bd68e\n--- /dev/null\n+++ b/list-objects-filter-options.h\n@@ -0,0 +1,50 @@\n+#ifndef LIST_OBJECTS_FILTER_OPTIONS_H\n+#define LIST_OBJECTS_FILTER_OPTIONS_H\n+\n+#include \"parse-options.h\"\n+\n+/*\n+ * Common declarations and utilities for filtering objects (such as omitting\n+ * large blobs) in list_objects:traverse_commit_list() and git-rev-list.\n+ */\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+};\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 * 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(struct list_objects_filter_options *filter_options,\n+\t\t\t const 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 */\n-- \n2.9.3\n\n"},{"id":"330947","messageId":"20171024185332.57261-4-git@jeffhostetler.com","threadId":"47024","inReplyTo":"20171024185332.57261-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-10-24T18:53:22Z","receivedAt":"2017-10-24T18:54:40Z","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 | 32 +++++++++++++++++++++++++++-\n 2 files changed, 81 insertions(+), 17 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..43a06fb 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-#endif\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 /* LIST_OBJECTS_H */\n-- \n2.9.3\n\n"},{"id":"330982","messageId":"CAGf8dg+_AewifMR8wnrQdJKXK0GuwdhMb8QAMrGoVCJzhysiRw@mail.gmail.com","threadId":"47024","inReplyTo":"20171024185332.57261-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-10-25T04:05:17Z","receivedAt":"2017-10-25T04:05:24Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote:\n\n> +enum list_objects_filter_result {\n> + LOFR_ZERO = 0,\n> + LOFR_MARK_SEEN = 1<<0,\n\nProbably worth documenting, something like /* Mark this object so that\nit is skipped for the rest of the traversal. */\n\n> + LOFR_SHOW = 1<<1,\n\nAnd something like /* Invoke show_object_fn on this object. This\nobject may be revisited unless LOFR_MARK_SEEN is also set. */\n\n> +};\n> +\n> +/* See object.h and revision.h */\n> +#define FILTER_REVISIT (1<<25)\n\nI think this should be declared closer to its use - in the sparse\nfilter code or in the file that uses it. Wherever it is, also update\nthe chart in object.h to indicate that we're using this 25th bit.\n\n> +\n> +enum list_objects_filter_type {\n> + LOFT_BEGIN_TREE,\n> + LOFT_END_TREE,\n> + LOFT_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\nI don't think we typedef enums in Git code.\n\n> +\n> +typedef list_objects_filter_result (*filter_object_fn)(\n> + list_objects_filter_type filter_type,\n> + struct object *obj,\n> + const char *pathname,\n> + const char *filename,\n> + void *filter_data);\n> +\n> +void traverse_commit_list_worker(\n> + struct rev_info *,\n> + show_commit_fn, show_object_fn, void *show_data,\n> + filter_object_fn filter, void *filter_data);\n\nI think things would be much clearer if a default filter was declared\n(matching the behavior as of this patch when filter == NULL), say:\nstatic inline default_filter(args) { switch(filter_type) { case\nLOFT_BEGIN_TREE: return LOFR_MARK_SEEN | LOFR_SHOW; case\nLOFT_END_TREE: return LOFT_ZERO; ...\n\nAnd inline traverse_commit_list() instead of putting it in the .c file.\n\nThis would reduce or eliminate the need to document\ntraverse_commit_list_worker, including what happens if filter is NULL,\nand explain how a user would make their own filter_object_fn.\n\n> +\n> +#endif /* LIST_OBJECTS_H */\n> --\n> 2.9.3\n>\n"},{"id":"330983","messageId":"CAPig+cSPwSq1s5EhnQA+K-zQJ_+LrTLJFA9n8fj+W9jtdpA+PQ@mail.gmail.com","threadId":"47024","inReplyTo":"20171024185332.57261-2-git@jeffhostetler.com","subject":"Re: [PATCH 01/13] dir: allow exclusions from blob in addition to file","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-10-25T04:05:20Z","receivedAt":"2017-10-25T04:05:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Oct 24, 2017 at 2:53 PM, Jeff Hostetler wrote:\n> Refactor add_excludes() to separate the reading of the\n> exclude file into a buffer and the parsing of the buffer\n> into exclude_list items.\n>\n> Add add_excludes_from_blob_to_list() to allow an exclude\n> file be specified with an OID.\n>\n> Signed-off-by: Jeff Hostetler \n> ---\n> diff --git a/dir.c b/dir.c\n> @@ -841,6 +856,38 @@ int add_excludes_from_file_to_list(const char *fname, const char *base,\n> +int add_excludes_from_blob_to_list(\n> + struct object_id *oid,\n> + const char *base, int baselen,\n> + struct exclude_list *el)\n> +{\n> + char *buf;\n> + unsigned long size;\n> + enum object_type type;\n> +\n> + buf = read_sha1_file(oid->hash, &type, &size);\n> + if (!buf)\n> + return -1;\n> +\n> + if (type != OBJ_BLOB) {\n> + free(buf);\n> + return -1;\n> + }\n> +\n> + if (size == 0) {\n> + free(buf);\n> + return 0;\n> + }\n> +\n> + if (buf[size - 1] != '\\n') {\n> + buf = xrealloc(buf, st_add(size, 1));\n> + buf[size++] = '\\n';\n> + }\n> +\n> + add_excludes_from_buffer(buf, size, base, baselen, el);\n\nSeeing all the free()'s above, at first glance, one wonders why 'buf'\nisn't freed here after add_excludes_from_buffer(), however an\nexamination of that function shows that 'buf' is assigned to\nel->filebuf and later freed by clear_exclude_list(). Okay.\n\n> + return 0;\n\nShould this be returning the result of add_excludes_from_buffer()\nrather than unconditionally returning 0?\n\n> +}\n"},{"id":"330985","messageId":"CAGf8dgJmwtN-v9SQfjUJOBreGQ==iM1G-oGBh3oYH_DE4NYyLA@mail.gmail.com","threadId":"47024","inReplyTo":"20171024185332.57261-8-git@jeffhostetler.com","subject":"Re: [PATCH 07/13] list-objects-filter-options: common argument parsing","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-10-25T04:14:27Z","receivedAt":"2017-10-25T04:14:34Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote:\n> + * ::= blob:none\n> + * blob:limit:[kmg]\n> + * sparse:oid:\n> + * sparse:path:\n\nI notice in the code below that there are some usages of \"=\" instead\nof \":\" - could you clarify which one it is? (Ideally this would point\nto one point of documentation which serves as both user and technical\ndocumentation.)\n\n> + */\n> +int parse_list_objects_filter(struct list_objects_filter_options *filter_options,\n> + const char *arg)\n> +{\n> + struct object_context oc;\n> + struct object_id sparse_oid;\n> + const char *v0;\n> + const char *v1;\n> +\n> + if (filter_options->choice)\n> + die(_(\"multiple object filter types cannot be combined\"));\n> +\n> + /*\n> + * TODO consider rejecting 'arg' if it contains any\n> + * TODO injection characters (since we might send this\n> + * TODO to a sub-command or to the server and we don't\n> + * TODO want to deal with legacy quoting/escaping for\n> + * TODO a new feature).\n> + */\n> +\n> + filter_options->raw_value = strdup(arg);\n> +\n> + if (skip_prefix(arg, \"blob:\", &v0) || skip_prefix(arg, \"blobs:\", &v0)) {\n\nI know that some people prefer leniency, but I think it's better to\nstandardize on one form (\"blob\" instead of both \"blob\" and \"blobs\").\n\n> + if (!strcmp(v0, \"none\")) {\n> + filter_options->choice = LOFC_BLOB_NONE;\n> + return 0;\n> + }\n> +\n> + if (skip_prefix(v0, \"limit=\", &v1) &&\n> + git_parse_ulong(v1, &filter_options->blob_limit_value)) {\n> + filter_options->choice = LOFC_BLOB_LIMIT;\n> + return 0;\n> + }\n> + }\n> + else if (skip_prefix(arg, \"sparse:\", &v0)) {\n> + if (skip_prefix(v0, \"oid=\", &v1)) {\n> + filter_options->choice = LOFC_SPARSE_OID;\n> + if (!get_oid_with_context(v1, GET_OID_BLOB,\n> + &sparse_oid, &oc)) {\n> + /*\n> + * We successfully converted the \n> + * into an actual OID. Rewrite the raw_value\n> + * in canonoical form with just the OID.\n> + * (If we send this request to the server, we\n> + * want an absolute expression rather than a\n> + * local-ref-relative expression.)\n> + */\n> + free((char *)filter_options->raw_value);\n> + filter_options->raw_value =\n> + xstrfmt(\"sparse:oid=%s\",\n> + oid_to_hex(&sparse_oid));\n> + filter_options->sparse_oid_value =\n> + oiddup(&sparse_oid);\n> + } else {\n> + /*\n> + * We could not turn the into an\n> + * OID. Leave the raw_value as is in case\n> + * the server can parse it. (It may refer to\n> + * a branch, commit, or blob we don't have.)\n> + */\n> + }\n> + return 0;\n> + }\n> +\n> + if (skip_prefix(v0, \"path=\", &v1)) {\n> + filter_options->choice = LOFC_SPARSE_PATH;\n> + filter_options->sparse_path_value = strdup(v1);\n> + return 0;\n> + }\n> + }\n> +\n> + die(_(\"invalid filter expression '%s'\"), arg);\n> + return 0;\n> +}\n> +\n> +int opt_parse_list_objects_filter(const struct option *opt,\n> + const char *arg, int unset)\n> +{\n> + struct list_objects_filter_options *filter_options = opt->value;\n> +\n> + assert(arg);\n> + assert(!unset);\n> +\n> + return parse_list_objects_filter(filter_options, arg);\n> +}\n> diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h\n> new file mode 100644\n> index 0000000..23bd68e\n> --- /dev/null\n> +++ b/list-objects-filter-options.h\n> @@ -0,0 +1,50 @@\n> +#ifndef LIST_OBJECTS_FILTER_OPTIONS_H\n> +#define LIST_OBJECTS_FILTER_OPTIONS_H\n> +\n> +#include \"parse-options.h\"\n> +\n> +/*\n> + * Common declarations and utilities for filtering objects (such as omitting\n> + * large blobs) in list_objects:traverse_commit_list() and git-rev-list.\n> + */\n> +\n> +enum list_objects_filter_choice {\n> + LOFC_DISABLED = 0,\n> + LOFC_BLOB_NONE,\n> + LOFC_BLOB_LIMIT,\n> + LOFC_SPARSE_OID,\n> + LOFC_SPARSE_PATH,\n> +};\n> +\n> +struct list_objects_filter_options {\n> + /*\n> + * The raw argument value given on the command line or\n> + * protocol request. (The part after the \"--keyword=\".)\n> + */\n> + char *raw_value;\n> +\n> + /*\n> + * Parsed values. Only 1 will be set depending on the flags below.\n> + */\n> + struct object_id *sparse_oid_value;\n> + char *sparse_path_value;\n> + unsigned long blob_limit_value;\n> +\n> + enum 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(struct list_objects_filter_options *filter_options,\n> + const char *arg);\n> +\n> +int opt_parse_list_objects_filter(const struct option *opt,\n> + const char *arg, int unset);\n> +\n> +#define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \\\n> + { OPTION_CALLBACK, 0, CL_ARG__FILTER, fo, N_(\"args\"), \\\n> + N_(\"object filtering\"), PARSE_OPT_NONEG, \\\n> + opt_parse_list_objects_filter }\n\nThanks - this does make it easier to have a standard argument name and\ndescription everywhere.\n\n> +\n> +#endif /* LIST_OBJECTS_FILTER_OPTIONS_H */\n> --\n> 2.9.3\n>\n"},{"id":"330986","messageId":"CAGf8dgLU_70vq8JNpr-cihgk9SbC_7GYcaEF6c9NgmJRXPAcmw@mail.gmail.com","threadId":"47024","inReplyTo":"20171024185332.57261-9-git@jeffhostetler.com","subject":"Re: [PATCH 08/13] list-objects: add traverse_commit_list_filtered method","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-10-25T04:24:55Z","receivedAt":"2017-10-25T04:25:01Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote:\n> +void traverse_commit_list_filtered(\n> + struct list_objects_filter_options *filter_options,\n> + struct rev_info *revs,\n> + show_commit_fn show_commit,\n> + show_object_fn show_object,\n> + list_objects_filter_map_foreach_cb print_omitted_object,\n> + void *show_data);\n\nSo the function call chain, if we wanted a filtered traversal, is:\ntraverse_commit_list_filtered -> traverse_commit_list__sparse_path\n(and friends, and each algorithm is in its own file) ->\ntraverse_commit_list_worker\n\nThis makes the implementation of each algorithm more easily understood\n(since they are all in their own files), but also increases the number\nof global functions and code files. I personally would combine the\ntraverse_commit_list__* functions into one file\n(list-objects-filtered.c), make them static, and also put\ntraverse_commit_list_filtered in there, but I understand that other\npeople in the Git project may differ on this.\n"},{"id":"330988","messageId":"CAGf8dgK3pW8T4Tst6LZuJxLZTDjk7Ak5iwK0WYBSLrWR1S8YBg@mail.gmail.com","threadId":"47024","inReplyTo":"20171024185332.57261-11-git@jeffhostetler.com","subject":"Re: [PATCH 10/13] rev-list: add list-objects filtering support","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-10-25T04:41:25Z","receivedAt":"2017-10-25T04:41:34Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote:\n> static void finish_object(struct object *obj, const char *name, void *cb_data)\n> {\n> struct rev_list_info *info = cb_data;\n> - if (obj->type == OBJ_BLOB && !has_object_file(&obj->oid))\n> + if (obj->type == OBJ_BLOB && !has_object_file(&obj->oid)) {\n> + if (arg_print_missing) {\n> + list_objects_filter_map_insert(\n> + &missing_objects, &obj->oid, name, obj->type);\n> + return;\n> + }\n> +\n> + /*\n> + * Relax consistency checks when we expect missing\n> + * objects because of partial-clone or a previous\n> + * partial-fetch.\n> + *\n> + * Note that this is independent of any filtering that\n> + * we are doing in this run.\n> + */\n> + if (is_partial_clone_registered())\n> + return;\n> +\n> die(\"missing blob object '%s'\", oid_to_hex(&obj->oid));\n\nI'm fine with arg_print_missing suppressing lazy fetching (when I\nrebase my patches on this, I'll have to ensure that fetch_if_missing\nis set to 0 if arg_print_missing is true), but I think that the\nbehavior when arg_print_missing is false should be the opposite - we\nshould let has_object_file() perform the lazy fetching, and die if it\nreturns false (that is, if the fetching failed).\n\n> + }\n> if (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n> parse_object(&obj->oid);\n> }\n"},{"id":"330990","messageId":"CAGf8dg+8AR=XfSV92ODAtKTNjBnD1+oVZp9rs4Y4Otz_eZyTfg@mail.gmail.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"Re: [PATCH 00/13] WIP Partial clone part 1: object filtering","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-10-25T04:57:47Z","receivedAt":"2017-10-25T04:57:53Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote:\n> From: Jeff Hostetler \n>\n> I've been working with Jonathan Tan to combine our partial clone\n> proposals. This patch series represents a first step in that effort\n> and introduces an object filtering mechanism to select unwanted\n> objects.\n>\n> [1] traverse_commit_list and list-objects is extended to allow\n> various filters.\n> [2] rev-list is extended to expose filtering. This allows testing\n> of the filtering options. And can be used later to predict\n> missing objects before commands like checkout or merge.\n> [3] pack-objects is extended to use filtering parameters and build\n> packfiles that omit unwanted objects.\n>\n> This patch series lays the ground work for subsequent parts which\n> will extend clone, fetch, fetch-pack, upload-pack, fsck, and etc.\n\nThanks - I've taken a look at all of them except for the partialclone\nextension one, which I've only glanced over briefly. Apart from some\nstyle issues (indentation and typedef-ing of enums) I think that they\ngenerally look all right.\n\nOne possible issue with using a formatted filter string (e.g.\nblob:none) directly passed to the server as opposed to actual\nclient-interpreted flags (e.g. --blob-byte-limit=0) is that a user may\nbe confused if the version of Git they're using supports fancier\nfilters, which will work if they're running rev-list locally, but not\nwhen they're fetching from a not-so-fancy Git server. Maybe that is\nfine, though - something we've discussed internally is an ability to\noffload some calculations (e.g. git log -S) to Git servers, and if we\nend up doing something similar to that, users will need to deal with\nthis problem anyway.\n\nThe reason why I only glanced over the partialclone patch is because I\nthink that that change needs more discussion than the rest, and it\nwould be good to get the others in first. I know that you included the\npartialclone patch because it is needed by the rev-list one, but as I\ncommented in the rev-list one, I think that it does not need to be\naware of partial clones, at least for now. The overall ideas in the\npartialclone patch seem good, though - in particular, one conceptual\ndifference from my patch [1] is that the filter setting is a property\nof the repository as opposed to the remote, which does seem like an\nimprovement, because it makes it easier to make and explain e.g. a\n\"git log --use-repo-filter -S\" command that uses the preconfigured\nconfig.\n\n[1] https://public-inbox.org/git/407a298b52a9e0a2ee4135fe844e35b9a14c0f7b.1506714999.git.jonathantanmy@google.com/\n"},{"id":"330991","messageId":"xmqqmv4fizyn.fsf@gitster.mtv.corp.google.com","threadId":"47024","inReplyTo":"20171024185332.57261-1-git@jeffhostetler.com","subject":"Re: [PATCH 00/13] WIP Partial clone part 1: object filtering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-25T05:00:32Z","receivedAt":"2017-10-25T05:00:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> From: Jeff Hostetler \n>\n> I've been working with Jonathan Tan to combine our partial clone\n> proposals. This patch series represents a first step in that effort\n> and introduces an object filtering mechanism to select unwanted\n> objects.\n>\n> [1] traverse_commit_list and list-objects is extended to allow\n> various filters.\n> [2] rev-list is extended to expose filtering. This allows testing\n> of the filtering options. And can be used later to predict\n> missing objects before commands like checkout or merge.\n> [3] pack-objects is extended to use filtering parameters and build\n> packfiles that omit unwanted objects.\n>\n> This patch series lays the ground work for subsequent parts which\n> will extend clone, fetch, fetch-pack, upload-pack, fsck, and etc.\n\nOK, thanks for working well together. So does this (1) build on\nJonathan's fsck-squelching series, or (2) ignores that and builds\nfiltering first, potentially leaving the codebase to a broken state\nwhere it can create fsck-unclean repository until Jonathan's series\nis rebased on top of this, or (3) something else? [*1*]\n\nI also saw a patch marked as \"this is from Jonathan's earlier work\",\ntaking the authorship (which to me implies that the changes were\nextensive enough), so I am a bit at loss envisioning how this piece\nfits in the bigger picture together with the other piece.\n\n\n[Footnote]\n\n*1* Not having the answer to this question does bother me, but it is\n perfectly fine if the answer is (2), especially while the series\n is in a WIP state.\n\n"},{"id":"330995","messageId":"xmqq60b3iv79.fsf@gitster.mtv.corp.google.com","threadId":"47024","inReplyTo":"20171024185332.57261-2-git@jeffhostetler.com","subject":"Re: [PATCH 01/13] dir: allow exclusions from blob in addition to file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-25T06:43:22Z","receivedAt":"2017-10-25T06:43:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\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 +758,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\nThe post-context of this hunk is quite interesting in that there is\na call to read_skip_worktree_file_from_index(); which essentially \npretends as if we read from the filesystem but in fact it grabs the\nblob object name registered in the index and reads from it.\n\nThe reason why it is interesting is because this patch adds yet\nnother \"let's instead read from a blob object\" function and there is\nno sign to make the existing one take advantage of the new function\nseen in this patch.\n"},{"id":"330996","messageId":"CAGf8dg+cK3WpEqosgkbdcrDzrMXJxVYHiBZda6UM7k8Ggq=eBw@mail.gmail.com","threadId":"47024","inReplyTo":"xmqqmv4fizyn.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 00/13] WIP Partial clone part 1: object filtering","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-10-25T06:46:18Z","receivedAt":"2017-10-25T06:46:25Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, Oct 24, 2017 at 10:00 PM, Junio C Hamano wrote:\n> OK, thanks for working well together. So does this (1) build on\n> Jonathan's fsck-squelching series, or (2) ignores that and builds\n> filtering first, potentially leaving the codebase to a broken state\n> where it can create fsck-unclean repository until Jonathan's series\n> is rebased on top of this, or (3) something else? [*1*]\n\nExcluding the partialclone patch (patch 9), I think that the answer is\n(2), but I don't think that it leaves the codebase in a broken state.\nIn particular, none of the code modifies the repo, so it can't create\na fsck-unclean one.\n\nMaybe one could say that this leaves the codebase with features that\nno one will ever use in the absence of partial clone, but I don't\nthink that's true - rev-list with blob-size/sparse-specification\nfilter seems independently useful, at least (for example, when\ndesiring to operate on a repo in a sparse way without going through a\nworkdir), and if we're planning to allow listing of objects, we\nprobably should allow packing as well (especially since this doesn't\nadd much code).\n\nThe above is relevant only if we can exclude the partialclone patch,\nbut I think that we can and we should, as I wrote in my reply to Jeff\nHostetler [1].\n\nAs for how this patch set (excluding the partialclone patch) interacts\nwith my fsck series, they are relatively independent, as far as I can\ntell. I'll rebase my fsck, gc, and lazy object fetch patches (but not\nthe fetch and clone parts, which we plan to instead adapt from Jeff\nHostetler's patches, as far as I know) on top of these and resend\nthose out once discussion on this has settled.\n\n[1] https://public-inbox.org/git/CAGf8dg+8AR=XfSV92ODAtKTNjBnD1+oVZp9rs4Y4Otz_eZyTfg@mail.gmail.com/\n\n> I also saw a patch marked as \"this is from Jonathan's earlier work\",\n> taking the authorship (which to me implies that the changes were\n> extensive enough), so I am a bit at loss envisioning how this piece\n> fits in the bigger picture together with the other piece.\n\nThe patch you mentioned is the partialclone patch, which I think can\nbe considered separately from the rest (as I said above).\n"},{"id":"330997","messageId":"xmqqwp3jhfct.fsf@gitster.mtv.corp.google.com","threadId":"47024","inReplyTo":"20171024185332.57261-3-git@jeffhostetler.com","subject":"Re: [PATCH 02/13] list-objects-filter-map: extend oidmap to collect omitted objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-25T07:10:58Z","receivedAt":"2017-10-25T07:11:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> From: Jeff Hostetler \n>\n> Create helper class to extend oidmap to collect a list of\n> omitted or missing objects during traversal.\n\nThe reason why oidmap itself cannot be used is because the code\nwants to record not just the object name but something else about\nthe object. And attributes that the code may care about we can see\nin this patch are the object type and the path it found. \n\nIs the plan to extend this set of attributes over time as different\n\"omitter\"s are added? Why was \"path\" chosen as a member of the\ninitial set and how it will be useful (also, what path would we\nrecord for tags and commits)?\n\nThese \"future plans\" needs revealed upfront, instead of (or in\naddition to) \"will be used in a later commit\". As it is hard to\njudge if \"filter map\" is an appropriate name for this thing without\nknowing _how_ it is envisioned to be used. \"filter map\" sounds more\nlike a map function that is consulted when we decide if we want to\ndrop the object, but from the looks of the code, it is used more to\nrecord what was done to these objects.\n\nIs it really a \"map\" (i.e. whose primary focus is to find out what\nan object name is \"mapped to\" when we get an object name---e.g. we\nnotice an otherwise connected object is missing, and consult this\n\"map\" to learn what the type/path is because we want to do X)? Or\nis it more like a \"set of known-to-be-missing object\" (i.e. whose\nprimary point is to serve as a set of object names and what a name\nmaps to is primarily for debugging)? These are easier to answer if\nwe know how it will be used.\n\n> This will be used in a later commit by the list-object filtering\n> code.\n>\n> Signed-off-by: Jeff Hostetler \n> ---\n> diff --git a/list-objects-filter-map.c b/list-objects-filter-map.c\n> new file mode 100644\n> index 0000000..7e496b3\n> --- /dev/null\n> +++ b/list-objects-filter-map.c\n> @@ -0,0 +1,63 @@\n> +#include \"cache.h\"\n> +#include \"list-objects-filter-map.h\"\n> +\n> +int list_objects_filter_map_insert(struct oidmap *map,\n> +\t\t\t\t const struct object_id *oid,\n> +\t\t\t\t const char *pathname, enum object_type type)\n> +{\n> +\tsize_t len, size;\n> +\tstruct list_objects_filter_map_entry *e;\n> +\n> +\tif (oidmap_get(map, oid))\n> +\t\treturn 1;\n\nIt is OK for the existing entry to record a path that is totally\ndifferent from what the caller has. It is hard to judge without\nknowing what pathname the callers are expected to call this function\nwith, but I am guessing that it is similar to the path shown in the\noutput from \"rev-list --objects\"---and if that is the case, it is\ncorrect that the same object may be reached at different paths\ndepending on what tree the traversal begins at, so pathname recorded\nin the map is merely \"there is one tree somewhere that has this\nobject at this path\".\n\nFor that matter, the caller may have a completely different type\nfrom the object we saw earlier; not checking and flagging it as a\npossible error makes me feel somewhat uneasy, but there probably is\nlittle you can do at this layer of the code if you noticed such a\ndiscrepancy so it may be OK to punt.\n\n> +\tlen = ((pathname && *pathname) ? strlen(pathname) : 0);\n> +\tsize = (offsetof(struct list_objects_filter_map_entry, pathname) + len + 1);\n> +\te = xcalloc(1, size);\n> +\n> +\toidcpy(&e->entry.oid, oid);\n> +\te->type = type;\n> +\tif (pathname && *pathname)\n> +\t\tstrcpy(e->pathname, pathname);\n> +\n> +\toidmap_put(map, e);\n> +\treturn 0;\n> +}\n\nThe return value from the function needs to be documented in the\nheader to help callers. It is not apparent why \"we did already have\none\" and \"we now newly added\" is interesting to the callers, for\nexample. An obvious alternative implementation of this function\nwould return the pointer to an entry that records the object id\n(i.e. either the one that was already there, or the one we created\nbecause we saw this object for the first time), so that the caller\ncan do something interesting to it---again, because the reason why\nwe want this \"filter map\" is not explained at this stage, it is hard\nto tell what that \"sometehing interesting\" would be.\n\n> +static int my_cmp(const void *a, const void *b)\n> +{\n> +\tconst struct oidmap_entry *ea, *eb;\n> +\n> +\tea = *(const struct oidmap_entry **)a;\n> +\teb = *(const struct oidmap_entry **)b;\n> +\n> +\treturn oidcmp(&ea->oid, &eb->oid);\n> +}\n> +\n> +void list_objects_filter_map_foreach(struct oidmap *map,\n> +\t\t\t\t list_objects_filter_map_foreach_cb cb,\n\nName a typedef of a function as something_fn, not something_cb;\nsomething_cb is often the type of a struct to be fed to the callback\nfunction. And call such a parameter of type something_fn just fn.\n\n> +\t\t\t\t void *cb_data)\n> +{\n> +\tstruct hashmap_iter iter;\n> +\tstruct list_objects_filter_map_entry **array;\n> +\tstruct list_objects_filter_map_entry *e;\n> +\tint k, nr;\n> +\n> +\tnr = hashmap_get_size(&map->map);\n> +\tif (!nr)\n> +\t\treturn;\n> +\n> +\tarray = xcalloc(nr, sizeof(*e));\n> +\n> +\tk = 0;\n> +\thashmap_iter_init(&map->map, &iter);\n> +\twhile ((e = hashmap_iter_next(&iter)))\n> +\t\tarray[k++] = e;\n> +\n> +\tQSORT(array, nr, my_cmp);\n\nIt is entirely unclear why foreach() must return the object names in\norder.\n\n> +\tfor (k = 0; k < nr; k++)\n> +\t\tcb(k, nr, array[k], cb_data);\n\nAlso it is not clear if you wanted to expose the type of the\nentry to the callback function.\n\nAn obvious alternative\n\n\tfn(&array[k].entry.oid, cb_data);\n\nwould allow you to keep the type of map-entry private to the map,\nand also the callback does not need to know about k or nr.\n\nI guess you are giving k and nr to allow the callers to do a\nprogress bar? If that is the case, that's fine by me. I still do\nnot understand why we uncondtionally sort, though.\n\n> +\n> +\tfree(array);\n> +}\n> diff --git a/list-objects-filter-map.h b/list-objects-filter-map.h\n> new file mode 100644\n> index 0000000..794fc81\n> --- /dev/null\n> +++ b/list-objects-filter-map.h\n> @@ -0,0 +1,26 @@\n> +#ifndef LIST_OBJECTS_FILTER_MAP_H\n> +#define LIST_OBJECTS_FILTER_MAP_H\n> +\n> +#include \"oidmap.h\"\n> +\n> +struct list_objects_filter_map_entry {\n> +\tstruct oidmap_entry entry; /* must be first */\n> +\tenum object_type type;\n> +\tchar pathname[FLEX_ARRAY];\n> +};\n> +\n> +extern int list_objects_filter_map_insert(\n> +\tstruct oidmap *map,\n> +\tconst struct object_id *oid,\n> +\tconst char *pathname, enum object_type type);\n> +\n> +typedef void (*list_objects_filter_map_foreach_cb)(\n> +\tint i, int i_limit,\n> +\tstruct list_objects_filter_map_entry *e, void *cb_data);\n> +\n> +extern void list_objects_filter_map_foreach(\n> +\tstruct oidmap *map,\n> +\tlist_objects_filter_map_foreach_cb cb,\n> +\tvoid *cb_data);\n> +\n> +#endif /* LIST_OBJECTS_FILTER_MAP_H */\n"},{"id":"331020","messageId":"b063dec4-b9c4-7145-9b57-7df22ccbc655@jeffhostetler.com","threadId":"47024","inReplyTo":"xmqq60b3iv79.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 01/13] dir: allow exclusions from blob in addition to file","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-25T14:54:34Z","receivedAt":"2017-10-25T14:54:43Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 10/25/2017 2:43 AM, Junio C Hamano wrote:\n> Jeff Hostetler writes:\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 +758,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> \n> The post-context of this hunk is quite interesting in that there is\n> a call to read_skip_worktree_file_from_index(); which essentially\n> pretends as if we read from the filesystem but in fact it grabs the\n> blob object name registered in the index and reads from it.\n> \n> The reason why it is interesting is because this patch adds yet\n> nother \"let's instead read from a blob object\" function and there is\n> no sign to make the existing one take advantage of the new function\n> seen in this patch.\n> \n\nThe existing code handles use cases where you want to read the\nexclusion list from a pathname in the worktree -- or from blob\nnamed in the index when the pathname is not populated (presumably\nbecause of the skip-worktree bit).\n\nI was wanting to add a more general case (and perhaps my commit\nmessage should be improved). I want to be able to read it from\na blob not necessarily associated with the current commit or\nnot necessarily available on the local client, but yet known to\nexist. I'm thinking of the case the client could ask the server\nto do a partial clone using a sparse-checkout specification stored\nin a well-known location on the server. The reason for this is\nthat, in this case, the client is pre-clone and doesn't have a\nworktree or index.\n\nWith my \"add_excludes_from_blob_to_list()\", we can request a\nblob-ish expression, such as \"master:enlistments/foo\". In my\nlater commits associated with clone and fetch, we can use this\nmechanism to let the client ask the server to filter using the\nblob associated with this blob-ish. If the client has the blob\n(such as during a later fetch) and can resolve it, then it can\nand send the server the OID, but it can also send the blob-ish\nto the server and let it resolve it.\n\nJeff\n\n\n"},{"id":"331021","messageId":"4ac6ffd8-60bd-d2bc-6bc1-6c5bb1473416@jeffhostetler.com","threadId":"47024","inReplyTo":"CAGf8dg+cK3WpEqosgkbdcrDzrMXJxVYHiBZda6UM7k8Ggq=eBw@mail.gmail.com","subject":"Re: [PATCH 00/13] WIP Partial clone part 1: object filtering","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-25T15:39:00Z","receivedAt":"2017-10-25T15:39:06Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 10/25/2017 2:46 AM, Jonathan Tan wrote:\n> On Tue, Oct 24, 2017 at 10:00 PM, Junio C Hamano wrote:\n>> OK, thanks for working well together. So does this (1) build on\n>> Jonathan's fsck-squelching series, or (2) ignores that and builds\n>> filtering first, potentially leaving the codebase to a broken state\n>> where it can create fsck-unclean repository until Jonathan's series\n>> is rebased on top of this, or (3) something else? [*1*]\n> \n> Excluding the partialclone patch (patch 9), I think that the answer is\n> (2), but I don't think that it leaves the codebase in a broken state.\n> In particular, none of the code modifies the repo, so it can't create\n> a fsck-unclean one.\n\nMy part 1 series starts with filtering, rev-list, and pack-objects.\nSo, yes, it add new features that no one will use yet. But it is useful\nby itself. For example, you can use rev-list to ask for the set of\nmissing objects that you would need to do a checkout (assuming you had\ncommits and trees, but no blobs or no large blobs) *before* actually\nstarting the checkout.\n\nI was then going to lay Jonathan's fsck/gc/dynamic object fetch\non top of that. I started that here:\n\thttps://github.com/jeffhostetler/git/pull/7\n\nPatch 9 just adds the \"extensions.partialclone*\" fields and is prep\nfor my rev-list and his fsck changes. I included it sooner rather than\nlater so I can test rev-list on a repo with hand-deleted blobs.\nBut yes, it can be omitted from this series and included with the fsck\nchanges.\n\n\n> \n> Maybe one could say that this leaves the codebase with features that\n> no one will ever use in the absence of partial clone, but I don't\n> think that's true - rev-list with blob-size/sparse-specification\n> filter seems independently useful, at least (for example, when\n> desiring to operate on a repo in a sparse way without going through a\n> workdir), and if we're planning to allow listing of objects, we\n> probably should allow packing as well (especially since this doesn't\n> add much code).\n> \n> The above is relevant only if we can exclude the partialclone patch,\n> but I think that we can and we should, as I wrote in my reply to Jeff\n> Hostetler [1].\n> \n> As for how this patch set (excluding the partialclone patch) interacts\n> with my fsck series, they are relatively independent, as far as I can\n> tell. I'll rebase my fsck, gc, and lazy object fetch patches (but not\n> the fetch and clone parts, which we plan to instead adapt from Jeff\n> Hostetler's patches, as far as I know) on top of these and resend\n> those out once discussion on this has settled.\n\nYes, I want to get Jonathan's fsck/gc/lazy changes built into part 2.\nThey came over easily and are independent of how/why there are missing\nobjects.\n\nFor part 3, I'd like to take my version of clone, fetch, fetch-pack,\nand upload-pack (that talks with the filters from part 1) and incorporate\nJonathan's promisor concepts. That merge is a little messier, so I'd\nlike to parts 1 and 2 a chance to get some feedback first.\n\n\n> [1] https://public-inbox.org/git/CAGf8dg+8AR=XfSV92ODAtKTNjBnD1+oVZp9rs4Y4Otz_eZyTfg@mail.gmail.com/\n> \n>> I also saw a patch marked as \"this is from Jonathan's earlier work\",\n>> taking the authorship (which to me implies that the changes were\n>> extensive enough), so I am a bit at loss envisioning how this piece\n>> fits in the bigger picture together with the other piece.\n> \n> The patch you mentioned is the partialclone patch, which I think can\n> be considered separately from the rest (as I said above).\n\nA question of mailing-list etiquette: in patch 9, I took Jonathan's\nideas for adding the \"extensions.partialclone\" setting and extended it\nwith some helper functions. His change was part of a larger change\nwith other code (fsck, IIRC) that I wasn't ready for. What is the\npreferred way to give credit for something like this?\n\n\nThanks\nJeff\n\n\n"},{"id":"331033","messageId":"2f7ad5dc-821e-3fd3-bb7c-205ea5016457@jeffhostetler.com","threadId":"47024","inReplyTo":"xmqqwp3jhfct.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 02/13] list-objects-filter-map: extend oidmap to collect omitted objects","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-25T19:22:04Z","receivedAt":"2017-10-25T19:22:12Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 10/25/2017 3:10 AM, Junio C Hamano wrote:\n> Jeff Hostetler writes:\n> \n>> From: Jeff Hostetler \n>>\n>> Create helper class to extend oidmap to collect a list of\n>> omitted or missing objects during traversal.\n> \n> The reason why oidmap itself cannot be used is because the code\n> wants to record not just the object name but something else about\n> the object. And attributes that the code may care about we can see\n> in this patch are the object type and the path it found.\n\nI recently simplified the code in this version to not completely\nsub-class oidmap, but to just use it along with a custom\n_insert method that takes care of allocating the _entry\ndata. I should update the commit message to reflect that.\n\n> \n> Is the plan to extend this set of attributes over time as different\n> \"omitter\"s are added? Why was \"path\" chosen as a member of the\n> initial set and how it will be useful (also, what path would we\n> record for tags and commits)?\n\nI envisioned this to let rev-list print the pathname of omitted\nobjects -- like \"rev-list --objects\" does for regular blobs.\nI would leave the pathname NULL for tags and commits.\n\nThe pathname helps with debugging and testing, but also is\nused by the sparse filter to avoid some expensive duplicate\nis-excluded lookups.\n\nCurrently the 3 filters I have defined all use the same extra\ndata. I suppose a future filter could want additional fields,\nso maybe it would be better to refactor my \"map-entry\" to be\nper-filter specific.\n\n> \n> These \"future plans\" needs revealed upfront, instead of (or in\n> addition to) \"will be used in a later commit\". As it is hard to\n> judge if \"filter map\" is an appropriate name for this thing without\n> knowing _how_ it is envisioned to be used. \"filter map\" sounds more\n> like a map function that is consulted when we decide if we want to\n> drop the object, but from the looks of the code, it is used more to\n> record what was done to these objects.\n\nSorry, I meant a later commit in this patch series. It is used by\ncommits 4, 5, 6, and 10 to actually do the filtering and collect a\nlist of omitted or missing objects.\n\n> \n> Is it really a \"map\" (i.e. whose primary focus is to find out what\n> an object name is \"mapped to\" when we get an object name---e.g. we\n> notice an otherwise connected object is missing, and consult this\n> \"map\" to learn what the type/path is because we want to do X)? Or\n> is it more like a \"set of known-to-be-missing object\" (i.e. whose\n> primary point is to serve as a set of object names and what a name\n> maps to is primarily for debugging)? These are easier to answer if\n> we know how it will be used.\n\nI think of a \"set\" as a member? or not-member? class.\nI think of a \"map\" as a member? or not-member? class but where each\nmember also has a value. Sometimes map lookups just want to know\nmembership and sometimes the lookup wants the value.\n\nGranted, having the key and value data stuffed into the same entry\n(from hashmap's point of view, rather than a key having a pointer\nto a value) does kind of blur the line, but I was thinking about\na map here. (And I was building on oidmap which builds on hashmap,\nso it seemed appropriate.)\n\n> \n>> This will be used in a later commit by the list-object filtering\n>> code.\n>>\n>> Signed-off-by: Jeff Hostetler \n>> ---\n>> diff --git a/list-objects-filter-map.c b/list-objects-filter-map.c\n>> new file mode 100644\n>> index 0000000..7e496b3\n>> --- /dev/null\n>> +++ b/list-objects-filter-map.c\n>> @@ -0,0 +1,63 @@\n>> +#include \"cache.h\"\n>> +#include \"list-objects-filter-map.h\"\n>> +\n>> +int list_objects_filter_map_insert(struct oidmap *map,\n>> +\t\t\t\t const struct object_id *oid,\n>> +\t\t\t\t const char *pathname, enum object_type type)\n>> +{\n>> +\tsize_t len, size;\n>> +\tstruct list_objects_filter_map_entry *e;\n>> +\n>> +\tif (oidmap_get(map, oid))\n>> +\t\treturn 1;\n> \n> It is OK for the existing entry to record a path that is totally\n> different from what the caller has. It is hard to judge without\n> knowing what pathname the callers are expected to call this function\n> with, but I am guessing that it is similar to the path shown in the\n> output from \"rev-list --objects\"---and if that is the case, it is\n> correct that the same object may be reached at different paths\n> depending on what tree the traversal begins at, so pathname recorded\n> in the map is merely \"there is one tree somewhere that has this\n> object at this path\".\n\nRight, the first observed pathname is as good as any.\n\n> \n> For that matter, the caller may have a completely different type\n> from the object we saw earlier; not checking and flagging it as a\n> possible error makes me feel somewhat uneasy, but there probably is\n> little you can do at this layer of the code if you noticed such a\n> discrepancy so it may be OK to punt.\n\nI could assert() that the types match, but right there's not much\nwe can do about it at this layer.\n\n> \n>> +\tlen = ((pathname && *pathname) ? strlen(pathname) : 0);\n>> +\tsize = (offsetof(struct list_objects_filter_map_entry, pathname) + len + 1);\n>> +\te = xcalloc(1, size);\n>> +\n>> +\toidcpy(&e->entry.oid, oid);\n>> +\te->type = type;\n>> +\tif (pathname && *pathname)\n>> +\t\tstrcpy(e->pathname, pathname);\n>> +\n>> +\toidmap_put(map, e);\n>> +\treturn 0;\n>> +}\n> \n> The return value from the function needs to be documented in the\n> header to help callers. It is not apparent why \"we did already have\n> one\" and \"we now newly added\" is interesting to the callers, for\n> example. An obvious alternative implementation of this function\n> would return the pointer to an entry that records the object id\n> (i.e. either the one that was already there, or the one we created\n> because we saw this object for the first time), so that the caller\n> can do something interesting to it---again, because the reason why\n> we want this \"filter map\" is not explained at this stage, it is hard\n> to tell what that \"sometehing interesting\" would be.\n\ngood point. thanks.\n\n> \n>> +static int my_cmp(const void *a, const void *b)\n>> +{\n>> +\tconst struct oidmap_entry *ea, *eb;\n>> +\n>> +\tea = *(const struct oidmap_entry **)a;\n>> +\teb = *(const struct oidmap_entry **)b;\n>> +\n>> +\treturn oidcmp(&ea->oid, &eb->oid);\n>> +}\n>> +\n>> +void list_objects_filter_map_foreach(struct oidmap *map,\n>> +\t\t\t\t list_objects_filter_map_foreach_cb cb,\n> \n> Name a typedef of a function as something_fn, not something_cb;\n> something_cb is often the type of a struct to be fed to the callback\n> function. And call such a parameter of type something_fn just fn.\n> \n\nok.\n\n\n>> +\t\t\t\t void *cb_data)\n>> +{\n>> +\tstruct hashmap_iter iter;\n>> +\tstruct list_objects_filter_map_entry **array;\n>> +\tstruct list_objects_filter_map_entry *e;\n>> +\tint k, nr;\n>> +\n>> +\tnr = hashmap_get_size(&map->map);\n>> +\tif (!nr)\n>> +\t\treturn;\n>> +\n>> +\tarray = xcalloc(nr, sizeof(*e));\n>> +\n>> +\tk = 0;\n>> +\thashmap_iter_init(&map->map, &iter);\n>> +\twhile ((e = hashmap_iter_next(&iter)))\n>> +\t\tarray[k++] = e;\n>> +\n>> +\tQSORT(array, nr, my_cmp);\n> \n> It is entirely unclear why foreach() must return the object names in\n> order.\n> \n>> +\tfor (k = 0; k < nr; k++)\n>> +\t\tcb(k, nr, array[k], cb_data);\n> \n> Also it is not clear if you wanted to expose the type of the\n> entry to the callback function.\n> \n> An obvious alternative\n> \n> \tfn(&array[k].entry.oid, cb_data);\n> \n> would allow you to keep the type of map-entry private to the map,\n> and also the callback does not need to know about k or nr.\n> \n> I guess you are giving k and nr to allow the callers to do a\n> progress bar? If that is the case, that's fine by me. I still do\n> not understand why we uncondtionally sort, though.\n\nThe thought was that we would sort the OIDs so that things\nlike rev-list could print the omitted/missing objects in OID\norder. Not critical that we do it here, but I thought it would\nhelp callers.\n\nI included the {k, nr} so that the callback could dump header/trailer\ninformation when reporting the results or pre-allocate an array.\nI'll look at refactoring this -- I never quite liked how it turned\nout anyway -- especially with the oidmap simplifications.\n\n>> +\n>> +\tfree(array);\n>> +}\n>> diff --git a/list-objects-filter-map.h b/list-objects-filter-map.h\n>> new file mode 100644\n>> index 0000000..794fc81\n>> --- /dev/null\n>> +++ b/list-objects-filter-map.h\n>> @@ -0,0 +1,26 @@\n>> +#ifndef LIST_OBJECTS_FILTER_MAP_H\n>> +#define LIST_OBJECTS_FILTER_MAP_H\n>> +\n>> +#include \"oidmap.h\"\n>> +\n>> +struct list_objects_filter_map_entry {\n>> +\tstruct oidmap_entry entry; /* must be first */\n>> +\tenum object_type type;\n>> +\tchar pathname[FLEX_ARRAY];\n>> +};\n>> +\n>> +extern int list_objects_filter_map_insert(\n>> +\tstruct oidmap *map,\n>> +\tconst struct object_id *oid,\n>> +\tconst char *pathname, enum object_type type);\n>> +\n>> +typedef void (*list_objects_filter_map_foreach_cb)(\n>> +\tint i, int i_limit,\n>> +\tstruct list_objects_filter_map_entry *e, void *cb_data);\n>> +\n>> +extern void list_objects_filter_map_foreach(\n>> +\tstruct oidmap *map,\n>> +\tlist_objects_filter_map_foreach_cb cb,\n>> +\tvoid *cb_data);\n>> +\n>> +#endif /* LIST_OBJECTS_FILTER_MAP_H */\n"},{"id":"331034","messageId":"69ca62a0-ac4a-f821-0652-4340ce7b2fb1@jeffhostetler.com","threadId":"47024","inReplyTo":"CAGf8dg+_AewifMR8wnrQdJKXK0GuwdhMb8QAMrGoVCJzhysiRw@mail.gmail.com","subject":"Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-25T19:25:27Z","receivedAt":"2017-10-25T19:25:33Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 10/25/2017 12:05 AM, Jonathan Tan wrote:\n> On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote:\n> \n>> +enum list_objects_filter_result {\n>> + LOFR_ZERO = 0,\n>> + LOFR_MARK_SEEN = 1<<0,\n> \n> Probably worth documenting, something like /* Mark this object so that\n> it is skipped for the rest of the traversal. */\n> \n>> + LOFR_SHOW = 1<<1,\n> \n> And something like /* Invoke show_object_fn on this object. This\n> object may be revisited unless LOFR_MARK_SEEN is also set. */\n> \n>> +};\n>> +\n>> +/* See object.h and revision.h */\n>> +#define FILTER_REVISIT (1<<25)\n> \n> I think this should be declared closer to its use - in the sparse\n> filter code or in the file that uses it. Wherever it is, also update\n> the chart in object.h to indicate that we're using this 25th bit.\n> \n>> +\n>> +enum list_objects_filter_type {\n>> + LOFT_BEGIN_TREE,\n>> + LOFT_END_TREE,\n>> + LOFT_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> I don't think we typedef enums in Git code.\n> \n>> +\n>> +typedef list_objects_filter_result (*filter_object_fn)(\n>> + list_objects_filter_type filter_type,\n>> + struct object *obj,\n>> + const char *pathname,\n>> + const char *filename,\n>> + void *filter_data);\n>> +\n>> +void traverse_commit_list_worker(\n>> + struct rev_info *,\n>> + show_commit_fn, show_object_fn, void *show_data,\n>> + filter_object_fn filter, void *filter_data);\n> \n> I think things would be much clearer if a default filter was declared\n> (matching the behavior as of this patch when filter == NULL), say:\n> static inline default_filter(args) { switch(filter_type) { case\n> LOFT_BEGIN_TREE: return LOFR_MARK_SEEN | LOFR_SHOW; case\n> LOFT_END_TREE: return LOFT_ZERO; ...\n> \n> And inline traverse_commit_list() instead of putting it in the .c file.\n> \n> This would reduce or eliminate the need to document\n> traverse_commit_list_worker, including what happens if filter is NULL,\n> and explain how a user would make their own filter_object_fn.\n> \n>> +\n>> +#endif /* LIST_OBJECTS_H */\n>> --\n>> 2.9.3\n>>\n\nI'll give that a try. Thanks!\n\nJeff\n"},{"id":"331035","messageId":"a262e30b-1221-17b2-54dc-ccb6c710391f@jeffhostetler.com","threadId":"47024","inReplyTo":"CAGf8dgJmwtN-v9SQfjUJOBreGQ==iM1G-oGBh3oYH_DE4NYyLA@mail.gmail.com","subject":"Re: [PATCH 07/13] list-objects-filter-options: common argument parsing","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-25T19:28:03Z","receivedAt":"2017-10-25T19:28:09Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 10/25/2017 12:14 AM, Jonathan Tan wrote:\n> On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote:\n>> + * ::= blob:none\n>> + * blob:limit:[kmg]\n>> + * sparse:oid:\n>> + * sparse:path:\n> \n> I notice in the code below that there are some usages of \"=\" instead\n> of \":\" - could you clarify which one it is? (Ideally this would point\n> to one point of documentation which serves as both user and technical\n> documentation.)\n\ngood catch. thanks!\n \n>> + */\n>> +int parse_list_objects_filter(struct list_objects_filter_options *filter_options,\n>> + const char *arg)\n>> +{\n>> + struct object_context oc;\n>> + struct object_id sparse_oid;\n>> + const char *v0;\n>> + const char *v1;\n>> +\n>> + if (filter_options->choice)\n>> + die(_(\"multiple object filter types cannot be combined\"));\n>> +\n>> + /*\n>> + * TODO consider rejecting 'arg' if it contains any\n>> + * TODO injection characters (since we might send this\n>> + * TODO to a sub-command or to the server and we don't\n>> + * TODO want to deal with legacy quoting/escaping for\n>> + * TODO a new feature).\n>> + */\n>> +\n>> + filter_options->raw_value = strdup(arg);\n>> +\n>> + if (skip_prefix(arg, \"blob:\", &v0) || skip_prefix(arg, \"blobs:\", &v0)) {\n> \n> I know that some people prefer leniency, but I think it's better to\n> standardize on one form (\"blob\" instead of both \"blob\" and \"blobs\").\n\nI could go either way on this. (I kept mistyping it during interactive testing,\nso I added both cases...)\n\n> \n>> + if (!strcmp(v0, \"none\")) {\n>> + filter_options->choice = LOFC_BLOB_NONE;\n>> + return 0;\n>> + }\n>> +\n>> + if (skip_prefix(v0, \"limit=\", &v1) &&\n>> + git_parse_ulong(v1, &filter_options->blob_limit_value)) {\n>> + filter_options->choice = LOFC_BLOB_LIMIT;\n>> + return 0;\n>> + }\n>> + }\n>> + else if (skip_prefix(arg, \"sparse:\", &v0)) {\n>> + if (skip_prefix(v0, \"oid=\", &v1)) {\n>> + filter_options->choice = LOFC_SPARSE_OID;\n>> + if (!get_oid_with_context(v1, GET_OID_BLOB,\n>> + &sparse_oid, &oc)) {\n>> + /*\n>> + * We successfully converted the \n>> + * into an actual OID. Rewrite the raw_value\n>> + * in canonoical form with just the OID.\n>> + * (If we send this request to the server, we\n>> + * want an absolute expression rather than a\n>> + * local-ref-relative expression.)\n>> + */\n>> + free((char *)filter_options->raw_value);\n>> + filter_options->raw_value =\n>> + xstrfmt(\"sparse:oid=%s\",\n>> + oid_to_hex(&sparse_oid));\n>> + filter_options->sparse_oid_value =\n>> + oiddup(&sparse_oid);\n>> + } else {\n>> + /*\n>> + * We could not turn the into an\n>> + * OID. Leave the raw_value as is in case\n>> + * the server can parse it. (It may refer to\n>> + * a branch, commit, or blob we don't have.)\n>> + */\n>> + }\n>> + return 0;\n>> + }\n>> +\n>> + if (skip_prefix(v0, \"path=\", &v1)) {\n>> + filter_options->choice = LOFC_SPARSE_PATH;\n>> + filter_options->sparse_path_value = strdup(v1);\n>> + return 0;\n>> + }\n>> + }\n>> +\n>> + die(_(\"invalid filter expression '%s'\"), arg);\n>> + return 0;\n>> +}\n>> +\n>> +int opt_parse_list_objects_filter(const struct option *opt,\n>> + const char *arg, int unset)\n>> +{\n>> + struct list_objects_filter_options *filter_options = opt->value;\n>> +\n>> + assert(arg);\n>> + assert(!unset);\n>> +\n>> + return parse_list_objects_filter(filter_options, arg);\n>> +}\n>> diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h\n>> new file mode 100644\n>> index 0000000..23bd68e\n>> --- /dev/null\n>> +++ b/list-objects-filter-options.h\n>> @@ -0,0 +1,50 @@\n>> +#ifndef LIST_OBJECTS_FILTER_OPTIONS_H\n>> +#define LIST_OBJECTS_FILTER_OPTIONS_H\n>> +\n>> +#include \"parse-options.h\"\n>> +\n>> +/*\n>> + * Common declarations and utilities for filtering objects (such as omitting\n>> + * large blobs) in list_objects:traverse_commit_list() and git-rev-list.\n>> + */\n>> +\n>> +enum list_objects_filter_choice {\n>> + LOFC_DISABLED = 0,\n>> + LOFC_BLOB_NONE,\n>> + LOFC_BLOB_LIMIT,\n>> + LOFC_SPARSE_OID,\n>> + LOFC_SPARSE_PATH,\n>> +};\n>> +\n>> +struct list_objects_filter_options {\n>> + /*\n>> + * The raw argument value given on the command line or\n>> + * protocol request. (The part after the \"--keyword=\".)\n>> + */\n>> + char *raw_value;\n>> +\n>> + /*\n>> + * Parsed values. Only 1 will be set depending on the flags below.\n>> + */\n>> + struct object_id *sparse_oid_value;\n>> + char *sparse_path_value;\n>> + unsigned long blob_limit_value;\n>> +\n>> + enum 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(struct list_objects_filter_options *filter_options,\n>> + const char *arg);\n>> +\n>> +int opt_parse_list_objects_filter(const struct option *opt,\n>> + const char *arg, int unset);\n>> +\n>> +#define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \\\n>> + { OPTION_CALLBACK, 0, CL_ARG__FILTER, fo, N_(\"args\"), \\\n>> + N_(\"object filtering\"), PARSE_OPT_NONEG, \\\n>> + opt_parse_list_objects_filter }\n> \n> Thanks - this does make it easier to have a standard argument name and\n> description everywhere.\n> \n>> +\n>> +#endif /* LIST_OBJECTS_FILTER_OPTIONS_H */\n>> --\n>> 2.9.3\n>>\n"},{"id":"331036","messageId":"39e85992-6e3b-2059-17a1-784c92072944@jeffhostetler.com","threadId":"47024","inReplyTo":"CAGf8dgLU_70vq8JNpr-cihgk9SbC_7GYcaEF6c9NgmJRXPAcmw@mail.gmail.com","subject":"Re: [PATCH 08/13] list-objects: add traverse_commit_list_filtered method","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-25T19:29:45Z","receivedAt":"2017-10-25T19:29:50Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 10/25/2017 12:24 AM, Jonathan Tan wrote:\n> On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote:\n>> +void traverse_commit_list_filtered(\n>> + struct list_objects_filter_options *filter_options,\n>> + struct rev_info *revs,\n>> + show_commit_fn show_commit,\n>> + show_object_fn show_object,\n>> + list_objects_filter_map_foreach_cb print_omitted_object,\n>> + void *show_data);\n> \n> So the function call chain, if we wanted a filtered traversal, is:\n> traverse_commit_list_filtered -> traverse_commit_list__sparse_path\n> (and friends, and each algorithm is in its own file) ->\n> traverse_commit_list_worker\n> \n> This makes the implementation of each algorithm more easily understood\n> (since they are all in their own files), but also increases the number\n> of global functions and code files. I personally would combine the\n> traverse_commit_list__* functions into one file\n> (list-objects-filtered.c), make them static, and also put\n> traverse_commit_list_filtered in there, but I understand that other\n> people in the Git project may differ on this.\n> \n\nI'll do a round of refactoring to include your suggestion of\na default null filter. Then with that see what collapsing this\nlooks like.\n\nThanks,\nJeff\n"},{"id":"331037","messageId":"4afe76f0-a243-126b-b3e0-d2d08640e73d@jeffhostetler.com","threadId":"47024","inReplyTo":"CAGf8dgK3pW8T4Tst6LZuJxLZTDjk7Ak5iwK0WYBSLrWR1S8YBg@mail.gmail.com","subject":"Re: [PATCH 10/13] rev-list: add list-objects filtering support","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-25T19:37:50Z","receivedAt":"2017-10-25T19:37:56Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 10/25/2017 12:41 AM, Jonathan Tan wrote:\n> On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote:\n>> static void finish_object(struct object *obj, const char *name, void *cb_data)\n>> {\n>> struct rev_list_info *info = cb_data;\n>> - if (obj->type == OBJ_BLOB && !has_object_file(&obj->oid))\n>> + if (obj->type == OBJ_BLOB && !has_object_file(&obj->oid)) {\n>> + if (arg_print_missing) {\n>> + list_objects_filter_map_insert(\n>> + &missing_objects, &obj->oid, name, obj->type);\n>> + return;\n>> + }\n>> +\n>> + /*\n>> + * Relax consistency checks when we expect missing\n>> + * objects because of partial-clone or a previous\n>> + * partial-fetch.\n>> + *\n>> + * Note that this is independent of any filtering that\n>> + * we are doing in this run.\n>> + */\n>> + if (is_partial_clone_registered())\n>> + return;\n>> +\n>> die(\"missing blob object '%s'\", oid_to_hex(&obj->oid));\n> \n> I'm fine with arg_print_missing suppressing lazy fetching (when I\n> rebase my patches on this, I'll have to ensure that fetch_if_missing\n> is set to 0 if arg_print_missing is true), but I think that the\n> behavior when arg_print_missing is false should be the opposite - we\n> should let has_object_file() perform the lazy fetching, and die if it\n> returns false (that is, if the fetching failed).\n\nRight. This is a point where our different approaches need\nto come together. My \"is_partial_clone_registered\" is essentially\na placeholder for your lazy fetching. so we can delete this call\nwhen your changes are in. Basically, you set:\n\tfetch_if_missing = !arg_print_missing\nat the top.\n\n> \n>> + }\n>> if (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n>> parse_object(&obj->oid);\n>> }\n"},{"id":"331058","messageId":"xmqq7evihdkl.fsf@gitster.mtv.corp.google.com","threadId":"47024","inReplyTo":"CAGf8dg+cK3WpEqosgkbdcrDzrMXJxVYHiBZda6UM7k8Ggq=eBw@mail.gmail.com","subject":"Re: [PATCH 00/13] WIP Partial clone part 1: object filtering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-26T02:01:46Z","receivedAt":"2017-10-26T02:01:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> On Tue, Oct 24, 2017 at 10:00 PM, Junio C Hamano wrote:\n>> OK, thanks for working well together. So does this (1) build on\n>> Jonathan's fsck-squelching series, or (2) ignores that and builds\n>> filtering first, potentially leaving the codebase to a broken state\n>> where it can create fsck-unclean repository until Jonathan's series\n>> is rebased on top of this, or (3) something else? [*1*]\n>\n> Excluding the partialclone patch (patch 9), I think that the answer is\n> (2), but I don't think that it leaves the codebase in a broken state.\n> In particular, none of the code modifies the repo, so it can't create\n> a fsck-unclean one.\n\nOK. It is not dangerous enough to matter until we start using the\nupdated features in repack->rev-list|pack-objects ;-) As I said, I\nwas mostly interested in learning what the big-picture direction was\nand also seeing you two were more-or-less in agreement.\n\n> The above is relevant only if we can exclude the partialclone patch,\n> but I think that we can and we should, as I wrote in my reply to Jeff\n> Hostetler [1].\n\nOK.\n\n> As for how this patch set (excluding the partialclone patch) interacts\n> with my fsck series, they are relatively independent, as far as I can\n> tell. I'll rebase my fsck, gc, and lazy object fetch patches (but not\n> the fetch and clone parts, which we plan to instead adapt from Jeff\n> Hostetler's patches, as far as I know) on top of these and resend\n> those out once discussion on this has settled.\n\nOK. Thanks, I think tht is a reasonable way forward.\n\n> [1] https://public-inbox.org/git/CAGf8dg+8AR=XfSV92ODAtKTNjBnD1+oVZp9rs4Y4Otz_eZyTfg@mail.gmail.com/\n>\n>> I also saw a patch marked as \"this is from Jonathan's earlier work\",\n>> taking the authorship (which to me implies that the changes were\n>> extensive enough), so I am a bit at loss envisioning how this piece\n>> fits in the bigger picture together with the other piece.\n>\n> The patch you mentioned is the partialclone patch, which I think can\n> be considered separately from the rest (as I said above).\n\nGood, that lets us sidestep Jeff's question about \"how should the\ncredits for this change attributed?\", too.\n"},{"id":"331059","messageId":"xmqq3766hd7v.fsf@gitster.mtv.corp.google.com","threadId":"47024","inReplyTo":"4ac6ffd8-60bd-d2bc-6bc1-6c5bb1473416@jeffhostetler.com","subject":"Re: [PATCH 00/13] WIP Partial clone part 1: object filtering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-26T02:09:24Z","receivedAt":"2017-10-26T02:09:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> A question of mailing-list etiquette: in patch 9, I took Jonathan's\n> ideas for adding the \"extensions.partialclone\" setting and extended it\n> with some helper functions. His change was part of a larger change\n> with other code (fsck, IIRC) that I wasn't ready for. What is the\n> preferred way to give credit for something like this?\n\nI think the note you left in the proposed log message\n\n This patch is part of a patch originally authored by:\n Jonathan Tan \n\nwas a bit misleading. The phrasing makes it sound as if it is\nmore-or-less verbatim copy (either of the whole thing or just a\nsubset) of Jonathan's patch, in which case, keeping the authorship\nintact, i.e.\n\n\tFrom: Jonathan Tan \n\n\t... log message taken from the original, with additional\n ... note to describe any adjustment you did\n\n\tSigned-off-by: Jonathan Tan \n\tSigned-off-by: Jeff Hostetler \n\nwould have been more appropriate. But if you just were inspired by\nthe idea in his patch and wrote a one that is similar to but\ndifferent from it that suits the need of your series better, then a\nnote left in the log that instead does s/is part of/was inspired by/\nwould have been perfectly fine.\n\nThanks.\n\n\n\n"},{"id":"331063","messageId":"xmqqpo9afu3s.fsf@gitster.mtv.corp.google.com","threadId":"47024","inReplyTo":"b063dec4-b9c4-7145-9b57-7df22ccbc655@jeffhostetler.com","subject":"Re: [PATCH 01/13] dir: allow exclusions from blob in addition to file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-26T03:47:35Z","receivedAt":"2017-10-26T03:47:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> The existing code handles use cases where you want to read the\n> exclusion list from a pathname in the worktree -- or from blob\n> named in the index when the pathname is not populated (presumably\n> because of the skip-worktree bit).\n>\n> I was wanting to add a more general case (and perhaps my commit\n> message should be improved). I want to be able to read it from\n> a blob not necessarily associated with the current commit or\n> not necessarily available on the local client, but yet known to\n> exist. \n\nOh, I understand the above two paragraphs perfectly well, and I\nagree with you that such a helper to read from an arbitrary blob is\na worthy thing to have. I was merely commenting on the fact that\nsuch a helper that is meant to be able to handle more general cases\nis not used to help the more specific case that we already have,\nwhich was a bit curious.\n\nI guess the reason why it is not done is (besides expediency)\nbecause the model the new helper operates in would not fit well with\nthe existing logic flow, where everything is loaded into core\n(either from the filesystem or from a blob) and then a common code\nparses and registers; the helper wants to do the reading (only) from\nthe blob, the parsing and the registration all by itself, so there\nis not much that can be shared even if the existing code wanted to\nreuse what the helper offers.\n\nThe new helper mimicks the read_skip_worktree_file_from_index()\ncodepath to massage the data it reads from the blob to buf[] but not\nreally (e.g. even though it copies and pastes a lot, it forgets to\ncall skip_utf8_bom(), for example). We may still want to see if we\ncan share more so that we do not have to worry about these tiny\ndifferences between codepaths.\n\n> With my \"add_excludes_from_blob_to_list()\", we can request a\n> blob-ish expression, such as \"master:enlistments/foo\". In my\n> later commits associated with clone and fetch, we can use this\n> mechanism to let the client ask the server to filter using the\n> blob associated with this blob-ish. If the client has the blob\n> (such as during a later fetch) and can resolve it, then it can\n> and send the server the OID, but it can also send the blob-ish\n> to the server and let it resolve it.\n\nSecurity-minded people may want to keep an eye or two open for these\nlater patches---extended SHA-1 expressions is a new attack surface\nwe would want to carefully polish and protect.\n"},{"id":"331064","messageId":"xmqqefpqfsxm.fsf@gitster.mtv.corp.google.com","threadId":"47024","inReplyTo":"2f7ad5dc-821e-3fd3-bb7c-205ea5016457@jeffhostetler.com","subject":"Re: [PATCH 02/13] list-objects-filter-map: extend oidmap to collect omitted objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-26T04:12:53Z","receivedAt":"2017-10-26T04:13:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> Sorry, I meant a later commit in this patch series. It is used by\n> commits 4, 5, 6, and 10 to actually do the filtering and collect a\n> list of omitted or missing objects.\n\nI know you meant \"later commits in the series\" ;-). \n\nIt does not change the fact that readers of 02/13 haven't seen them\nyet to understand patch 02/13, if the changes that drove the design\nof this step is in the same series or if they are not yet posted.\n\n> I think of a \"set\" as a member? or not-member? class.\n> I think of a \"map\" as a member? or not-member? class but where each\n> member also has a value. Sometimes map lookups just want to know\n> membership and sometimes the lookup wants the value.\n>\n> Granted, having the key and value data stuffed into the same entry\n> (from hashmap's point of view, rather than a key having a pointer\n> to a value) does kind of blur the line, but I was thinking about\n> a map here. (And I was building on oidmap which builds on hashmap,\n> so it seemed appropriate.)\n\nMy question was mostly about \"if this is a map, then a caller that\nqueries the map with an oid does so because it wants to know the\ndata associated to the oid; if this is just a set, it is mostly\ninterested in the membership\" and \"I cannot quite tell which was\nmeant without the caller\". \n\nIt seems that some callers do care about the \"path\" name from your\nresponse above, so calling this \"map\" sounds more appropriate.\n\nThe answer \"it can be used to speed up 'is this path excluded?'\ncheck\" is a bit worrisome, though. A blob can appear at more than\none path, and unless all the appearances of it are in an excluded\npath, omitting the blob from the repository would lead to an aborted\n\"rev-list --objects\" run, and this \"map\" can record at most one path\nper each object; we need to wait until seeing the optimization code\nto actually see how effectively this data helps optimization and\ncomment on the code ;-)\n\n>>> +\tlen = ((pathname && *pathname) ? strlen(pathname) : 0);\n>>> +\tsize = (offsetof(struct list_objects_filter_map_entry, pathname) + len + 1);\n>>> +\te = xcalloc(1, size);\n>>> +\n>>> +\toidcpy(&e->entry.oid, oid);\n>>> +\te->type = type;\n>>> +\tif (pathname && *pathname)\n>>> +\t\tstrcpy(e->pathname, pathname);\n>>> +\n>>> +\toidmap_put(map, e);\n>>> +\treturn 0;\n>>> +}\n>>\n>> The return value from the function needs to be documented in the\n>> header to help callers. It is not apparent why \"we did already have\n>> one\" and \"we now newly added\" is interesting to the callers, for\n>> example. An obvious alternative implementation of this function\n>> would return the pointer to an entry that records the object id\n>> (i.e. either the one that was already there, or the one we created\n>> because we saw this object for the first time), so that the caller\n>> can do something interesting to it---again, because the reason why\n>> we want this \"filter map\" is not explained at this stage, it is hard\n>> to tell what that \"sometehing interesting\" would be.\n>\n> good point. thanks.\n\nI am more confused by the response ;-) But as we established that\nthis is a map (not a set that borrows the implementation of map),\nwhere the data recorded in 'e' is quite useful to the caller, it\nprobably makes sense to make 'e' available to the caller? It is\nstill unclear if the caller finds \"it is the first time I saw the\nobject you gave me\" vs \"I've seen that object before already\"\nuseful.\n\n>>> +\tfor (k = 0; k < nr; k++)\n>>> +\t\tcb(k, nr, array[k], cb_data);\n>>\n>> Also it is not clear if you wanted to expose the type of the\n>> entry to the callback function.\n>\n> The thought was that we would sort the OIDs so that things\n> like rev-list could print the omitted/missing objects in OID\n> order. Not critical that we do it here, but I thought it would\n> help callers.\n\nI can foresee some callers would want sorted, while others do not.\nI was primarily wondering why \"my_cmp\" is not a parameter that can\nbe NULL (in which case we do not sort at all).\n\n>> An obvious alternative\n>>\n>> \tfn(&array[k].entry.oid, cb_data);\n>>\n>> would allow you to keep the type of map-entry private to the map,\n>> and also the callback does not need to know about k or nr.\n>> ...\n> I included the {k, nr} so that the callback could dump header/trailer\n> information when reporting the results or pre-allocate an array.\n> I'll look at refactoring this -- I never quite liked how it turned\n> out anyway -- especially with the oidmap simplifications.\n\nAnd as we established that this is a map, where the data associated\nwith each oid is interesting to the caller, we do not want to hide\nthe type of array[] element by passing only &array[k].entry.oid, I\nguess?\n\nThanks.\n"},{"id":"331105","messageId":"c881d4d1-f9c0-0360-a149-2cee12f144ee@jeffhostetler.com","threadId":"47024","inReplyTo":"xmqqpo9afu3s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 01/13] dir: allow exclusions from blob in addition to file","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-10-26T18:11:25Z","receivedAt":"2017-10-26T18:11:32Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 10/25/2017 11:47 PM, Junio C Hamano wrote:\n> Jeff Hostetler writes:\n> \n>> The existing code handles use cases where you want to read the\n>> exclusion list from a pathname in the worktree -- or from blob\n>> named in the index when the pathname is not populated (presumably\n>> because of the skip-worktree bit).\n>>\n>> I was wanting to add a more general case (and perhaps my commit\n>> message should be improved). I want to be able to read it from\n>> a blob not necessarily associated with the current commit or\n>> not necessarily available on the local client, but yet known to\n>> exist.\n> \n> Oh, I understand the above two paragraphs perfectly well, and I\n> agree with you that such a helper to read from an arbitrary blob is\n> a worthy thing to have. I was merely commenting on the fact that\n> such a helper that is meant to be able to handle more general cases\n> is not used to help the more specific case that we already have,\n> which was a bit curious.\n> \n> I guess the reason why it is not done is (besides expediency)\n> because the model the new helper operates in would not fit well with\n> the existing logic flow, where everything is loaded into core\n> (either from the filesystem or from a blob) and then a common code\n> parses and registers; the helper wants to do the reading (only) from\n> the blob, the parsing and the registration all by itself, so there\n> is not much that can be shared even if the existing code wanted to\n> reuse what the helper offers.\n> \n> The new helper mimicks the read_skip_worktree_file_from_index()\n> codepath to massage the data it reads from the blob to buf[] but not\n> really (e.g. even though it copies and pastes a lot, it forgets to\n> call skip_utf8_bom(), for example). We may still want to see if we\n> can share more so that we do not have to worry about these tiny\n> differences between codepaths.\n\nI'm going to extract this commit, refactor it to try to share\nmore code with the existing read_skip_worktree_file_from_index()\nand submit it as a separate patch series so that we can discuss\nit in isolation without the rest of the partial-clone code getting\nin the way.\n\nThe call to skip_utf8_bom() was captured in the new\nadd_excludes_from_buffer() routine that both add_excludes()\nand my new add_excludes_from_blob_to_list() call.\n\n> \n>> With my \"add_excludes_from_blob_to_list()\", we can request a\n>> blob-ish expression, such as \"master:enlistments/foo\". In my\n>> later commits associated with clone and fetch, we can use this\n>> mechanism to let the client ask the server to filter using the\n>> blob associated with this blob-ish. If the client has the blob\n>> (such as during a later fetch) and can resolve it, then it can\n>> and send the server the OID, but it can also send the blob-ish\n>> to the server and let it resolve it.\n> \n> Security-minded people may want to keep an eye or two open for these\n> later patches---extended SHA-1 expressions is a new attack surface\n> we would want to carefully polish and protect.\n> \n"},{"id":"331420","messageId":"20171030222726.g26nryjxktyj27sd@aiede.mtv.corp.google.com","threadId":"47024","inReplyTo":"CAGf8dg+cK3WpEqosgkbdcrDzrMXJxVYHiBZda6UM7k8Ggq=eBw@mail.gmail.com","subject":"Re: [PATCH 00/13] WIP Partial clone part 1: object filtering","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-10-30T22:27:26Z","receivedAt":"2017-10-30T22:27:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJonathan Tan wrote:\n\n> As for how this patch set (excluding the partialclone patch) interacts\n> with my fsck series, they are relatively independent, as far as I can\n> tell. I'll rebase my fsck, gc, and lazy object fetch patches (but not\n> the fetch and clone parts, which we plan to instead adapt from Jeff\n> Hostetler's patches, as far as I know) on top of these and resend\n> those out once discussion on this has settled.\n\nSelfishly, I'll make a request here. The only part of the series that\noverlaps is the max-blob-bytes part, right? Would you mind re-sending\nthe remainder of the series so it can go through the \"next\" ->\n\"master\" -> etc process in the usual way?\n\nMy selfishness comes in because this would reduce the set of patches I\nhave to apply locally to just the max-blob-bytes part. If I\nunderstood correctly, the rest of the series was something everyone\nagreed about, so there's no reason not to pursue including it in\n\"next\".\n\nI'd have the same request for this object filtering series, but I\nthink it's already happening: the patches in this thread so far do not\nallow omitting some blobs from the local object store, so they should\nbe able to go through the \"next\" -> \"master\" -> etc process as well\nwithout having to wait for the fsck patches.\n\nThanks,\nJonathan\n"}]}