{"thread":{"id":"4900","subject":"[RFH/RFC] typechange tests for git apply (currently failing)","startedAt":"2006-07-16T10:38:40Z","lastAt":"2006-07-17T06:21:39Z","messageCount":2,"participants":["Eric Wong","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"23876","messageId":"1153046320538-git-send-email-normalperson@yhbt.net","threadId":"4900","inReplyTo":null,"subject":"[RFH/RFC] typechange tests for git apply (currently failing)","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2006-07-16T10:38:40Z","receivedAt":"2006-07-16T10:38:40Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"I've found that git apply is incapable of handling patches\ninvolving object type changes to the same path.\n\nOf course git itself is perfectly capable of making commits that\ngenerate these changes, as it only tracks trees states.  It's\njust that the diffs between them are less useful if they can't\nbe applied.\n\nSome of these are rare, but I've hit one of them (file becoming\na symlink) recently in real-world usage, and was inspired to\nfind more potential breakages :)\n\nI'm not sure when I'll have time to fix these myself and I'm not\nvery familiar with the apply code.   So if someone could get\nsome or all of these cases working, they would be my hero :)\n\nSome of these are what I would refer to as corner-cases from\nhell.  Most (if not all) other systems fail some of these.  In\nfact, they aren't even capable of representing most of these\nchanges in their histories; much less being able to handle\npatches to that effect.\n\nSigned-off-by: Eric Wong <normalperson@yhbt.net>\n---\n t/t4114-apply-typechange.sh |  105 +++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 105 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh\nnew file mode 100755\nindex 0000000..ca81d72\n--- /dev/null\n+++ b/t/t4114-apply-typechange.sh\n@@ -0,0 +1,105 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2006 Eric Wong\n+#\n+\n+test_description='git-apply should not get confused with type changes.\n+\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup repository and commits' '\n+\techo \"hello world\" > foo &&\n+\techo \"hi planet\" > bar &&\n+\tgit update-index --add foo bar &&\n+\tgit commit -m initial &&\n+\tgit branch initial &&\n+\trm -f foo &&\n+\tln -s bar foo &&\n+\tgit update-index foo &&\n+\tgit commit -m \"foo symlinked to bar\" &&\n+\tgit branch foo-symlinked-to-bar &&\n+\trm -f foo &&\n+\techo \"how far is the sun?\" > foo &&\n+\tgit update-index foo &&\n+\tgit commit -m \"foo back to file\" &&\n+\tgit branch foo-back-to-file &&\n+\trm -f foo &&\n+\tgit update-index --remove foo &&\n+\tmkdir foo &&\n+\techo \"if only I knew\" > foo/baz &&\n+\tgit update-index --add foo/baz &&\n+\tgit commit -m \"foo becomes a directory\" &&\n+\tgit branch \"foo-becomes-a-directory\" &&\n+\techo \"hello world\" > foo/baz &&\n+\tgit update-index foo/baz &&\n+\tgit commit -m \"foo/baz is the original foo\" &&\n+\tgit branch foo-baz-renamed-from-foo\n+\t'\n+\n+test_expect_success 'file renamed from foo to foo/baz' '\n+\tgit checkout -f initial &&\n+\tgit diff-tree -M -p HEAD foo-baz-renamed-from-foo > patch &&\n+\tgit apply --index < patch\n+\t'\n+test_debug 'cat patch'\n+\n+\n+test_expect_success 'file renamed from foo/baz to foo' '\n+\tgit checkout -f foo-baz-renamed-from-foo &&\n+\tgit diff-tree -M -p HEAD initial > patch &&\n+\tgit apply --index < patch\n+\t'\n+test_debug 'cat patch'\n+\n+\n+test_expect_success 'directory becomes file' '\n+\tgit checkout -f foo-becomes-a-directory &&\n+\tgit diff-tree -p HEAD initial > patch &&\n+\tgit apply --index < patch\n+\t'\n+test_debug 'cat patch'\n+\n+\n+test_expect_success 'file becomes directory' '\n+\tgit checkout -f initial &&\n+\tgit diff-tree -p HEAD foo-becomes-a-directory > patch &&\n+\tgit apply --index < patch\n+\t'\n+test_debug 'cat patch'\n+\n+\n+test_expect_success 'file becomes symlink' '\n+\tgit checkout -f initial &&\n+\tgit diff-tree -p HEAD foo-symlinked-to-bar > patch &&\n+\tgit apply --index < patch\n+\t'\n+test_debug 'cat patch'\n+\n+\n+test_expect_success 'symlink becomes file' '\n+\tgit checkout -f foo-symlinked-to-bar &&\n+\tgit diff-tree -p HEAD foo-back-to-file > patch &&\n+\tgit apply --index < patch\n+\t'\n+test_debug 'cat patch'\n+\n+\n+test_expect_success 'symlink becomes directory' '\n+\tgit checkout -f foo-symlinked-to-bar &&\n+\tgit diff-tree -p HEAD foo-becomes-a-directory > patch &&\n+\tgit apply --index < patch\n+\t'\n+test_debug 'cat patch'\n+\n+\n+test_expect_success 'directory becomes symlink' '\n+\tgit checkout -f foo-becomes-a-directory &&\n+\tgit diff-tree -p HEAD foo-symlinked-to-bar > patch &&\n+\tgit apply --index < patch\n+\t'\n+test_debug 'cat patch'\n+\n+\n+test_done\n-- \n1.4.1.g9d8f\n"},{"id":"23892","messageId":"7vwtaciwek.fsf@assigned-by-dhcp.cox.net","threadId":"4900","inReplyTo":"1153046320538-git-send-email-normalperson@yhbt.net","subject":"Re: [RFH/RFC] typechange tests for git apply (currently failing)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-17T06:21:39Z","receivedAt":"2006-07-17T06:21:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <normalperson@yhbt.net> writes:\n\n> I've found that git apply is incapable of handling patches\n> involving object type changes to the same path.\n\nIt's more of directory vs file conflicts -- and we do not track\ndirectories.  Some are pure bugs and relatively simple to fix\n(and important), some others I am not sure if they are worth\ndealing with.\n\n> +test_expect_success 'file renamed from foo to foo/baz' '\n> +\tgit checkout -f initial &&\n> +\tgit diff-tree -M -p HEAD foo-baz-renamed-from-foo > patch &&\n> +\tgit apply --index < patch\n> +\t'\n\nIf you look at where it fails closely you would notice this\nfirst fails during git-checkout (without failing, I should\nadd).  Try adding \"git diff\" immediately after \"git checkout\".\n\nA fix for read-tree is in this message to fix this, but this has\nonly been very lightly tested, so please check it thoroughly.\n\nAfter that is cleared, this and the next one uncover a few bugs\nin \"git apply\".\n\n> +test_expect_success 'file renamed from foo/baz to foo' '\n> +\tgit checkout -f foo-baz-renamed-from-foo &&\n> +\tgit diff-tree -M -p HEAD initial > patch &&\n> +\tgit apply --index < patch\n> +\t'\n> +test_debug 'cat patch'\n\none-way merge used in \"git checkout -f\" does not remove existing\ndirectory when checking out a file.  \"git reset --hard\" used to\nbe more careful but recent rewrite made them more or less\nequivalent, and now has the same problem.  This patch to read-tree\nshould fix it.\n\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 6df5d7c..122b6f1 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -507,7 +507,7 @@ static int merged_entry(struct cache_ent\n \t}\n \n \tmerge->ce_flags &= ~htons(CE_STAGEMASK);\n-\tadd_cache_entry(merge, ADD_CACHE_OK_TO_ADD);\n+\tadd_cache_entry(merge, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n \treturn 1;\n }\n \n@@ -518,7 +518,7 @@ static int deleted_entry(struct cache_en\n \telse\n \t\tverify_absent(ce->name, \"removed\");\n \tce->ce_mode = 0;\n-\tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD);\n+\tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n \tinvalidate_ce_path(ce);\n \treturn 1;\n }\n\n\nThen apply, when applying to create a file where a directory\nlies, or vice versa, was not careful and did not remove\nconflicting one.  This patch makes the first few tests to work,\nbut it is not enough.\n\nCurrently, builtin-apply.c::write_out_one_result() says \"remove\nthe old, write the new\" for each \"struct patch\" (which\ncorresponds to \"diff --git\" line in your patch file).  I think\nthe loop write_out_results() should be modified to first remove\nwhat we are going to remove in all patches, and then create what\nwe are going to create.\n\nWhat causes the fourth test to fail is that you have foo/baz in\nthe working tree and the index, and the patch creates file foo\nand removes file foo/baz.  The current loop to deal with one\npatch at a time means we try to create file \"foo\" first, which\nwould not work without removing directory \"foo\" first.\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex c903146..9727442 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1732,9 +1732,14 @@ static int check_patch(struct patch *pat\n \t\tif (check_index && cache_name_pos(new_name, strlen(new_name)) >= 0)\n \t\t\treturn error(\"%s: already exists in index\", new_name);\n \t\tif (!cached) {\n-\t\t\tif (!lstat(new_name, &st))\n-\t\t\t\treturn error(\"%s: already exists in working directory\", new_name);\n-\t\t\tif (errno != ENOENT)\n+\t\t\tstruct stat nst;\n+\t\t\tif (!lstat(new_name, &nst)) {\n+\t\t\t\tif (S_ISDIR(nst.st_mode))\n+\t\t\t\t\t; /* ok */\n+\t\t\t\telse\n+\t\t\t\t\treturn error(\"%s: already exists in working directory\", new_name);\n+\t\t\t}\n+\t\t\telse if ((errno != ENOENT) && (errno != ENOTDIR))\n \t\t\t\treturn error(\"%s: %s\", new_name, strerror(errno));\n \t\t}\n \t\tif (!patch->new_mode) {\n@@ -2011,6 +2016,16 @@ static void create_one_file(char *path, \n \t}\n \n \tif (errno == EEXIST) {\n+\t\t/* We may be trying to create a file where a directory\n+\t\t * used to be.\n+\t\t */\n+\t\tstruct stat st;\n+\t\terrno = 0;\n+\t\tif (!lstat(path, &st) && S_ISDIR(st.st_mode) && !rmdir(path))\n+\t\t\terrno = EEXIST;\n+\t}\n+\n+\tif (errno == EEXIST) {\n \t\tunsigned int nr = getpid();\n \n \t\tfor (;;) {\n"}]}