[PATCH] git-p4: support exclusively locked files

By default, newly added binary files are exclusively locked by Perforce:

'add default change (binary+l) *exclusive*'

This results in a 'Could not determine file type' error as the regex
expects
the line to end after the file type matching group. Some repositories are
also configured to always require exclusive locks, so may be a problem for
all revisions in some cases.

Signed-off-by: Danny Thomas <redacted>
---
git-p4.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index 0682e61..ffba294 100755
--- a/git-p4.py
+++ b/git-p4.py

@@ -326,7 +326,7 @@ def getP4OpenedType(file):
     # Returns the perforce file type for the given file.

     result = p4_read_pipe(["opened", wildcard_encode(file)])
-    match = re.match(".*\((.+)\)\r?$", result)
+    match = re.match(".*\((.+)\)(?:.+)?\r?$", result)
     if match:
         return match.group(1)
     else:
--

1.8.1.5





This 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.

Danny.Thomas@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:

By default, newly added binary files are exclusively locked by Perforce:

'add default change (binary+l) *exclusive*'

This results in a 'Could not determine file type' error as the regex
expects
the line to end after the file type matching group. Some repositories are
also configured to always require exclusive locks, so may be a problem for
all revisions in some cases.

Can you explain how to configure p4d to default everything to
binary+l?  Also, what version are you using ("p4 info")?  I'm
trying to write a test case for this.

I did find a way to play with typemap to get +l, as:

    ( p4 typemap -o ; printf "\tbinary+l\t//.../bash*" ) | p4 typemap -i

With this, the 2011.1 here just says:

    tic-git-test$ p4 opened bash
    //depot/bash#1 - add default change (binary+l)

I've not been able to make it say " *exclusive*" too.

     result = p4_read_pipe(["opened", wildcard_encode(file)])
-    match = re.match(".*\((.+)\)\r?$", result)
+    match = re.match(".*\((.+)\)(?:.+)?\r?$", result)

I think this whole function would be less brittle
using p4's "-G" output, like:

    d = p4Cmd(["fstat", "-T", "type", wildcard_encode(file)])
    # some error checking
    return d['type']

But I'm curious if your p4d includes " *exclusive*" in the
type there too, in which case we'll have to strip it.

Thanks for starting the patch on this.  It's good if we can keep
others from running into the same problem.

		-- Pete

Interesting. 'Implementing sitewide pessimistic locking with p4 typemap',
http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.htm
l seems to suggest this is all that's needed. I believe we're using the
default configuration, the exclusive lock on all files behaviour was
researching the exclusive locking problem,
http://ericlathrop.com/2012/12/how-to-set-up-git-p4-in-windows/, so I
thought I'd mention it.

You might be onto something w/ fstat, it doesn't include the exclusive
indicator:

... type binary+l

Latest P4 client, and fairly recent server build:

P4/DARWIN90X86_64/2012.2/536738 (2012/10/16)
P4D/LINUX26X86_64/2012.2/538478 (2012/10/16)


Danny

On 17/03/2013 20:04, "Pete Wyckoff" [off-list ref] wrote:

Danny.Thomas@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:

quoted

By default, newly added binary files are exclusively locked by Perforce:

'add default change (binary+l) *exclusive*'

This results in a 'Could not determine file type' error as the regex
expects
the line to end after the file type matching group. Some repositories
are
also configured to always require exclusive locks, so may be a problem
for
all revisions in some cases.

Can you explain how to configure p4d to default everything to
binary+l?  Also, what version are you using ("p4 info")?  I'm
trying to write a test case for this.

I did find a way to play with typemap to get +l, as:

   ( p4 typemap -o ; printf "\tbinary+l\t//.../bash*" ) | p4 typemap -i

With this, the 2011.1 here just says:
0
   tic-git-test$ p4 opened bash
   //depot/bash#1 - add default change (binary+l)

I've not been able to make it say " *exclusive*" too.

quoted

     result = p4_read_pipe(["opened", wildcard_encode(file)])
-    match = re.match(".*\((.+)\)\r?$", result)
+    match = re.match(".*\((.+)\)(?:.+)?\r?$", result)

I think this whole function would be less brittle
using p4's "-G" output, like:

   d = p4Cmd(["fstat", "-T", "type", wildcard_encode(file)])
   # some error checking
   return d['type']

But I'm curious if your p4d includes " *exclusive*" in the
type there too, in which case we'll have to strip it.

Thanks for starting the patch on this.  It's good if we can keep
others from running into the same problem.

              -- Pete

This 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.

Danny.Thomas@blackboard.com wrote on Mon, 18 Mar 2013 09:26 -0400:

