Re: [PATCH v6 03/11] ref-filter: implement '--points-at' option

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 29, 2015, 17:40 UTC
Message-ID: <xmqqmvzibegt.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1435222633-32007-3-git-send-email-karthik.188@gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 23 quoted lines

> +/*
> + * Given a ref (sha1, refname) see if it points to one of the sha1s
> + * in a sha1_array.
> + */
> +static int match_points_at(struct sha1_array *points_at, const unsigned char *sha1,
> +			   const char *refname)
> +{
> +	struct object *obj;
> +
> +	if (!points_at || !points_at->nr)
> +		return 1;
> +
> +	if (sha1_array_lookup(points_at, sha1) >= 0)
> +		return 1;
> +
> +	obj = parse_object_or_die(sha1, refname);
> +	if (obj->type == OBJ_TAG &&
> +	    sha1_array_lookup(points_at, ((struct tag *)obj)->tagged->sha1) >= 0)
> +		return 1;
> +
> +	return 0;
> +}
> +

Interesting. I think the change done while copying the code does not change anything from the original (other than that the helper lost its ability to return the peeled object name), and I think you shouldn't make any change while copying the code that would change the benaviour, but I notice a few things that we might want to keep in mind and revisit them later (i.e. might be a good idea to add NEEDSWORK comment to record them near the function):

 - The original only peeled one level of indirection, so does this
   implementation.  But is that really what we want, I wonder?

   After doing:

   $ git tag -a -m 'annotated' atag $commit
   $ git tag -a -m 'annotated doubly' dtag atag

   atag^0, dtag^0 and $commit all refer to the same commit object.
   Do we want to miss dtag with --point-at=$commit?

 - As we are in for-each-ref (or eventually tag -l) that is walking
   the cached refs, we may know what refname peels to without
   parsing the object at all.  Could it be more efficient to ask
   peel_ref() for the pointee without doing parse_object()
   ourselves?