{"thread":{"id":"39177","subject":"[PATCH] git-p4: add failing tests for case-folding p4d","startedAt":"2015-04-28T09:08:00Z","lastAt":"2015-04-29T08:19:43Z","messageCount":5,"participants":["Luke Diamand","Lex Spoon"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"260118","messageId":"1430212081-16146-1-git-send-email-luke@diamand.org","threadId":"39177","inReplyTo":null,"subject":"[PATCH] git-p4: add failing tests for case-folding in p4d","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-04-28T09:08:00Z","receivedAt":"2015-04-28T09:08:00Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Lex found out recently that when git-p4 is asked to clone a repo\nand the case of the repo is incorrect (but otherwise correct) that\ngit-p4, instead of reporting an error, appears to work fine, but\nactually produces an empty repo. This can be quite confusing.\n\nThis patch adds a couple of failing test cases that illustrate the\nproblem.\n\nThe next step is to figure out where it's going wrong, and how it\nshould actually behave.\n\nLuke Diamand (1):\n  git-p4: add failing tests for case-folding p4d\n\n t/lib-git-p4.sh                |  2 +-\n t/t9819-git-p4-case-folding.sh | 54 ++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 55 insertions(+), 1 deletion(-)\n create mode 100755 t/t9819-git-p4-case-folding.sh\n\n-- \n2.4.0.rc3.380.g8e2ddc7\n"},{"id":"260117","messageId":"1430212081-16146-2-git-send-email-luke@diamand.org","threadId":"39177","inReplyTo":"1430212081-16146-1-git-send-email-luke@diamand.org","subject":"[PATCH] git-p4: add failing tests for case-folding p4d","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-04-28T09:08:01Z","receivedAt":"2015-04-28T09:08:01Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"When p4d runs on a case-folding OS, git-p4 can end up getting\nvery confused. This adds failing tests to demonstrate the problem.\n\nSigned-off-by: Luke Diamand <luke@diamand.org>\n---\n t/lib-git-p4.sh                |  2 +-\n t/t9819-git-p4-case-folding.sh | 54 ++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 55 insertions(+), 1 deletion(-)\n create mode 100755 t/t9819-git-p4-case-folding.sh\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 5aa8adc..7548225 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -69,7 +69,7 @@ start_p4d() {\n \t(\n \t\tcd \"$db\" &&\n \t\t{\n-\t\t\tp4d -q -p $P4DPORT &\n+\t\t\tp4d -q -p $P4DPORT \"$@\" &\n \t\t\techo $! >\"$pidfile\"\n \t\t}\n \t) &&\ndiff --git a/t/t9819-git-p4-case-folding.sh b/t/t9819-git-p4-case-folding.sh\nnew file mode 100755\nindex 0000000..78f1d0f\n--- /dev/null\n+++ b/t/t9819-git-p4-case-folding.sh\n@@ -0,0 +1,54 @@\n+#!/bin/sh\n+\n+test_description='interaction with P4 case-folding'\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d with case folding enabled' '\n+\tstart_p4d -C1\n+'\n+\n+test_expect_success 'Create a repo, name is lowercase' '\n+\t(\n+\t\tclient_view \"//depot/... //client/...\" &&\n+\t\tcd \"$cli\" &&\n+\t\tmkdir -p lc UC &&\n+\t\t>lc/file.txt && >UC/file.txt &&\n+\t\tp4 add lc/file.txt UC/file.txt &&\n+\t\tp4 submit -d \"Add initial lc and UC repos\"\n+\t)\n+'\n+\n+test_expect_success 'Check p4 is in case-folding mode' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\t>lc/FILE.TXT &&\n+\t\tp4 add lc/FILE.TXT &&\n+\t\ttest_must_fail p4 submit -d \"Cannot add file differing only in case\" lc/FILE.TXT\n+\t)\n+'\n+\n+# Check we created the repo properly\n+test_expect_success 'Clone lc repo using lc name' '\n+\tgit p4 clone //depot/lc/... &&\n+\ttest_path_is_file lc/file.txt &&\n+\tgit p4 clone //depot/UC/... &&\n+\ttest_path_is_file UC/file.txt\n+'\n+\n+# The clone should fail, since there is no repo called LC, but because\n+# we have case-insensitive p4d enabled, it appears to go ahead and work,\n+# but leaves an empty git repo in place.\n+test_expect_failure 'Clone lc repo using uc name' '\n+\ttest_must_fail git p4 clone //depot/LC/...\n+'\n+\n+test_expect_failure 'Clone UC repo with lc name' '\n+\ttest_must_fail git p4 clone //depot/uc/...\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n2.4.0.rc3.380.g8e2ddc7\n"},{"id":"260119","messageId":"553F508E.3010603@diamand.org","threadId":"39177","inReplyTo":"1430212081-16146-1-git-send-email-luke@diamand.org","subject":"Re: [PATCH] git-p4: add failing tests for case-folding in p4d","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-04-28T09:19:10Z","receivedAt":"2015-04-28T09:19:10Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 28/04/15 10:08, Luke Diamand wrote:\n> Lex found out recently that when git-p4 is asked to clone a repo\n> and the case of the repo is incorrect (but otherwise correct) that\n> git-p4, instead of reporting an error, appears to work fine, but\n> actually produces an empty repo. This can be quite confusing.\n\nOops, not Lex, but Fusion, noticed the problem!\n\nThanks!\nLuke\n"},{"id":"260149","messageId":"CALM2Sna0OqxYWzEj94SY61ZsL8cB+SyuiJ0EsAzq+kbiw87QLQ@mail.gmail.com","threadId":"39177","inReplyTo":"1430212081-16146-2-git-send-email-luke@diamand.org","subject":"Re: [PATCH] git-p4: add failing tests for case-folding p4d","fromName":"Lex Spoon","fromEmail":"lex@lexspoon.org","sentAt":"2015-04-28T23:01:46Z","receivedAt":"2015-04-28T23:01:46Z","isPatch":true,"sender":{"key":"lex@lexspoon.org","avatar":"https://avatars.githubusercontent.com/u/274906?v=4"},"body":"The last comment in the test took me a minute to decipher. I would\nsuggest \"no repo path called LC\" instead of \"no repo called LC\". Also,\nit would have helped me to either have a little comment on the \"UC\"\nversion of the test, or to make the previous comment a little more\nneutral so that it will apply to both test cases.\n\nOtherwise, while I am not a regular maintainer of this code, the patch\ndoes LGTM. Certainly it's good to have more test coverage.\n\nFor the underlying problem, I haven't thought about it very much, but\nit looks like a plausible first step might be to simply probe the\ngiven file name and see if it comes back the same way. If it comes\nback differently, then maybe the command should abort?\n\n\nWhat a tough problem all around...\n\nLex\n"},{"id":"260160","messageId":"5540941F.9090507@diamand.org","threadId":"39177","inReplyTo":"CALM2Sna0OqxYWzEj94SY61ZsL8cB+SyuiJ0EsAzq+kbiw87QLQ@mail.gmail.com","subject":"Re: [PATCH] git-p4: add failing tests for case-folding p4d","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-04-29T08:19:43Z","receivedAt":"2015-04-29T08:19:43Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"(Adding Pete, Vitor, and Fusion in case they have any thoughts on \nworking with P4 servers that do case-folding, or at least failing \ngracefully).\n\nOn 29/04/15 00:01, Lex Spoon wrote:\n> The last comment in the test took me a minute to decipher. I would\n> suggest \"no repo path called LC\" instead of \"no repo called LC\". Also,\n> it would have helped me to either have a little comment on the \"UC\"\n> version of the test, or to make the previous comment a little more\n> neutral so that it will apply to both test cases.\n\nOK, thanks!\n\n>\n> Otherwise, while I am not a regular maintainer of this code, the patch\n> does LGTM. Certainly it's good to have more test coverage.\n>\n> For the underlying problem, I haven't thought about it very much, but\n> it looks like a plausible first step might be to simply probe the\n> given file name and see if it comes back the same way. If it comes\n> back differently, then maybe the command should abort?\n\nI think the problem may be a bit trickier than that.\n\nI think what's happening when cloning is that when files come back from \nthe server, git-p4 checks that they are contained within the directory \nit is cloning. This happens in p4StartsWith(), (called from \nextractFilesFromCommit()) which already tries to fix this problem by \nchecking 'core.ignorecase'. However, that won't work if the local \nmachine is case sensitive but the server isn't (e.g. Linux client, \nWindows server).\n\ngit-p4 does this because it's fetching *commits* from Perforce, and a \ncommit might have files that are outside the directory being cloned.\n\nI tried teaching p4StartsWith() to ask the server if it is case-folding \n('p4 info') and that then means that the git-p4 clone actually succeeds. \nHowever, git-p4 submit then fails because it gets terribly confused \nabout pathnames - it probably needs to do some lowercasing somewhere. So \nthat might be worth pursuing.\n\nOpen to other suggestions though!\n\n>\n>\n> What a tough problem all around...\n\nIndeed!\n\nLuke\n"}]}