Re: [PATCH] Add git-findtags

From: Junio C Hamano <junkio@cox.net>
Date: Oct 12, 2005, 05:19 UTC
Message-ID: <7vek6rmhw5.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <1129087169926-git-send-email-martin@catalyst.net.nz>

Martin Langhoff <martin@catalyst.net.nz> writes:

> +my @tagfiles   = `find $git_dir/refs/tags -follow -type f`; # haystack

Please do this with File::Find(), unless you have a compelling reason not to. $git_dir could contain shell metacharacters and/or whitespace.

> +    # grab the first 2 lines (the whole tag could be large)
> +    my $tagobj = `git-cat-file tag $tagid | head -n2 `;

Careful; $tagfile could be a lightweight tag, and you would want to consider it a hit if ($tagid eq $target).

BTW, does `` in Perl behave well upon SIGPIPE (hehe, we now have Merlyn on the list so I can ask these silly Perl questions ;-))?

> +    if ($tagobj =~  m/^type commit$/m) { # only deal with commits
> +
> +	if ($tagobj =~ m/^object $target$/m) { # match on the commit
> +	    print basename($tagfile) . "\n";

Please show "tags/bl/ah" when ".git/refs/tags/bl/ah" points at the target, not just "ah". Alternatively, "bl/ah" may also be acceptable, since this command is about tags.

> +	} elsif ( $opt_t &&                      # follow the commit
> +		 $tagobj =~ m/^object (\S+)$/m) { # and try to match trees

I am not sure how useful '-t' would be in practice. If it is, I wonder if it would also be useful to look for a subtree match..

> +sub quickread {
> +    my $file = shift;
> +    local $/; undef $/; # slurp mode

Nit -- "local $/;" without "undef $/;" would do just fine.

BTW, wouldn't it be easier for this particular script, and more useful in general, if something like what 'git-rev-parse' does for commit objects when given "REV^0" is supported for tags?

I am uncertain the about syntax/notation, but just like "^0" is a postfix operator for "peel the onion repeatedly until you get a commit, or barf if you ended up with a non-commit", maybe if we had "peel the onion repeatedly until you get a non-tag" operator, let's call it '%', then you could say something like this:

        # This is pseudo-code -- it does not handle tagname with
        # embedded $IFS letter very well.

        # list local refs
	git-rev-parse --symbolic --all |

        # limit only to tags
        sed -ne '/^refs\/tags\/p' |
        while read tagname
        do
        	if test $(git-rev-parse --verify "${tagname}%") = "$target"
		then
                	echo "$tagname"
		fi
	done