[PATCH 0/2] non-incremental mode for fast-export

[PATCH 0/2] non-incremental mode for fast-export

[newren]

This patch series fixes silently dropped files in uses of fast-export
involving both --import-marks and limiting files by path.  It also
adds a new feature: a --full-tree option to switch from incremental to
comprehensive handling of files in commits.  The two changes are
related in that the bugfix consists of simply automatically activating
the new feature in a case where it is required for correct operation.

This obsoletes my previous (single) patch submission about the
--full-tree option.  Changes since that submission include splitting
this into two patches, automatically enabling the feature when needed
for correct behavior, and providing more explicit testcases that
probably explain the feature better.

Since we're in feature freeze, it may make sense to split this patch
series so that the first patch can be applied now and keep the second
patch in pu until feature freeze is over (the series is currently
based on maint).  Since the second patch depends on the first, though,
I thought it made sense to submit them together; let me know if you'd
rather I submitted them in some other manner.

(I'm not sure who to cc on this; other than Dscho who is out, I appear
to be the biggest contributor to fast-export.)

Elijah Newren (2):
      fast-export: Fix dropping of files with --import-marks and path limiting
      fast-export: Add a --full-tree option

 Documentation/git-fast-export.txt |    6 +++
 builtin/fast-export.c             |   11 +++++-
 t/t9350-fast-export.sh            |   79 +++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 1 deletions(-)

[PATCHv2 1/2] fast-export: Fix dropping of files with --import-marks and path limiting

[newren]

From: Elijah Newren <newren@gmail.com>

Since fast-export operates by listing file changes since the (first) parent
commit, when using --import-marks and path limiting and using a wider list
of paths than in previous runs, files from the new path(s) will silently be
omitted from the result unless or until a commit which explicitly changes
those files.  The resulting repository in such cases is broken and makes no
sense.

This commit fixes this by having fast-export work with complete trees
instead of incremental changes (when both --import-marks and path limiting
are used).  It works by issuing a 'deleteall' directive with each commit and
then listing the full set of files that make up that commit, rather than
just showing the list of files that have changed since the (first) parent
commit.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  |    9 ++++++++-
 t/t9350-fast-export.sh |    9 +++++++++
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c6dd71a..25d13a1 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -27,6 +27,7 @@ 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 no_data;
+static int full_tree = 0;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -241,7 +242,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 		message += 2;
 
 	if (commit->parents &&
-	    get_object_mark(&commit->parents->item->object) != 0) {
+	    get_object_mark(&commit->parents->item->object) != 0 &&
+	    !full_tree) {
 		parse_commit(commit->parents->item);
 		diff_tree_sha1(commit->parents->item->tree->object.sha1,
 			       commit->tree->object.sha1, "", &rev->diffopt);
@@ -281,6 +283,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 		i++;
 	}
 
+	if (full_tree)
+		printf("deleteall\n");
 	log_tree_diff_flush(rev);
 	rev->diffopt.output_format = saved_output_format;
 
@@ -608,6 +612,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (import_filename)
 		import_marks(import_filename);
 
+	if (import_filename && revs.prune_data)
+		full_tree = 1;
+
 	get_tags_and_duplicates(&revs.pending, &extra_refs);
 
 	if (prepare_revision_walk(&revs))
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index d43f37c..6069e1f 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -355,6 +355,15 @@ test_expect_failure 'no exact-ref revisions included' '
 	)
 '
 
+test_expect_success 'path limiting with import-marks does not lose unmodified files'        '
+	git checkout -b simple marks~2 &&
+	git fast-export --export-marks=marks simple -- file > /dev/null &&
+	echo more content >> file &&
+	test_tick &&
+	git commit -mnext file &&
+	git fast-export --import-marks=marks simple -- file file0 | grep file0
+'
+
 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.7.2.rc1.14.g19914

[PATCHv2 2/2] fast-export: Add a --full-tree option

