A few fast export fixups -- round 3

A few fast export fixups -- round 3

[newren]

This patch series fixes a few fast-export bugs I have come across,
plus some new testcases to verify the fixes, a testcase to document a
pre-existing failure, and finally a small addition to the
documentation.

Changes since v1:
  - fixed test issues identified by Johannes Sixt and Stephen Boyd
  - squashed patch #2 into patch #1

Changes since v2:
  - several changes and improvements suggested by Junio (moving code
    to different functions, avoiding exporting functionality from
    revision.c, improved commit messages)
  - new patch to ensure invariants created by setup_revisions are
    respected (set revs flags before calling setup_revisions, rather
    than after)
  - now also catches and omits tags of tags of trees, tags of tags of
    tags of trees, etc.
  - remove some now-obsoleted code
  - There are two new cases I'm aware of in which fast-export produces
    broken "(null)" refs; I'm not sure how/where to fix these (or even
    what correct behavior is in one of the cases), but they are no worse
    than before and I've at least added a test documenting that we know
    that there's a case that is (still) broken.

Elijah Newren (7):
      fast-export: Set revs.topo_order before calling setup_revisions
      fast-export: Omit tags that tag trees
      fast-export: Make sure we show actual ref names instead of "(null)"
      fast-export: Do parent rewriting to avoid dropping relevant commits
      fast-export: Add a --tag-of-filtered-object option for newly dangling tags
      Add new fast-export testcases
      fast-export: Document the fact that git-rev-list arguments are accepted

 Documentation/git-fast-export.txt |   17 +++++++
 builtin-fast-export.c             |   82 +++++++++++++++++++++++++++++---
 t/t9301-fast-export.sh            |   96 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 187 insertions(+), 8 deletions(-)

[PATCHv3 1/7] fast-export: Set revs.topo_order before calling setup_revisions

[newren]

From: Elijah Newren <newren@gmail.com>

setup_revisions sets a variety of flags based on the setting of other
flags, such as setting the limited flag when topo_order is set.  To avoid
circumventing any invariants created by setup_revisions, we set
revs.topo_order before calling it rather than after.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
This patch is new since the last series.

 builtin-fast-export.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 6cef810..e0cfa60 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -514,6 +514,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	git_config(git_default_config, NULL);
 
 	init_revisions(&revs, prefix);
+	revs.topo_order = 1;
 	argc = setup_revisions(argc, argv, &revs, NULL);
 	argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);
 	if (argc > 1)
@@ -524,7 +525,6 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 
 	get_tags_and_duplicates(&revs.pending, &extra_refs);
 
-	revs.topo_order = 1;
 	if (prepare_revision_walk(&revs))
 		die("revision walk setup failed");
 	revs.diffopt.format_callback = show_filemodify;
-- 
1.6.3.2.323.geb889

[PATCHv3 2/7] fast-export: Omit tags that tag trees

[newren]

From: Elijah Newren <newren@gmail.com>

Commit c0582c53bcf4e83bba70e1ad23abbad31f96ebc8 introduced logic to just
omit tags that point to tree objects.  However, these objects were still
being output and were pointing at "mark :0", which caused fast-import to
crash.  This patch makes sure such tags (including deeper nestings such
as tags of tags of trees), are omitted.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
Since the last series, the code has been moved to the handle_tag
function, as suggested by Junio.  Also, it now correctly catches and
omits tags of tags of trees, tags of tags of tags of trees, etc.

 builtin-fast-export.c  |   15 +++++++++++++++
 t/t9301-fast-export.sh |    8 +++++++-
 2 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index e0cfa60..8c90a2d 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -289,6 +289,21 @@ static void handle_tag(const char *name, struct tag *tag)
 	char *buf;
 	const char *tagger, *tagger_end, *message;
 	size_t message_size = 0;
+	struct object *tagged;
+
+	/* Trees have no identifer in fast-export output, thus we have no way
+	 * to output tags of trees, tags of tags of trees, etc.  Simply omit
+	 * such tags.
+	 */
+	tagged = tag->tagged;
+	while (tagged->type == OBJ_TAG) {
+		tagged = ((struct tag *)tagged)->tagged;
+	}
+	if (tagged->type == OBJ_TREE) {
+		warning("Omitting tag %s,\nsince tags of trees (or tags of tags of trees, etc.) are not supported.",
+			sha1_to_hex(tag->object.sha1));
+		return;
+	}
 
 	buf = read_sha1_file(tag->object.sha1, &type, &size);
 	if (!buf)
diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 8c8a9e6..3f13e6b 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -271,8 +271,14 @@ test_expect_success 'set-up a few more tags for tag export tests' '
 	git tag -a tag-obj_tag-obj -m "tagging a tag" tree_tag-obj
 '
 
+test_expect_success 'tree_tag'        '
+	mkdir result &&
+	(cd result && git init) &&
+	git fast-export tree_tag > fe-stream &&
+	(cd result && git fast-import < ../fe-stream)
+'
+
 # NEEDSWORK: not just check return status, but validate the output
-test_expect_success 'tree_tag'        'git fast-export tree_tag'
 test_expect_success 'tree_tag-obj'    'git fast-export tree_tag-obj'
 test_expect_success 'tag-obj_tag'     'git fast-export tag-obj_tag'
 test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'
-- 
1.6.3.2.323.geb889

[PATCHv3 3/7] fast-export: Make sure we show actual ref names instead of "(null)"

[newren]

From: Elijah Newren <newren@gmail.com>

The code expects a ref name to be provided in commit->util.  While there
was some code to set commit->util, it only worked in cases where there was
an unbroken chain of revisions from a ref to the relevant commit.  In
cases such as running
   git fast-export --parents master -- COPYING
commit->util would fail to be set.  The old method of setting commit->util
has been removed in favor of requesting show_source from the revision
traversal machinery (related to the "--source" option of "git log" family
of commands.)

However, this change does not fix cases like
   git fast export master~1
or
   git fast export :/arguments
since in such cases commit->util will be "master~1" or ":/arguments" while
we need the actual ref (e.g. "refs/heads/master")

Signed-off-by: Elijah Newren <newren@gmail.com>
---
Since the last series, some now-obsolete code has been removed.  Also,
the commit message now points out two cases that continue to fail with
this patch.  For the second of those cases (the :/arguments one), I'm
not sure that I even know what correct behavior would be.

 builtin-fast-export.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 8c90a2d..43a7e17 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -530,6 +530,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 
 	init_revisions(&revs, prefix);
 	revs.topo_order = 1;
+	revs.show_source = 1;
 	argc = setup_revisions(argc, argv, &revs, NULL);
 	argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);
 	if (argc > 1)
