Re: [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
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 ande...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