[newren]

From: Elijah Newren <newren@gmail.com>

This option adds symmetry with fast-import, enabling it to also work with
complete trees instead of just incremental changes.  It works by issuing a
'deleteall' directive with each commit and then listing the full set of
files that make up that commit, rather than just showing the list of files
that have changed since the (first) parent commit.  Note that this
functionality is automatically turned on when using --import-marks together
with path limiting in order to avoid dropping important but unchanged
files.

This functionality is desired when using hand-written filters along with
'fast-export | some-filter | fast-import' as it can be easier to write
<some-filter> in terms of complete trees than incremental changes.

We could avoid the need to add this option by simply always turning it on.
While the end result would be identical, it would slow things down slightly
by printing many more filenames per commit which goes somewhat against the
'fast' in 'fast-export'.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-export.txt |    6 +++
 builtin/fast-export.c             |    4 ++-
 t/t9350-fast-export.sh            |   70 +++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 98ec6b5..8a6a3cb 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -90,6 +90,12 @@ marks the same across runs.
 	resulting stream can only be used by a repository which
 	already contains the necessary objects.
 
+--full-tree::
+	This option will cause fast-export to issue a "deleteall"
+	directive for each commit followed by a full list of all files
+	in the commit (as opposed to just listing the files which are
+	different from the commit's first parent).
+
 [git-rev-list-args...]::
        A list of arguments, acceptable to 'git rev-parse' and
        'git rev-list', that specifies the specific objects and references
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 25d13a1..8c77602 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -27,7 +27,7 @@ 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 no_data;
-static int full_tree = 0;
+static int full_tree;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -588,6 +588,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			     "Import marks from this file"),
 		OPT_BOOLEAN(0, "fake-missing-tagger", &fake_missing_tagger,
 			     "Fake a tagger when tags lack one"),
+		OPT_BOOLEAN(0, "full-tree", &full_tree,
+			     "Output full tree for each commit"),
 		{ OPTION_NEGBIT, 0, "data", &no_data, NULL,
 			"Skip output of blob data",
 			PARSE_OPT_NOARG | PARSE_OPT_NEGHELP, NULL, 1 },
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 6069e1f..b08954e 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -364,6 +364,76 @@ test_expect_success 'path limiting with import-marks does not lose unmodified fi
 	git fast-export --import-marks=marks simple -- file file0 | grep file0
 '
 
+cat > expected << EOF
+blob
+mark :1
+data 8
+Wohlauf
+
+blob
+mark :2
+data 9
+break it
+
+reset refs/heads/simple
+commit refs/heads/simple
+mark :3
+author A U Thor <author@example.com> 1112912053 -0700
+committer C O Mitter <committer@example.com> 1112912053 -0700
+data 8
+initial
+deleteall
+M 100644 :1 file
+M 100644 :2 file0
+
+blob
+mark :4
+data 9
+die Luft
+
+blob
+mark :5
+data 12
+geht frisch
+
+commit refs/heads/simple
+mark :6
+author A U Thor <author@example.com> 1112912113 -0700
+committer C O Mitter <committer@example.com> 1112912113 -0700
+data 7
+second
+from :3
+deleteall
+M 100644 :4 file
+M 100644 :2 file0
+M 100644 :5 file2
+
+blob
+mark :7
+data 22
+die Luft
+more content
+
+commit refs/heads/simple
+mark :8
+author A U Thor <author@example.com> 1112912773 -0700
+committer C O Mitter <committer@example.com> 1112912773 -0700
+data 5
+next
+from :6
+deleteall
+M 100644 :7 file
+M 100644 :2 file0
+M 100644 :5 file2
+
+EOF
+
+test_expect_success 'full-tree shows all files in commits'        '
+	git checkout -f simple &&
+	git fast-export --full-tree simple > output &&
+	test_cmp output expected
+'
+
 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.7.2.rc1.14.g19914

