[PATCHv2 0/4] git-p4: fix filetype detection on files opened exclusively

[PATCHv2 0/4] git-p4: fix filetype detection on files opened exclusively

[Luke Diamand]

This is a followup series to Blair's patch to fix filetype detection on files
opened exclusively. It updates the git-p4 unit tests to catchup with that
fix, fixing a couple of small bugs in the original tests.

Holloway, Blair (1):
  git-p4: fix filetype detection on files opened exclusively

Luke Diamand (3):
  git-p4: fix small bug in locked test scripts
  git-p4: small fix for locked-file-move-test
  git-p4: update locked test case

 git-p4.py                |  2 +-
 t/t9816-git-p4-locked.sh | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.3.0.rc1.30.g76afe74

[PATCHv2 1/4] git-p4: fix small bug in locked test scripts

[Luke Diamand]

Test script t9816-git-p4-locked.sh test #4 tests for
adding a file that is locked by Perforce automatially.
This is currently not supported by git-p4 and so is
expected to fail.

However, a small typo meant it always failed, even with
a fixed git-p4. Fix the typo to resolve this.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 t/t9816-git-p4-locked.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index e71e543..ce0eb22 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -41,7 +41,7 @@ test_expect_failure 'add with lock not taken' '
 	(
 		cd "$git" &&
 		echo line1 >>add-lock-not-taken &&
-		git add file2 &&
+		git add add-lock-not-taken &&
 		git commit -m "add add-lock-not-taken" &&
 		git config git-p4.skipSubmitEdit true &&
 		git p4 submit --verbose
-- 
2.3.0.rc1.30.g76afe74

Re: [PATCHv2 1/4] git-p4: fix small bug in locked test scripts

[Eric Sunshine]

On Fri, Apr 3, 2015 at 2:53 PM, Luke Diamand <luke@diamand.org> wrote:
> Test script t9816-git-p4-locked.sh test #4 tests for
> adding a file that is locked by Perforce automatially.

s/automatially/automatically/

> This is currently not supported by git-p4 and so is
> expected to fail.
>
> However, a small typo meant it always failed, even with
> a fixed git-p4. Fix the typo to resolve this.
>
> Signed-off-by: Luke Diamand <luke@diamand.org>
> ---
>  t/t9816-git-p4-locked.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
> index e71e543..ce0eb22 100755
> --- a/t/t9816-git-p4-locked.sh
> +++ b/t/t9816-git-p4-locked.sh
> @@ -41,7 +41,7 @@ test_expect_failure 'add with lock not taken' '
>         (
>                 cd "$git" &&
>                 echo line1 >>add-lock-not-taken &&
> -               git add file2 &&
> +               git add add-lock-not-taken &&
>                 git commit -m "add add-lock-not-taken" &&
>                 git config git-p4.skipSubmitEdit true &&
>                 git p4 submit --verbose
> --
> 2.3.0.rc1.30.g76afe74

[PATCHv2 2/4] git-p4: small fix for locked-file-move-test

[Luke Diamand]

The test for handling of failure when trying to move a file
that is locked by another client was not quite correct - it
failed early on because the target file in the move already
existed.

The test now fails because git-p4 does not properly detect
that p4 has rejected the move, and instead just crashes. At
present, git-p4 has no support for detecting that a file
has been locked and reporting it to the user, so this is
the expected outcome.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 t/t9816-git-p4-locked.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index ce0eb22..464f10b 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -130,8 +130,8 @@ test_expect_failure 'move with lock taken' '
 	git p4 clone --dest="$git" //depot &&
 	(
 		cd "$git" &&
-		git mv file1 file2 &&
-		git commit -m "mv file1 to file2" &&
+		git mv file1 file3 &&
+		git commit -m "mv file1 to file3" &&
 		git config git-p4.skipSubmitEdit true &&
 		git config git-p4.detectRenames true &&
 		git p4 submit --verbose
-- 
2.3.0.rc1.30.g76afe74

[PATCHv2 3/4] git-p4: fix filetype detection on files opened exclusively

[Luke Diamand]

From: "Holloway, Blair" <Blair_Holloway@playstation.sony.com>

If a Perforce server is configured to automatically set +l (exclusive lock) on
add of certain file types, git p4 submit will fail during getP4OpenedType, as
the regex doesn't expect the trailing '*exclusive*' from p4 opened:

//depot/file.png#1 - add default change (binary+l) *exclusive*

Signed-off-by: Blair Holloway <blair_holloway@playstation.sony.com>
Acked-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index ff132b2..d43482a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -368,7 +368,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(".*\((.+)\)( \*exclusive\*)?\r?$", result)
     if match:
         return match.group(1)
     else:
-- 
2.3.0.rc1.30.g76afe74

[PATCHv2 4/4] git-p4: update locked test case

[Luke Diamand]

The add-new-file and copy-existing-file tests from
t9816-git-p4-locked.sh now pass. Update the test
case accordingly.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 t/t9816-git-p4-locked.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index 464f10b..d048bd3 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -35,7 +35,7 @@ test_expect_success 'edit with lock not taken' '
 	)
 '
 
-test_expect_failure 'add with lock not taken' '
+test_expect_success 'add with lock not taken' '
 	test_when_finished cleanup_git &&
 	git p4 clone --dest="$git" //depot &&
 	(
@@ -107,7 +107,7 @@ test_expect_failure 'chmod with lock taken' '
 	)
 '
 
-test_expect_failure 'copy with lock taken' '
+test_expect_success 'copy with lock taken' '
 	lock_in_another_client &&
 	test_when_finished cleanup_git &&
 	test_when_finished "cd \"$cli\" && p4 revert file2 && rm -f file2" &&
-- 
2.3.0.rc1.30.g76afe74

Re: [PATCHv2 4/4] git-p4: update locked test case

[Eric Sunshine]

On Fri, Apr 3, 2015 at 2:53 PM, Luke Diamand <luke@diamand.org> wrote:
> The add-new-file and copy-existing-file tests from
> t9816-git-p4-locked.sh now pass. Update the test
> case accordingly.

This patch should be folded into patch 3/4, shouldn't it?

> Signed-off-by: Luke Diamand <luke@diamand.org>
> ---
>  t/t9816-git-p4-locked.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
> index 464f10b..d048bd3 100755
> --- a/t/t9816-git-p4-locked.sh
> +++ b/t/t9816-git-p4-locked.sh
> @@ -35,7 +35,7 @@ test_expect_success 'edit with lock not taken' '
>         )
>  '
>
> -test_expect_failure 'add with lock not taken' '
> +test_expect_success 'add with lock not taken' '
>         test_when_finished cleanup_git &&
>         git p4 clone --dest="$git" //depot &&
>         (
> @@ -107,7 +107,7 @@ test_expect_failure 'chmod with lock taken' '
>         )
>  '
>
> -test_expect_failure 'copy with lock taken' '
> +test_expect_success 'copy with lock taken' '
>         lock_in_another_client &&
>         test_when_finished cleanup_git &&
>         test_when_finished "cd \"$cli\" && p4 revert file2 && rm -f file2" &&
> --
> 2.3.0.rc1.30.g76afe74

Re: [PATCHv2 4/4] git-p4: update locked test case

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Fri, Apr 3, 2015 at 2:53 PM, Luke Diamand <luke@diamand.org> wrote:
>> The add-new-file and copy-existing-file tests from
>> t9816-git-p4-locked.sh now pass. Update the test
>> case accordingly.
>
> This patch should be folded into patch 3/4, shouldn't it?

If the fix and tests were done by the same person, we strongly
prefer to see the code fix and test expectation update in the same
commit.

What Luke is doing here is to spread the credits/blame to two
people.  If I were playing Luke's role, I might have squashed them
into a single commit, kept the authorship of the whole thing under
Blair Holloway's name, and extend the log message by mentioning that
I flipped the test expectations, but I'd say either is fine for this
change.

>> Signed-off-by: Luke Diamand <luke@diamand.org>
>> ---
>>  t/t9816-git-p4-locked.sh | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
>> index 464f10b..d048bd3 100755
>> --- a/t/t9816-git-p4-locked.sh
>> +++ b/t/t9816-git-p4-locked.sh
>> @@ -35,7 +35,7 @@ test_expect_success 'edit with lock not taken' '
>>         )
>>  '
>>
>> -test_expect_failure 'add with lock not taken' '
>> +test_expect_success 'add with lock not taken' '
>>         test_when_finished cleanup_git &&
>>         git p4 clone --dest="$git" //depot &&
>>         (
>> @@ -107,7 +107,7 @@ test_expect_failure 'chmod with lock taken' '
>>         )
>>  '
>>
>> -test_expect_failure 'copy with lock taken' '
>> +test_expect_success 'copy with lock taken' '
>>         lock_in_another_client &&
>>         test_when_finished cleanup_git &&
>>         test_when_finished "cd \"$cli\" && p4 revert file2 && rm -f file2" &&
>> --
>> 2.3.0.rc1.30.g76afe74