[PATCH V2 0/2] git-p4: improve client path detection with branches used

[PATCH V2 0/2] git-p4: improve client path detection with branches used

[Vitor Antunes]

Robustness improvement in regards to previous patches, both in the test case and
git-p4 functionality.

1. Test case now includes a mapped sub-file, to guarantee that git-p4 copes with
   such client view.

2. git-p4 now searches for paths starting with the branch's depot path and
   ending with "/..." to guarantee that the correct client side path is
   identified.

Vitor Antunes (2):
  t9801: check git-p4's branch detection and client view together
  git-p4: improve client path detection when branches are used

 git-p4.py                |   13 ++++--
 t/t9801-git-p4-branch.sh |  106 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+), 4 deletions(-)

-- 
1.7.10.4

[PATCH V2 1/2] t9801: check git-p4's branch detection and client view together

[Vitor Antunes]

Add failing scenario where branch detection is enabled together with
use client view. In this specific scenario git-p4 will break when the
perforce client view removes part of the depot path.

The test case also includes an extra sub-file mapping to enforce
robustness check on git-p4 implementation.

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t9801-git-p4-branch.sh |  106 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index 2bf142d..36a7f51 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -504,6 +504,112 @@ test_expect_success 'use-client-spec detect-branches skips files in branches' '
 	)
 '
 
+test_expect_success 'restart p4d' '
+	kill_p4d &&
+	start_p4d
+'
+
+#
+# 1: //depot/branch1/base/file1
+#    //depot/branch1/base/file2
+#    //depot/branch1/base/dir/sub_file1
+# 2: integrate //depot/branch1/base/... -> //depot/branch2/base/...
+# 3: //depot/branch1/base/file3
+# 4: //depot/branch1/base/file2 (edit)
+# 5: integrate //depot/branch1/base/... -> //depot/branch3/base/...
+#
+# Note: the client view removes the "base" folder from the workspace
+#       and moves sub_file1 one level up.
+test_expect_success 'add simple p4 branches with common base folder on each branch' '
+	(
+		cd "$cli" &&
+		client_view "//depot/branch1/base/... //client/branch1/..." \
+			    "//depot/branch1/base/dir/sub_file1 //client/branch1/sub_file1" \
+			    "//depot/branch2/base/... //client/branch2/..." \
+			    "//depot/branch3/base/... //client/branch3/..." &&
+		mkdir -p branch1 &&
+		cd branch1 &&
+		echo file1 >file1 &&
+		echo file2 >file2 &&
+		mkdir dir &&
+		echo sub_file1 >sub_file1 &&
+		p4 add file1 file2 sub_file1 &&
+		p4 submit -d "Create branch1" &&
+		p4 integrate //depot/branch1/base/... //depot/branch2/base/... &&
+		p4 submit -d "Integrate branch2 from branch1" &&
+		echo file3 >file3 &&
+		p4 add file3 &&
+		p4 submit -d "add file3 in branch1" &&
+		p4 open file2 &&
+		echo update >>file2 &&
+		p4 submit -d "update file2 in branch1" &&
+		p4 integrate //depot/branch1/base/... //depot/branch3/base/... &&
+		p4 submit -d "Integrate branch3 from branch1"
+	)
+'
+
+# Configure branches through git-config and clone them.
+# All files are tested to make sure branches were cloned correctly.
+# Finally, make an update to branch1 on P4 side to check if it is imported
+# correctly by git p4.
+# git p4 is expected to use the client view to also not include the common
+# "base" folder in the imported directory structure.
+test_expect_success 'git p4 clone simple branches with base folder on server side' '
+	test_create_repo "$git" &&
+	(
+		cd "$git" &&
+		git config git-p4.branchList branch1:branch2 &&
+		git config --add git-p4.branchList branch1:branch3 &&
+		git p4 clone --dest=. --use-client-spec  --detect-branches //depot@all &&
+		git log --all --graph --decorate --stat &&
+		git reset --hard p4/depot/branch1 &&
+		test -f file1 &&
+		test -f file2 &&
+		test -f file3 &&
+		test -f sub_file1 &&
+		grep update file2 &&
+		git reset --hard p4/depot/branch2 &&
+		test -f file1 &&
+		test -f file2 &&
+		test ! -f file3 &&
+		test -f sub_file1 &&
+		! grep update file2 &&
+		git reset --hard p4/depot/branch3 &&
+		test -f file1 &&
+		test -f file2 &&
+		test -f file3 &&
+		test -f sub_file1 &&
+		grep update file2 &&
+		cd "$cli" &&
+		cd branch1 &&
+		p4 edit file2 &&
+		echo file2_ >>file2 &&
+		p4 submit -d "update file2 in branch1" &&
+		cd "$git" &&
+		git reset --hard p4/depot/branch1 &&
+		git p4 rebase &&
+		grep file2_ file2
+	)
+'
+
+# Now update a file in one of the branches in git and submit to P4
+test_expect_failure 'Update a file in git side and submit to P4 using client view' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git reset --hard p4/depot/branch1 &&
+		echo "client spec" >> file1 &&
+		git add -u . &&
+		git commit -m "update file1 in branch1" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit --verbose &&
+		cd "$cli" &&
+		p4 sync ... &&
+		cd branch1 &&
+		grep "client spec" file1
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.10.4

