Re: [RFD PATCH 3/3] name-rev: Allow lightweight tags and branch refs

[Junio C Hamano <gitster@pobox.com>]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> I'm wondering whether I'm overlooking any side-effects that our test
> suite doesn't cover, though. In any case, we may want to have
> lightweight tags allowed based on an extra flag (like the
> existing --tags for describe, which means something else for name-rev).
>
>  builtin/name-rev.c     | 2 ++
>  t/t9903-bash-prompt.sh | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index 8bdc3eaa6f..75ba7bbad5 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -207,6 +207,8 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
>  	if (o && o->type == OBJ_COMMIT) {
>  		struct commit *commit = (struct commit *)o;
>  
> +		if (taggerdate == ULONG_MAX) /* lightweight tag */
> +			taggerdate = commit->date;
>  		path = name_ref_abbrev(path, can_abbreviate_output);
>  		name_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);
>  	}

I think this comment indicates where this change will bite us ;-)  

This could have been an invocation of "name-rev" without "--tags",
where _any_ tip of ref can be used to name a revision, and in such a
case, retaining commit->date may still be valuable, but it is
probably wrong to use it for nothing more than tie-breaking.

In an extreme case, imagine that your repository does not have any
tag and you have a commit that can be reached from both 'maint' and
'master' branches.  Because the current code assigns the same
useless taggerdate for these tips of refs, the commit is described
solely based on the distance from the tip of 'maint' or 'master'
with today's code.

If you assign commit date to taggerdate variable here, that date is
used to trump the distance when deciding whether to name the commit
based on 'maint' and 'master', and I doubt it is what you want.
Perhaps I regularly advance 'master', but still add a few selected
fixes to 'maint' every few days---when these two branches both are
active like that, do you want the same commit that is topologically
much closer to 'maint' than 'master' to be described in terms of
'maint' on some days (i.e. those days I added fixes to it) and
'master' on some other days?

I think the expectation by end-users who do not use "--tags" is that
they still prefer a commit to be named in terms of the oldest tag
that is fewer number of hops away, and the naming to fall back to
non-tag refs only when there is no tags that reach the commit.  And
between two or more non-tag tips that are _known_ to be still active
(iow, branch tips), they want "fewer number of hops away" to be the
primary deciding factor to choose among them.  The ULONG_MAX setting
the code currently uses is very suitable for that purpose.  It
penalizes the non-tag refs, and it disables the use of timestamp for
deciding among these tips and uses only the topological distance.

I think you can do this change _ONLY_ when we are operating under
the "--tags" option.  That would place unannotated tags at a better
location in the timestamp order than the current code does, without
introducing issues with refs that are actively moving.