Re: [PATCH v5 03/14] packfile-uri: support for excluding commit objects

> Currently packfile-uri supports the exclusion of blob objects, but in
> some scenarios, users may wish to exclude more types of objects, such as
> commit and tree objects, not only because packfile itself supports
> storing these object types, but also on the other hand, to make
> configuration items maintainable and simpler.
>
> This commit is used to support the recursive exclusion of a commit
> object, which means that if the exclusion of a commit is configured as
> packfile-uri, the commit itself and all the objects it contains will
> also be recursively excluded. In addition, to support this feature, a
> new configuration  `uploadpack.excludeobject` is introduced.
>
> The reason for bringing a new configuration is for two considerations.
> First, the old configuration supports a single object type (blob), which
> limits the use of this feature. Secondly, the name of the old
> configuration is not abstract enough, this make extension difficult. If
> different object types use different configuration names, the
> configuration items will be bloated and difficult to maintain, so the
> new configuration is more abstract in name and easy to extend.
>
> Although a new configuration has been introduced, the old one is
> still available and compatible with the new configuration. The old
> configuration `uploadpack.blobpackfileuri` only supports excluding
> blobs. The new configuration `uploadpack.excludeobject` not only
> supports excluding blob objects, but also supports excluding commit
> objects, as well as recursively excluding tree objects and blob objects
> they contain.