Re: [PATCH V2 1/2] t9801: check git-p4's branch detection and client view together

[Junio C Hamano]

Vitor Antunes <vitor.hda@gmail.com> writes:

> Add failing scenario where branch detection is enabled together with
> use client view. In this specific scenario git-p4 will break when the
> perforce client view removes part of the depot path.

I somehow cannot parse "together with use client view", especially
the word "use".  Is it "user client view" or something (I am not
familiar with p4 lingo), or perhaps "use of client view"?

Re: [PATCH V2 1/2] t9801: check git-p4's branch detection and client view together

[Vitor Antunes]

On April 20, 2015 6:43:49 AM GMT+01:00, Junio C Hamano <gitster@pobox.com> wrote:
>Vitor Antunes <vitor.hda@gmail.com> writes:
>
>> Add failing scenario where branch detection is enabled together with
>> use client view. In this specific scenario git-p4 will break when the
>> perforce client view removes part of the depot path.
>
>I somehow cannot parse "together with use client view", especially
>the word "use".  Is it "user client view" or something (I am not
>familiar with p4 lingo), or perhaps "use of client view"?

I meant "spec" instead of "view". As in - -use-client-spec.

In perforce you need to configure your workspace using a client specification.
One of the configured values is the client view, which maps files/folders in the
server to locations in your local workspace. What I'm trying to fix with these
patches is the ability of git-p4 to process the client view definition through
the use of "p4 where" to determine the correct location of the local files, such
that it is able to apply the necessary patches for submission to the perforce
server.

While thinking about client views I completely forgot that the git-p4 argument
that enables thos feature uses "spec" and not "view".

Re: [PATCH V2 1/2] t9801: check git-p4's branch detection and client view together

[Junio C Hamano]

Vitor Antunes <vitor.hda@gmail.com> writes:

> On April 20, 2015 6:43:49 AM GMT+01:00, Junio C Hamano <gitster@pobox.com> wrote:
>>Vitor Antunes <vitor.hda@gmail.com> writes:
>>
>>> Add failing scenario where branch detection is enabled together with
>>> use client view. In this specific scenario git-p4 will break when the
>>> perforce client view removes part of the depot path.
>>
>>I somehow cannot parse "together with use client view", especially
>>the word "use".  Is it "user client view" or something (I am not
>>familiar with p4 lingo), or perhaps "use of client view"?
>
> I meant "spec" instead of "view". As in - -use-client-spec.
>
> In perforce you need to configure your workspace using a client specification.
> One of the configured values is the client view, which maps files/folders in the
> server to locations in your local workspace. What I'm trying to fix with these
> patches is the ability of git-p4 to process the client view definition through
> the use of "p4 where" to determine the correct location of the local files, such
> that it is able to apply the necessary patches for submission to the perforce
> server.
>
> While thinking about client views I completely forgot that the git-p4 argument
> that enables thos feature uses "spec" and not "view".

So,... what's the conclusion?  Should the log message be written
like this perhaps?

    t9801: check git-p4's branch detection and client spec together
    
    Add failing scenario where branch detection is enabled together
    with use of client spec.  In this specific scenario git-p4 will
    break when the perforce client spec removes part of the depot
    path.
    
    The test case also includes an extra sub-file mapping to enforce
    robustness check on git-p4 implementation.
    
    Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

Re: [PATCH V2 1/2] t9801: check git-p4's branch detection and client view together

