I'm adding a test case for a scenario I was confronted with when using branch
detection and a client view specification. It is possible that the implemented
fix may not cover all possible scenarios, but there is no regression in the
available tests.
Vitor Antunes (2):
git-p4: Check branch detection and client view together
git-p4: Improve client path detection when branches are used
git-p4.py | 11 ++++--
t/t9801-git-p4-branch.sh | 98 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 105 insertions(+), 4 deletions(-)
--
1.7.10.4
Add failing scenario where using branch detection and a client view will break
git p4 submit functionality.
Signed-off-by: Vitor Antunes <redacted>
---
t/t9801-git-p4-branch.sh | 98 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index 2bf142d..2f0361a 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -504,6 +504,104 @@ 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
+# 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 remove the "base" folder from the workspace
+test_expect_success 'add simple p4 branches with common base folder on each branch' '
+ (
+ cd "$cli" &&
+ client_view "//depot/branch1/base/... //client/branch1/..." \
+ "//depot/branch2/base/... //client/branch2/..." \
+ "//depot/branch3/base/... //client/branch3/..." &&
+ mkdir -p branch1 &&
+ cd branch1 &&
+ echo file1 >file1 &&
+ echo file2 >file2 &&
+ p4 add file1 file2 &&
+ 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 &&
+ grep update file2 &&
+ git reset --hard p4/depot/branch2 &&
+ test -f file1 &&
+ test -f file2 &&
+ test ! -f file3 &&
+ ! grep update file2 &&
+ git reset --hard p4/depot/branch3 &&
+ test -f file1 &&
+ test -f file2 &&
+ test -f file3 &&
+ 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
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.
Signed-off-by: Vitor Antunes <redacted>
---
git-p4.py | 11 +++++++----
t/t9801-git-p4-branch.sh | 2 +-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 549022e..6954549 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -502,12 +502,12 @@ 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:
+ if entry["depotFile"].find(depotPath) >= 0:
output = entry
break
elif "data" in entry:@@ -1627,7 +1627,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 2f0361a..4fe4e18 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -585,7 +585,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
* Modify source file (file2) before copying the file.
* Check that only file2 is the source in the output of "p4 filelog".
* Remove all "case" statements and replace them simple tests to check that
source is "file2".
Signed-off-by: Vitor Antunes <redacted>
---
t/t9814-git-p4-rename.sh | 46 +++++++++++++++++++++++++++++++---------------
1 file changed, 31 insertions(+), 15 deletions(-)
diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 8b9c295..d8fb22d 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -132,6 +132,9 @@ test_expect_success 'detect copies' '
cd "$git" &&
git config git-p4.skipSubmitEdit true &&
+ echo "file8" >> file2 &&
+ git commit -a -m "Differentiate file2" &&
+ git p4 submit &&
cp file2 file8 &&
git add file8 &&
git commit -a -m "Copy file2 to file8" &&
@@ -140,6 +143,10 @@ test_expect_success 'detect copies' '
p4 filelog //depot/file8 &&
p4 filelog //depot/file8 | test_must_fail grep -q "branch from" &&
+ echo "file9" >> file2 &&
+ git commit -a -m "Differentiate file2" &&
+ git p4 submit &&
+
cp file2 file9 &&
git add file9 &&
git commit -a -m "Copy file2 to file9" &&
@@ -149,28 +156,39 @@ test_expect_success 'detect copies' '
p4 filelog //depot/file9 &&
p4 filelog //depot/file9 | test_must_fail grep -q "branch from" &&
+ echo "file10" >> file2 &&
+ git commit -a -m "Differentiate file2" &&
+ git p4 submit &&
+
echo "file2" >>file2 &&
cp file2 file10 &&
git add file2 file10 &&
git commit -a -m "Modify and copy file2 to file10" &&
git diff-tree -r -C HEAD &&
+ src=$(git diff-tree -r -C HEAD | sed 1d | sed 2d | cut -f2) &&
+ test "$src" = file2 &&
git p4 submit &&
p4 filelog //depot/file10 &&
- p4 filelog //depot/file10 | grep -q "branch from //depot/file" &&
+ p4 filelog //depot/file10 | grep -q "branch from //depot/file2" &&
+
+ echo "file11" >> file2 &&
+ git commit -a -m "Differentiate file2" &&
+ git p4 submit &&
cp file2 file11 &&
git add file11 &&
git commit -a -m "Copy file2 to file11" &&
git diff-tree -r -C --find-copies-harder HEAD &&
src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
- case "$src" in
- file2 | file10) : ;; # happy
- *) false ;; # not
- esac &&
+ test "$src" = file2 &&
git config git-p4.detectCopiesHarder true &&
git p4 submit &&
p4 filelog //depot/file11 &&
- p4 filelog //depot/file11 | grep -q "branch from //depot/file" &&
+ p4 filelog //depot/file11 | grep -q "branch from //depot/file2" &&
+
+ echo "file12" >> file2 &&
+ git commit -a -m "Differentiate file2" &&
+ git p4 submit &&
cp file2 file12 &&
echo "some text" >>file12 &&
@@ -180,15 +198,16 @@ test_expect_success 'detect copies' '
level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
- case "$src" in
- file10 | file11) : ;; # happy
- *) false ;; # not
- esac &&
+ test "$src" = file2 &&
git config git-p4.detectCopies $(($level + 2)) &&
git p4 submit &&
p4 filelog //depot/file12 &&
p4 filelog //depot/file12 | test_must_fail grep -q "branch from" &&
+ echo "file13" >> file2 &&
+ git commit -a -m "Differentiate file2" &&
+ git p4 submit &&
+
cp file2 file13 &&
echo "different text" >>file13 &&
git add file13 &&
@@ -197,14 +216,11 @@ test_expect_success 'detect copies' '
level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
- case "$src" in
- file10 | file11 | file12) : ;; # happy
- *) false ;; # not
- esac &&
+ test "$src" = file2 &&
git config git-p4.detectCopies $(($level - 2)) &&
git p4 submit &&
p4 filelog //depot/file13 &&
- p4 filelog //depot/file13 | grep -q "branch from //depot/file"
+ p4 filelog //depot/file13 | grep -q "branch from //depot/file2"
)
'
--
1.7.10.4
On 28/03/15 12:28, Vitor Antunes wrote:
I'm adding a test case for a scenario I was confronted with when using branch
detection and a client view specification. It is possible that the implemented
fix may not cover all possible scenarios, but there is no regression in the
available tests.
Vitor, one thing I wondered about with this part of the change:
- if entry["depotFile"] == depotPath:
+ if entry["depotFile"].find(depotPath) >= 0:
Does this mean that if 'p4 where' produces multiple lines of output that
this will get confused, as it's just going to search for an instance of
depotPath.
The example in the Perforce man page for 'p4 where' would trigger this
for example:
http://www.perforce.com/perforce/r14.2/manuals/cmdref/p4_where.html
-//a/b/file.txt //client/a/b/file.txt //home/user/root/a/b/file.txt
//a/b/file.txt //client/b/file.txt /home/user/root/b/file.txt
As an experiment, I hacked git-p4 to always use p4Where rather than
getClientRoot(), which I would have thought ought to work, but while
most of the tests passed, Pete's client-spec torture tests failed.
Luke
Vitor Antunes (2):
git-p4: Check branch detection and client view together
git-p4: Improve client path detection when branches are used
git-p4.py | 11 ++++--
t/t9801-git-p4-branch.sh | 98 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 105 insertions(+), 4 deletions(-)
Luke Diamand [off-list ref] wrote on Sun, 05 Apr 2015 20:27:11 +0100
On 28/03/15 12:28, Vitor Antunes wrote:
quoted
I'm adding a test case for a scenario I was confronted with when using branch
detection and a client view specification. It is possible that the implemented
fix may not cover all possible scenarios, but there is no regression in the
available tests.
Vitor, one thing I wondered about with this part of the change:
- if entry["depotFile"] == depotPath:
+ if entry["depotFile"].find(depotPath) >= 0:
Does this mean that if 'p4 where' produces multiple lines of output that
this will get confused, as it's just going to search for an instance of
depotPath.
The reason why I introduced that was because in the test case I implemented (and
which reflects a scenario I am confronted with in my workplace) the branches
have a base directory that is removed in the client view mapping.
As such, we will have a situation where depotPath is //depot/branch1/ while
runninng "p4 where" will result in //depot/branch1/base/. To overcome this I
used find() instead of a direct comparison. Now that I think about that, I could
probably have used the simpler `if depotPath in entry["depotFile"]`...
The example in the Perforce man page for 'p4 where' would trigger this
for example:
http://www.perforce.com/perforce/r14.2/manuals/cmdref/p4_where.html
-//a/b/file.txt //client/a/b/file.txt //home/user/root/a/b/file.txt
//a/b/file.txt //client/b/file.txt /home/user/root/b/file.txt
These are examples where a simple comparison as was implemented would work.
As an experiment, I hacked git-p4 to always use p4Where rather than
getClientRoot(), which I would have thought ought to work, but while
most of the tests passed, Pete's client-spec torture tests failed.
That was exactly my first approach and got to the same conclusion. I would have
investigated it further but since I haven't had much free time to invest in
solving this problem I decided to implement an intermediary solution that would
not introduce any regressions.
Vitor