On Wed, Aug 19, 2015 at 03:46:27PM -0400, Anders Kaseorg wrote:
> git fast-export | git fast-import fails to preserve a commit that replaces
> a symlink with a directory. Add a failing test case demonstrating this
> bug.
>
> The fast-export output for the commit in question looks like
>
> commit refs/heads/master
> mark :4
> author …
> committer …
> data 4
> two
> M 100644 :1 foo/world
> D foo
>
> fast-import deletes the symlink foo and ignores foo/world. Swapping the M
> line with the D line would give the correct result.
Thanks for providing a patch to the tests. That is my favorite form of
bug report. :)
The problem seems to be that we output the entries in a "depth first"
way; "foo/bar" always comes before "foo", to cover the cases explained
in 060df62 (fast-export: Fix output order of D/F changes, 2010-07-09).
I'm tempted to say we would want to do all deletions (at any level)
first, to make room for new files. That patch looks like:
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d23f3be..336fd6f 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -267,6 +267,14 @@ static int depth_first(const void *a_, const void *b_)
const char *name_a, *name_b;
int len_a, len_b, len;
int cmp;
+ int deletion;
+
+ /*
+ * Move all deletions first, to make room for any later modifications.
+ */
+ deletion = (b->status == 'D') - (a->status == 'D');
+ if (deletion)
+ return deletion;
name_a = a->one ? a->one->path : a->two->path;
name_b = b->one ? b->one->path : b->two->path;
and does indeed pass your test. But I'm not sure that covers all cases,
and I'm not sure it doesn't make some cases worse:
- if we moved a deletion to the front, is it possible for that path to
have been the source side of a copy or rename, that is now broken? I
don't _think_ so. If it's a copy, then the file by definition cannot
also be deleted (that would make it a rename, not a copy). We could
have a copy along with a rename, but again, then we don't have a
delete (we have a rename, which is explicitly bumped to the end for
this reason).
- we may still have the opposite problem with renames. That is, a
rename is _also_ a deletion, but will go to the end. So I would
expect renaming the symlink "foo" to "bar" and then adding
"foo/world" would end up with:
M 100644 :3 foo/world
R foo bar
(because we push renames to the end in our sort). And indeed,
importing that does seem to get it wrong (we end up with "bar/world"
and no symlink).
We can't fix the ordering in the second case without breaking the first
case. So I'm not sure it's fixable on the fast-export end.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html