[PATCH] fast-export: Avoid dropping files from commits

From: Elijah Newren <redacted>

When exporting a subset of commits on a branch that do not go back to a
root commit (e.g. master~2..master), we still want each exported commit to
have the same files in the exported tree as in the original tree.

Signed-off-by: Elijah Newren <redacted>
---
 builtin-fast-export.c  |    3 ++-
 t/t9301-fast-export.sh |    7 +++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index fdf4ae9..34a419c 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c

@@ -221,7 +221,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 	if (message)
 		message += 2;
 
-	if (commit->parents) {
+	if (commit->parents &&
+	    get_object_mark(&commit->parents->item->object) != 0) {
 		parse_commit(commit->parents->item);
 		diff_tree_sha1(commit->parents->item->tree->object.sha1,
 			       commit->tree->object.sha1, "", &rev->diffopt);

diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 86c3760..b860626 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh

@@ -8,6 +8,9 @@ test_description='git fast-export'
 
 test_expect_success 'setup' '
 
+	echo break it > file0 &&
+	git add file0 &&
+	test_tick &&
 	echo Wohlauf > file &&
 	git add file &&
 	test_tick &&

@@ -57,8 +60,8 @@ test_expect_success 'fast-export master~2..master' '
 		(cd new &&
 		 git fast-import &&
 		 test $MASTER != $(git rev-parse --verify refs/heads/partial) &&
-		 git diff master..partial &&
-		 git diff master^..partial^ &&
+		 git diff --exit-code master partial &&
+		 git diff --exit-code master^ partial^ &&
 		 test_must_fail git rev-parse partial~2)
 
 '

-- 
1.6.0.6

Hi,

On Wed, 25 Mar 2009, newren@gmail.com wrote:

From: Elijah Newren <redacted>

When exporting a subset of commits on a branch that do not go back to a
root commit (e.g. master~2..master), we still want each exported commit to
have the same files in the exported tree as in the original tree.

Signed-off-by: Elijah Newren <redacted>
---

Makes sense.

I do not understand that change.

A good explanation in the commit message might help this stupid developer.

Ciao,
Dscho

From: Elijah Newren <redacted>

When exporting a subset of commits on a branch that do not go back to a
root commit (e.g. master~2..master), we still want each exported commit to
have the same files in the exported tree as in the original tree.

Previously, when given such a range, we would omit master~2 as a parent of
master~1, but we would still diff against master~2 when selecting the list
of files to include in master~1.  This would result in only files that
had changed in the given range showing up in the resulting export.  In such
cases, we should diff master~1 against the root instead (i.e. use
diff_root_tree_sha1 instead of diff_tree_sha1).

There's a special case to consider here: incremental exports (i.e. exports
where the --import-marks flag is specified).  If master~2 is an imported
mark, then we still want to diff master~1 against master~2 when selecting
the list of files to include.

We can handle all cases, including the special case, by just checking
whether master~2 corresponds to a known object mark when deciding what to
diff against.

Signed-off-by: Elijah Newren <redacted>
---
 builtin-fast-export.c  |    3 ++-
 t/t9301-fast-export.sh |    7 +++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index fdf4ae9..34a419c 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c

@@ -221,7 +221,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 	if (message)
 		message += 2;
 
-	if (commit->parents) {
+	if (commit->parents &&
+	    get_object_mark(&commit->parents->item->object) != 0) {
 		parse_commit(commit->parents->item);
 		diff_tree_sha1(commit->parents->item->tree->object.sha1,
 			       commit->tree->object.sha1, "", &rev->diffopt);

diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 86c3760..b860626 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh

@@ -8,6 +8,9 @@ test_description='git fast-export'
 
 test_expect_success 'setup' '
 
+	echo break it > file0 &&
+	git add file0 &&
+	test_tick &&
 	echo Wohlauf > file &&
 	git add file &&
 	test_tick &&

@@ -57,8 +60,8 @@ test_expect_success 'fast-export master~2..master' '
 		(cd new &&
 		 git fast-import &&
 		 test $MASTER != $(git rev-parse --verify refs/heads/partial) &&
-		 git diff master..partial &&
-		 git diff master^..partial^ &&
+		 git diff --exit-code master partial &&
+		 git diff --exit-code master^ partial^ &&
 		 test_must_fail git rev-parse partial~2)
 
 '

-- 
1.6.0.6

Hi,

On Wed, Mar 25, 2009 at 4:13 PM, Johannes Schindelin
[off-list ref] wrote:

quoted

-     if (commit->parents) {
+     if (commit->parents &&
+         get_object_mark(&commit->parents->item->object) != 0) {
              parse_commit(commit->parents->item);
              diff_tree_sha1(commit->parents->item->tree->object.sha1,
                             commit->tree->object.sha1, "", &rev->diffopt);

I do not understand that change.

A good explanation in the commit message might help this stupid developer.

I resent the patch in another email (sorry for the duplication, but I
don't trust gmail to preserve patches, and responding inline to
comments via git-send-email isn't so great either).  Let me know if
the explanation is missing anything, is too detailed, or is using
incorrect terminology.  In two cases I was sufficiently unsure about
my wording that I provided extra wording to try to make it clear what
I was talking about.

Thanks,
Elijah

Hi,

On Wed, 25 Mar 2009, Elijah Newren wrote:

On Wed, Mar 25, 2009 at 4:13 PM, Johannes Schindelin
[off-list ref] wrote:

quoted

quoted

-     if (commit->parents) {
+     if (commit->parents &&
+         get_object_mark(&commit->parents->item->object) != 0) {
              parse_commit(commit->parents->item);
              diff_tree_sha1(commit->parents->item->tree->object.sha1,
                             commit->tree->object.sha1, "", &rev->diffopt);

I do not understand that change.

A good explanation in the commit message might help this stupid 
developer.

I resent the patch in another email (sorry for the duplication, but I 
don't trust gmail to preserve patches, and responding inline to comments 
via git-send-email isn't so great either).  Let me know if the 
explanation is missing anything, is too detailed, or is using incorrect 
terminology.  In two cases I was sufficiently unsure about my wording 
that I provided extra wording to try to make it clear what I was talking 
about.

I am pretty tired, but I still have the impression that I understood it, 
so yes, I like it.

You might want to skip the != 0, though, as we avoid that in the rest of 
Git's source code, too.

Thanks,
Dscho