Re: [PATCH 0/2] non-incremental mode for fast-export

[Elijah Newren]

On Wed, Jul 7, 2010 at 2:46 PM,  <newren@gmail.com> wrote:
> This patch series fixes silently dropped files in uses of fast-export
> involving both --import-marks and limiting files by path.  It also
> adds a new feature: a --full-tree option to switch from incremental to
> comprehensive handling of files in commits.  The two changes are
> related in that the bugfix consists of simply automatically activating
> the new feature in a case where it is required for correct operation.

Hmm..no comments.  Am I (and others at my $dayjob) unique enough in
our combination of options we use that no one else ever runs into this
mangling of exported repositories?

Elijah

Re: [PATCH 0/2] non-incremental mode for fast-export

[Sverre Rabbelier]

Heya,

On Wed, Jul 14, 2010 at 20:03, Elijah Newren <newren@gmail.com> wrote:
> Hmm..no comments.  Am I (and others at my $dayjob) unique enough in
> our combination of options we use that no one else ever runs into this
> mangling of exported repositories?

The first patch makes sense to me and should probably go into maint.
The second one is so trivial that even if you are the only user of it
I can see no reason _not_ to include it. So even if I'm not exactly an
expert on fast-export, I think I know enough of fast-import to be able
to say this should be merged :).

-- 
Cheers,

Sverre Rabbelier

[PATCHv2 0/2] non-incremental mode for fast-export

[Elijah Newren]

This patch series fixes silently dropped files in uses of fast-export
involving both --import-marks and limiting files by path.  It also
adds a new feature: a --full-tree option to switch from incremental to
comprehensive handling of files in commits.  The two changes are
related in that the bugfix consists of simply automatically activating
the new feature in a case where it is required for correct operation.

There is just one change since the last submission
  * modify one of the new testcases to not depend on the full exact
    output of fast-export (since it is no longer the same in next)
This change enables the series to apply cleanly to next.  All tests
pass with this series applied to either maint or next now.

I'll again note that it may make sense to split this patch series
since the first is a bugfix and thus makes sense to apply to maint,
while the second (whose testcase was the fragile one) is a feature
addition, though a fairly trivial one.

Elijah Newren (2):
      fast-export: Fix dropping of files with --import-marks and path limiting
      fast-export: Add a --full-tree option

 Documentation/git-fast-export.txt |    6 ++++++
 builtin/fast-export.c             |   11 ++++++++++-
 t/t9350-fast-export.sh            |   14 ++++++++++++++
 3 files changed, 30 insertions(+), 1 deletions(-)

[PATCHv2 1/2] fast-export: Fix dropping of files with --import-marks and path limiting

[Elijah Newren]

Since fast-export operates by listing file changes since the (first) parent
commit, when using --import-marks and path limiting and using a wider list
of paths than in previous runs, files from the new path(s) will silently be
omitted from the result unless or until a commit which explicitly changes
those files.  The resulting repository in such cases is broken and makes no
sense.

This commit fixes this by having fast-export work with complete trees
instead of incremental changes (when both --import-marks and path limiting
are used).  It works by issuing a 'deleteall' directive with each commit and
then listing the full set of files that make up that commit, rather than
just showing the list of files that have changed since the (first) parent
commit.

Signed-off-by: Elijah Newren <newren@gmail.com>
Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
---
 builtin/fast-export.c  |    9 ++++++++-
 t/t9350-fast-export.sh |    9 +++++++++
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c6dd71a..25d13a1 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -27,6 +27,7 @@ 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 no_data;
+static int full_tree = 0;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -241,7 +242,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 		message += 2;
 
 	if (commit->parents &&
-	    get_object_mark(&commit->parents->item->object) != 0) {
+	    get_object_mark(&commit->parents->item->object) != 0 &&
+	    !full_tree) {
 		parse_commit(commit->parents->item);
 		diff_tree_sha1(commit->parents->item->tree->object.sha1,
 			       commit->tree->object.sha1, "", &rev->diffopt);
@@ -281,6 +283,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 		i++;
 	}
 
