{"thread":{"id":"38933","subject":"[PATCH 1/2] git-p4: Check branch detection and client view together","startedAt":"2015-03-28T12:28:48Z","lastAt":"2015-04-20T05:32:19Z","messageCount":18,"participants":["Vitor Antunes","Junio C Hamano","Luke Diamand"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"258632","messageId":"1427545730-3563-1-git-send-email-vitor.hda@gmail.com","threadId":"38933","inReplyTo":null,"subject":"[PATCH 0/2] git-p4: Improve client path detection","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-28T12:28:48Z","receivedAt":"2015-03-28T12:28:48Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"I'm adding a test case for a scenario I was confronted with when using branch\ndetection and a client view specification. It is possible that the implemented\nfix may not cover all possible scenarios, but there is no regression in the\navailable tests.\n\nVitor Antunes (2):\n  git-p4: Check branch detection and client view together\n  git-p4: Improve client path detection when branches are used\n\n git-p4.py                |   11 ++++--\n t/t9801-git-p4-branch.sh |   98 ++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 105 insertions(+), 4 deletions(-)\n\n-- \n1.7.10.4\n"},{"id":"258631","messageId":"1427545730-3563-2-git-send-email-vitor.hda@gmail.com","threadId":"38933","inReplyTo":"1427545730-3563-1-git-send-email-vitor.hda@gmail.com","subject":"[PATCH 1/2] git-p4: Check branch detection and client view together","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-28T12:28:49Z","receivedAt":"2015-03-28T12:28:49Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Add failing scenario where using branch detection and a client view will break\ngit p4 submit functionality.\n\nSigned-off-by: Vitor Antunes <vitor.hda@gmail.com>\n---\n t/t9801-git-p4-branch.sh |   98 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 98 insertions(+)\n\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 2bf142d..2f0361a 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -504,6 +504,104 @@ test_expect_success 'use-client-spec detect-branches skips files in branches' '\n \t)\n '\n \n+test_expect_success 'restart p4d' '\n+\tkill_p4d &&\n+\tstart_p4d\n+'\n+\n+#\n+# 1: //depot/branch1/base/file1\n+#    //depot/branch1/base/file2\n+# 2: integrate //depot/branch1/base/... -> //depot/branch2/base/...\n+# 3: //depot/branch1/base/file3\n+# 4: //depot/branch1/base/file2 (edit)\n+# 5: integrate //depot/branch1/base/... -> //depot/branch3/base/...\n+#\n+# Note: the client view remove the \"base\" folder from the workspace\n+test_expect_success 'add simple p4 branches with common base folder on each branch' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tclient_view \"//depot/branch1/base/... //client/branch1/...\" \\\n+\t\t\t    \"//depot/branch2/base/... //client/branch2/...\" \\\n+\t\t\t    \"//depot/branch3/base/... //client/branch3/...\" &&\n+\t\tmkdir -p branch1 &&\n+\t\tcd branch1 &&\n+\t\techo file1 >file1 &&\n+\t\techo file2 >file2 &&\n+\t\tp4 add file1 file2 &&\n+\t\tp4 submit -d \"Create branch1\" &&\n+\t\tp4 integrate //depot/branch1/base/... //depot/branch2/base/... &&\n+\t\tp4 submit -d \"Integrate branch2 from branch1\" &&\n+\t\techo file3 >file3 &&\n+\t\tp4 add file3 &&\n+\t\tp4 submit -d \"add file3 in branch1\" &&\n+\t\tp4 open file2 &&\n+\t\techo update >>file2 &&\n+\t\tp4 submit -d \"update file2 in branch1\" &&\n+\t\tp4 integrate //depot/branch1/base/... //depot/branch3/base/... &&\n+\t\tp4 submit -d \"Integrate branch3 from branch1\"\n+\t)\n+'\n+\n+# Configure branches through git-config and clone them.\n+# All files are tested to make sure branches were cloned correctly.\n+# Finally, make an update to branch1 on P4 side to check if it is imported\n+# correctly by git p4.\n+# git p4 is expected to use the client view to also not include the common\n+# \"base\" folder in the imported directory structure.\n+test_expect_success 'git p4 clone simple branches with base folder on server side' '\n+\ttest_create_repo \"$git\" &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.branchList branch1:branch2 &&\n+\t\tgit config --add git-p4.branchList branch1:branch3 &&\n+\t\tgit p4 clone --dest=. --use-client-spec  --detect-branches //depot@all &&\n+\t\tgit log --all --graph --decorate --stat &&\n+\t\tgit reset --hard p4/depot/branch1 &&\n+\t\ttest -f file1 &&\n+\t\ttest -f file2 &&\n+\t\ttest -f file3 &&\n+\t\tgrep update file2 &&\n+\t\tgit reset --hard p4/depot/branch2 &&\n+\t\ttest -f file1 &&\n+\t\ttest -f file2 &&\n+\t\ttest ! -f file3 &&\n+\t\t! grep update file2 &&\n+\t\tgit reset --hard p4/depot/branch3 &&\n+\t\ttest -f file1 &&\n+\t\ttest -f file2 &&\n+\t\ttest -f file3 &&\n+\t\tgrep update file2 &&\n+\t\tcd \"$cli\" &&\n+\t\tcd branch1 &&\n+\t\tp4 edit file2 &&\n+\t\techo file2_ >>file2 &&\n+\t\tp4 submit -d \"update file2 in branch1\" &&\n+\t\tcd \"$git\" &&\n+\t\tgit reset --hard p4/depot/branch1 &&\n+\t\tgit p4 rebase &&\n+\t\tgrep file2_ file2\n+\t)\n+'\n+\n+# Now update a file in one of the branches in git and submit to P4\n+test_expect_failure 'Update a file in git side and submit to P4 using client view' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit reset --hard p4/depot/branch1 &&\n+\t\techo \"client spec\" >> file1 &&\n+\t\tgit add -u . &&\n+\t\tgit commit -m \"update file1 in branch1\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit --verbose &&\n+\t\tcd \"$cli\" &&\n+\t\tp4 sync ... &&\n+\t\tcd branch1 &&\n+\t\tgrep \"client spec\" file1\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n1.7.10.4\n"},{"id":"258633","messageId":"1427545730-3563-3-git-send-email-vitor.hda@gmail.com","threadId":"38933","inReplyTo":"1427545730-3563-1-git-send-email-vitor.hda@gmail.com","subject":"[PATCH 2/2] git-p4: Improve client path detection when branches are used","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-28T12:28:50Z","receivedAt":"2015-03-28T12:28:50Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"A client view can be used to remap folder locations in the workspace. To support\nthis when branch detection is enabled it is necessary to get the client path\nthrough \"p4 where\". This patch does two things to achieve this:\n\n1. Force usage of \"p4 where\" when P4 branches exist in the git repository.\n2. Search for mappings that contain the depot path, instead of requiring an\n   exact match.\n\nSigned-off-by: Vitor Antunes <vitor.hda@gmail.com>\n---\n git-p4.py                |   11 +++++++----\n t/t9801-git-p4-branch.sh |    2 +-\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 549022e..6954549 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -502,12 +502,12 @@ def p4Cmd(cmd):\n def p4Where(depotPath):\n     if not depotPath.endswith(\"/\"):\n         depotPath += \"/\"\n-    depotPath = depotPath + \"...\"\n-    outputList = p4CmdList([\"where\", depotPath])\n+    depotPathLong = depotPath + \"...\"\n+    outputList = p4CmdList([\"where\", depotPathLong])\n     output = None\n     for entry in outputList:\n         if \"depotFile\" in entry:\n-            if entry[\"depotFile\"] == depotPath:\n+            if entry[\"depotFile\"].find(depotPath) >= 0:\n                 output = entry\n                 break\n         elif \"data\" in entry:\n@@ -1627,7 +1627,10 @@ class P4Submit(Command, P4UserMap):\n         if self.useClientSpec:\n             self.clientSpecDirs = getClientSpec()\n \n-        if self.useClientSpec:\n+        # Check for the existance of P4 branches\n+        branchesDetected = (len(p4BranchesInGit().keys()) > 1)\n+\n+        if self.useClientSpec and not branchesDetected:\n             # all files are relative to the client spec\n             self.clientPath = getClientRoot()\n         else:\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 2f0361a..4fe4e18 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -585,7 +585,7 @@ test_expect_success 'git p4 clone simple branches with base folder on server sid\n '\n \n # Now update a file in one of the branches in git and submit to P4\n-test_expect_failure 'Update a file in git side and submit to P4 using client view' '\n+test_expect_success 'Update a file in git side and submit to P4 using client view' '\n \ttest_when_finished cleanup_git &&\n \t(\n \t\tcd \"$git\" &&\n-- \n1.7.10.4\n"},{"id":"258664","messageId":"1427671914-12131-1-git-send-email-vitor.hda@gmail.com","threadId":"38933","inReplyTo":"1427545730-3563-1-git-send-email-vitor.hda@gmail.com","subject":"[PATCH] t9814: Guarantee only one source exists in git-p4 copy tests","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-29T23:31:54Z","receivedAt":"2015-03-29T23:31:54Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"* Modify source file (file2) before copying the file.\n* Check that only file2 is the source in the output of \"p4 filelog\".\n* Remove all \"case\" statements and replace them simple tests to check that\n  source is \"file2\".\n\nSigned-off-by: Vitor Antunes <vitor.hda@gmail.com>\n---\n t/t9814-git-p4-rename.sh |   46 +++++++++++++++++++++++++++++++---------------\n 1 file changed, 31 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\nindex 8b9c295..d8fb22d 100755\n--- a/t/t9814-git-p4-rename.sh\n+++ b/t/t9814-git-p4-rename.sh\n@@ -132,6 +132,9 @@ test_expect_success 'detect copies' '\n \t\tcd \"$git\" &&\n \t\tgit config git-p4.skipSubmitEdit true &&\n \n+\t\techo \"file8\" >> file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n \t\tcp file2 file8 &&\n \t\tgit add file8 &&\n \t\tgit commit -a -m \"Copy file2 to file8\" &&\n@@ -140,6 +143,10 @@ test_expect_success 'detect copies' '\n \t\tp4 filelog //depot/file8 &&\n \t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n \n+\t\techo \"file9\" >> file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n+\n \t\tcp file2 file9 &&\n \t\tgit add file9 &&\n \t\tgit commit -a -m \"Copy file2 to file9\" &&\n@@ -149,28 +156,39 @@ test_expect_success 'detect copies' '\n \t\tp4 filelog //depot/file9 &&\n \t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n \n+\t\techo \"file10\" >> file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n+\n \t\techo \"file2\" >>file2 &&\n \t\tcp file2 file10 &&\n \t\tgit add file2 file10 &&\n \t\tgit commit -a -m \"Modify and copy file2 to file10\" &&\n \t\tgit diff-tree -r -C HEAD &&\n+\t\tsrc=$(git diff-tree -r -C HEAD | sed 1d | sed 2d | cut -f2) &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file10 &&\n-\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file\" &&\n+\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file2\" &&\n+\n+\t\techo \"file11\" >> file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n \n \t\tcp file2 file11 &&\n \t\tgit add file11 &&\n \t\tgit commit -a -m \"Copy file2 to file11\" &&\n \t\tgit diff-tree -r -C --find-copies-harder HEAD &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\tcase \"$src\" in\n-\t\tfile2 | file10) : ;; # happy\n-\t\t*) false ;; # not\n-\t\tesac &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopiesHarder true &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file11 &&\n-\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file\" &&\n+\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file2\" &&\n+\n+\t\techo \"file12\" >> file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n \n \t\tcp file2 file12 &&\n \t\techo \"some text\" >>file12 &&\n@@ -180,15 +198,16 @@ test_expect_success 'detect copies' '\n \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n \t\ttest -n \"$level\" && test \"$level\" -gt 0 && test \"$level\" -lt 98 &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\tcase \"$src\" in\n-\t\tfile10 | file11) : ;; # happy\n-\t\t*) false ;; # not\n-\t\tesac &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file12 &&\n \t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n \n+\t\techo \"file13\" >> file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n+\n \t\tcp file2 file13 &&\n \t\techo \"different text\" >>file13 &&\n \t\tgit add file13 &&\n@@ -197,14 +216,11 @@ test_expect_success 'detect copies' '\n \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n \t\ttest -n \"$level\" && test \"$level\" -gt 2 && test \"$level\" -lt 100 &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\tcase \"$src\" in\n-\t\tfile10 | file11 | file12) : ;; # happy\n-\t\t*) false ;; # not\n-\t\tesac &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopies $(($level - 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file13 &&\n-\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file\"\n+\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file2\"\n \t)\n '\n \n-- \n1.7.10.4\n"},{"id":"258665","messageId":"xmqqk2xzxk3y.fsf@gitster.dls.corp.google.com","threadId":"38933","inReplyTo":"1427671914-12131-1-git-send-email-vitor.hda@gmail.com","subject":"Re: [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-30T03:03:13Z","receivedAt":"2015-03-30T03:03:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vitor Antunes <vitor.hda@gmail.com> writes:\n\n> * Modify source file (file2) before copying the file.\n> * Check that only file2 is the source in the output of \"p4 filelog\".\n> * Remove all \"case\" statements and replace them simple tests to check that\n>   source is \"file2\".\n>\n> Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>\n> ---\n\nI am not a Perfoce user, so I'd like to ask Pete's and Luke's\ncomments on these changes.\n\n>  t/t9814-git-p4-rename.sh |   46 +++++++++++++++++++++++++++++++---------------\n>  1 file changed, 31 insertions(+), 15 deletions(-)\n>\n> diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\n> index 8b9c295..d8fb22d 100755\n> --- a/t/t9814-git-p4-rename.sh\n> +++ b/t/t9814-git-p4-rename.sh\n> @@ -132,6 +132,9 @@ test_expect_success 'detect copies' '\n>  \t\tcd \"$git\" &&\n>  \t\tgit config git-p4.skipSubmitEdit true &&\n>  \n> +\t\techo \"file8\" >> file2 &&\n\nStyle: please lose SP between redirection and its target, i.e.\n\n\techo file8 >>file2 &&\n\nThe same comment applies to everywhere else.\n\n> +\t\tgit commit -a -m \"Differentiate file2\" &&\n> +\t\tgit p4 submit &&\n>  \t\tcp file2 file8 &&\n>  \t\tgit add file8 &&\n>  \t\tgit commit -a -m \"Copy file2 to file8\" &&\n> @@ -140,6 +143,10 @@ test_expect_success 'detect copies' '\n>  \t\tp4 filelog //depot/file8 &&\n>  \t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n>  \n> +\t\techo \"file9\" >> file2 &&\n> +\t\tgit commit -a -m \"Differentiate file2\" &&\n> +\t\tgit p4 submit &&\n> +\n>  \t\tcp file2 file9 &&\n>  \t\tgit add file9 &&\n>  \t\tgit commit -a -m \"Copy file2 to file9\" &&\n> @@ -149,28 +156,39 @@ test_expect_success 'detect copies' '\n>  \t\tp4 filelog //depot/file9 &&\n>  \t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n>  \n> +\t\techo \"file10\" >> file2 &&\n> +\t\tgit commit -a -m \"Differentiate file2\" &&\n> +\t\tgit p4 submit &&\n> +\n>  \t\techo \"file2\" >>file2 &&\n>  \t\tcp file2 file10 &&\n>  \t\tgit add file2 file10 &&\n>  \t\tgit commit -a -m \"Modify and copy file2 to file10\" &&\n>  \t\tgit diff-tree -r -C HEAD &&\n> +\t\tsrc=$(git diff-tree -r -C HEAD | sed 1d | sed 2d | cut -f2) &&\n> +\t\ttest \"$src\" = file2 &&\n>  \t\tgit p4 submit &&\n>  \t\tp4 filelog //depot/file10 &&\n> -\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file\" &&\n> +\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file2\" &&\n> +\n> +\t\techo \"file11\" >> file2 &&\n> +\t\tgit commit -a -m \"Differentiate file2\" &&\n> +\t\tgit p4 submit &&\n>  \n>  \t\tcp file2 file11 &&\n>  \t\tgit add file11 &&\n>  \t\tgit commit -a -m \"Copy file2 to file11\" &&\n>  \t\tgit diff-tree -r -C --find-copies-harder HEAD &&\n>  \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n> -\t\tcase \"$src\" in\n> -\t\tfile2 | file10) : ;; # happy\n> -\t\t*) false ;; # not\n> -\t\tesac &&\n> +\t\ttest \"$src\" = file2 &&\n>  \t\tgit config git-p4.detectCopiesHarder true &&\n>  \t\tgit p4 submit &&\n>  \t\tp4 filelog //depot/file11 &&\n> -\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file\" &&\n> +\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file2\" &&\n> +\n> +\t\techo \"file12\" >> file2 &&\n> +\t\tgit commit -a -m \"Differentiate file2\" &&\n> +\t\tgit p4 submit &&\n>  \n>  \t\tcp file2 file12 &&\n>  \t\techo \"some text\" >>file12 &&\n> @@ -180,15 +198,16 @@ test_expect_success 'detect copies' '\n>  \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n>  \t\ttest -n \"$level\" && test \"$level\" -gt 0 && test \"$level\" -lt 98 &&\n>  \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n> -\t\tcase \"$src\" in\n> -\t\tfile10 | file11) : ;; # happy\n> -\t\t*) false ;; # not\n> -\t\tesac &&\n> +\t\ttest \"$src\" = file2 &&\n>  \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n>  \t\tgit p4 submit &&\n>  \t\tp4 filelog //depot/file12 &&\n>  \t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n>  \n> +\t\techo \"file13\" >> file2 &&\n> +\t\tgit commit -a -m \"Differentiate file2\" &&\n> +\t\tgit p4 submit &&\n> +\n>  \t\tcp file2 file13 &&\n>  \t\techo \"different text\" >>file13 &&\n>  \t\tgit add file13 &&\n> @@ -197,14 +216,11 @@ test_expect_success 'detect copies' '\n>  \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n>  \t\ttest -n \"$level\" && test \"$level\" -gt 2 && test \"$level\" -lt 100 &&\n>  \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n> -\t\tcase \"$src\" in\n> -\t\tfile10 | file11 | file12) : ;; # happy\n> -\t\t*) false ;; # not\n> -\t\tesac &&\n> +\t\ttest \"$src\" = file2 &&\n>  \t\tgit config git-p4.detectCopies $(($level - 2)) &&\n>  \t\tgit p4 submit &&\n>  \t\tp4 filelog //depot/file13 &&\n> -\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file\"\n> +\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file2\"\n>  \t)\n>  '\n"},{"id":"258742","messageId":"CAE5ih7-jrh2dE=LS7dWyM-xxpGe5yFPnDfZUYGX-=0o4uaM72A@mail.gmail.com","threadId":"38933","inReplyTo":"xmqqk2xzxk3y.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-03-31T13:26:56Z","receivedAt":"2015-03-31T13:26:56Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"I'm on holiday this week, so I'll not get a chance to look at these\nproperly until next week.\n\nLuke\n\n\nOn 30 March 2015 at 04:03, Junio C Hamano <gitster@pobox.com> wrote:\n> Vitor Antunes <vitor.hda@gmail.com> writes:\n>\n>> * Modify source file (file2) before copying the file.\n>> * Check that only file2 is the source in the output of \"p4 filelog\".\n>> * Remove all \"case\" statements and replace them simple tests to check that\n>>   source is \"file2\".\n>>\n>> Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>\n>> ---\n>\n> I am not a Perfoce user, so I'd like to ask Pete's and Luke's\n> comments on these changes.\n>\n>>  t/t9814-git-p4-rename.sh |   46 +++++++++++++++++++++++++++++++---------------\n>>  1 file changed, 31 insertions(+), 15 deletions(-)\n>>\n>> diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\n>> index 8b9c295..d8fb22d 100755\n>> --- a/t/t9814-git-p4-rename.sh\n>> +++ b/t/t9814-git-p4-rename.sh\n>> @@ -132,6 +132,9 @@ test_expect_success 'detect copies' '\n>>               cd \"$git\" &&\n>>               git config git-p4.skipSubmitEdit true &&\n>>\n>> +             echo \"file8\" >> file2 &&\n>\n> Style: please lose SP between redirection and its target, i.e.\n>\n>         echo file8 >>file2 &&\n>\n> The same comment applies to everywhere else.\n>\n>> +             git commit -a -m \"Differentiate file2\" &&\n>> +             git p4 submit &&\n>>               cp file2 file8 &&\n>>               git add file8 &&\n>>               git commit -a -m \"Copy file2 to file8\" &&\n>> @@ -140,6 +143,10 @@ test_expect_success 'detect copies' '\n>>               p4 filelog //depot/file8 &&\n>>               p4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n>>\n>> +             echo \"file9\" >> file2 &&\n>> +             git commit -a -m \"Differentiate file2\" &&\n>> +             git p4 submit &&\n>> +\n>>               cp file2 file9 &&\n>>               git add file9 &&\n>>               git commit -a -m \"Copy file2 to file9\" &&\n>> @@ -149,28 +156,39 @@ test_expect_success 'detect copies' '\n>>               p4 filelog //depot/file9 &&\n>>               p4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n>>\n>> +             echo \"file10\" >> file2 &&\n>> +             git commit -a -m \"Differentiate file2\" &&\n>> +             git p4 submit &&\n>> +\n>>               echo \"file2\" >>file2 &&\n>>               cp file2 file10 &&\n>>               git add file2 file10 &&\n>>               git commit -a -m \"Modify and copy file2 to file10\" &&\n>>               git diff-tree -r -C HEAD &&\n>> +             src=$(git diff-tree -r -C HEAD | sed 1d | sed 2d | cut -f2) &&\n>> +             test \"$src\" = file2 &&\n>>               git p4 submit &&\n>>               p4 filelog //depot/file10 &&\n>> -             p4 filelog //depot/file10 | grep -q \"branch from //depot/file\" &&\n>> +             p4 filelog //depot/file10 | grep -q \"branch from //depot/file2\" &&\n>> +\n>> +             echo \"file11\" >> file2 &&\n>> +             git commit -a -m \"Differentiate file2\" &&\n>> +             git p4 submit &&\n>>\n>>               cp file2 file11 &&\n>>               git add file11 &&\n>>               git commit -a -m \"Copy file2 to file11\" &&\n>>               git diff-tree -r -C --find-copies-harder HEAD &&\n>>               src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n>> -             case \"$src\" in\n>> -             file2 | file10) : ;; # happy\n>> -             *) false ;; # not\n>> -             esac &&\n>> +             test \"$src\" = file2 &&\n>>               git config git-p4.detectCopiesHarder true &&\n>>               git p4 submit &&\n>>               p4 filelog //depot/file11 &&\n>> -             p4 filelog //depot/file11 | grep -q \"branch from //depot/file\" &&\n>> +             p4 filelog //depot/file11 | grep -q \"branch from //depot/file2\" &&\n>> +\n>> +             echo \"file12\" >> file2 &&\n>> +             git commit -a -m \"Differentiate file2\" &&\n>> +             git p4 submit &&\n>>\n>>               cp file2 file12 &&\n>>               echo \"some text\" >>file12 &&\n>> @@ -180,15 +198,16 @@ test_expect_success 'detect copies' '\n>>               level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n>>               test -n \"$level\" && test \"$level\" -gt 0 && test \"$level\" -lt 98 &&\n>>               src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n>> -             case \"$src\" in\n>> -             file10 | file11) : ;; # happy\n>> -             *) false ;; # not\n>> -             esac &&\n>> +             test \"$src\" = file2 &&\n>>               git config git-p4.detectCopies $(($level + 2)) &&\n>>               git p4 submit &&\n>>               p4 filelog //depot/file12 &&\n>>               p4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n>>\n>> +             echo \"file13\" >> file2 &&\n>> +             git commit -a -m \"Differentiate file2\" &&\n>> +             git p4 submit &&\n>> +\n>>               cp file2 file13 &&\n>>               echo \"different text\" >>file13 &&\n>>               git add file13 &&\n>> @@ -197,14 +216,11 @@ test_expect_success 'detect copies' '\n>>               level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n>>               test -n \"$level\" && test \"$level\" -gt 2 && test \"$level\" -lt 100 &&\n>>               src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n>> -             case \"$src\" in\n>> -             file10 | file11 | file12) : ;; # happy\n>> -             *) false ;; # not\n>> -             esac &&\n>> +             test \"$src\" = file2 &&\n>>               git config git-p4.detectCopies $(($level - 2)) &&\n>>               git p4 submit &&\n>>               p4 filelog //depot/file13 &&\n>> -             p4 filelog //depot/file13 | grep -q \"branch from //depot/file\"\n>> +             p4 filelog //depot/file13 | grep -q \"branch from //depot/file2\"\n>>       )\n>>  '\n"},{"id":"258801","messageId":"1427844582-29749-1-git-send-email-vitor.hda@gmail.com","threadId":"38933","inReplyTo":"xmqqk2xzxk3y.fsf@gitster.dls.corp.google.com","subject":"[PATCH V2] t9814: Guarantee only one source exists in git-p4 copy tests","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-31T23:29:42Z","receivedAt":"2015-03-31T23:29:42Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"* Modify source file (file2) before copying the file.\n* Check that only file2 is the source in the output of \"p4 filelog\".\n* Remove all \"case\" statements and replace them with simple tests to check\n  that source is \"file2\".\n\nSigned-off-by: Vitor Antunes <vitor.hda@gmail.com>\n---\n t/t9814-git-p4-rename.sh |   46 +++++++++++++++++++++++++++++++---------------\n 1 file changed, 31 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\nindex 8b9c295..99bb71b 100755\n--- a/t/t9814-git-p4-rename.sh\n+++ b/t/t9814-git-p4-rename.sh\n@@ -132,6 +132,9 @@ test_expect_success 'detect copies' '\n \t\tcd \"$git\" &&\n \t\tgit config git-p4.skipSubmitEdit true &&\n \n+\t\techo \"file8\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n \t\tcp file2 file8 &&\n \t\tgit add file8 &&\n \t\tgit commit -a -m \"Copy file2 to file8\" &&\n@@ -140,6 +143,10 @@ test_expect_success 'detect copies' '\n \t\tp4 filelog //depot/file8 &&\n \t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n \n+\t\techo \"file9\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n+\n \t\tcp file2 file9 &&\n \t\tgit add file9 &&\n \t\tgit commit -a -m \"Copy file2 to file9\" &&\n@@ -149,28 +156,39 @@ test_expect_success 'detect copies' '\n \t\tp4 filelog //depot/file9 &&\n \t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n \n+\t\techo \"file10\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n+\n \t\techo \"file2\" >>file2 &&\n \t\tcp file2 file10 &&\n \t\tgit add file2 file10 &&\n \t\tgit commit -a -m \"Modify and copy file2 to file10\" &&\n \t\tgit diff-tree -r -C HEAD &&\n+\t\tsrc=$(git diff-tree -r -C HEAD | sed 1d | sed 2d | cut -f2) &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file10 &&\n-\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file\" &&\n+\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file2\" &&\n+\n+\t\techo \"file11\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n \n \t\tcp file2 file11 &&\n \t\tgit add file11 &&\n \t\tgit commit -a -m \"Copy file2 to file11\" &&\n \t\tgit diff-tree -r -C --find-copies-harder HEAD &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\tcase \"$src\" in\n-\t\tfile2 | file10) : ;; # happy\n-\t\t*) false ;; # not\n-\t\tesac &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopiesHarder true &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file11 &&\n-\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file\" &&\n+\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file2\" &&\n+\n+\t\techo \"file12\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n \n \t\tcp file2 file12 &&\n \t\techo \"some text\" >>file12 &&\n@@ -180,15 +198,16 @@ test_expect_success 'detect copies' '\n \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n \t\ttest -n \"$level\" && test \"$level\" -gt 0 && test \"$level\" -lt 98 &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\tcase \"$src\" in\n-\t\tfile10 | file11) : ;; # happy\n-\t\t*) false ;; # not\n-\t\tesac &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file12 &&\n \t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n \n+\t\techo \"file13\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n+\n \t\tcp file2 file13 &&\n \t\techo \"different text\" >>file13 &&\n \t\tgit add file13 &&\n@@ -197,14 +216,11 @@ test_expect_success 'detect copies' '\n \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n \t\ttest -n \"$level\" && test \"$level\" -gt 2 && test \"$level\" -lt 100 &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\tcase \"$src\" in\n-\t\tfile10 | file11 | file12) : ;; # happy\n-\t\t*) false ;; # not\n-\t\tesac &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopies $(($level - 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file13 &&\n-\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file\"\n+\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file2\"\n \t)\n '\n \n-- \n1.7.10.4\n"},{"id":"258985","messageId":"551FA15D.30304@diamand.org","threadId":"38933","inReplyTo":"xmqqk2xzxk3y.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-04-04T08:31:25Z","receivedAt":"2015-04-04T08:31:25Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 30/03/15 04:03, Junio C Hamano wrote:\n> Vitor Antunes <vitor.hda@gmail.com> writes:\n>\n>> * Modify source file (file2) before copying the file.\n>> * Check that only file2 is the source in the output of \"p4 filelog\".\n>> * Remove all \"case\" statements and replace them simple tests to check that\n>>    source is \"file2\".\n>>\n>> Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>\n>> ---\n>\n> I am not a Perfoce user, so I'd like to ask Pete's and Luke's\n> comments on these changes.\n\nIt's much clearer now that the guessing of file source has been cleaned \nup, thanks. Ack.\n\nLuke\n\n\n\n>\n>>   t/t9814-git-p4-rename.sh |   46 +++++++++++++++++++++++++++++++---------------\n>>   1 file changed, 31 insertions(+), 15 deletions(-)\n>>\n>> diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\n>> index 8b9c295..d8fb22d 100755\n>> --- a/t/t9814-git-p4-rename.sh\n>> +++ b/t/t9814-git-p4-rename.sh\n>> @@ -132,6 +132,9 @@ test_expect_success 'detect copies' '\n>>   \t\tcd \"$git\" &&\n>>   \t\tgit config git-p4.skipSubmitEdit true &&\n>>\n>> +\t\techo \"file8\" >> file2 &&\n>\n> Style: please lose SP between redirection and its target, i.e.\n>\n> \techo file8 >>file2 &&\n>\n> The same comment applies to everywhere else.\n>\n>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>> +\t\tgit p4 submit &&\n>>   \t\tcp file2 file8 &&\n>>   \t\tgit add file8 &&\n>>   \t\tgit commit -a -m \"Copy file2 to file8\" &&\n>> @@ -140,6 +143,10 @@ test_expect_success 'detect copies' '\n>>   \t\tp4 filelog //depot/file8 &&\n>>   \t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n>>\n>> +\t\techo \"file9\" >> file2 &&\n>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>> +\t\tgit p4 submit &&\n>> +\n>>   \t\tcp file2 file9 &&\n>>   \t\tgit add file9 &&\n>>   \t\tgit commit -a -m \"Copy file2 to file9\" &&\n>> @@ -149,28 +156,39 @@ test_expect_success 'detect copies' '\n>>   \t\tp4 filelog //depot/file9 &&\n>>   \t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n>>\n>> +\t\techo \"file10\" >> file2 &&\n>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>> +\t\tgit p4 submit &&\n>> +\n>>   \t\techo \"file2\" >>file2 &&\n>>   \t\tcp file2 file10 &&\n>>   \t\tgit add file2 file10 &&\n>>   \t\tgit commit -a -m \"Modify and copy file2 to file10\" &&\n>>   \t\tgit diff-tree -r -C HEAD &&\n>> +\t\tsrc=$(git diff-tree -r -C HEAD | sed 1d | sed 2d | cut -f2) &&\n>> +\t\ttest \"$src\" = file2 &&\n>>   \t\tgit p4 submit &&\n>>   \t\tp4 filelog //depot/file10 &&\n>> -\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file\" &&\n>> +\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file2\" &&\n>> +\n>> +\t\techo \"file11\" >> file2 &&\n>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>> +\t\tgit p4 submit &&\n>>\n>>   \t\tcp file2 file11 &&\n>>   \t\tgit add file11 &&\n>>   \t\tgit commit -a -m \"Copy file2 to file11\" &&\n>>   \t\tgit diff-tree -r -C --find-copies-harder HEAD &&\n>>   \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n>> -\t\tcase \"$src\" in\n>> -\t\tfile2 | file10) : ;; # happy\n>> -\t\t*) false ;; # not\n>> -\t\tesac &&\n>> +\t\ttest \"$src\" = file2 &&\n>>   \t\tgit config git-p4.detectCopiesHarder true &&\n>>   \t\tgit p4 submit &&\n>>   \t\tp4 filelog //depot/file11 &&\n>> -\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file\" &&\n>> +\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file2\" &&\n>> +\n>> +\t\techo \"file12\" >> file2 &&\n>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>> +\t\tgit p4 submit &&\n>>\n>>   \t\tcp file2 file12 &&\n>>   \t\techo \"some text\" >>file12 &&\n>> @@ -180,15 +198,16 @@ test_expect_success 'detect copies' '\n>>   \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n>>   \t\ttest -n \"$level\" && test \"$level\" -gt 0 && test \"$level\" -lt 98 &&\n>>   \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n>> -\t\tcase \"$src\" in\n>> -\t\tfile10 | file11) : ;; # happy\n>> -\t\t*) false ;; # not\n>> -\t\tesac &&\n>> +\t\ttest \"$src\" = file2 &&\n>>   \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n>>   \t\tgit p4 submit &&\n>>   \t\tp4 filelog //depot/file12 &&\n>>   \t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n>>\n>> +\t\techo \"file13\" >> file2 &&\n>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>> +\t\tgit p4 submit &&\n>> +\n>>   \t\tcp file2 file13 &&\n>>   \t\techo \"different text\" >>file13 &&\n>>   \t\tgit add file13 &&\n>> @@ -197,14 +216,11 @@ test_expect_success 'detect copies' '\n>>   \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n>>   \t\ttest -n \"$level\" && test \"$level\" -gt 2 && test \"$level\" -lt 100 &&\n>>   \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n>> -\t\tcase \"$src\" in\n>> -\t\tfile10 | file11 | file12) : ;; # happy\n>> -\t\t*) false ;; # not\n>> -\t\tesac &&\n>> +\t\ttest \"$src\" = file2 &&\n>>   \t\tgit config git-p4.detectCopies $(($level - 2)) &&\n>>   \t\tgit p4 submit &&\n>>   \t\tp4 filelog //depot/file13 &&\n>> -\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file\"\n>> +\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file2\"\n>>   \t)\n>>   '\n"},{"id":"258996","messageId":"xmqq384ffzqm.fsf@gitster.dls.corp.google.com","threadId":"38933","inReplyTo":"551FA15D.30304@diamand.org","subject":"Re: [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-04T19:41:05Z","receivedAt":"2015-04-04T19:41:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand <luke@diamand.org> writes:\n\n> On 30/03/15 04:03, Junio C Hamano wrote:\n>> Vitor Antunes <vitor.hda@gmail.com> writes:\n>>\n>>> * Modify source file (file2) before copying the file.\n>>> * Check that only file2 is the source in the output of \"p4 filelog\".\n>>> * Remove all \"case\" statements and replace them simple tests to check that\n>>>    source is \"file2\".\n>>>\n>>> Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>\n>>> ---\n>>\n>> I am not a Perfoce user, so I'd like to ask Pete's and Luke's\n>> comments on these changes.\n>\n> It's much clearer now that the guessing of file source has been\n> cleaned up, thanks. Ack.\n\nThanks.\n\nVitor, when resubmitting v2 to fix style nits, please add Luke's\nAcked-by just after your Sign-off, perhaps like this:\n\n    t9814: guarantee only one source exists in git-p4 copy tests\n    \n    By using a tree with multiple identical files and allowing copy\n    detection to choose any one of them, the check in the test is\n    unnecessarily complex.  We can simplify by:\n    \n     * Modify source file (file2) before copying the file.\n    \n     * Check that only file2 is the source in the output of \"p4 filelog\".\n    \n     * Remove all \"case\" statements and replace them simple tests to\n       check that source is \"file2\".\n    \n    Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>\n    Acked-by: Luke Diamand <luke@diamand.org>\n\n\n>>> diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\n>>> index 8b9c295..d8fb22d 100755\n>>> --- a/t/t9814-git-p4-rename.sh\n>>> +++ b/t/t9814-git-p4-rename.sh\n>>> @@ -132,6 +132,9 @@ test_expect_success 'detect copies' '\n>>>   \t\tcd \"$git\" &&\n>>>   \t\tgit config git-p4.skipSubmitEdit true &&\n>>>\n>>> +\t\techo \"file8\" >> file2 &&\n>>\n>> Style: please lose SP between redirection and its target, i.e.\n>>\n>> \techo file8 >>file2 &&\n>>\n>> The same comment applies to everywhere else.\n>>\n>>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>>> +\t\tgit p4 submit &&\n>>>   \t\tcp file2 file8 &&\n>>>   \t\tgit add file8 &&\n>>>   \t\tgit commit -a -m \"Copy file2 to file8\" &&\n>>> @@ -140,6 +143,10 @@ test_expect_success 'detect copies' '\n>>>   \t\tp4 filelog //depot/file8 &&\n>>>   \t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n>>>\n>>> +\t\techo \"file9\" >> file2 &&\n>>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>>> +\t\tgit p4 submit &&\n>>> +\n>>>   \t\tcp file2 file9 &&\n>>>   \t\tgit add file9 &&\n>>>   \t\tgit commit -a -m \"Copy file2 to file9\" &&\n>>> @@ -149,28 +156,39 @@ test_expect_success 'detect copies' '\n>>>   \t\tp4 filelog //depot/file9 &&\n>>>   \t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n>>>\n>>> +\t\techo \"file10\" >> file2 &&\n>>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>>> +\t\tgit p4 submit &&\n>>> +\n>>>   \t\techo \"file2\" >>file2 &&\n>>>   \t\tcp file2 file10 &&\n>>>   \t\tgit add file2 file10 &&\n>>>   \t\tgit commit -a -m \"Modify and copy file2 to file10\" &&\n>>>   \t\tgit diff-tree -r -C HEAD &&\n>>> +\t\tsrc=$(git diff-tree -r -C HEAD | sed 1d | sed 2d | cut -f2) &&\n>>> +\t\ttest \"$src\" = file2 &&\n>>>   \t\tgit p4 submit &&\n>>>   \t\tp4 filelog //depot/file10 &&\n>>> -\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file\" &&\n>>> +\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file2\" &&\n>>> +\n>>> +\t\techo \"file11\" >> file2 &&\n>>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>>> +\t\tgit p4 submit &&\n>>>\n>>>   \t\tcp file2 file11 &&\n>>>   \t\tgit add file11 &&\n>>>   \t\tgit commit -a -m \"Copy file2 to file11\" &&\n>>>   \t\tgit diff-tree -r -C --find-copies-harder HEAD &&\n>>>   \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n>>> -\t\tcase \"$src\" in\n>>> -\t\tfile2 | file10) : ;; # happy\n>>> -\t\t*) false ;; # not\n>>> -\t\tesac &&\n>>> +\t\ttest \"$src\" = file2 &&\n>>>   \t\tgit config git-p4.detectCopiesHarder true &&\n>>>   \t\tgit p4 submit &&\n>>>   \t\tp4 filelog //depot/file11 &&\n>>> -\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file\" &&\n>>> +\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file2\" &&\n>>> +\n>>> +\t\techo \"file12\" >> file2 &&\n>>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>>> +\t\tgit p4 submit &&\n>>>\n>>>   \t\tcp file2 file12 &&\n>>>   \t\techo \"some text\" >>file12 &&\n>>> @@ -180,15 +198,16 @@ test_expect_success 'detect copies' '\n>>>   \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n>>>   \t\ttest -n \"$level\" && test \"$level\" -gt 0 && test \"$level\" -lt 98 &&\n>>>   \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n>>> -\t\tcase \"$src\" in\n>>> -\t\tfile10 | file11) : ;; # happy\n>>> -\t\t*) false ;; # not\n>>> -\t\tesac &&\n>>> +\t\ttest \"$src\" = file2 &&\n>>>   \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n>>>   \t\tgit p4 submit &&\n>>>   \t\tp4 filelog //depot/file12 &&\n>>>   \t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n>>>\n>>> +\t\techo \"file13\" >> file2 &&\n>>> +\t\tgit commit -a -m \"Differentiate file2\" &&\n>>> +\t\tgit p4 submit &&\n>>> +\n>>>   \t\tcp file2 file13 &&\n>>>   \t\techo \"different text\" >>file13 &&\n>>>   \t\tgit add file13 &&\n>>> @@ -197,14 +216,11 @@ test_expect_success 'detect copies' '\n>>>   \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n>>>   \t\ttest -n \"$level\" && test \"$level\" -gt 2 && test \"$level\" -lt 100 &&\n>>>   \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n>>> -\t\tcase \"$src\" in\n>>> -\t\tfile10 | file11 | file12) : ;; # happy\n>>> -\t\t*) false ;; # not\n>>> -\t\tesac &&\n>>> +\t\ttest \"$src\" = file2 &&\n>>>   \t\tgit config git-p4.detectCopies $(($level - 2)) &&\n>>>   \t\tgit p4 submit &&\n>>>   \t\tp4 filelog //depot/file13 &&\n>>> -\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file\"\n>>> +\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file2\"\n>>>   \t)\n>>>   '\n"},{"id":"259030","messageId":"55218C8F.209@diamand.org","threadId":"38933","inReplyTo":"1427545730-3563-1-git-send-email-vitor.hda@gmail.com","subject":"Re: [PATCH 0/2] git-p4: Improve client path detection","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-04-05T19:27:11Z","receivedAt":"2015-04-05T19:27:11Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 28/03/15 12:28, Vitor Antunes wrote:\n> I'm adding a test case for a scenario I was confronted with when using branch\n> detection and a client view specification. It is possible that the implemented\n> fix may not cover all possible scenarios, but there is no regression in the\n> available tests.\n\nVitor, one thing I wondered about with this part of the change:\n\n-            if entry[\"depotFile\"] == depotPath:\n+            if entry[\"depotFile\"].find(depotPath) >= 0:\n\nDoes this mean that if 'p4 where' produces multiple lines of output that \nthis will get confused, as it's just going to search for an instance of \ndepotPath.\n\nThe example in the Perforce man page for 'p4 where' would trigger this \nfor example:\n\nhttp://www.perforce.com/perforce/r14.2/manuals/cmdref/p4_where.html\n\n-//a/b/file.txt //client/a/b/file.txt //home/user/root/a/b/file.txt\n//a/b/file.txt //client/b/file.txt /home/user/root/b/file.txt\n\nAs an experiment, I hacked git-p4 to always use p4Where rather than \ngetClientRoot(), which I would have thought ought to work, but while \nmost of the tests passed, Pete's client-spec torture tests failed.\n\nLuke\n\n\n>\n> Vitor Antunes (2):\n>    git-p4: Check branch detection and client view together\n>    git-p4: Improve client path detection when branches are used\n>\n>   git-p4.py                |   11 ++++--\n>   t/t9801-git-p4-branch.sh |   98 ++++++++++++++++++++++++++++++++++++++++++++++\n>   2 files changed, 105 insertions(+), 4 deletions(-)\n>\n"},{"id":"259039","messageId":"20150405235759.392c0f2b@pt-vhugo","threadId":"38933","inReplyTo":"55218C8F.209@diamand.org","subject":"Re: [PATCH 0/2] git-p4: Improve client path detection","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-04-05T22:57:59Z","receivedAt":"2015-04-05T22:57:59Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Luke Diamand <luke@diamand.org> wrote on Sun, 05 Apr 2015 20:27:11 +0100\n> On 28/03/15 12:28, Vitor Antunes wrote:\n> > I'm adding a test case for a scenario I was confronted with when using branch\n> > detection and a client view specification. It is possible that the implemented\n> > fix may not cover all possible scenarios, but there is no regression in the\n> > available tests.\n>\n> Vitor, one thing I wondered about with this part of the change:\n>\n> -            if entry[\"depotFile\"] == depotPath:\n> +            if entry[\"depotFile\"].find(depotPath) >= 0:\n>\n> Does this mean that if 'p4 where' produces multiple lines of output that\n> this will get confused, as it's just going to search for an instance of\n> depotPath.\n\nThe reason why I introduced that was because in the test case I implemented (and\nwhich reflects a scenario I am confronted with in my workplace) the branches\nhave a base directory that is removed in the client view mapping.\nAs such, we will have a situation where depotPath is //depot/branch1/ while\nrunninng \"p4 where\" will result in //depot/branch1/base/. To overcome this I\nused find() instead of a direct comparison. Now that I think about that, I could\nprobably have used the simpler `if depotPath in entry[\"depotFile\"]`...\n\n> The example in the Perforce man page for 'p4 where' would trigger this\n> for example:\n>\n> http://www.perforce.com/perforce/r14.2/manuals/cmdref/p4_where.html\n>\n> -//a/b/file.txt //client/a/b/file.txt //home/user/root/a/b/file.txt\n> //a/b/file.txt //client/b/file.txt /home/user/root/b/file.txt\n\nThese are examples where a simple comparison as was implemented would work.\n\n> As an experiment, I hacked git-p4 to always use p4Where rather than\n> getClientRoot(), which I would have thought ought to work, but while\n> most of the tests passed, Pete's client-spec torture tests failed.\n\nThat was exactly my first approach and got to the same conclusion. I would have\ninvestigated it further but since I haven't had much free time to invest in\nsolving this problem I decided to implement an intermediary solution that would\nnot introduce any regressions.\n\nVitor\n"},{"id":"259041","messageId":"1428275315-15425-1-git-send-email-vitor.hda@gmail.com","threadId":"38933","inReplyTo":"xmqq384ffzqm.fsf@gitster.dls.corp.google.com","subject":"[PATCH V3] t9814: Guarantee only one source exists in git-p4 copy tests","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-04-05T23:08:35Z","receivedAt":"2015-04-05T23:08:35Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"By using a tree with multiple identical files and allowing copy detection to\nchoose any one of them, the check in the test is unnecessarily complex.  We can\nsimplify by:\n\n* Modify source file (file2) before copying the file.\n* Check that only file2 is the source in the output of \"p4 filelog\".\n* Remove all \"case\" statements and replace them with simple tests to check\n  that source is \"file2\".\n\nSigned-off-by: Vitor Antunes <vitor.hda@gmail.com>\nAcked-by: Luke Diamand <luke@diamand.org>\n---\n t/t9814-git-p4-rename.sh |   46 +++++++++++++++++++++++++++++++---------------\n 1 file changed, 31 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\nindex 8b9c295..99bb71b 100755\n--- a/t/t9814-git-p4-rename.sh\n+++ b/t/t9814-git-p4-rename.sh\n@@ -132,6 +132,9 @@ test_expect_success 'detect copies' '\n \t\tcd \"$git\" &&\n \t\tgit config git-p4.skipSubmitEdit true &&\n \n+\t\techo \"file8\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n \t\tcp file2 file8 &&\n \t\tgit add file8 &&\n \t\tgit commit -a -m \"Copy file2 to file8\" &&\n@@ -140,6 +143,10 @@ test_expect_success 'detect copies' '\n \t\tp4 filelog //depot/file8 &&\n \t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n \n+\t\techo \"file9\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n+\n \t\tcp file2 file9 &&\n \t\tgit add file9 &&\n \t\tgit commit -a -m \"Copy file2 to file9\" &&\n@@ -149,28 +156,39 @@ test_expect_success 'detect copies' '\n \t\tp4 filelog //depot/file9 &&\n \t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n \n+\t\techo \"file10\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n+\n \t\techo \"file2\" >>file2 &&\n \t\tcp file2 file10 &&\n \t\tgit add file2 file10 &&\n \t\tgit commit -a -m \"Modify and copy file2 to file10\" &&\n \t\tgit diff-tree -r -C HEAD &&\n+\t\tsrc=$(git diff-tree -r -C HEAD | sed 1d | sed 2d | cut -f2) &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file10 &&\n-\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file\" &&\n+\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file2\" &&\n+\n+\t\techo \"file11\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n \n \t\tcp file2 file11 &&\n \t\tgit add file11 &&\n \t\tgit commit -a -m \"Copy file2 to file11\" &&\n \t\tgit diff-tree -r -C --find-copies-harder HEAD &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\tcase \"$src\" in\n-\t\tfile2 | file10) : ;; # happy\n-\t\t*) false ;; # not\n-\t\tesac &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopiesHarder true &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file11 &&\n-\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file\" &&\n+\t\tp4 filelog //depot/file11 | grep -q \"branch from //depot/file2\" &&\n+\n+\t\techo \"file12\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n \n \t\tcp file2 file12 &&\n \t\techo \"some text\" >>file12 &&\n@@ -180,15 +198,16 @@ test_expect_success 'detect copies' '\n \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n \t\ttest -n \"$level\" && test \"$level\" -gt 0 && test \"$level\" -lt 98 &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\tcase \"$src\" in\n-\t\tfile10 | file11) : ;; # happy\n-\t\t*) false ;; # not\n-\t\tesac &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file12 &&\n \t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n \n+\t\techo \"file13\" >>file2 &&\n+\t\tgit commit -a -m \"Differentiate file2\" &&\n+\t\tgit p4 submit &&\n+\n \t\tcp file2 file13 &&\n \t\techo \"different text\" >>file13 &&\n \t\tgit add file13 &&\n@@ -197,14 +216,11 @@ test_expect_success 'detect copies' '\n \t\tlevel=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d\" \" -f5 | sed \"s/C0*//\") &&\n \t\ttest -n \"$level\" && test \"$level\" -gt 2 && test \"$level\" -lt 100 &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\tcase \"$src\" in\n-\t\tfile10 | file11 | file12) : ;; # happy\n-\t\t*) false ;; # not\n-\t\tesac &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopies $(($level - 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file13 &&\n-\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file\"\n+\t\tp4 filelog //depot/file13 | grep -q \"branch from //depot/file2\"\n \t)\n '\n \n-- \n1.7.10.4\n"},{"id":"259271","messageId":"xmqqsic44rw5.fsf@gitster.dls.corp.google.com","threadId":"38933","inReplyTo":"20150405235759.392c0f2b@pt-vhugo","subject":"Re: [PATCH 0/2] git-p4: Improve client path detection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-13T03:40:58Z","receivedAt":"2015-04-13T03:40:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vitor Antunes <vitor.hda@gmail.com> writes:\n\n> Luke Diamand <luke@diamand.org> wrote on Sun, 05 Apr 2015 20:27:11 +0100\n>> On 28/03/15 12:28, Vitor Antunes wrote:\n>> > I'm adding a test case for a scenario I was confronted with when using branch\n>> > detection and a client view specification. It is possible that the implemented\n>> > fix may not cover all possible scenarios, but there is no regression in the\n>> > available tests.\n>>\n>> Vitor, one thing I wondered about with this part of the change:\n>>\n>> -            if entry[\"depotFile\"] == depotPath:\n>> +            if entry[\"depotFile\"].find(depotPath) >= 0:\n>>\n>> Does this mean that if 'p4 where' produces multiple lines of output that\n>> this will get confused, as it's just going to search for an instance of\n>> depotPath.\n>\n> The reason why I introduced that was because in the test case I implemented (and\n> which reflects a scenario I am confronted with in my workplace) the branches\n> have a base directory that is removed in the client view mapping.\n> As such, we will have a situation where depotPath is //depot/branch1/ while\n> runninng \"p4 where\" will result in //depot/branch1/base/. To overcome this I\n> used find() instead of a direct comparison. Now that I think about that, I could\n> probably have used the simpler `if depotPath in entry[\"depotFile\"]`...\n\nHmph, is this find() under discussion the string.find() that finds a\nsubstring?  You are doing >=0 comparison here, but with your example\nthat entry[\"depotFile\"] may have \"base/\" appended to what you\nexpect, the result of running string.find() must yield \"0\", i.e. no\nextra prefix string, no?  I kind of find it hard to believe that it\nis OK to have any extra prefix is fine ...\n\n>> The example in the Perforce man page for 'p4 where' would trigger this\n>> for example:\n>>\n>> http://www.perforce.com/perforce/r14.2/manuals/cmdref/p4_where.html\n>>\n>> -//a/b/file.txt //client/a/b/file.txt //home/user/root/a/b/file.txt\n>> //a/b/file.txt //client/b/file.txt /home/user/root/b/file.txt\n>\n> These are examples where a simple comparison as was implemented would work.\n\n... so is this \"find()\" an attempt to catch prefix like \"-\"?  Even\nif it that were the reason why you do not limit the acceptable\nreturn value from find() to zero, it feels a bit too loose to allow\nanything if the only thing you want to allow is a single \"-\" prefix.\n\nCan you explain this a bit better?  I cannot quite tell what is\ngoing on from what was written in the log message.\n\n>> As an experiment, I hacked git-p4 to always use p4Where rather than\n>> getClientRoot(), which I would have thought ought to work, but while\n>> most of the tests passed, Pete's client-spec torture tests failed.\n>\n> That was exactly my first approach and got to the same conclusion. I would have\n> investigated it further but since I haven't had much free time to invest in\n> solving this problem I decided to implement an intermediary solution that would\n> not introduce any regressions.\n\nThanks.\n"},{"id":"259634","messageId":"20150418234044.3adfcff0@pt-vhugo","threadId":"38933","inReplyTo":"xmqqsic44rw5.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/2] git-p4: Improve client path detection","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-04-18T22:40:44Z","receivedAt":"2015-04-18T22:40:44Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Hi Junio,\n\nJunio C Hamano <gitster@pobox.com> wrote on Sun, 12 Apr 2015 20:40:58 -0700\n> Vitor Antunes <vitor.hda@gmail.com> writes:\n>> Luke Diamand <luke@diamand.org> wrote on Sun, 05 Apr 2015 20:27:11 +0100\n>>> Vitor, one thing I wondered about with this part of the change:\n>>>\n>>> -            if entry[\"depotFile\"] == depotPath:\n>>> +            if entry[\"depotFile\"].find(depotPath) >= 0:\n>>>\n>>> Does this mean that if 'p4 where' produces multiple lines of output that\n>>> this will get confused, as it's just going to search for an instance of\n>>> depotPath.\n>>\n>> The reason why I introduced that was because in the test case I implemented (and\n>> which reflects a scenario I am confronted with in my workplace) the branches\n>> have a base directory that is removed in the client view mapping.\n>> As such, we will have a situation where depotPath is //depot/branch1/ while\n>> runninng \"p4 where\" will result in //depot/branch1/base/. To overcome this I\n>> used find() instead of a direct comparison. Now that I think about that, I could\n>> probably have used the simpler `if depotPath in entry[\"depotFile\"]`...\n>\n> Hmph, is this find() under discussion the string.find() that finds a\n> substring?  You are doing >=0 comparison here, but with your example\n> that entry[\"depotFile\"] may have \"base/\" appended to what you\n> expect, the result of running string.find() must yield \"0\", i.e. no\n> extra prefix string, no?  I kind of find it hard to believe that it\n> is OK to have any extra prefix is fine ...\n\nAs usual, you're correct about your assumption. I should in fact be\nusing \"== 0\" because what I really want is to guarantee that the path\n_starts_ with //depot/branch1.\n\n>>> The example in the Perforce man page for 'p4 where' would trigger this\n>>> for example:\n>>>\n>>> http://www.perforce.com/perforce/r14.2/manuals/cmdref/p4_where.html\n>>>\n>>> -//a/b/file.txt //client/a/b/file.txt //home/user/root/a/b/file.txt\n>>> //a/b/file.txt //client/b/file.txt /home/user/root/b/file.txt\n>>\n>> These are examples where a simple comparison as was implemented would work.\n>\n> ... so is this \"find()\" an attempt to catch prefix like \"-\"?  Even\n> if it that were the reason why you do not limit the acceptable\n> return value from find() to zero, it feels a bit too loose to allow\n> anything if the only thing you want to allow is a single \"-\" prefix.\n\nAgain, it was just a bad coding from my part.\n\n> Can you explain this a bit better?  I cannot quite tell what is\n> going on from what was written in the log message.\n\nI've temporarily modified the script to print out the output of \"p4\nwhere\", for future reference:\n\n[{'clientFile': '//client/branch1/...',           'code': 'stat',              'depotFile': '//depot/branch1/base/...',           'path': '/path/to/git/t/trash directory.t9801-git-p4-branch/cli/branch1/...'},\n {'clientFile': '//client/branch1/sub_file1',     'code': 'stat', 'unmap': '', 'depotFile': '//depot/branch1/base/sub_file1',     'path': '/path/to/git/t/trash directory.t9801-git-p4-branch/cli/branch1/sub_file1'},\n {'clientFile': '//client/branch1/dir/sub_file1', 'code': 'stat', 'unmap': '', 'depotFile': '//depot/branch1/base/dir/sub_file1', 'path': '/path/to/git/t/trash directory.t9801-git-p4-branch/cli/branch1/dir/sub_file1'},\n {'clientFile': '//client/branch1/sub_file1',     'code': 'stat',              'depotFile': '//depot/branch1/base/dir/sub_file1', 'path': '/path/to/git/t/trash directory.t9801-git-p4-branch/cli/branch1/sub_file1'}]\n\nNote that this is from a modified test case. As you can see, there are\nno paths starting with \"-\", instead there a new attribute called \"unmap\"\nthat implements that description.\n\nIn the latest version of this update I'm searching for a path starting\nwith \"//depot/branch1\" and ending in \"/...\". This is a much more robust\nsolution, so I am really grateful for your review.\n\n>>> As an experiment, I hacked git-p4 to always use p4Where rather than\n>>> getClientRoot(), which I would have thought ought to work, but while\n>>> most of the tests passed, Pete's client-spec torture tests failed.\n>>\n>> That was exactly my first approach and got to the same conclusion. I would have\n>> investigated it further but since I haven't had much free time to invest in\n>> solving this problem I decided to implement an intermediary solution that would\n>> not introduce any regressions.\n\nSince I'm looking at this more carefully now, I'll also try to see if I\nam able to make p4 where work even when not using branch detection.\n\n> Thanks.\n\nNo, thank _you_!\n"},{"id":"259635","messageId":"1429399445-11024-1-git-send-email-vitor.hda@gmail.com","threadId":"38933","inReplyTo":"xmqqsic44rw5.fsf@gitster.dls.corp.google.com","subject":"[PATCH] git-p4: Improve client path detection when branches are used","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-04-18T23:24:05Z","receivedAt":"2015-04-18T23:24:05Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"This patch makes the client path detection more robust by limiting the valid\nresults from p4 where. The test case is also made more complex, to guarantee\nthat such client views are supported.\n\nSigned-off-by: Vitor Antunes <vitor.hda@gmail.com>\n---\n git-p4.py                |    4 +++-\n t/t9801-git-p4-branch.sh |   12 ++++++++++--\n 2 files changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 262a95b..28d0d90 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -507,7 +507,9 @@ def p4Where(depotPath):\n     output = None\n     for entry in outputList:\n         if \"depotFile\" in entry:\n-            if entry[\"depotFile\"].find(depotPath) >= 0:\n+            # Search for the base client side depot path, as long as it starts with the branch's P4 path.\n+            # The base path always ends with \"/...\".\n+            if entry[\"depotFile\"].find(depotPath) == 0 and entry[\"depotFile\"][-4:] == \"/...\":\n                 output = entry\n                 break\n         elif \"data\" in entry:\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 4fe4e18..0aafd03 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -512,23 +512,28 @@ test_expect_success 'restart p4d' '\n #\n # 1: //depot/branch1/base/file1\n #    //depot/branch1/base/file2\n+#    //depot/branch1/base/dir/sub_file1\n # 2: integrate //depot/branch1/base/... -> //depot/branch2/base/...\n # 3: //depot/branch1/base/file3\n # 4: //depot/branch1/base/file2 (edit)\n # 5: integrate //depot/branch1/base/... -> //depot/branch3/base/...\n #\n-# Note: the client view remove the \"base\" folder from the workspace\n+# Note: the client view removes the \"base\" folder from the workspace\n+#       and moves sub_file1 one level up.\n test_expect_success 'add simple p4 branches with common base folder on each branch' '\n \t(\n \t\tcd \"$cli\" &&\n \t\tclient_view \"//depot/branch1/base/... //client/branch1/...\" \\\n+\t\t\t    \"//depot/branch1/base/dir/sub_file1 //client/branch1/sub_file1\" \\\n \t\t\t    \"//depot/branch2/base/... //client/branch2/...\" \\\n \t\t\t    \"//depot/branch3/base/... //client/branch3/...\" &&\n \t\tmkdir -p branch1 &&\n \t\tcd branch1 &&\n \t\techo file1 >file1 &&\n \t\techo file2 >file2 &&\n-\t\tp4 add file1 file2 &&\n+\t\tmkdir dir &&\n+\t\techo sub_file1 >sub_file1 &&\n+\t\tp4 add file1 file2 sub_file1 &&\n \t\tp4 submit -d \"Create branch1\" &&\n \t\tp4 integrate //depot/branch1/base/... //depot/branch2/base/... &&\n \t\tp4 submit -d \"Integrate branch2 from branch1\" &&\n@@ -561,16 +566,19 @@ test_expect_success 'git p4 clone simple branches with base folder on server sid\n \t\ttest -f file1 &&\n \t\ttest -f file2 &&\n \t\ttest -f file3 &&\n+\t\ttest -f sub_file1 &&\n \t\tgrep update file2 &&\n \t\tgit reset --hard p4/depot/branch2 &&\n \t\ttest -f file1 &&\n \t\ttest -f file2 &&\n \t\ttest ! -f file3 &&\n+\t\ttest -f sub_file1 &&\n \t\t! grep update file2 &&\n \t\tgit reset --hard p4/depot/branch3 &&\n \t\ttest -f file1 &&\n \t\ttest -f file2 &&\n \t\ttest -f file3 &&\n+\t\ttest -f sub_file1 &&\n \t\tgrep update file2 &&\n \t\tcd \"$cli\" &&\n \t\tcd branch1 &&\n-- \n1.7.10.4\n"},{"id":"259637","messageId":"xmqqk2x9ndcs.fsf@gitster.dls.corp.google.com","threadId":"38933","inReplyTo":"1429399445-11024-1-git-send-email-vitor.hda@gmail.com","subject":"Re: [PATCH] git-p4: Improve client path detection when branches are used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-19T00:58:11Z","receivedAt":"2015-04-19T00:58:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vitor Antunes <vitor.hda@gmail.com> writes:\n\n> This patch makes the client path detection more robust by limiting the valid\n> results from p4 where. The test case is also made more complex, to guarantee\n> that such client views are supported.\n>\n> Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>\n> ---\n\nWas this designed to be squashed into the previous 2/2 patch?  I\ndo not think either 1/2 or 2/2 is in 'next' yet, and if this was\nto correct mistakes in the 2/2 that was posted earlier, it would\nbe nicer to have a replacement patch with corrected log message.\n\nThanks.\n\n\n>  git-p4.py                |    4 +++-\n>  t/t9801-git-p4-branch.sh |   12 ++++++++++--\n>  2 files changed, 13 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 262a95b..28d0d90 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -507,7 +507,9 @@ def p4Where(depotPath):\n>      output = None\n>      for entry in outputList:\n>          if \"depotFile\" in entry:\n> -            if entry[\"depotFile\"].find(depotPath) >= 0:\n> +            # Search for the base client side depot path, as long as it starts with the branch's P4 path.\n> +            # The base path always ends with \"/...\".\n> +            if entry[\"depotFile\"].find(depotPath) == 0 and entry[\"depotFile\"][-4:] == \"/...\":\n>                  output = entry\n>                  break\n>          elif \"data\" in entry:\n> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\n> index 4fe4e18..0aafd03 100755\n> --- a/t/t9801-git-p4-branch.sh\n> +++ b/t/t9801-git-p4-branch.sh\n> @@ -512,23 +512,28 @@ test_expect_success 'restart p4d' '\n>  #\n>  # 1: //depot/branch1/base/file1\n>  #    //depot/branch1/base/file2\n> +#    //depot/branch1/base/dir/sub_file1\n>  # 2: integrate //depot/branch1/base/... -> //depot/branch2/base/...\n>  # 3: //depot/branch1/base/file3\n>  # 4: //depot/branch1/base/file2 (edit)\n>  # 5: integrate //depot/branch1/base/... -> //depot/branch3/base/...\n>  #\n> -# Note: the client view remove the \"base\" folder from the workspace\n> +# Note: the client view removes the \"base\" folder from the workspace\n> +#       and moves sub_file1 one level up.\n>  test_expect_success 'add simple p4 branches with common base folder on each branch' '\n>  \t(\n>  \t\tcd \"$cli\" &&\n>  \t\tclient_view \"//depot/branch1/base/... //client/branch1/...\" \\\n> +\t\t\t    \"//depot/branch1/base/dir/sub_file1 //client/branch1/sub_file1\" \\\n>  \t\t\t    \"//depot/branch2/base/... //client/branch2/...\" \\\n>  \t\t\t    \"//depot/branch3/base/... //client/branch3/...\" &&\n>  \t\tmkdir -p branch1 &&\n>  \t\tcd branch1 &&\n>  \t\techo file1 >file1 &&\n>  \t\techo file2 >file2 &&\n> -\t\tp4 add file1 file2 &&\n> +\t\tmkdir dir &&\n> +\t\techo sub_file1 >sub_file1 &&\n> +\t\tp4 add file1 file2 sub_file1 &&\n>  \t\tp4 submit -d \"Create branch1\" &&\n>  \t\tp4 integrate //depot/branch1/base/... //depot/branch2/base/... &&\n>  \t\tp4 submit -d \"Integrate branch2 from branch1\" &&\n> @@ -561,16 +566,19 @@ test_expect_success 'git p4 clone simple branches with base folder on server sid\n>  \t\ttest -f file1 &&\n>  \t\ttest -f file2 &&\n>  \t\ttest -f file3 &&\n> +\t\ttest -f sub_file1 &&\n>  \t\tgrep update file2 &&\n>  \t\tgit reset --hard p4/depot/branch2 &&\n>  \t\ttest -f file1 &&\n>  \t\ttest -f file2 &&\n>  \t\ttest ! -f file3 &&\n> +\t\ttest -f sub_file1 &&\n>  \t\t! grep update file2 &&\n>  \t\tgit reset --hard p4/depot/branch3 &&\n>  \t\ttest -f file1 &&\n>  \t\ttest -f file2 &&\n>  \t\ttest -f file3 &&\n> +\t\ttest -f sub_file1 &&\n>  \t\tgrep update file2 &&\n>  \t\tcd \"$cli\" &&\n>  \t\tcd branch1 &&\n"},{"id":"259645","messageId":"20150419115949.770008db@pt-vhugo","threadId":"38933","inReplyTo":"xmqqk2x9ndcs.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git-p4: Improve client path detection when branches are used","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-04-19T10:59:49Z","receivedAt":"2015-04-19T10:59:49Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote on Sat, 18 Apr 2015 17:58:11 -0700\n> Vitor Antunes <vitor.hda@gmail.com> writes:\n>\n>> This patch makes the client path detection more robust by limiting the valid\n>> results from p4 where. The test case is also made more complex, to guarantee\n>> that such client views are supported.\n>>\n>> Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>\n>> ---\n>\n>Was this designed to be squashed into the previous 2/2 patch?  I\n>do not think either 1/2 or 2/2 is in 'next' yet, and if this was\n>to correct mistakes in the 2/2 that was posted earlier, it would\n>be nicer to have a replacement patch with corrected log message.\n>\n>Thanks.\n\nHope I got everything right this time :)\n\nThanks for your help,\nVitor\n"},{"id":"259657","messageId":"xmqq383vnz4s.fsf@gitster.dls.corp.google.com","threadId":"38933","inReplyTo":"20150419115949.770008db@pt-vhugo","subject":"Re: [PATCH] git-p4: Improve client path detection when branches are used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-20T05:32:19Z","receivedAt":"2015-04-20T05:32:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vitor Antunes <vitor.hda@gmail.com> writes:\n\n> Hope I got everything right this time :)\n>\n> Thanks for your help,\n\nThanks.\n"}]}