Re: [PATCH 04/10] fast-export: avoid dying when filtering by paths and old tags exist

From: Jeff King <peff@peff.net>
Date: Nov 11, 2018, 06:44 UTC
Message-ID: <20181111064442.GD30850@sigill.intra.peff.net>
In-Reply-To: <20181111062312.16342-5-newren@gmail.com>

On Sat, Nov 10, 2018 at 10:23:06PM -0800, Elijah Newren wrote:

Show 12 quoted lines

> If --tag-of-filtered-object=rewrite is specified along with a set of
> paths to limit what is exported, then any tags pointing to old commits
> that do not contain any of those specified paths cause problems.  Since
> the old tagged commit is not exported, fast-export attempts to rewrite
> such tags to an ancestor commit which was exported.  If no such commit
> exists, then fast-export currently die()s.  Five years after the tag
> rewriting logic was added to fast-export (see commit 2d8ad4691921,
> "fast-export: Add a --tag-of-filtered-object  option for newly dangling
> tags", 2009-06-25), fast-import gained the ability to delete refs (see
> commit 4ee1b225b99f, "fast-import: add support to delete refs",
> 2014-04-20), so now we do have a valid option to rewrite the tag to.
> Delete these tags instead of dying.

Hmm. That's the right thing to do if we're considering the export to be an independent unit. But what if I'm just rewriting a portion of history like:

  git fast-export HEAD~5..HEAD | some_filter | git fast-import

? If I have a tag pointing to HEAD~10, will this delete that? Ideally I think it would be left alone.

Show 6 quoted lines

> +test_expect_success 'rewrite tag predating pathspecs to nothing' '
> +	test_create_repo rewrite_tag_predating_pathspecs &&
> +	(
> +		cd rewrite_tag_predating_pathspecs &&
> +
> +		touch ignored &&

We usually prefer ">ignored" to create an empty file rather than "touch".

> +		git add ignored &&
> +		test_commit initial &&

What do we need this "ignored" for? test_commit should create a file "initial.t".

> +		echo foo >bar &&
> +		git add bar &&
> +		test_commit add-bar &&

Likewise, "test_commit bar" should work by itself (though note the filename is "bar.t" in your fast-export command).

> +		git fast-export --tag-of-filtered-object=rewrite --all -- bar >output &&
> +		grep -A 1 refs/tags/v0.0.0.0.0.0.1 output | grep -E ^from.0{40}

I don't think "grep -A" is portable (and we don't seem to otherwise use it). You can probably do something similar with sed.

Use $ZERO_OID instead of hard-coding 40, which future-proofs for the hash transition (though I suppose the hash is not likely to get _shorter_ ;) ).

-Peff