+	if (full_tree)
+		printf("deleteall\n");
 	log_tree_diff_flush(rev);
 	rev->diffopt.output_format = saved_output_format;
 
@@ -608,6 +612,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (import_filename)
 		import_marks(import_filename);
 
+	if (import_filename && revs.prune_data)
+		full_tree = 1;
+
 	get_tags_and_duplicates(&revs.pending, &extra_refs);
 
 	if (prepare_revision_walk(&revs))
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index d43f37c..6069e1f 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -355,6 +355,15 @@ test_expect_failure 'no exact-ref revisions included' '
 	)
 '
 
+test_expect_success 'path limiting with import-marks does not lose unmodified files'        '
+	git checkout -b simple marks~2 &&
+	git fast-export --export-marks=marks simple -- file > /dev/null &&
+	echo more content >> file &&
+	test_tick &&
+	git commit -mnext file &&
+	git fast-export --import-marks=marks simple -- file file0 | grep file0
+'
+
 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.6.1

[PATCHv2 2/2] fast-export: Add a --full-tree option

[Elijah Newren]

This option adds symmetry with fast-import, enabling it to also work with
complete trees instead of just incremental changes.  It works by issuing a
'deleteall' directive with each commit and then listing the full set of
files that make up that commit, rather than just showing the list of files
that have changed since the (first) parent commit.  Note that this
functionality is automatically turned on when using --import-marks together
with path limiting in order to avoid dropping important but unchanged
files.

This functionality is desired when using hand-written filters along with
'fast-export | some-filter | fast-import' as it can be easier to write
<some-filter> in terms of complete trees than incremental changes.

We could avoid the need to add this option by simply always turning it on.
While the end result would be identical, it would slow things down slightly
by printing many more filenames per commit which goes somewhat against the
'fast' in 'fast-export'.

Signed-off-by: Elijah Newren <newren@gmail.com>
Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
---
 Documentation/git-fast-export.txt |    6 ++++++
 builtin/fast-export.c             |    4 +++-
 t/t9350-fast-export.sh            |    5 +++++
 3 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 98ec6b5..8a6a3cb 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -90,6 +90,12 @@ marks the same across runs.
 	resulting stream can only be used by a repository which
 	already contains the necessary objects.
 
+--full-tree::
+	This option will cause fast-export to issue a "deleteall"
+	directive for each commit followed by a full list of all files
+	in the commit (as opposed to just listing the files which are
+	different from the commit's first parent).
+
 [git-rev-list-args...]::
        A list of arguments, acceptable to 'git rev-parse' and
        'git rev-list', that specifies the specific objects and references
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 25d13a1..8c77602 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -27,7 +27,7 @@ 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 no_data;
-static int full_tree = 0;
+static int full_tree;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -588,6 +588,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			     "Import marks from this file"),
 		OPT_BOOLEAN(0, "fake-missing-tagger", &fake_missing_tagger,
 			     "Fake a tagger when tags lack one"),
+		OPT_BOOLEAN(0, "full-tree", &full_tree,
+			     "Output full tree for each commit"),
 		{ OPTION_NEGBIT, 0, "data", &no_data, NULL,
 			"Skip output of blob data",
 			PARSE_OPT_NOARG | PARSE_OPT_NEGHELP, NULL, 1 },
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 6069e1f..d831404 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -364,6 +364,11 @@ test_expect_success 'path limiting with import-marks does not lose unmodified fi
 	git fast-export --import-marks=marks simple -- file file0 | grep file0
 '
 
+test_expect_success 'full-tree re-shows unmodified files'        '
+	git checkout -f simple &&
+	test $(git fast-export --full-tree simple | grep -c file0) -eq 3
+'
+
 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.6.1