{"thread":{"id":"33169","subject":"[PATCH] git-p4: support exclusively locked files","startedAt":"2013-03-13T17:51:11Z","lastAt":"2013-03-24T22:19:05Z","messageCount":6,"participants":["Danny Thomas","Pete Wyckoff"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"211238","messageId":"CD666D0F.1DC6E%danny.thomas@blackboard.com","threadId":"33169","inReplyTo":null,"subject":"[PATCH] git-p4: support exclusively locked files","fromName":"Danny Thomas","fromEmail":"danny.thomas@blackboard.com","sentAt":"2013-03-13T17:51:11Z","receivedAt":"2013-03-13T17:51:11Z","isPatch":true,"sender":{"key":"danny.thomas@blackboard.com","avatar":null},"body":"By default, newly added binary files are exclusively locked by Perforce:\n\n'add default change (binary+l) *exclusive*'\n\nThis results in a 'Could not determine file type' error as the regex\nexpects\nthe line to end after the file type matching group. Some repositories are\nalso configured to always require exclusive locks, so may be a problem for\nall revisions in some cases.\n\nSigned-off-by: Danny Thomas <dthomas@blackboard.com>\n---\ngit-p4.py | 2 +-\n1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 0682e61..ffba294 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -326,7 +326,7 @@ def getP4OpenedType(file):\n     # Returns the perforce file type for the given file.\n\n     result = p4_read_pipe([\"opened\", wildcard_encode(file)])\n-    match = re.match(\".*\\((.+)\\)\\r?$\", result)\n+    match = re.match(\".*\\((.+)\\)(?:.+)?\\r?$\", result)\n     if match:\n         return match.group(1)\n     else:\n--\n1.8.1.5\n\n\n\n\n\nThis email and any attachments may contain confidential and proprietary information of Blackboard that is for the sole use of the intended recipient. If you are not the intended recipient, disclosure, copying, re-distribution or other use of any of this information is strictly prohibited. Please immediately notify the sender and delete this transmission if you received this email in error.\n"},{"id":"211525","messageId":"20130317200437.GA29115@padd.com","threadId":"33169","inReplyTo":"CD666D0F.1DC6E%danny.thomas@blackboard.com","subject":"Re: [PATCH] git-p4: support exclusively locked files","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-03-17T20:04:37Z","receivedAt":"2013-03-17T20:04:37Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Danny.Thomas@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:\n> By default, newly added binary files are exclusively locked by Perforce:\n> \n> 'add default change (binary+l) *exclusive*'\n> \n> This results in a 'Could not determine file type' error as the regex\n> expects\n> the line to end after the file type matching group. Some repositories are\n> also configured to always require exclusive locks, so may be a problem for\n> all revisions in some cases.\n\nCan you explain how to configure p4d to default everything to\nbinary+l?  Also, what version are you using (\"p4 info\")?  I'm\ntrying to write a test case for this.\n\nI did find a way to play with typemap to get +l, as:\n\n    ( p4 typemap -o ; printf \"\\tbinary+l\\t//.../bash*\" ) | p4 typemap -i\n\nWith this, the 2011.1 here just says:\n\n    tic-git-test$ p4 opened bash\n    //depot/bash#1 - add default change (binary+l)\n\nI've not been able to make it say \" *exclusive*\" too.\n\n>      result = p4_read_pipe([\"opened\", wildcard_encode(file)])\n> -    match = re.match(\".*\\((.+)\\)\\r?$\", result)\n> +    match = re.match(\".*\\((.+)\\)(?:.+)?\\r?$\", result)\n\nI think this whole function would be less brittle\nusing p4's \"-G\" output, like:\n\n    d = p4Cmd([\"fstat\", \"-T\", \"type\", wildcard_encode(file)])\n    # some error checking\n    return d['type']\n\nBut I'm curious if your p4d includes \" *exclusive*\" in the\ntype there too, in which case we'll have to strip it.\n\nThanks for starting the patch on this.  It's good if we can keep\nothers from running into the same problem.\n\n\t\t-- Pete\n"},{"id":"211569","messageId":"CD6CC10C.1DE4E%danny.thomas@blackboard.com","threadId":"33169","inReplyTo":"20130317200437.GA29115@padd.com","subject":"Re: [PATCH] git-p4: support exclusively locked files","fromName":"Danny Thomas","fromEmail":"danny.thomas@blackboard.com","sentAt":"2013-03-18T13:26:18Z","receivedAt":"2013-03-18T13:26:18Z","isPatch":true,"sender":{"key":"danny.thomas@blackboard.com","avatar":null},"body":"Interesting. 'Implementing sitewide pessimistic locking with p4 typemap',\nhttp://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.htm\nl seems to suggest this is all that's needed. I believe we're using the\ndefault configuration, the exclusive lock on all files behaviour was\nresearching the exclusive locking problem,\nhttp://ericlathrop.com/2012/12/how-to-set-up-git-p4-in-windows/, so I\nthought I'd mention it.\n\nYou might be onto something w/ fstat, it doesn't include the exclusive\nindicator:\n\n... type binary+l\n\nLatest P4 client, and fairly recent server build:\n\nP4/DARWIN90X86_64/2012.2/536738 (2012/10/16)\nP4D/LINUX26X86_64/2012.2/538478 (2012/10/16)\n\n\nDanny\n\nOn 17/03/2013 20:04, \"Pete Wyckoff\" <pw@padd.com> wrote:\n\n\n>Danny.Thomas@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:\n>> By default, newly added binary files are exclusively locked by Perforce:\n>>\n>> 'add default change (binary+l) *exclusive*'\n>>\n>> This results in a 'Could not determine file type' error as the regex\n>> expects\n>> the line to end after the file type matching group. Some repositories\n>>are\n>> also configured to always require exclusive locks, so may be a problem\n>>for\n>> all revisions in some cases.\n>\n>Can you explain how to configure p4d to default everything to\n>binary+l?  Also, what version are you using (\"p4 info\")?  I'm\n>trying to write a test case for this.\n>\n>I did find a way to play with typemap to get +l, as:\n>\n>    ( p4 typemap -o ; printf \"\\tbinary+l\\t//.../bash*\" ) | p4 typemap -i\n>\n>With this, the 2011.1 here just says:\n>0\n>    tic-git-test$ p4 opened bash\n>    //depot/bash#1 - add default change (binary+l)\n>\n>I've not been able to make it say \" *exclusive*\" too.\n>\n>>      result = p4_read_pipe([\"opened\", wildcard_encode(file)])\n>> -    match = re.match(\".*\\((.+)\\)\\r?$\", result)\n>> +    match = re.match(\".*\\((.+)\\)(?:.+)?\\r?$\", result)\n>\n>I think this whole function would be less brittle\n>using p4's \"-G\" output, like:\n>\n>    d = p4Cmd([\"fstat\", \"-T\", \"type\", wildcard_encode(file)])\n>    # some error checking\n>    return d['type']\n>\n>But I'm curious if your p4d includes \" *exclusive*\" in the\n>type there too, in which case we'll have to strip it.\n>\n>Thanks for starting the patch on this.  It's good if we can keep\n>others from running into the same problem.\n>\n>               -- Pete\n\n\nThis email and any attachments may contain confidential and proprietary information of Blackboard that is for the sole use of the intended recipient. If you are not the intended recipient, disclosure, copying, re-distribution or other use of any of this information is strictly prohibited. Please immediately notify the sender and delete this transmission if you received this email in error.\n"},{"id":"211703","messageId":"20130319192312.GA16872@padd.com","threadId":"33169","inReplyTo":"CD6CC10C.1DE4E%danny.thomas@blackboard.com","subject":"Re: [PATCH] git-p4: support exclusively locked files","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-03-19T19:23:12Z","receivedAt":"2013-03-19T19:23:12Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Danny.Thomas@blackboard.com wrote on Mon, 18 Mar 2013 09:26 -0400:\n> Interesting. 'Implementing sitewide pessimistic locking with p4 typemap',\n> http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.htm\n> l seems to suggest this is all that's needed. I believe we're using the\n> default configuration, the exclusive lock on all files behaviour was\n> researching the exclusive locking problem,\n> http://ericlathrop.com/2012/12/how-to-set-up-git-p4-in-windows/, so I\n> thought I'd mention it.\n> \n> You might be onto something w/ fstat, it doesn't include the exclusive\n> indicator:\n> \n> ... type binary+l\n> \n> Latest P4 client, and fairly recent server build:\n> \n> P4/DARWIN90X86_64/2012.2/536738 (2012/10/16)\n> P4D/LINUX26X86_64/2012.2/538478 (2012/10/16)\n\nGreat, thanks for the pointer and explanation.  Do you want to\nreroll your patch to use fstat?  I'll work on the tests, and\nalso look into potential failure modes of \"git p4 submit\" when somebody\nelse has the exclusive file open.\n\n\t\t-- Pete\n\n> On 17/03/2013 20:04, \"Pete Wyckoff\" <pw@padd.com> wrote:\n> \n> >Danny.Thomas@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:\n> >> By default, newly added binary files are exclusively locked by Perforce:\n> >>\n> >> 'add default change (binary+l) *exclusive*'\n> >>\n> >> This results in a 'Could not determine file type' error as the regex\n> >> expects\n> >> the line to end after the file type matching group. Some repositories\n> >>are\n> >> also configured to always require exclusive locks, so may be a problem\n> >>for\n> >> all revisions in some cases.\n> >\n> >Can you explain how to configure p4d to default everything to\n> >binary+l?  Also, what version are you using (\"p4 info\")?  I'm\n> >trying to write a test case for this.\n> >\n> >I did find a way to play with typemap to get +l, as:\n> >\n> >    ( p4 typemap -o ; printf \"\\tbinary+l\\t//.../bash*\" ) | p4 typemap -i\n> >\n> >With this, the 2011.1 here just says:\n> >0\n> >    tic-git-test$ p4 opened bash\n> >    //depot/bash#1 - add default change (binary+l)\n> >\n> >I've not been able to make it say \" *exclusive*\" too.\n> >\n> >>      result = p4_read_pipe([\"opened\", wildcard_encode(file)])\n> >> -    match = re.match(\".*\\((.+)\\)\\r?$\", result)\n> >> +    match = re.match(\".*\\((.+)\\)(?:.+)?\\r?$\", result)\n> >\n> >I think this whole function would be less brittle\n> >using p4's \"-G\" output, like:\n> >\n> >    d = p4Cmd([\"fstat\", \"-T\", \"type\", wildcard_encode(file)])\n> >    # some error checking\n> >    return d['type']\n> >\n> >But I'm curious if your p4d includes \" *exclusive*\" in the\n> >type there too, in which case we'll have to strip it.\n> >\n> >Thanks for starting the patch on this.  It's good if we can keep\n> >others from running into the same problem.\n"},{"id":"211733","messageId":"CD6F44FF.1DFE4%danny.thomas@blackboard.com","threadId":"33169","inReplyTo":"20130319192312.GA16872@padd.com","subject":"Re: [PATCH] git-p4: support exclusively locked files","fromName":"Danny Thomas","fromEmail":"danny.thomas@blackboard.com","sentAt":"2013-03-20T11:33:29Z","receivedAt":"2013-03-20T11:33:29Z","isPatch":true,"sender":{"key":"danny.thomas@blackboard.com","avatar":null},"body":"Sounds good to me. I've got a couple of busy days coming up, but should\nhave time this week.\n\nDanny\n\n\n\n\nOn 19/03/2013 19:23, \"Pete Wyckoff\" <pw@padd.com> wrote:\n\n>Danny.Thomas@blackboard.com wrote on Mon, 18 Mar 2013 09:26 -0400:\n>> Interesting. 'Implementing sitewide pessimistic locking with p4\n>>typemap',\n>>\n>>http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.h\n>>tm\n>> l seems to suggest this is all that's needed. I believe we're using the\n>> default configuration, the exclusive lock on all files behaviour was\n>> researching the exclusive locking problem,\n>> http://ericlathrop.com/2012/12/how-to-set-up-git-p4-in-windows/, so I\n>> thought I'd mention it.\n>>\n>> You might be onto something w/ fstat, it doesn't include the exclusive\n>> indicator:\n>>\n>> ... type binary+l\n>>\n>> Latest P4 client, and fairly recent server build:\n>>\n>> P4/DARWIN90X86_64/2012.2/536738 (2012/10/16)\n>> P4D/LINUX26X86_64/2012.2/538478 (2012/10/16)\n>\n>Great, thanks for the pointer and explanation.  Do you want to\n>reroll your patch to use fstat?  I'll work on the tests, and\n>also look into potential failure modes of \"git p4 submit\" when somebody\n>else has the exclusive file open.\n>\n>               -- Pete\n>\n>> On 17/03/2013 20:04, \"Pete Wyckoff\" <pw@padd.com> wrote:\n>>\n>> >Danny.Thomas@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:\n>> >> By default, newly added binary files are exclusively locked by\n>>Perforce:\n>> >>\n>> >> 'add default change (binary+l) *exclusive*'\n>> >>\n>> >> This results in a 'Could not determine file type' error as the regex\n>> >> expects\n>> >> the line to end after the file type matching group. Some repositories\n>> >>are\n>> >> also configured to always require exclusive locks, so may be a\n>>problem\n>> >>for\n>> >> all revisions in some cases.\n>> >\n>> >Can you explain how to configure p4d to default everything to\n>> >binary+l?  Also, what version are you using (\"p4 info\")?  I'm\n>> >trying to write a test case for this.\n>> >\n>> >I did find a way to play with typemap to get +l, as:\n>> >\n>> >    ( p4 typemap -o ; printf \"\\tbinary+l\\t//.../bash*\" ) | p4 typemap\n>>-i\n>> >\n>> >With this, the 2011.1 here just says:\n>> >0\n>> >    tic-git-test$ p4 opened bash\n>> >    //depot/bash#1 - add default change (binary+l)\n>> >\n>> >I've not been able to make it say \" *exclusive*\" too.\n>> >\n>> >>      result = p4_read_pipe([\"opened\", wildcard_encode(file)])\n>> >> -    match = re.match(\".*\\((.+)\\)\\r?$\", result)\n>> >> +    match = re.match(\".*\\((.+)\\)(?:.+)?\\r?$\", result)\n>> >\n>> >I think this whole function would be less brittle\n>> >using p4's \"-G\" output, like:\n>> >\n>> >    d = p4Cmd([\"fstat\", \"-T\", \"type\", wildcard_encode(file)])\n>> >    # some error checking\n>> >    return d['type']\n>> >\n>> >But I'm curious if your p4d includes \" *exclusive*\" in the\n>> >type there too, in which case we'll have to strip it.\n>> >\n>> >Thanks for starting the patch on this.  It's good if we can keep\n>> >others from running into the same problem.\n\n\nThis email and any attachments may contain confidential and proprietary information of Blackboard that is for the sole use of the intended recipient. If you are not the intended recipient, disclosure, copying, re-distribution or other use of any of this information is strictly prohibited. Please immediately notify the sender and delete this transmission if you received this email in error.\n"},{"id":"212137","messageId":"20130324221905.GA32504@padd.com","threadId":"33169","inReplyTo":"CD6F44FF.1DFE4%danny.thomas@blackboard.com","subject":"Re: [PATCH] git-p4: support exclusively locked files","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-03-24T22:19:05Z","receivedAt":"2013-03-24T22:19:05Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Danny.Thomas@blackboard.com wrote on Wed, 20 Mar 2013 07:33 -0400:\n> Sounds good to me. I've got a couple of busy days coming up, but should\n> have time this week.\n\nHere's what I'm playing with for test cases, by the way.  The fix\nyou're working on is definitely part of it, but there are more\nissues as well.  I'll address them once you've taken care of the\nopened/fstat issue.  Thanks,\n\n\t\t-- Pete\n\n--- 8< ---\n\n>From c6691126ae75c364763ab4d774c75045285b8ddd Mon Sep 17 00:00:00 2001\nFrom: Pete Wyckoff <pw@padd.com>\nDate: Sun, 17 Mar 2013 16:05:07 -0400\nSubject: [PATCH] git p4 test: examine behavior with locked (+l) files\n\nThe p4 server can enforce file locking, so that only one user\ncan edit a file at a time.  Git p4 is unable to submit changes\nto locked files.  Currently it exits poorly.  Ideally it would\nnotice the locked condition and clean up nicely.\n---\n t/t9816-git-p4-locked.sh | 145 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 145 insertions(+)\n create mode 100755 t/t9816-git-p4-locked.sh\n\ndiff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh\nnew file mode 100755\nindex 0000000..e71e543\n--- /dev/null\n+++ b/t/t9816-git-p4-locked.sh\n@@ -0,0 +1,145 @@\n+#!/bin/sh\n+\n+test_description='git p4 locked file behavior'\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+# See\n+# http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.html#1088563\n+# for suggestions on how to configure \"sitewide pessimistic locking\"\n+# where only one person can have a file open for edit at a time.\n+test_expect_success 'init depot' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo \"TypeMap: +l //depot/...\" | p4 typemap -i &&\n+\t\techo file1 >file1 &&\n+\t\tp4 add file1 &&\n+\t\tp4 submit -d \"add file1\"\n+\t)\n+'\n+\n+test_expect_success 'edit with lock not taken' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo line2 >>file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m \"line2 in file1\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit\n+\t)\n+'\n+\n+test_expect_failure 'add with lock not taken' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo line1 >>add-lock-not-taken &&\n+\t\tgit add file2 &&\n+\t\tgit commit -m \"add add-lock-not-taken\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit --verbose\n+\t)\n+'\n+\n+lock_in_another_client() {\n+\t# build a different client\n+\tcli2=\"$TRASH_DIRECTORY/cli2\" &&\n+\tmkdir -p \"$cli2\" &&\n+\ttest_when_finished \"p4 client -f -d client2 && rm -rf \\\"$cli2\\\"\" &&\n+\t(\n+\t\tcd \"$cli2\" &&\n+\t\tP4CLIENT=client2 &&\n+\t\tcli=\"$cli2\" &&\n+\t\tclient_view \"//depot/... //client2/...\" &&\n+\t\tp4 sync &&\n+\t\tp4 open file1\n+\t)\n+}\n+\n+test_expect_failure 'edit with lock taken' '\n+\tlock_in_another_client &&\n+\ttest_when_finished cleanup_git &&\n+\ttest_when_finished \"cd \\\"$cli\\\" && p4 sync -f file1\" &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo line3 >>file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m \"line3 in file1\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit --verbose\n+\t)\n+'\n+\n+test_expect_failure 'delete with lock taken' '\n+\tlock_in_another_client &&\n+\ttest_when_finished cleanup_git &&\n+\ttest_when_finished \"cd \\\"$cli\\\" && p4 sync -f file1\" &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit rm file1 &&\n+\t\tgit commit -m \"delete file1\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit --verbose\n+\t)\n+'\n+\n+test_expect_failure 'chmod with lock taken' '\n+\tlock_in_another_client &&\n+\ttest_when_finished cleanup_git &&\n+\ttest_when_finished \"cd \\\"$cli\\\" && p4 sync -f file1\" &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tchmod +x file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m \"chmod +x file1\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit --verbose\n+\t)\n+'\n+\n+test_expect_failure 'copy with lock taken' '\n+\tlock_in_another_client &&\n+\ttest_when_finished cleanup_git &&\n+\ttest_when_finished \"cd \\\"$cli\\\" && p4 revert file2 && rm -f file2\" &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tcp file1 file2 &&\n+\t\tgit add file2 &&\n+\t\tgit commit -m \"cp file1 to file2\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.detectCopies true &&\n+\t\tgit p4 submit --verbose\n+\t)\n+'\n+\n+test_expect_failure 'move with lock taken' '\n+\tlock_in_another_client &&\n+\ttest_when_finished cleanup_git &&\n+\ttest_when_finished \"cd \\\"$cli\\\" && p4 sync file1 && rm -f file2\" &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit mv file1 file2 &&\n+\t\tgit commit -m \"mv file1 to file2\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.detectRenames true &&\n+\t\tgit p4 submit --verbose\n+\t)\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n1.8.2.rc2.65.g92f3e2d\n"}]}