Interesting. 'Implementing sitewide pessimistic locking with p4 typemap',
http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.htm
l seems to suggest this is all that's needed. I believe we're using the
default configuration, the exclusive lock on all files behaviour was
researching the exclusive locking problem,
http://ericlathrop.com/2012/12/how-to-set-up-git-p4-in-windows/, so I
thought I'd mention it.

You might be onto something w/ fstat, it doesn't include the exclusive
indicator:

... type binary+l

Latest P4 client, and fairly recent server build:

P4/DARWIN90X86_64/2012.2/536738 (2012/10/16)
P4D/LINUX26X86_64/2012.2/538478 (2012/10/16)

Great, thanks for the pointer and explanation.  Do you want to
reroll your patch to use fstat?  I'll work on the tests, and
also look into potential failure modes of "git p4 submit" when somebody
else has the exclusive file open.

		-- Pete

On 17/03/2013 20:04, "Pete Wyckoff" [off-list ref] wrote:

quoted

Danny.Thomas@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:

quoted

By default, newly added binary files are exclusively locked by Perforce:

'add default change (binary+l) *exclusive*'

This results in a 'Could not determine file type' error as the regex
expects
the line to end after the file type matching group. Some repositories
are
also configured to always require exclusive locks, so may be a problem
for
all revisions in some cases.

Can you explain how to configure p4d to default everything to
binary+l?  Also, what version are you using ("p4 info")?  I'm
trying to write a test case for this.

I did find a way to play with typemap to get +l, as:

   ( p4 typemap -o ; printf "\tbinary+l\t//.../bash*" ) | p4 typemap -i

With this, the 2011.1 here just says:
0
   tic-git-test$ p4 opened bash
   //depot/bash#1 - add default change (binary+l)

I've not been able to make it say " *exclusive*" too.

quoted

     result = p4_read_pipe(["opened", wildcard_encode(file)])
-    match = re.match(".*\((.+)\)\r?$", result)
+    match = re.match(".*\((.+)\)(?:.+)?\r?$", result)

I think this whole function would be less brittle
using p4's "-G" output, like:

   d = p4Cmd(["fstat", "-T", "type", wildcard_encode(file)])
   # some error checking
   return d['type']

But I'm curious if your p4d includes " *exclusive*" in the
type there too, in which case we'll have to strip it.

Thanks for starting the patch on this.  It's good if we can keep
others from running into the same problem.

Sounds good to me. I've got a couple of busy days coming up, but should
have time this week.

Danny




On 19/03/2013 19:23, "Pete Wyckoff" [off-list ref] wrote:

Danny.Thomas@blackboard.com wrote on Mon, 18 Mar 2013 09:26 -0400:

quoted

Interesting. 'Implementing sitewide pessimistic locking with p4
typemap',

http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.h
tm
l seems to suggest this is all that's needed. I believe we're using the
default configuration, the exclusive lock on all files behaviour was
researching the exclusive locking problem,
http://ericlathrop.com/2012/12/how-to-set-up-git-p4-in-windows/, so I
thought I'd mention it.

You might be onto something w/ fstat, it doesn't include the exclusive
indicator:

... type binary+l

Latest P4 client, and fairly recent server build:

P4/DARWIN90X86_64/2012.2/536738 (2012/10/16)
P4D/LINUX26X86_64/2012.2/538478 (2012/10/16)

Great, thanks for the pointer and explanation.  Do you want to
reroll your patch to use fstat?  I'll work on the tests, and
also look into potential failure modes of "git p4 submit" when somebody
else has the exclusive file open.

              -- Pete

quoted

On 17/03/2013 20:04, "Pete Wyckoff" [off-list ref] wrote:

quoted

Danny.Thomas@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:

quoted

By default, newly added binary files are exclusively locked by

Perforce:

quoted

quoted

'add default change (binary+l) *exclusive*'

This results in a 'Could not determine file type' error as the regex
expects
the line to end after the file type matching group. Some repositories
are
also configured to always require exclusive locks, so may be a

problem

quoted

quoted

for
all revisions in some cases.

Can you explain how to configure p4d to default everything to
binary+l?  Also, what version are you using ("p4 info")?  I'm
trying to write a test case for this.

I did find a way to play with typemap to get +l, as:

   ( p4 typemap -o ; printf "\tbinary+l\t//.../bash*" ) | p4 typemap

-i

quoted

With this, the 2011.1 here just says:
0
   tic-git-test$ p4 opened bash
   //depot/bash#1 - add default change (binary+l)

I've not been able to make it say " *exclusive*" too.

quoted

     result = p4_read_pipe(["opened", wildcard_encode(file)])
-    match = re.match(".*\((.+)\)\r?$", result)
+    match = re.match(".*\((.+)\)(?:.+)?\r?$", result)

I think this whole function would be less brittle
using p4's "-G" output, like:

   d = p4Cmd(["fstat", "-T", "type", wildcard_encode(file)])
   # some error checking
   return d['type']