[Vitor Antunes]

On April 20, 2015 11:45:15 PM GMT+01:00, Junio C Hamano <gitster@pobox.com> wrote:
>Vitor Antunes <vitor.hda@gmail.com> writes:
>
>> On April 20, 2015 6:43:49 AM GMT+01:00, Junio C Hamano
><gitster@pobox.com> wrote:
>>>Vitor Antunes <vitor.hda@gmail.com> writes:
>>>
>>>> Add failing scenario where branch detection is enabled together
>with
>>>> use client view. In this specific scenario git-p4 will break when
>the
>>>> perforce client view removes part of the depot path.
>>>
>>>I somehow cannot parse "together with use client view", especially
>>>the word "use".  Is it "user client view" or something (I am not
>>>familiar with p4 lingo), or perhaps "use of client view"?
>>
>> I meant "spec" instead of "view". As in - -use-client-spec.
>>
>> In perforce you need to configure your workspace using a client
>specification.
>> One of the configured values is the client view, which maps
>files/folders in the
>> server to locations in your local workspace. What I'm trying to fix
>with these
>> patches is the ability of git-p4 to process the client view
>definition through
>> the use of "p4 where" to determine the correct location of the local
>files, such
>> that it is able to apply the necessary patches for submission to the
>perforce
>> server.
>>
>> While thinking about client views I completely forgot that the git-p4
>argument
>> that enables thos feature uses "spec" and not "view".
>
>So,... what's the conclusion?  Should the log message be written
>like this perhaps?
>
>    t9801: check git-p4's branch detection and client spec together
>    
>    Add failing scenario where branch detection is enabled together
>    with use of client spec.  In this specific scenario git-p4 will
>    break when the perforce client spec removes part of the depot
>    path.
>    
>    The test case also includes an extra sub-file mapping to enforce
>    robustness check on git-p4 implementation.
>    
>    Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
>    Signed-off-by: Junio C Hamano <gitster@pobox.com>

Unfortunately at the moment I have limited computer access at
home. I will, obviously, update the descriptions as soon as
possible.

[PATCH V2 2/2] git-p4: improve client path detection when branches are used

[Vitor Antunes]

A client view can be used to remap folder locations in the
workspace. To support this when branch detection is enabled,
it is necessary to get the client path through "p4 where".

This patch does two things to achieve this:

 1. Force usage of "p4 where" when P4 branches exist in the
    git repository.

 2. Search for mappings that contain the depot path, instead
    of requiring an exact match. To guarantee robustness,
    paths only match if ending in "/...".

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-p4.py                |   13 +++++++++----
 t/t9801-git-p4-branch.sh |    2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 549022e..34e4fdd 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -502,12 +502,14 @@ def p4Cmd(cmd):
 def p4Where(depotPath):
     if not depotPath.endswith("/"):
         depotPath += "/"
-    depotPath = depotPath + "..."
-    outputList = p4CmdList(["where", depotPath])
+    depotPathLong = depotPath + "..."
+    outputList = p4CmdList(["where", depotPathLong])
     output = None
     for entry in outputList:
         if "depotFile" in entry:
-            if entry["depotFile"] == depotPath:
+            # Search for the base client side depot path, as long as it starts with the branch's P4 path.
+            # The base path always ends with "/...".
+            if entry["depotFile"].find(depotPath) == 0 and entry["depotFile"][-4:] == "/...":
                 output = entry
                 break
         elif "data" in entry:
@@ -1627,7 +1629,10 @@ class P4Submit(Command, P4UserMap):
         if self.useClientSpec:
             self.clientSpecDirs = getClientSpec()
 
-        if self.useClientSpec:
+        # Check for the existance of P4 branches
+        branchesDetected = (len(p4BranchesInGit().keys()) > 1)
+
+        if self.useClientSpec and not branchesDetected:
             # all files are relative to the client spec
             self.clientPath = getClientRoot()
         else:
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index 36a7f51..0aafd03 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -593,7 +593,7 @@ test_expect_success 'git p4 clone simple branches with base folder on server sid
 '
 
 # Now update a file in one of the branches in git and submit to P4
-test_expect_failure 'Update a file in git side and submit to P4 using client view' '
+test_expect_success 'Update a file in git side and submit to P4 using client view' '
 	test_when_finished cleanup_git &&
 	(
 		cd "$git" &&
-- 
1.7.10.4