* [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
@ 2015-08-19 19:46 Anders Kaseorg
2015-08-21 14:58 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Anders Kaseorg @ 2015-08-19 19:46 UTC (permalink / raw)
To: gitster; +Cc: git
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.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
t/t9350-fast-export.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 66c8b0a..5fb8a04 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -419,6 +419,30 @@ test_expect_success 'directory becomes symlink' '
(cd result && git show master:foo)
'
+test_expect_failure 'symlink becomes directory' '
+ git init symlinktodir &&
+ git init symlinktodirresult &&
+ (
+ cd symlinktodir &&
+ mkdir bar &&
+ echo hello > bar/world &&
+ test_ln_s_add bar foo &&
+ git add foo bar/world &&
+ git commit -q -mone &&
+ git rm foo &&
+ mkdir foo &&
+ echo hello > foo/world &&
+ git add foo/world &&
+ git commit -q -mtwo
+ ) &&
+ (
+ cd symlinktodir &&
+ git fast-export master -- foo |
+ (cd ../symlinktodirresult && git fast-import --quiet)
+ ) &&
+ (cd symlinktodirresult && git show master:foo)
+'
+
test_expect_success 'fast-export quotes pathnames' '
git init crazy-paths &&
(cd crazy-paths &&
--
2.5.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
2015-08-19 19:46 [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory Anders Kaseorg
@ 2015-08-21 14:58 ` Jeff King
2015-08-21 16:47 ` Anders Kaseorg
0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2015-08-21 14:58 UTC (permalink / raw)
To: Anders Kaseorg; +Cc: gitster, git
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
2015-08-21 14:58 ` Jeff King
@ 2015-08-21 16:47 ` Anders Kaseorg
2015-08-24 5:25 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Anders Kaseorg @ 2015-08-21 16:47 UTC (permalink / raw)
To: Jeff King; +Cc: gitster, git
On Fri, 21 Aug 2015, Jeff King wrote:
> - 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.
Hmm, renames have a more fundamental ordering problem: swapping two
(normal) files and using fast-export -C -B results in
R foo bar
R bar foo
which cannot be reimported correctly without fast-import fixes.
Anders
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
2015-08-21 16:47 ` Anders Kaseorg
@ 2015-08-24 5:25 ` Jeff King
0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2015-08-24 5:25 UTC (permalink / raw)
To: Anders Kaseorg; +Cc: gitster, git
On Fri, Aug 21, 2015 at 12:47:30PM -0400, Anders Kaseorg wrote:
> On Fri, 21 Aug 2015, Jeff King wrote:
> > - 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.
>
> Hmm, renames have a more fundamental ordering problem: swapping two
> (normal) files and using fast-export -C -B results in
>
> R foo bar
> R bar foo
>
> which cannot be reimported correctly without fast-import fixes.
Yeah, you're right. Fast-export's view of the world comes from diff,
which is that the "source" side is immutable. Whereas fast-import seems
to mutate the tree in-place as it reads the set of operations. I wonder
what would break if we simply fixed that. I.e., is anybody else
depending on:
R foo bar
M bar ...
to modify "foo" and not "bar". I kind of wonder if it is insane to turn
on renames at all in fast-export.
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-08-24 5:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-19 19:46 [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory Anders Kaseorg
2015-08-21 14:58 ` Jeff King
2015-08-21 16:47 ` Anders Kaseorg
2015-08-24 5:25 ` Jeff King
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).