Am 05.06.2013 00:04, schrieb Junio C Hamano: > Johannes Sixt <j6t@xxxxxxxx> writes: > >> @@ -62,12 +57,7 @@ test_expect_success 'git update-index to add conflicting file path2 should fail' >> >> test_expect_success 'git update-index to add conflicting symlink path3 should fail' ' >> >> - if test_have_prereq SYMLINKS >> - then >> - ln -s xyzzy path3 >> - else >> - date >path3 >> - fi && >> + test_ln_s xyzzy path3 && >> test_must_fail git update-index --add -- path3 >> ' > > This is also borderline questionable. With path2, we are already > testing that adding a regular file (one variant of "non directory") > at a path that the index expects to see a directory (the index has > path2/file2 in it at this point) fails,... You are right, of course. But again, the issue is not new with this patch. > If we want to really test the equivalent on a filesystem without > symbolic links, it would be a more faithful test to attempt to add > it using "--add --cacheinfo" and see it fail, i.e. > > test_must_fail_to_ln_s_add xyzzy path3 > > which would be a copy of test_ln_s_add but has test_must_fail before > two calls to git_update_index it makes. That's not necessary, IMO. We can just add an unconditional --cache-info test here and add SYMLINKS to the above test. > I think all the test_ln_s_add conversion in the series make sense, > but many uses of test_ln_s are questionable, and I suspect it would > invite similar confusion down the road. Incidentally, I've been running the test_ln_s_add conversions since, whoa!, two years now, but added test_ln_s only when I cleaned up the patch for submission. Perhaps it's better when I rip out test_ln_s again. -- Hannes -- 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