> Signed-off-by: Teng Long <dyroneteng@gmail.com>
> ---
>  builtin/pack-objects.c | 50 +++++++++++++++++++++++++-----------------
>  list-objects.c         | 37 +++++++++++++++++--------------
>  object.c               | 15 ++++++++++---
>  object.h               |  4 +++-
>  revision.c             | 34 ++++++++++++++++++++--------
>  revision.h             |  3 +++
>  upload-pack.c          |  7 ++++++
>  7 files changed, 101 insertions(+), 49 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 5f9ec3566f..63f3aed70a 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1311,11 +1311,14 @@ static int want_object_in_pack_one(struct packed_git *p,
>  static int want_object_in_pack(const struct object_id *oid,
>  			       int exclude,
>  			       struct packed_git **found_pack,
> -			       off_t *found_offset)
> +			       off_t *found_offset,
> +			       struct object *referred_commit)
>  {
>  	int want;
>  	struct list_head *pos;
>  	struct multi_pack_index *m;
> +	struct configured_exclusion *ex;
> +	struct configured_exclusion *referred_ex;
>  
>  	if (!exclude && local && has_loose_object_nonlocal(oid))
>  		return 0;
> @@ -1351,9 +1354,13 @@ static int want_object_in_pack(const struct object_id *oid,
>  	}
>  
>  	if (uri_protocols.nr) {
> -		struct configured_exclusion *ex =
> -			oidmap_get(&configured_exclusions, oid);
> +		if (referred_commit) {
> +			referred_ex = oidmap_get(&configured_exclusions, &referred_commit->oid);
> +			if (referred_ex && match_packfile_uri_exclusions(referred_ex))
> +				return 0;
> +		}
>  
> +		ex = oidmap_get(&configured_exclusions, oid);
>  		if (ex && match_packfile_uri_exclusions(ex)) {
>  			oidset_insert(&excluded_by_config, oid);
>  			return 0;
> @@ -1393,7 +1400,8 @@ static const char no_closure_warning[] = N_(
>  );
>  
>  static int add_object_entry(const struct object_id *oid, enum object_type type,
> -			    const char *name, int exclude)
> +			    const char *name, int exclude,
> +			    struct object *referred_commit)
>  {
>  	struct packed_git *found_pack = NULL;
>  	off_t found_offset = 0;
> @@ -1403,7 +1411,7 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
>  	if (have_duplicate_entry(oid, exclude))
>  		return 0;
>  
> -	if (!want_object_in_pack(oid, exclude, &found_pack, &found_offset)) {
> +	if (!want_object_in_pack(oid, exclude, &found_pack, &found_offset, referred_commit)) {
>  		/* The pack is missing an object, so it will not have closure */
>  		if (write_bitmap_index) {
>  			if (write_bitmap_index != WRITE_BITMAP_QUIET)
> @@ -1429,7 +1437,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
>  	if (have_duplicate_entry(oid, 0))
>  		return 0;
>  
> -	if (!want_object_in_pack(oid, 0, &pack, &offset))
> +	if (!want_object_in_pack(oid, 0, &pack, &offset, NULL))
>  		return 0;
>  
>  	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
> @@ -1569,7 +1577,7 @@ static void add_pbase_object(struct tree_desc *tree,
>  		if (name[cmplen] != '/') {
>  			add_object_entry(&entry.oid,
>  					 object_type(entry.mode),
> -					 fullname, 1);
> +					 fullname, 1, NULL);
>  			return;
>  		}
>  		if (S_ISDIR(entry.mode)) {
> @@ -1637,7 +1645,7 @@ static void add_preferred_base_object(const char *name)
>  	cmplen = name_cmp_len(name);
>  	for (it = pbase_tree; it; it = it->next) {
>  		if (cmplen == 0) {
> -			add_object_entry(&it->pcache.oid, OBJ_TREE, NULL, 1);
> +			add_object_entry(&it->pcache.oid, OBJ_TREE, NULL, 1, NULL);
>  		}
>  		else {
>  			struct tree_desc tree;
> @@ -2839,7 +2847,7 @@ static void add_tag_chain(const struct object_id *oid)
>  			die(_("unable to pack objects reachable from tag %s"),
>  			    oid_to_hex(oid));
>  
> -		add_object_entry(&tag->object.oid, OBJ_TAG, NULL, 0);
> +		add_object_entry(&tag->object.oid, OBJ_TAG, NULL, 0, NULL);
>  
>  		if (tag->tagged->type != OBJ_TAG)
>  			return;
> @@ -2994,7 +3002,7 @@ static int git_pack_config(const char *k, const char *v, void *cb)
>  			pack_idx_opts.flags &= ~WRITE_REV;
>  		return 0;
>  	}
> -	if (!strcmp(k, "uploadpack.blobpackfileuri")) {
> +	if (!strcmp(k, "uploadpack.excludeobject") || !strcmp(k, "uploadpack.blobpackfileuri")) {
>  		struct configured_exclusion *ex = xmalloc(sizeof(*ex));
>  		const char *oid_end, *pack_end;
>  		/*
> @@ -3007,11 +3015,11 @@ static int git_pack_config(const char *k, const char *v, void *cb)
>  		    *oid_end != ' ' ||
>  		    parse_oid_hex(oid_end + 1, &pack_hash, &pack_end) ||
>  		    *pack_end != ' ')
> -			die(_("value of uploadpack.blobpackfileuri must be "
> +			die(_("value of uploadpack.excludeobject or uploadpack.blobpackfileuri must be "
>  			      "of the form '<object-hash> <pack-hash> <uri>' (got '%s')"), v);
>  		if (oidmap_get(&configured_exclusions, &ex->e.oid))
> -			die(_("object already configured in another "
> -			      "uploadpack.blobpackfileuri (got '%s')"), v);
> +			die(_("object already configured by an earlier "
> +			      "uploadpack.excludeobject or uploadpack.blobpackfileuri (got '%s')"), v);
>  		ex->pack_hash_hex = xcalloc(1, pack_end - oid_end);
>  		memcpy(ex->pack_hash_hex, oid_end + 1, pack_end - oid_end - 1);
>  		ex->uri = xstrdup(pack_end + 1);
> @@ -3040,7 +3048,7 @@ static int add_object_entry_from_pack(const struct object_id *oid,
>  		return 0;
>  
>  	ofs = nth_packed_object_offset(p, pos);
> -	if (!want_object_in_pack(oid, 0, &p, &ofs))
> +	if (!want_object_in_pack(oid, 0, &p, &ofs, NULL))
>  		return 0;
>  
>  	oi.typep = &type;
> @@ -3233,7 +3241,7 @@ static void read_object_list_from_stdin(void)
>  			die(_("expected object ID, got garbage:\n %s"), line);
>  
>  		add_preferred_base_object(p + 1);
> -		add_object_entry(&oid, OBJ_NONE, p + 1, 0);
> +		add_object_entry(&oid, OBJ_NONE, p + 1, 0, NULL);
>  	}
>  }
>  
> @@ -3242,7 +3250,7 @@ static void read_object_list_from_stdin(void)
>  
>  static void show_commit(struct commit *commit, void *data)
>  {
> -	add_object_entry(&commit->object.oid, OBJ_COMMIT, NULL, 0);
> +	add_object_entry(&commit->object.oid, OBJ_COMMIT, NULL, 0, NULL);
>  	commit->object.flags |= OBJECT_ADDED;
>  
>  	if (write_bitmap_index)
> @@ -3254,8 +3262,9 @@ static void show_commit(struct commit *commit, void *data)
>  
>  static void show_object(struct object *obj, const char *name, void *show_data, void *carry_data)
>  {
> +	struct object *referred_commit = carry_data;
>  	add_preferred_base_object(name);
> -	add_object_entry(&obj->oid, obj->type, name, 0);
> +	add_object_entry(&obj->oid, obj->type, name, 0, referred_commit);
>  	obj->flags |= OBJECT_ADDED;
>  
>  	if (use_delta_islands) {
> @@ -3406,7 +3415,7 @@ static void add_objects_in_unpacked_packs(void)
>  		QSORT(in_pack.array, in_pack.nr, ofscmp);
>  		for (i = 0; i < in_pack.nr; i++) {
>  			struct object *o = in_pack.array[i].object;
> -			add_object_entry(&o->oid, o->type, "", 0);
> +			add_object_entry(&o->oid, o->type, "", 0, NULL);
>  		}
>  	}
>  	free(in_pack.array);
> @@ -3422,7 +3431,7 @@ static int add_loose_object(const struct object_id *oid, const char *path,
>  		return 0;
>  	}
>  
> -	add_object_entry(oid, type, "", 0);
> +	add_object_entry(oid, type, "", 0, NULL);
>  	return 0;
>  }
>  
> @@ -3841,7 +3850,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
>  			 N_("respect islands during delta compression")),
>  		OPT_STRING_LIST(0, "uri-protocol", &uri_protocols,
>  				N_("protocol"),
> -				N_("exclude any configured uploadpack.blobpackfileuri with this protocol")),
> +				N_("exclude any configured uploadpack.excludeobject or "
> +				   "uploadpack.blobpackfileuri with this protocol")),
>  		OPT_END(),
>  	};
>  
> diff --git a/list-objects.c b/list-objects.c
> index 427228a3ba..968d842ceb 100644
> --- a/list-objects.c
> +++ b/list-objects.c
> @@ -24,7 +24,8 @@ struct traversal_context {
>  static void process_blob(struct traversal_context *ctx,
>  			 struct blob *blob,
>  			 struct strbuf *path,
> -			 const char *name)
> +			 const char *name,
> +			 struct object *referred_commit)
>  {
>  	struct object *obj = &blob->object;
>  	size_t pathlen;
> @@ -60,7 +61,7 @@ static void process_blob(struct traversal_context *ctx,
>  	if (r & LOFR_MARK_SEEN)
>  		obj->flags |= SEEN;
>  	if (r & LOFR_DO_SHOW)
> -		ctx->show_object(obj, path->buf, ctx->show_data, NULL);
> +		ctx->show_object(obj, path->buf, ctx->show_data, referred_commit);
>  	strbuf_setlen(path, pathlen);
>  }
>  
> @@ -97,11 +98,13 @@ static void process_gitlink(struct traversal_context *ctx,
>  static void process_tree(struct traversal_context *ctx,
>  			 struct tree *tree,
>  			 struct strbuf *base,
> -			 const char *name);
> +			 const char *name,
> +			 struct object *referred_commit);
>  
>  static void process_tree_contents(struct traversal_context *ctx,
>  				  struct tree *tree,
> -				  struct strbuf *base)
> +				  struct strbuf *base,
> +				  struct object *referred_commit)
>  {
>  	struct tree_desc desc;
>  	struct name_entry entry;
> @@ -129,7 +132,7 @@ static void process_tree_contents(struct traversal_context *ctx,
>  				    entry.path, oid_to_hex(&tree->object.oid));
>  			}
>  			t->object.flags |= NOT_USER_GIVEN;
> -			process_tree(ctx, t, base, entry.path);
> +			process_tree(ctx, t, base, entry.path, referred_commit);
>  		}
>  		else if (S_ISGITLINK(entry.mode))
>  			process_gitlink(ctx, entry.oid.hash,
> @@ -142,7 +145,7 @@ static void process_tree_contents(struct traversal_context *ctx,
>  				    entry.path, oid_to_hex(&tree->object.oid));
>  			}
>  			b->object.flags |= NOT_USER_GIVEN;
> -			process_blob(ctx, b, base, entry.path);
> +			process_blob(ctx, b, base, entry.path, referred_commit);
>  		}
>  	}
>  }
> @@ -150,7 +153,8 @@ static void process_tree_contents(struct traversal_context *ctx,
>  static void process_tree(struct traversal_context *ctx,
>  			 struct tree *tree,
>  			 struct strbuf *base,
> -			 const char *name)
> +			 const char *name,
> +			 struct object *referred_commit)
>  {
>  	struct object *obj = &tree->object;
>  	struct rev_info *revs = ctx->revs;
> @@ -191,14 +195,14 @@ static void process_tree(struct traversal_context *ctx,
>  	if (r & LOFR_MARK_SEEN)
>  		obj->flags |= SEEN;
>  	if (r & LOFR_DO_SHOW)
> -		ctx->show_object(obj, base->buf, ctx->show_data, NULL);
> +		ctx->show_object(obj, base->buf, ctx->show_data, referred_commit);
>  	if (base->len)
>  		strbuf_addch(base, '/');
>  
>  	if (r & LOFR_SKIP_TREE)
>  		trace_printf("Skipping contents of tree %s...\n", base->buf);
>  	else if (!failed_parse)
> -		process_tree_contents(ctx, tree, base);
> +		process_tree_contents(ctx, tree, base, referred_commit);
>  
>  	r = list_objects_filter__filter_object(ctx->revs->repo,
>  					       LOFS_END_TREE, obj,
> @@ -207,7 +211,7 @@ static void process_tree(struct traversal_context *ctx,
>  	if (r & LOFR_MARK_SEEN)
>  		obj->flags |= SEEN;
>  	if (r & LOFR_DO_SHOW)
> -		ctx->show_object(obj, base->buf, ctx->show_data, NULL);
> +		ctx->show_object(obj, base->buf, ctx->show_data, referred_commit);
>  
>  	strbuf_setlen(base, baselen);
>  	free_tree_buffer(tree);
> @@ -314,9 +318,9 @@ void mark_edges_uninteresting(struct rev_info *revs,
>  	}
>  }
>  
> -static void add_pending_tree(struct rev_info *revs, struct tree *tree)
> +static void add_pending_tree(struct rev_info *revs, struct tree *tree, struct object *referred_commit)
>  {
> -	add_pending_object(revs, &tree->object, "");
> +	add_pending_object_with_referred_commit(revs, &tree->object, "", referred_commit);
>  }
>  
>  static void traverse_trees_and_blobs(struct traversal_context *ctx,
> @@ -329,23 +333,24 @@ static void traverse_trees_and_blobs(struct traversal_context *ctx,
>  	for (i = 0; i < ctx->revs->pending.nr; i++) {
>  		struct object_array_entry *pending = ctx->revs->pending.objects + i;
>  		struct object *obj = pending->item;
> +		struct object *referred_commit = pending->referred_commit;
>  		const char *name = pending->name;
>  		const char *path = pending->path;
>  		if (obj->flags & (UNINTERESTING | SEEN))
>  			continue;
>  		if (obj->type == OBJ_TAG) {
>  			obj->flags |= SEEN;
> -			ctx->show_object(obj, name, ctx->show_data, NULL);
> +			ctx->show_object(obj, name, ctx->show_data, referred_commit);
>  			continue;
>  		}
>  		if (!path)
>  			path = "";
>  		if (obj->type == OBJ_TREE) {
> -			process_tree(ctx, (struct tree *)obj, base, path);
> +			process_tree(ctx, (struct tree *)obj, base, path, referred_commit);
>  			continue;
>  		}
>  		if (obj->type == OBJ_BLOB) {
> -			process_blob(ctx, (struct blob *)obj, base, path);
> +			process_blob(ctx, (struct blob *)obj, base, path, referred_commit);
>  			continue;
>  		}
>  		die("unknown pending object %s (%s)",
> @@ -370,7 +375,7 @@ static void do_traverse(struct traversal_context *ctx)
>  		else if (get_commit_tree(commit)) {
>  			struct tree *tree = get_commit_tree(commit);
>  			tree->object.flags |= NOT_USER_GIVEN;
> -			add_pending_tree(ctx->revs, tree);
> +			add_pending_tree(ctx->revs, tree, &commit->object);
>  		} else if (commit->object.parsed) {
>  			die(_("unable to load root tree for commit %s"),
>  			      oid_to_hex(&commit->object.oid));
> diff --git a/object.c b/object.c
> index 14188453c5..6b1ce2fcde 100644
> --- a/object.c
> +++ b/object.c
> @@ -322,9 +322,10 @@ void object_list_free(struct object_list **list)
>   */
>  static char object_array_slopbuf[1];
>  
> -void add_object_array_with_path(struct object *obj, const char *name,
> -				struct object_array *array,
> -				unsigned mode, const char *path)
> +void add_object_array_with_path_and_referred_commit(struct object *obj, const char *name,
> +						    struct object_array *array,
> +						    unsigned mode, const char *path,
> +						    struct object *referred_commit)
>  {
>  	unsigned nr = array->nr;
>  	unsigned alloc = array->alloc;
> @@ -339,6 +340,7 @@ void add_object_array_with_path(struct object *obj, const char *name,
>  	}
>  	entry = &objects[nr];
>  	entry->item = obj;
> +	entry->referred_commit = referred_commit;
>  	if (!name)
>  		entry->name = NULL;
>  	else if (!*name)
> @@ -354,6 +356,13 @@ void add_object_array_with_path(struct object *obj, const char *name,
>  	array->nr = ++nr;
>  }
>  
> +void add_object_array_with_path(struct object *obj, const char *name,
> +				struct object_array *array,
> +				unsigned mode, const char *path)
> +{
> +	add_object_array_with_path_and_referred_commit(obj, name, array, mode, path, NULL);
> +}
> +
>  void add_object_array(struct object *obj, const char *name, struct object_array *array)
>  {
>  	add_object_array_with_path(obj, name, array, S_IFINVALID, NULL);
> diff --git a/object.h b/object.h
> index 87a6da47c8..d63819ab91 100644
> --- a/object.h
> +++ b/object.h
> @@ -52,6 +52,7 @@ struct object_array {
>  		char *name;
>  		char *path;
>  		unsigned mode;
> +		struct object *referred_commit;
>  	} *objects;
>  };
>  
> @@ -157,7 +158,8 @@ void object_list_free(struct object_list **list);
>  /* Object array handling .. */
>  void add_object_array(struct object *obj, const char *name, struct object_array *array);
>  void add_object_array_with_path(struct object *obj, const char *name, struct object_array *array, unsigned mode, const char *path);
> -
> +void add_object_array_with_path_and_referred_commit(struct object *obj, const char *name, struct object_array *array,
> +						    unsigned mode, const char *path, struct object *referred_commit);
>  /*
>   * Returns NULL if the array is empty. Otherwise, returns the last object
>   * after removing its entry from the array. Other resources associated
> diff --git a/revision.c b/revision.c
> index 4853c85d0b..65e0926d25 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -304,10 +304,11 @@ void mark_parents_uninteresting(struct commit *commit)
>  	commit_stack_clear(&pending);
>  }
>  
> -static void add_pending_object_with_path(struct rev_info *revs,
> -					 struct object *obj,
> -					 const char *name, unsigned mode,
> -					 const char *path)
> +static void add_pending_object_with_path_and_referred_commit(struct rev_info *revs,
> +							     struct object *obj,
> +							     const char *name, unsigned mode,
> +							     const char *path,
> +							     struct object *referred_commit)
>  {
>  	struct interpret_branch_name_options options = { 0 };
>  	if (!obj)
> @@ -326,20 +327,35 @@ static void add_pending_object_with_path(struct rev_info *revs,
>  		strbuf_release(&buf);
>  		return; /* do not add the commit itself */
>  	}
> -	add_object_array_with_path(obj, name, &revs->pending, mode, path);
> +	add_object_array_with_path_and_referred_commit(obj, name, &revs->pending, mode, path, referred_commit);
>  }
>  
> +static void add_pending_object_with_path(struct rev_info *revs,
> +					 struct object *obj,
> +					 const char *name, unsigned mode,
> +					 const char *path)
> +{
> +	add_pending_object_with_path_and_referred_commit(revs, obj, name, mode, path, NULL);
> +}
>  static void add_pending_object_with_mode(struct rev_info *revs,
>  					 struct object *obj,
> -					 const char *name, unsigned mode)
> +					 const char *name, unsigned mode,
> +					 struct object *referred_commit)
> +{
> +	add_pending_object_with_path_and_referred_commit(revs, obj, name, mode, NULL, referred_commit);
> +}
> +
> +void add_pending_object_with_referred_commit(struct rev_info *revs,
> +					     struct object *obj, const char *name,
> +					     struct object *referred_commit)
>  {
> -	add_pending_object_with_path(revs, obj, name, mode, NULL);
> +	add_pending_object_with_mode(revs, obj, name, S_IFINVALID, referred_commit);
>  }
>  
>  void add_pending_object(struct rev_info *revs,
>  			struct object *obj, const char *name)
>  {
> -	add_pending_object_with_mode(revs, obj, name, S_IFINVALID);
> +	add_pending_object_with_mode(revs, obj, name, S_IFINVALID, NULL);
>  }
>  
>  void add_head_to_pending(struct rev_info *revs)
> @@ -2817,7 +2833,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>  		if (get_oid_with_context(revs->repo, revs->def, 0, &oid, &oc))
>  			diagnose_missing_default(revs->def);
>  		object = get_reference(revs, revs->def, &oid, 0);
> -		add_pending_object_with_mode(revs, object, revs->def, oc.mode);
> +		add_pending_object_with_mode(revs, object, revs->def, oc.mode, NULL);
>  	}
>  
>  	/* Did the user ask for any diff output? Run the diff! */
> diff --git a/revision.h b/revision.h
> index a24f72dcd1..f9c9628ed8 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -423,6 +423,9 @@ void show_object_with_name(FILE *, struct object *, const char *);
>   */
>  void add_pending_object(struct rev_info *revs,
>  			struct object *obj, const char *name);
> +void add_pending_object_with_referred_commit(struct rev_info *revs,
> +					     struct object *obj, const char *name,
> +					     struct object *referred_commit);
>  
>  void add_pending_oid(struct rev_info *revs,
>  		     const char *name, const struct object_id *oid,
> diff --git a/upload-pack.c b/upload-pack.c
> index 5c1cd19612..d26fb351a3 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -1751,6 +1751,13 @@ int upload_pack_advertise(struct repository *r,
>  			strbuf_addstr(value, " packfile-uris");
>  			free(str);
>  		}
> +
> +		if (!repo_config_get_string(the_repository,
> +					    "uploadpack.excludeobject",
> +					    &str) && str) {
> +			strbuf_addstr(value, " packfile-uris");
> +			free(str);
> +		}
>  	}
>  
>  	return 1;