@@ -546,11 +547,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	DIFF_OPT_SET(&revs.diffopt, RECURSIVE);
 	while ((commit = get_revision(&revs))) {
 		if (has_unshown_parent(commit)) {
-			struct commit_list *parent = commit->parents;
 			add_object_array(&commit->object, NULL, &commits);
-			for (; parent; parent = parent->next)
-				if (!parent->item->util)
-					parent->item->util = commit->util;
 		}
 		else {
 			handle_commit(commit, &revs);
-- 
1.6.3.2.323.geb889

[PATCHv3 4/7] fast-export: Do parent rewriting to avoid dropping relevant commits

[newren]

From: Elijah Newren <newren@gmail.com>

When specifying paths to export, parent rewriting must be turned on for
fast-export to output anything at all.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
No change to this patch since the last series.

 builtin-fast-export.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 43a7e17..9b8bd37 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -531,6 +531,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	init_revisions(&revs, prefix);
 	revs.topo_order = 1;
 	revs.show_source = 1;
+	revs.rewrite_parents = 1;
 	argc = setup_revisions(argc, argv, &revs, NULL);
 	argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);
 	if (argc > 1)
-- 
1.6.3.2.323.geb889

[PATCHv3 5/7] fast-export: Add a --tag-of-filtered-object option for newly dangling tags

[newren]

From: Elijah Newren <newren@gmail.com>

When providing a list of paths to limit what is exported, the object that
a tag points to can be filtered out entirely.  This new switch allows
the user to specify what should happen to the tag in such a case.  The
default action, 'abort' will exit with an error message.  With 'drop', the
tag will simply be omitted from the output.  With 'rewrite', if the object
tagged was a commit, the tag will be modified to tag an alternate commit.
The alternate commit is determined by treating the original commit as the
"parent" of the tag and then using the parent rewriting algorithm of the
revision traversal machinery (related to the "--parents" option of "git
rev-list")

Signed-off-by: Elijah Newren <newren@gmail.com>
---
Since the last series, this patch was changed to simply copy the
relevant code from rewrite_one() rather than exporting that function
out of revision.c, as requested by Junio.  The commit message was also
clarified a bit.

 Documentation/git-fast-export.txt |   11 +++++++
 builtin-fast-export.c             |   59 +++++++++++++++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 0c9eb56..194abde 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -36,6 +36,17 @@ when encountering a signed tag.  With 'strip', the tags will be made
 unsigned, with 'verbatim', they will be silently exported
 and with 'warn', they will be exported, but you will see a warning.
 
+--tag-of-filtered-object=(abort|drop|rewrite)::
+	Specify how to handle tags whose tagged objectis filtered out.
+	Since revisions and files to export can be limited by path,
+	tagged objects may be filtered completely.
++
+When asking to 'abort' (which is the default), this program will die
+when encountering such a tag.  With 'drop' it will omit such tags from
+the output.  With 'rewrite', if the tagged object is a commit, it will
+rewrite the tag to tag an ancestor commit (via parent rewriting; see
+linkgit:git-rev-list[1])
+
 -M::
 -C::
 	Perform move and/or copy detection, as described in the
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 9b8bd37..dc2c6ab 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -23,7 +23,8 @@ static const char *fast_export_usage[] = {
 };
 
 static int progress;
-static enum { VERBATIM, WARN, STRIP, ABORT } signed_tag_mode = ABORT;
+static enum { ABORT, VERBATIM, WARN, STRIP } signed_tag_mode = ABORT;
+static enum { ERROR, DROP, REWRITE } tag_of_filtered_mode = ABORT;
 static int fake_missing_tagger;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
@@ -42,6 +43,20 @@ static int parse_opt_signed_tag_mode(const struct option *opt,
 	return 0;
 }
 
+static int parse_opt_tag_of_filtered_mode(const struct option *opt,
+					  const char *arg, int unset)
+{
+	if (unset || !strcmp(arg, "abort"))
+		tag_of_filtered_mode = ABORT;
+	else if (!strcmp(arg, "drop"))
+		tag_of_filtered_mode = DROP;
+	else if (!strcmp(arg, "rewrite"))
+		tag_of_filtered_mode = REWRITE;
+	else
+		return error("Unknown tag-of-filtered mode: %s", arg);
+	return 0;
+}
+
 static struct decoration idnums;
 static uint32_t last_idnum;
 
@@ -290,6 +305,8 @@ static void handle_tag(const char *name, struct tag *tag)
 	const char *tagger, *tagger_end, *message;
 	size_t message_size = 0;
 	struct object *tagged;
+	int tagged_mark;
+	struct commit *p;
 
 	/* Trees have no identifer in fast-export output, thus we have no way
 	 * to output tags of trees, tags of tags of trees, etc.  Simply omit
@@ -348,10 +365,45 @@ static void handle_tag(const char *name, struct tag *tag)
 			}
 	}
 
+	/* handle tag->tagged having been filtered out due to paths specified */
+	tagged = tag->tagged;
+	tagged_mark = get_object_mark(tagged);
+	if (!tagged_mark) {
+		switch(tag_of_filtered_mode) {
+		case ABORT:
+			die ("Tag %s tags unexported object; use "
+			     "--tag-of-filtered-object=<mode> to handle it.",
+			     sha1_to_hex(tag->object.sha1));
+		case DROP:
+			/* Ignore this tag altogether */
+			return;
+		case REWRITE:
+			if (tagged->type != OBJ_COMMIT) {
+				die ("Tag %s tags unexported %s!",
+				     sha1_to_hex(tag->object.sha1),
+				     typename(tagged->type));
+			}
+			p = (struct commit *)tagged;
+			for (;;) {
+				if (p->parents && p->parents->next)
+					break;
+				if (p->object.flags & UNINTERESTING)
+					break;
+				if (!(p->object.flags & TREESAME))
+					break;
+				if (!p->parents)
+					die ("Can't find replacement commit for tag %s\n",
+					     sha1_to_hex(tag->object.sha1));
+				p = p->parents->item;
+			}
+			tagged_mark = get_object_mark(&p->object);
+		}
+	}
+
 	if (!prefixcmp(name, "refs/tags/"))
 		name += 10;
 	printf("tag %s\nfrom :%d\n%.*s%sdata %d\n%.*s\n",
-	       name, get_object_mark(tag->tagged),
+	       name, tagged_mark,
 	       (int)(tagger_end - tagger), tagger,
 	       tagger == tagger_end ? "" : "\n",
 	       (int)message_size, (int)message_size, message ? message : "");
@@ -513,6 +565,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		OPT_CALLBACK(0, "signed-tags", &signed_tag_mode, "mode",
 			     "select handling of signed tags",
 			     parse_opt_signed_tag_mode),
+		OPT_CALLBACK(0, "tag-of-filtered-object", &tag_of_filtered_mode, "mode",
+			     "select handling of tags that tag filtered objects",
+			     parse_opt_tag_of_filtered_mode),
 		OPT_STRING(0, "export-marks", &export_filename, "FILE",
 			     "Dump marks to this file"),
 		OPT_STRING(0, "import-marks", &import_filename, "FILE",
-- 
1.6.3.2.323.geb889

[PATCHv3 6/7] Add new fast-export testcases

[newren]

From: Elijah Newren <newren@gmail.com>

The testcases test the new --tag-of-filtered-object option, the output
when limiting what to export by path, and test behavior when no
exact-ref revision is included (e.g. master~8 present on command line
but not master).

Signed-off-by: Elijah Newren <newren@gmail.com>
---
Since the last series, one new testcase was added to document that running
with arguments like 'master~3' or 'master~7..master~2' are still broken.

 t/t9301-fast-export.sh |   88 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 88 insertions(+), 0 deletions(-)

diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 3f13e6b..356964e 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -262,6 +262,94 @@ test_expect_success 'cope with tagger-less tags' '
 
 '
 
+test_expect_success 'setup for limiting exports by PATH' '
+	mkdir limit-by-paths &&
+	cd limit-by-paths &&
+	git init &&
+	echo hi > there &&
+	git add there &&
+	git commit -m "First file" &&
+	echo foo > bar &&
+	git add bar &&
+	git commit -m "Second file" &&
+	git tag -a -m msg mytag &&
+	echo morefoo >> bar &&
+	git add bar &&
+	git commit -m "Change to second file" &&
+	cd ..
+'
+
+cat > limit-by-paths/expected << EOF
+blob
+mark :1
+data 3
+hi
+
+reset refs/tags/mytag
+commit refs/tags/mytag
+mark :2
+author A U Thor <author@example.com> 1112912713 -0700
+committer C O Mitter <committer@example.com> 1112912713 -0700
+data 11
+First file
+M 100644 :1 there
+
+EOF
+
+test_expect_success 'dropping tag of filtered out object' '
+	cd limit-by-paths &&
+	git fast-export --tag-of-filtered-object=drop mytag -- there > output &&
+	test_cmp output expected &&
+	cd ..
+'
+
+cat >> limit-by-paths/expected << EOF
+tag mytag
+from :2
+tagger C O Mitter <committer@example.com> 1112912713 -0700
+data 4
+msg
+
+EOF
+
+test_expect_success 'rewriting tag of filtered out object' '
+	cd limit-by-paths &&
+	git fast-export --tag-of-filtered-object=rewrite mytag -- there > output &&
+	test_cmp output expected &&
+	cd ..
+'
+
+cat > limit-by-paths/expected << EOF
+blob
+mark :1
+data 4
+foo
+
+blob
+mark :2
+data 3
+hi
+
+reset refs/heads/master
+commit refs/heads/master
+mark :3
+author A U Thor <author@example.com> 1112912713 -0700
+committer C O Mitter <committer@example.com> 1112912713 -0700
+data 12
+Second file
+M 100644 :1 bar
+M 100644 :2 there
+
+EOF
+
+test_expect_failure 'no exact-ref revisions included' '
+	cd limit-by-paths &&
+	git fast-export master~2..master~1 > output &&
+	test_cmp output expected &&
+	cd ..
+'
+
+
 test_expect_success 'set-up a few more tags for tag export tests' '
 	git checkout -f master &&
 	HEAD_TREE=`git show -s --pretty=raw HEAD | grep tree | sed "s/tree //"` &&
-- 
1.6.3.2.323.geb889

[PATCHv3 7/7] fast-export: Document the fact that git-rev-list arguments are accepted

[newren]

From: Elijah Newren <newren@gmail.com>


Signed-off-by: Elijah Newren <newren@gmail.com>
---
No change since the last series.

 Documentation/git-fast-export.txt |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 194abde..af2328d 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -82,6 +82,12 @@ marks the same across runs.
 	allow that.  So fake a tagger to be able to fast-import the
 	output.
 
+[git-rev-list-args...]::
+       A list of arguments, acceptable to 'git-rev-parse' and
+       'git-rev-list', that specifies the specific objects and references
+       to export.  For example, `master\~10..master` causes the
+       current master reference to be exported along with all objects
+       added since its 10th ancestor commit.
 
 EXAMPLES
 --------
-- 
1.6.3.2.323.geb889