Re: [PATCH v2] fast-export: Omit tags that tag trees

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

newren@gmail.com writes:

> From: Elijah Newren <newren@gmail.com>
>
> Commit c0582c53bcf4e83bba70e1ad23abbad31f96ebc8 introduced logic to just
> omit tags that point to tree objects.  However, a case was missed
> resulting in a tag being output which pointed at "mark :0", which would
> cause fast-import to crash.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> Includes changes suggested by Stephen Boyd (squashing commits together
> and moving the NEEDSWORK comment to where it is still relevant) and
> Johannes Sixt (fixing the testcase to remain in the right directory
> even if git init fails and to catch failures in git fast-export).
>
>  builtin-fast-export.c  |    8 +++++++-
>  t/t9301-fast-export.sh |    8 +++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/builtin-fast-export.c b/builtin-fast-export.c
> index 6cef810..891e2d4 100644
> --- a/builtin-fast-export.c
> +++ b/builtin-fast-export.c
> @@ -407,9 +407,15 @@ static void handle_tags_and_duplicates(struct string_list *extra_refs)
>  	for (i = extra_refs->nr - 1; i >= 0; i--) {
>  		const char *name = extra_refs->items[i].string;
>  		struct object *object = extra_refs->items[i].util;
> +		struct tag *tag;
>  		switch (object->type) {
>  		case OBJ_TAG:
> -			handle_tag(name, (struct tag *)object);
> +			tag = (struct tag *)object;
> +			if (tag->tagged->type == OBJ_TREE) {
> +				/* Ignore this tag altogether */
> +				return;
> +			}
> +			handle_tag(name, tag);
>  			break;

This seems to be the only callsite of handle_tag(), and I wonder why the
above special case is not in the called function instead.

If a newer version of fast-import ever supports a tag that points at
non-commits, a patch to add the corresponding support on the export side
logically should go to handle_tag(), no?