Re: [PATCH 2/4] blame: validate and peel the object names on the ignore list

Junio C Hamano [off-list ref] writes:

quoted

quoted

@@ -66,7 +72,8 @@ void oidset_parse_file(struct oidset *set, const char *path)
 		if (!sb.len)
 			continue;

-		if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0')
+		if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0' ||
+		    (fn && fn(&oid, cbdata)))

OK, so this turns the basic all-I-know-is-hashes oidset loader into a
flexible higher-order map function.  Fun, but wise?  Can't make up my
mind.

Fun and probably useful.  It is a different matter if it is wise to
use it to (1) peel tags to commits and (2) fail on an nonexistent
object.  My take on them is (1) is probably true, and (2) is Meh ;-)

If we choose to do (2) differently, we only need the following
one-liner patch.  I might suggest tweaking the semantics of the
callback function a bit to allow it to tell the caller (i.e.
oidset_parse_file_carefully()) that it wants to go on as usual, it
wants to omit the object from the hashtable, it replaced the given
object to something else, or it detected an error and wants to
abort, and if we were doing that, we'd be returning "do not add this
object to the table" signal, instead of 0 that signals "we are good
doing business as usual", from here.


 builtin/blame.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index baa5d979cc..8d7b66e970 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c

@@ -817,8 +817,19 @@ static int peel_to_commit_oid(struct object_id *oid_ret, void *cbdata)
 			oidcpy(oid_ret, &oid);
 			return 0;
 		}
+
+		/*
+		 * We can ignore a request to ignore any nonexistent
+		 * objects, trees and blobs by not doing anything
+		 * special, as the blame machinery works with commits,
+		 * so entries in the hashtable from these objects will
+		 * never be looked up.  But we do allow dereferencing
+		 * an annotated tag, as silently ignoring a request to
+		 * ignore v1.0.0 because it is an annotated tag is a
+		 * bit too unfriendly to end-users.
+		 */
 		if (kind != OBJ_TAG)
-			return -1;
+			return 0;
 		obj = deref_tag(r, parse_object(r, &oid), NULL, 0);
 		oidcpy(&oid, &obj->oid);
 	}