But I'm curious if your p4d includes " *exclusive*" in the
type there too, in which case we'll have to strip it.

Thanks for starting the patch on this.  It's good if we can keep
others from running into the same problem.

This 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.

Danny.Thomas@blackboard.com wrote on Wed, 20 Mar 2013 07:33 -0400:

Sounds good to me. I've got a couple of busy days coming up, but should
have time this week.

Here's what I'm playing with for test cases, by the way.  The fix
you're working on is definitely part of it, but there are more
issues as well.  I'll address them once you've taken care of the
opened/fstat issue.  Thanks,

		-- Pete

--- 8< ---

From c6691126ae75c364763ab4d774c75045285b8ddd Mon Sep 17 00:00:00 2001

From: Pete Wyckoff <redacted>
Date: Sun, 17 Mar 2013 16:05:07 -0400
Subject: [PATCH] git p4 test: examine behavior with locked (+l) files

The p4 server can enforce file locking, so that only one user
can edit a file at a time.  Git p4 is unable to submit changes
to locked files.  Currently it exits poorly.  Ideally it would
notice the locked condition and clean up nicely.
---
 t/t9816-git-p4-locked.sh | 145 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)
 create mode 100755 t/t9816-git-p4-locked.sh

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
new file mode 100755
index 0000000..e71e543
--- /dev/null
+++ b/t/t9816-git-p4-locked.sh

@@ -0,0 +1,145 @@
+#!/bin/sh
+
+test_description='git p4 locked file behavior'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+# See
+# http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.html#1088563
+# for suggestions on how to configure "sitewide pessimistic locking"
+# where only one person can have a file open for edit at a time.
+test_expect_success 'init depot' '
+	(
+		cd "$cli" &&
+		echo "TypeMap: +l //depot/..." | p4 typemap -i &&
+		echo file1 >file1 &&
+		p4 add file1 &&
+		p4 submit -d "add file1"
+	)
+'
+
+test_expect_success 'edit with lock not taken' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo line2 >>file1 &&
+		git add file1 &&
+		git commit -m "line2 in file1" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit
+	)
+'
+
+test_expect_failure 'add with lock not taken' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo line1 >>add-lock-not-taken &&
+		git add file2 &&
+		git commit -m "add add-lock-not-taken" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit --verbose
+	)
+'
+
+lock_in_another_client() {
+	# build a different client
+	cli2="$TRASH_DIRECTORY/cli2" &&
+	mkdir -p "$cli2" &&
+	test_when_finished "p4 client -f -d client2 && rm -rf \"$cli2\"" &&
+	(
+		cd "$cli2" &&
+		P4CLIENT=client2 &&
+		cli="$cli2" &&
+		client_view "//depot/... //client2/..." &&
+		p4 sync &&
+		p4 open file1
+	)
+}
+
+test_expect_failure 'edit with lock taken' '
+	lock_in_another_client &&
+	test_when_finished cleanup_git &&
+	test_when_finished "cd \"$cli\" && p4 sync -f file1" &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo line3 >>file1 &&
+		git add file1 &&
+		git commit -m "line3 in file1" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit --verbose
+	)
+'
+
+test_expect_failure 'delete with lock taken' '
+	lock_in_another_client &&
+	test_when_finished cleanup_git &&
+	test_when_finished "cd \"$cli\" && p4 sync -f file1" &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git rm file1 &&
+		git commit -m "delete file1" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit --verbose
+	)
+'
+
+test_expect_failure 'chmod with lock taken' '
+	lock_in_another_client &&
+	test_when_finished cleanup_git &&
+	test_when_finished "cd \"$cli\" && p4 sync -f file1" &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		chmod +x file1 &&
+		git add file1 &&
+		git commit -m "chmod +x file1" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit --verbose
+	)
+'
+
+test_expect_failure 'copy with lock taken' '
+	lock_in_another_client &&
+	test_when_finished cleanup_git &&
+	test_when_finished "cd \"$cli\" && p4 revert file2 && rm -f file2" &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		cp file1 file2 &&
+		git add file2 &&
+		git commit -m "cp file1 to file2" &&
+		git config git-p4.skipSubmitEdit true &&
+		git config git-p4.detectCopies true &&
+		git p4 submit --verbose
+	)
+'
+
+test_expect_failure 'move with lock taken' '
+	lock_in_another_client &&
+	test_when_finished cleanup_git &&
+	test_when_finished "cd \"$cli\" && p4 sync file1 && rm -f file2" &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git mv file1 file2 &&
+		git commit -m "mv file1 to file2" &&
+		git config git-p4.skipSubmitEdit true &&
+		git config git-p4.detectRenames true &&
+		git p4 submit --verbose
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done

-- 
1.8.2.rc2.65.g92f3e2d