Thread (2 messages) 2 messages, 2 authors, 2017-06-28

Re: [PATCH 1/3] list-objects: add filter_blob to traverse_commit_list

From: Junio C Hamano <hidden>
Date: 2017-06-28 16:23:43

Jeff Hostetler [off-list ref] writes:
quoted hunk
diff --git a/list-objects.c b/list-objects.c
index f3ca6aa..c9ca81c 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -24,11 +25,28 @@ static void process_blob(struct rev_info *revs,
 		die("bad blob object");
 	if (obj->flags & (UNINTERESTING | SEEN))
 		return;
-	obj->flags |= SEEN;
 
 	pathlen = path->len;
 	strbuf_addstr(path, name);
-	show(obj, path->buf, cb_data);
+	if (!filter_blob) {
+		/*
+		 * Normal processing is to imediately dedup blobs
+		 * during commit traversal, regardless of how many
+		 * times it appears in a single or multiple commits,
+		 * so we always set SEEN.
+		 */
+		obj->flags |= SEEN;
+		show(obj, path->buf, cb_data);
+	} else {
+		/*
+		 * Use the filter-proc to decide whether to show
+		 * the blob.  We only set SEEN if requested.  For
+		 * example, this could be used to omit a specific
+		 * blob until it appears with a ".git*" entryname.
+		 */
+		if (filter_blob(obj, path->buf, &path->buf[pathlen], cb_data))
+			obj->flags |= SEEN;
+	}
This somehow looks a bit surprising organization and division of
responsibility.  I would have expected

	if (!filter_blob || 
	    filter_blob(obj, path->buf, &path->buf[pathlen], cb_data) {
		obj->flags |= SEEN;
		show(obj, path->buf, cb_data);
	}

i.e. making the filter function responsible for only making a
decision to include or exclude, not giving it a chance to decide to
"show" anything different.
quoted hunk
@@ -67,6 +85,7 @@ static void process_gitlink(struct rev_info *revs,
 static void process_tree(struct rev_info *revs,
 			 struct tree *tree,
 			 show_object_fn show,
+			 filter_blob_fn filter_blob,
 			 struct strbuf *base,
 			 const char *name,
 			 void *cb_data)
@@ -111,7 +130,7 @@ static void process_tree(struct rev_info *revs,
 		if (S_ISDIR(entry.mode))
 			process_tree(revs,
 				     lookup_tree(entry.oid->hash),
-				     show, base, entry.path,
+				     show, filter_blob, base, entry.path,
 				     cb_data);
 		else if (S_ISGITLINK(entry.mode))
 			process_gitlink(revs, entry.oid->hash,
I wonder if we'll need filter_tree_fn in the future in this
codepath.  When somebody wants to do a "narrow fetch/clone", would
the approach taken by this series, i.e. decide not to show certain
objects during the "rev-list --objects" traversal, a good precedent
to follow?  Would this approach be a good foundation to build on
such a future?

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help