{"thread":{"id":"26392","subject":"untracked symlinks are less precious than untracked files?","startedAt":"2011-02-02T19:25:05Z","lastAt":"2011-02-22T20:01:48Z","messageCount":14,"participants":["Johannes Sixt","Junio C Hamano","Clemens Buchacher"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"160285","messageId":"201102022025.06140.j6t@kdbg.org","threadId":"26392","inReplyTo":null,"subject":"untracked symlinks are less precious than untracked files?","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2011-02-02T19:25:05Z","receivedAt":"2011-02-02T19:25:05Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"While I was poking around in t6035-merge-dir-to-symlink.sh, I noticed this:\n\ndiff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh\nindex 92e02d5..46b401b 100755\n--- a/t/t6035-merge-dir-to-symlink.sh\n+++ b/t/t6035-merge-dir-to-symlink.sh\n@@ -5,21 +5,22 @@ test_description='merging when a directory was replaced with \na symlink'\n \n test_expect_success SYMLINKS 'create a commit where dir a/b changed to \nsymlink' '\n \tmkdir -p a/b/c a/b-2/c &&\n \t> a/b/c/d &&\n \t> a/b-2/c/d &&\n \t> a/x &&\n \tgit add -A &&\n \tgit commit -m base &&\n \tgit tag start &&\n \trm -rf a/b &&\n-\tln -s b-2 a/b &&\n+\t# ln -s b-2 a/b &&\n+\t>a/b &&\n \tgit add -A &&\n \tgit commit -m \"dir to symlink\"\n '\n \n test_expect_success SYMLINKS 'keep a/b-2/c/d across checkout' '\n \tgit checkout HEAD^0 &&\n \tgit reset --hard master &&\n \tgit rm --cached a/b &&\n \tgit commit -m \"untracked symlink remains\" &&\n \t git checkout start^0 &&\n\nWith this change, where a symlink is replaced by a regular file, the 'git \ncheckout start^0' fails. At this time, a/b is untracked. When it is a \nsymlink, it is replaced by a directory. When it is a file, the test fails:\n\nerror: The following untracked working tree files would be overwritten by \ncheckout:\n a/b\n\nIs it by design that symlinks are less precious than files, or is it an \noversight?\n\n-- Hannes\n"},{"id":"160289","messageId":"7v7hdixkys.fsf@alter.siamese.dyndns.org","threadId":"26392","inReplyTo":"201102022025.06140.j6t@kdbg.org","subject":"Re: untracked symlinks are less precious than untracked files?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-02T20:03:39Z","receivedAt":"2011-02-02T20:03:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Is it by design that symlinks are less precious than files, or is it an \n> oversight?\n\nI don't recall making conscious distinction between symmlinks and regular\nfiles, so it is likely to be just a bug. Perhaps using stat() where\nlstat() should be used and mistaking an error return as missing, or\nsomething silly like that?\n"},{"id":"160295","messageId":"201102022324.22123.j6t@kdbg.org","threadId":"26392","inReplyTo":"7v7hdixkys.fsf@alter.siamese.dyndns.org","subject":"Re: untracked symlinks are less precious than untracked files?","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2011-02-02T22:24:22Z","receivedAt":"2011-02-02T22:24:22Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Mittwoch, 2. Februar 2011, Junio C Hamano wrote:\n> Johannes Sixt writes:\n> > Is it by design that symlinks are less precious than files, or is it an\n> > oversight?\n>\n> I don't recall making conscious distinction between symmlinks and regular\n> files, so it is likely to be just a bug. Perhaps using stat() where\n> lstat() should be used and mistaking an error return as missing, or\n> something silly like that?\n\nHm, I don't think so. It seems to interact with the lstat_cache. When lstat \nreports a symlink, this result is cached; but if it is a regular file, it is \nnot cached. I don't know, what the consequences are, though... I have to stop \nmy investigations for tonight.\n\n-- Hannes\n"},{"id":"160453","messageId":"201102051918.44848.j6t@kdbg.org","threadId":"26392","inReplyTo":"201102022324.22123.j6t@kdbg.org","subject":"[PATCH] Demonstrate breakage: checkout overwrites untracked symlink with directory","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2011-02-05T18:18:44Z","receivedAt":"2011-02-05T18:18:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"This adds tests where an untracked file and an untracked symlink are in the\nway where a directory should be created by 'git checkout'. Commit b1735b1a\n(do not overwrite files in leading path, 2010-12-14) fixed the case where\na file is in the way, but the untracked symlink is still removed silently.\n\nSigned-off-by: Johannes Sixt \n---\nOn Mittwoch, 2. Februar 2011, Johannes Sixt wrote:\n> It seems to interact with the lstat_cache. When lstat\n> reports a symlink, this result is cached; but if it is a regular file, it\n> is not cached.\n\nThe case where a file is in the way was fixed only in v1.7.3.4, but symlinks\nare still affected. Clemens, can you help?\n\n-- Hannes\n\nPS: When a date is given for commit reference in a commit message as above,\ndo you prefer the author date or the committer date? Above, I took the\ncommitter date, which is 2 months behind the author date.\n\n t/t2019-checkout-overwrite.sh | 50 +++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 50 insertions(+), 0 deletions(-)\n create mode 100755 t/t2019-checkout-overwrite.sh\n\ndiff --git a/t/t2019-checkout-overwrite.sh b/t/t2019-checkout-overwrite.sh\nnew file mode 100755\nindex 0000000..e4e529d\n--- /dev/null\n+++ b/t/t2019-checkout-overwrite.sh\n@@ -0,0 +1,50 @@\n+#!/bin/sh\n+\n+test_description='checkout must not overwrite an untracked objects'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\n+\tmkdir -p a/b/c &&\n+\t>a/b/c/d &&\n+\tgit add -A &&\n+\tgit commit -m base &&\n+\tgit tag start\n+'\n+\n+test_expect_success 'create a commit where dir a/b changed to file' '\n+\n+\tgit checkout -b file &&\n+\trm -rf a/b &&\n+\t>a/b &&\n+\tgit add -A &&\n+\tgit commit -m \"dir to file\"\n+'\n+\n+test_expect_success 'checkout commit with dir must not remove untracked a/b' '\n+\n+\tgit rm --cached a/b &&\n+\tgit commit -m \"un-track the file\" &&\n+\ttest_must_fail git checkout start &&\n+\ttest -f a/b\n+'\n+\n+test_expect_success 'create a commit where dir a/b changed to symlink' '\n+\n+\trm -rf a/b &&\t# cleanup if previous test failed\n+\tgit checkout -f -b symlink start &&\n+\trm -rf a/b &&\n+\tln -s foo a/b &&\n+\tgit add -A &&\n+\tgit commit -m \"dir to symlink\"\n+'\n+\n+test_expect_failure 'checkout commit with dir must not remove untracked a/b' '\n+\n+\tgit rm --cached a/b &&\n+\tgit commit -m \"un-track the symlink\" &&\n+\ttest_must_fail git checkout start &&\n+\ttest -h a/b\n+'\n+\n+test_done\n-- \n1.7.4.80.g89060\n"},{"id":"160455","messageId":"20110205183351.GA25717@localhost","threadId":"26392","inReplyTo":"201102051918.44848.j6t@kdbg.org","subject":"Re: [PATCH] Demonstrate breakage: checkout overwrites untracked symlink with directory","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2011-02-05T18:33:51Z","receivedAt":"2011-02-05T18:33:51Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Hi Hannes,\n\nOn Sat, Feb 05, 2011 at 07:18:44PM +0100, Johannes Sixt wrote:\n>\n> This adds tests where an untracked file and an untracked symlink are in the\n> way where a directory should be created by 'git checkout'. Commit b1735b1a\n> (do not overwrite files in leading path, 2010-12-14) fixed the case where\n> a file is in the way, but the untracked symlink is still removed silently.\n\nIndeed. It was my impression from reading the code that this\nbehavior is intentional. To protect symlinks from being overwritten\nas well, I believe we simply have to remove FL_SYMLINK from the\nfollowing line in check_leading_path().\n\ndiff --git a/symlinks.c b/symlinks.c\nindex 3cacebd..034943b 100644\n--- a/symlinks.c\n+++ b/symlinks.c\n@@ -223,7 +223,7 @@ int check_leading_path(const char *name, int len)\n int flags;\n int match_len = lstat_cache_matchlen(cache, name, len, &flags,\n FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);\n- if (flags & (FL_SYMLINK|FL_NOENT))\n+ if (flags & FL_NOENT)\n return 0;\n else if (flags & FL_DIR)\n return -1;\n\nIt does fix your testcase, but it may break others and I will have\nto review the code to be sure.\n\nClemens\n"},{"id":"160799","messageId":"7vpqr0n51f.fsf@alter.siamese.dyndns.org","threadId":"26392","inReplyTo":"20110205183351.GA25717@localhost","subject":"Re: [PATCH] Demonstrate breakage: checkout overwrites untracked symlink with directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-09T23:48:12Z","receivedAt":"2011-02-09T23:48:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> diff --git a/symlinks.c b/symlinks.c\n> index 3cacebd..034943b 100644\n> --- a/symlinks.c\n> +++ b/symlinks.c\n> @@ -223,7 +223,7 @@ int check_leading_path(const char *name, int len)\n> int flags;\n> int match_len = lstat_cache_matchlen(cache, name, len, &flags,\n> FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);\n> - if (flags & (FL_SYMLINK|FL_NOENT))\n> + if (flags & FL_NOENT)\n> return 0;\n> else if (flags & FL_DIR)\n> return -1;\n\nThis function used to be named has-symlink-or-noent-leading-path before\nf66caaf (do not overwrite files in leading path, 2010-10-09) and was used\nto check for exactly that condition. For example, verify_absent_1() used\nit to verify that a path A/B/C/D is not on the filesystem (e.g. in\npreparation for checking it out) by making sure that none of A, A/B, or\nA/B/C exists -- or is an untracked symlink. If one of them is a symlink\nleading elsewhere, even if lstat(\"A/B/C/D\") said the path exists, A/B/C/D\nis not something we have in the work tree, and we decide that we can check\nout the path by possibly removing intermediate symbolic link and running\nmkdir.\n\nNow you are changing the semantics of the function, so that we cannot\nclobber intermediate symbolic links when we check out a path. It may\nprobably be a good change.\n\nCan we rename this function to fix the naming regression introduced in\nf66caaf, by the way? \"check_leading_path()\" is a horrible name for a\nfunction that takes some parameters and returns a boolean, as the boolness\nof the function already says enough that it is about \"check\", giving the\nfirst part of the name 0-bit of information, and the remainder of the name\ndoesn't say much either: what aspect of leading-path is the function\nabout? Should the pathcomponents exist, should they not exist, why should\nthe caller care?\n\nA name that explains for what purpose the caller is expected to call it is\nprobably the best kind of name. As long as the purpose does not change,\neven though the implementation and the semantics are changed later, the\nname can stay the same without losing its meaning. The second best kind\nis a name that explains what it does. The old name of this function was\nof this kind, until f66caaf renamed it to a meaningless name.\n\nPerhaps can-clobber-to-checkout would be a good candidate.\n"},{"id":"160840","messageId":"20110210214901.GA25437@localhost","threadId":"26392","inReplyTo":"7vpqr0n51f.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Demonstrate breakage: checkout overwrites untracked symlink with directory","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2011-02-10T21:49:01Z","receivedAt":"2011-02-10T21:49:01Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Wed, Feb 09, 2011 at 03:48:12PM -0800, Junio C Hamano wrote:\n> \n> Perhaps can-clobber-to-checkout would be a good candidate.\n\nHow about leading_path_in_use() instead? Whether files in the path\ncan be clobbered or not is checked in a separate step, which is\ncurrently called ok_to_remove().\n\nClemens\n"},{"id":"161158","messageId":"4D5A2A34.7090008@viscovery.net","threadId":"26392","inReplyTo":"201102051918.44848.j6t@kdbg.org","subject":"Re: [PATCH] Demonstrate breakage: checkout overwrites untracked symlink with directory","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-02-15T07:24:36Z","receivedAt":"2011-02-15T07:24:36Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"I forgot to test the patch on Windows.. Would you please squash this\ninto js/checkout-untracked-symlink~1 ?\n\nThanks a lot!\n\n--- 8< ---\nFrom: Johannes Sixt \nSubject: [PATCH] fixup! Make test case number unique, mark tests with SYMLINKS prerequisite\n\nSigned-off-by: Johannes Sixt \n---\n ...ut-overwrite.sh => t2021-checkout-overwrite.sh} | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n rename t/{t2019-checkout-overwrite.sh => t2021-checkout-overwrite.sh} (83%)\n\ndiff --git a/t/t2019-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh\nsimilarity index 83%\nrename from t/t2019-checkout-overwrite.sh\nrename to t/t2021-checkout-overwrite.sh\nindex e4e529d..27db2ad 100755\n--- a/t/t2019-checkout-overwrite.sh\n+++ b/t/t2021-checkout-overwrite.sh\n@@ -29,7 +29,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' '\n \ttest -f a/b\n '\n \n-test_expect_success 'create a commit where dir a/b changed to symlink' '\n+test_expect_success SYMLINKS 'create a commit where dir a/b changed to symlink' '\n \n \trm -rf a/b &&\t# cleanup if previous test failed\n \tgit checkout -f -b symlink start &&\n@@ -39,7 +39,7 @@ test_expect_success 'create a commit where dir a/b changed to symlink' '\n \tgit commit -m \"dir to symlink\"\n '\n \n-test_expect_failure 'checkout commit with dir must not remove untracked a/b' '\n+test_expect_failure SYMLINKS 'checkout commit with dir must not remove untracked a/b' '\n \n \tgit rm --cached a/b &&\n \tgit commit -m \"un-track the symlink\" &&\n-- \n1.7.4.2.gb816c.dirty\n"},{"id":"161755","messageId":"20110220121343.GA21514@localhost","threadId":"26392","inReplyTo":"20110205183351.GA25717@localhost","subject":"[PATCH] do not overwrite untracked symlinks","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2011-02-20T12:13:43Z","receivedAt":"2011-02-20T12:13:43Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Git traditionally overwrites untracked symlinks silently. This will\ngenerally not cause massive data loss, but it is inconsistent with\nthe behavior for regular files, which are not silently overwritten.\n\nWith this change, git refuses to overwrite untracked symlinks by\ndefault. If the user really wants to overwrite the untracked\nsymlink, he has git-clean and git-checkout -f at his disposal.\n\nSigned-off-by: Clemens Buchacher \n---\n\nI checked and there are no undesireable side-effects. One test had\nto be modified slightly because it does overwrite an untracked\nsymlink.\n\n symlinks.c | 2 +-\n t/t6035-merge-dir-to-symlink.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/symlinks.c b/symlinks.c\nindex 3cacebd..034943b 100644\n--- a/symlinks.c\n+++ b/symlinks.c\n@@ -223,7 +223,7 @@ int check_leading_path(const char *name, int len)\n \tint flags;\n \tint match_len = lstat_cache_matchlen(cache, name, len, &flags,\n \t\t\t FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);\n-\tif (flags & (FL_SYMLINK|FL_NOENT))\n+\tif (flags & FL_NOENT)\n \t\treturn 0;\n \telse if (flags & FL_DIR)\n \t\treturn -1;\ndiff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh\nindex 92e02d5..1de285b 100755\n--- a/t/t6035-merge-dir-to-symlink.sh\n+++ b/t/t6035-merge-dir-to-symlink.sh\n@@ -22,7 +22,7 @@ test_expect_success SYMLINKS 'keep a/b-2/c/d across checkout' '\n \tgit reset --hard master &&\n \tgit rm --cached a/b &&\n \tgit commit -m \"untracked symlink remains\" &&\n-\t git checkout start^0 &&\n+\t git checkout -f start^0 &&\n \t test -f a/b-2/c/d\n '\n \n-- \n1.7.3.1.105.g84915\n"},{"id":"161777","messageId":"7vaahpluy9.fsf@alter.siamese.dyndns.org","threadId":"26392","inReplyTo":"20110220121343.GA21514@localhost","subject":"Re: [PATCH] do not overwrite untracked symlinks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-21T07:15:26Z","receivedAt":"2011-02-21T07:15:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> Git traditionally overwrites untracked symlinks silently. This will\n> generally not cause massive data loss, but it is inconsistent with\n> the behavior for regular files, which are not silently overwritten.\n>\n> With this change, git refuses to overwrite untracked symlinks by\n> default. If the user really wants to overwrite the untracked\n> symlink, he has git-clean and git-checkout -f at his disposal.\n>\n> Signed-off-by: Clemens Buchacher \n> ---\n>\n> I checked and there are no undesireable side-effects. One test had\n> to be modified slightly because it does overwrite an untracked\n> symlink.\n>\n> symlinks.c | 2 +-\n> t/t6035-merge-dir-to-symlink.sh | 2 +-\n> 2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/symlinks.c b/symlinks.c\n> index 3cacebd..034943b 100644\n> --- a/symlinks.c\n> +++ b/symlinks.c\n> @@ -223,7 +223,7 @@ int check_leading_path(const char *name, int len)\n> \tint flags;\n> \tint match_len = lstat_cache_matchlen(cache, name, len, &flags,\n> \t\t\t FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);\n> -\tif (flags & (FL_SYMLINK|FL_NOENT))\n> +\tif (flags & FL_NOENT)\n> \t\treturn 0;\n> \telse if (flags & FL_DIR)\n> \t\treturn -1;\n\n> diff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh\n> index 92e02d5..1de285b 100755\n> --- a/t/t6035-merge-dir-to-symlink.sh\n> +++ b/t/t6035-merge-dir-to-symlink.sh\n> @@ -22,7 +22,7 @@ test_expect_success SYMLINKS 'keep a/b-2/c/d across checkout' '\n> \tgit reset --hard master &&\n> \tgit rm --cached a/b &&\n> \tgit commit -m \"untracked symlink remains\" &&\n> -\t git checkout start^0 &&\n> +\t git checkout -f start^0 &&\n> \t test -f a/b-2/c/d\n> '\n\nThe title of the test says that checkout must keep a/b-2/c/d; if \"git\ncheckout\" without \"-f\" doesn't do so and you had to change it to \"git\ncheckout -f\", it would mean one of two things: (1) you broke \"checkout\",\nor (2) the behaviour the test wanted to keep working turned out to be\nunwanted (iow, \"git checkout\" without \"-f\" should fail under the initial\ncondition this test sets up).\n\nI suspect the situation is the latter.\n\nWhat this test in 6035 does before your patch is:\n\n 0. Prepare a tree with the following path (no intermediate symlinks)\n\n\ta/b/c/d\n a/b-2/c/d\n a/x\n\n and call that \"start\";\n\n prepare another tree with the following path\n\n\ta/b -> b-2 (symlink)\n a/b-2/c/d\n a/x\n\n make it a child of the \"start\".\n\n 1. Detach the head at the second commit (i.e. a/b symlink pointing at\n b-2), and then remove a/b symlink from the index. Make a commit with\n that index.\n\n The index at that point has:\n\n\ta/b-2/c/d\n a/x\n\n and in the working tree there is an untracked a/b symlink that point\n at b-2.\n\n 2. Try to switch to \"start\"'s tree. To be able to check it out, a\n directory needs to be created at a/b (as we need c/d underneath it),\n but untracked symbolic link in the working tree a/b interferes with\n it, and it _should_ fail.\n\n\nSo instead of testing \"with -f, the untracked path is nuked\" and still\ncalling the test to \"preserve a/b-2/c/d\", shouldn't you rename the test to\nmake sure the untracked symlink is preserved, and make sure checkout\nwithout \"-f\" fails?\n\nOf course, in addition to the above, it would be a good idea to add\nanother test that makes sure that with \"checkout -f\" the user can get rid\nof the untracked symlink and check out what he wanted to check out.\n\nHmm?\n"},{"id":"161833","messageId":"20110221194623.GA31181@localhost","threadId":"26392","inReplyTo":"7vaahpluy9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] do not overwrite untracked symlinks","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2011-02-21T19:46:23Z","receivedAt":"2011-02-21T19:46:23Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Hi Junio,\n\nOn Sun, Feb 20, 2011 at 11:15:26PM -0800, Junio C Hamano wrote:\n> \n> The title of the test says that checkout must keep a/b-2/c/d; if \"git\n> checkout\" without \"-f\" doesn't do so and you had to change it to \"git\n> checkout -f\", it would mean one of two things: (1) you broke \"checkout\",\n> or (2) the behaviour the test wanted to keep working turned out to be\n> unwanted (iow, \"git checkout\" without \"-f\" should fail under the initial\n> condition this test sets up).\n\nI didn't write the test, but to me it looks like the test wants to\nmake sure that while the symlink is removed, the tree it's pointing\nto is not removed. I am not sure why that was ever a concern. But\nby adding -f the test stays the same, except for the fact that it\nis now forcefully overwriting a symlink, which could be done\nsilently before.\n\nBut I am fine with removing the test if you think it's meaningless.\nAnd Hannes posted a number of tests for \"symlink preservation\"\nearlier in this thread:\n\n http://mid.gmane.org/201102051918.44848.j6t@kdbg.org\n\nWe should rename the test to 2020, since 2019 is by now already\ntaken. But otherwise I think the tests are fine as-is. I also used\nthem to test my patch.\n\nClemens\n"},{"id":"161860","messageId":"7v62scftjl.fsf@alter.siamese.dyndns.org","threadId":"26392","inReplyTo":"20110221194623.GA31181@localhost","subject":"Re: [PATCH] do not overwrite untracked symlinks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-22T06:54:38Z","receivedAt":"2011-02-22T06:54:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> I didn't write the test, but to me it looks like the test wants to\n> make sure that while the symlink is removed, the tree it's pointing\n> to is not removed.\n\nYes, I agree with that reading. I'd squash in this on top, first making\nsure that \"do not overwrite untracked symlinks\" (which is the title of\nthis patch) won't be broken, and then the original test that wanted to\nmake sure that checking out a/b/c/d when a/b is pointing to an unrelated\npart of the tree does not nuke whatever is pointed at that symbolic link.\n\nThanks.\n\nMy unhappiness with the undescriptive \"check_leading_path()\" still\nremains, though...\n\ndiff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh\nindex 1de285b..2599ae5 100755\n--- a/t/t6035-merge-dir-to-symlink.sh\n+++ b/t/t6035-merge-dir-to-symlink.sh\n@@ -17,13 +17,21 @@ test_expect_success SYMLINKS 'create a commit where dir a/b changed to symlink'\n \tgit commit -m \"dir to symlink\"\n '\n \n-test_expect_success SYMLINKS 'keep a/b-2/c/d across checkout' '\n+test_expect_success SYMLINKS 'checkout does not clobber untracked symlink' '\n \tgit checkout HEAD^0 &&\n \tgit reset --hard master &&\n \tgit rm --cached a/b &&\n \tgit commit -m \"untracked symlink remains\" &&\n-\t git checkout -f start^0 &&\n-\t test -f a/b-2/c/d\n+\ttest_must_fail git checkout start^0\n+'\n+\n+test_expect_success SYMLINKS 'a/b-2/c/d is kept when clobbering symlink b' '\n+\tgit checkout HEAD^0 &&\n+\tgit reset --hard master &&\n+\tgit rm --cached a/b &&\n+\tgit commit -m \"untracked symlink remains\" &&\n+\tgit checkout -f start^0 &&\n+\ttest -f a/b-2/c/d\n '\n \n test_expect_success SYMLINKS 'checkout should not have deleted a/b-2/c/d' '\n"},{"id":"161907","messageId":"20110222192632.GB4881@localhost","threadId":"26392","inReplyTo":"7v62scftjl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] do not overwrite untracked symlinks","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2011-02-22T19:26:32Z","receivedAt":"2011-02-22T19:26:32Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Mon, Feb 21, 2011 at 10:54:38PM -0800, Junio C Hamano wrote:\n> \n> Yes, I agree with that reading. I'd squash in this on top, first making\n> sure that \"do not overwrite untracked symlinks\" (which is the title of\n> this patch) won't be broken, and then the original test that wanted to\n> make sure that checking out a/b/c/d when a/b is pointing to an unrelated\n> part of the tree does not nuke whatever is pointed at that symbolic link.\n\nOk, perfect.\n\n> My unhappiness with the undescriptive \"check_leading_path()\" still\n> remains, though...\n\nOk. If I can't figure anything out this weekend, I'll rename it.\nPromise.\n\nIt's just that I'm even more unhappy about the actual code rather\nthan the function name. And each time I look at it I end up trying\nto fix it, only to find out that it affects so many parts of the\ncode that don't make any sense. \n\nFor example, we already have a function check_path() in entry.c,\nwhich does something very similar and is called by\ncheckout_entry(), which in turn does something similar to\nverify_absent(). In fact, I suspect that both functions are called\ntwice by the same code path around unpack_trees, for the same paths\nand for the same reason.\n\nI strongly feel that we should separate the merge process into two\nsteps.\n\n - First, do everything in the index, ignoring the work tree.\n\n - Second, checkout the index to the work tree while making sure no\n changes or untracked files get overwritten.\n\nI know I have talked about this before, and I don't know if I will\never find the time to implement such a major change. But I can't\nget myself to feel good about fixing the function name, but not the\nfunction.\n\nClemens\n"},{"id":"161912","messageId":"7vzkpnbzyr.fsf@alter.siamese.dyndns.org","threadId":"26392","inReplyTo":"20110222192632.GB4881@localhost","subject":"Re: [PATCH] do not overwrite untracked symlinks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-22T20:01:48Z","receivedAt":"2011-02-22T20:01:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> I strongly feel that we should separate the merge process into two\n> steps.\n>\n> - First, do everything in the index, ignoring the work tree.\n>\n> - Second, checkout the index to the work tree while making sure no\n> changes or untracked files get overwritten.\n\nYes, I think most people who are familiar with the internals agreed on\nthis long time ago.\n\n> ... But I can't\n> get myself to feel good about fixing the function name, but not the\n> function.\n\nI think I said what I wanted to say badly.\n\nI agree that the code structure is suboptimal; the badly named function is\njust a manifestation of the fact that the abstraction used in the\ncodepaths involved is not a good one, I think.\n"}]}