* [PATCH 0/2] git-p4: Improve client path detection
@ 2015-03-28 12:28 Vitor Antunes
2015-03-28 12:28 ` [PATCH 1/2] git-p4: Check branch detection and client view together Vitor Antunes
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Vitor Antunes @ 2015-03-28 12:28 UTC (permalink / raw)
To: git; +Cc: Vitor Antunes
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
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/2] git-p4: Check branch detection and client view together 2015-03-28 12:28 [PATCH 0/2] git-p4: Improve client path detection Vitor Antunes @ 2015-03-28 12:28 ` Vitor Antunes 2015-03-28 12:28 ` [PATCH 2/2] git-p4: Improve client path detection when branches are used Vitor Antunes ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Vitor Antunes @ 2015-03-28 12:28 UTC (permalink / raw) To: git; +Cc: Vitor Antunes Add failing scenario where using branch detection and a client view will break git p4 submit functionality. Signed-off-by: Vitor Antunes <vitor.hda@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] git-p4: Improve client path detection when branches are used 2015-03-28 12:28 [PATCH 0/2] git-p4: Improve client path detection Vitor Antunes 2015-03-28 12:28 ` [PATCH 1/2] git-p4: Check branch detection and client view together Vitor Antunes @ 2015-03-28 12:28 ` Vitor Antunes 2015-03-29 23:31 ` [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests Vitor Antunes 2015-04-05 19:27 ` [PATCH 0/2] git-p4: Improve client path detection Luke Diamand 3 siblings, 0 replies; 18+ messages in thread From: Vitor Antunes @ 2015-03-28 12:28 UTC (permalink / raw) To: git; +Cc: 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. Signed-off-by: Vitor Antunes <vitor.hda@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests 2015-03-28 12:28 [PATCH 0/2] git-p4: Improve client path detection Vitor Antunes 2015-03-28 12:28 ` [PATCH 1/2] git-p4: Check branch detection and client view together Vitor Antunes 2015-03-28 12:28 ` [PATCH 2/2] git-p4: Improve client path detection when branches are used Vitor Antunes @ 2015-03-29 23:31 ` Vitor Antunes 2015-03-30 3:03 ` Junio C Hamano 2015-04-05 19:27 ` [PATCH 0/2] git-p4: Improve client path detection Luke Diamand 3 siblings, 1 reply; 18+ messages in thread From: Vitor Antunes @ 2015-03-29 23:31 UTC (permalink / raw) To: git; +Cc: Vitor Antunes * 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 <vitor.hda@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests 2015-03-29 23:31 ` [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests Vitor Antunes @ 2015-03-30 3:03 ` Junio C Hamano 2015-03-31 13:26 ` Luke Diamand ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Junio C Hamano @ 2015-03-30 3:03 UTC (permalink / raw) To: Pete Wyckoff, Luke Diamand; +Cc: git, Vitor Antunes Vitor Antunes <vitor.hda@gmail.com> writes: > * 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 <vitor.hda@gmail.com> > --- I am not a Perfoce user, so I'd like to ask Pete's and Luke's comments on these changes. > 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 && Style: please lose SP between redirection and its target, i.e. echo file8 >>file2 && The same comment applies to everywhere else. > + 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" > ) > ' ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests 2015-03-30 3:03 ` Junio C Hamano @ 2015-03-31 13:26 ` Luke Diamand 2015-03-31 23:29 ` [PATCH V2] " Vitor Antunes 2015-04-04 8:31 ` [PATCH] " Luke Diamand 2 siblings, 0 replies; 18+ messages in thread From: Luke Diamand @ 2015-03-31 13:26 UTC (permalink / raw) To: Junio C Hamano; +Cc: Pete Wyckoff, Git Users, Vitor Antunes I'm on holiday this week, so I'll not get a chance to look at these properly until next week. Luke On 30 March 2015 at 04:03, Junio C Hamano <gitster@pobox.com> wrote: > Vitor Antunes <vitor.hda@gmail.com> writes: > >> * 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 <vitor.hda@gmail.com> >> --- > > I am not a Perfoce user, so I'd like to ask Pete's and Luke's > comments on these changes. > >> 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 && > > Style: please lose SP between redirection and its target, i.e. > > echo file8 >>file2 && > > The same comment applies to everywhere else. > >> + 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" >> ) >> ' ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V2] t9814: Guarantee only one source exists in git-p4 copy tests 2015-03-30 3:03 ` Junio C Hamano 2015-03-31 13:26 ` Luke Diamand @ 2015-03-31 23:29 ` Vitor Antunes 2015-04-04 8:31 ` [PATCH] " Luke Diamand 2 siblings, 0 replies; 18+ messages in thread From: Vitor Antunes @ 2015-03-31 23:29 UTC (permalink / raw) To: git; +Cc: Vitor Antunes * 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 with simple tests to check that source is "file2". Signed-off-by: Vitor Antunes <vitor.hda@gmail.com> --- 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..99bb71b 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests 2015-03-30 3:03 ` Junio C Hamano 2015-03-31 13:26 ` Luke Diamand 2015-03-31 23:29 ` [PATCH V2] " Vitor Antunes @ 2015-04-04 8:31 ` Luke Diamand 2015-04-04 19:41 ` Junio C Hamano 2 siblings, 1 reply; 18+ messages in thread From: Luke Diamand @ 2015-04-04 8:31 UTC (permalink / raw) To: Junio C Hamano, Pete Wyckoff; +Cc: git, Vitor Antunes On 30/03/15 04:03, Junio C Hamano wrote: > Vitor Antunes <vitor.hda@gmail.com> writes: > >> * 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 <vitor.hda@gmail.com> >> --- > > I am not a Perfoce user, so I'd like to ask Pete's and Luke's > comments on these changes. It's much clearer now that the guessing of file source has been cleaned up, thanks. Ack. Luke > >> 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 && > > Style: please lose SP between redirection and its target, i.e. > > echo file8 >>file2 && > > The same comment applies to everywhere else. > >> + 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" >> ) >> ' ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests 2015-04-04 8:31 ` [PATCH] " Luke Diamand @ 2015-04-04 19:41 ` Junio C Hamano 2015-04-05 23:08 ` [PATCH V3] " Vitor Antunes 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2015-04-04 19:41 UTC (permalink / raw) To: Vitor Antunes; +Cc: Pete Wyckoff, git, Luke Diamand Luke Diamand <luke@diamand.org> writes: > On 30/03/15 04:03, Junio C Hamano wrote: >> Vitor Antunes <vitor.hda@gmail.com> writes: >> >>> * 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 <vitor.hda@gmail.com> >>> --- >> >> I am not a Perfoce user, so I'd like to ask Pete's and Luke's >> comments on these changes. > > It's much clearer now that the guessing of file source has been > cleaned up, thanks. Ack. Thanks. Vitor, when resubmitting v2 to fix style nits, please add Luke's Acked-by just after your Sign-off, perhaps like this: t9814: guarantee only one source exists in git-p4 copy tests By using a tree with multiple identical files and allowing copy detection to choose any one of them, the check in the test is unnecessarily complex. We can simplify by: * 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 <vitor.hda@gmail.com> Acked-by: Luke Diamand <luke@diamand.org> >>> 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 && >> >> Style: please lose SP between redirection and its target, i.e. >> >> echo file8 >>file2 && >> >> The same comment applies to everywhere else. >> >>> + 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" >>> ) >>> ' ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V3] t9814: Guarantee only one source exists in git-p4 copy tests 2015-04-04 19:41 ` Junio C Hamano @ 2015-04-05 23:08 ` Vitor Antunes 0 siblings, 0 replies; 18+ messages in thread From: Vitor Antunes @ 2015-04-05 23:08 UTC (permalink / raw) To: git; +Cc: Vitor Antunes By using a tree with multiple identical files and allowing copy detection to choose any one of them, the check in the test is unnecessarily complex. We can simplify by: * 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 with simple tests to check that source is "file2". Signed-off-by: Vitor Antunes <vitor.hda@gmail.com> Acked-by: Luke Diamand <luke@diamand.org> --- 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..99bb71b 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] git-p4: Improve client path detection 2015-03-28 12:28 [PATCH 0/2] git-p4: Improve client path detection Vitor Antunes ` (2 preceding siblings ...) 2015-03-29 23:31 ` [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests Vitor Antunes @ 2015-04-05 19:27 ` Luke Diamand 2015-04-05 22:57 ` Vitor Antunes 3 siblings, 1 reply; 18+ messages in thread From: Luke Diamand @ 2015-04-05 19:27 UTC (permalink / raw) To: Vitor Antunes, git 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(-) > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] git-p4: Improve client path detection 2015-04-05 19:27 ` [PATCH 0/2] git-p4: Improve client path detection Luke Diamand @ 2015-04-05 22:57 ` Vitor Antunes 2015-04-13 3:40 ` Junio C Hamano 0 siblings, 1 reply; 18+ messages in thread From: Vitor Antunes @ 2015-04-05 22:57 UTC (permalink / raw) To: Luke Diamand; +Cc: git Luke Diamand <luke@diamand.org> wrote on Sun, 05 Apr 2015 20:27:11 +0100 > 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 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] git-p4: Improve client path detection 2015-04-05 22:57 ` Vitor Antunes @ 2015-04-13 3:40 ` Junio C Hamano 2015-04-18 22:40 ` Vitor Antunes 2015-04-18 23:24 ` [PATCH] git-p4: Improve client path detection when branches are used Vitor Antunes 0 siblings, 2 replies; 18+ messages in thread From: Junio C Hamano @ 2015-04-13 3:40 UTC (permalink / raw) To: Vitor Antunes; +Cc: Luke Diamand, git Vitor Antunes <vitor.hda@gmail.com> writes: > Luke Diamand <luke@diamand.org> wrote on Sun, 05 Apr 2015 20:27:11 +0100 >> 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 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"]`... Hmph, is this find() under discussion the string.find() that finds a substring? You are doing >=0 comparison here, but with your example that entry["depotFile"] may have "base/" appended to what you expect, the result of running string.find() must yield "0", i.e. no extra prefix string, no? I kind of find it hard to believe that it is OK to have any extra prefix is fine ... >> 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. ... so is this "find()" an attempt to catch prefix like "-"? Even if it that were the reason why you do not limit the acceptable return value from find() to zero, it feels a bit too loose to allow anything if the only thing you want to allow is a single "-" prefix. Can you explain this a bit better? I cannot quite tell what is going on from what was written in the log message. >> 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. Thanks. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] git-p4: Improve client path detection 2015-04-13 3:40 ` Junio C Hamano @ 2015-04-18 22:40 ` Vitor Antunes 2015-04-18 23:24 ` [PATCH] git-p4: Improve client path detection when branches are used Vitor Antunes 1 sibling, 0 replies; 18+ messages in thread From: Vitor Antunes @ 2015-04-18 22:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: Luke Diamand, git Hi Junio, Junio C Hamano <gitster@pobox.com> wrote on Sun, 12 Apr 2015 20:40:58 -0700 > Vitor Antunes <vitor.hda@gmail.com> writes: >> Luke Diamand <luke@diamand.org> wrote on Sun, 05 Apr 2015 20:27:11 +0100 >>> 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"]`... > > Hmph, is this find() under discussion the string.find() that finds a > substring? You are doing >=0 comparison here, but with your example > that entry["depotFile"] may have "base/" appended to what you > expect, the result of running string.find() must yield "0", i.e. no > extra prefix string, no? I kind of find it hard to believe that it > is OK to have any extra prefix is fine ... As usual, you're correct about your assumption. I should in fact be using "== 0" because what I really want is to guarantee that the path _starts_ with //depot/branch1. >>> 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. > > ... so is this "find()" an attempt to catch prefix like "-"? Even > if it that were the reason why you do not limit the acceptable > return value from find() to zero, it feels a bit too loose to allow > anything if the only thing you want to allow is a single "-" prefix. Again, it was just a bad coding from my part. > Can you explain this a bit better? I cannot quite tell what is > going on from what was written in the log message. I've temporarily modified the script to print out the output of "p4 where", for future reference: [{'clientFile': '//client/branch1/...', 'code': 'stat', 'depotFile': '//depot/branch1/base/...', 'path': '/path/to/git/t/trash directory.t9801-git-p4-branch/cli/branch1/...'}, {'clientFile': '//client/branch1/sub_file1', 'code': 'stat', 'unmap': '', 'depotFile': '//depot/branch1/base/sub_file1', 'path': '/path/to/git/t/trash directory.t9801-git-p4-branch/cli/branch1/sub_file1'}, {'clientFile': '//client/branch1/dir/sub_file1', 'code': 'stat', 'unmap': '', 'depotFile': '//depot/branch1/base/dir/sub_file1', 'path': '/path/to/git/t/trash directory.t9801-git-p4-branch/cli/branch1/dir/sub_file1'}, {'clientFile': '//client/branch1/sub_file1', 'code': 'stat', 'depotFile': '//depot/branch1/base/dir/sub_file1', 'path': '/path/to/git/t/trash directory.t9801-git-p4-branch/cli/branch1/sub_file1'}] Note that this is from a modified test case. As you can see, there are no paths starting with "-", instead there a new attribute called "unmap" that implements that description. In the latest version of this update I'm searching for a path starting with "//depot/branch1" and ending in "/...". This is a much more robust solution, so I am really grateful for your review. >>> 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. Since I'm looking at this more carefully now, I'll also try to see if I am able to make p4 where work even when not using branch detection. > Thanks. No, thank _you_! ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] git-p4: Improve client path detection when branches are used 2015-04-13 3:40 ` Junio C Hamano 2015-04-18 22:40 ` Vitor Antunes @ 2015-04-18 23:24 ` Vitor Antunes 2015-04-19 0:58 ` Junio C Hamano 1 sibling, 1 reply; 18+ messages in thread From: Vitor Antunes @ 2015-04-18 23:24 UTC (permalink / raw) To: git; +Cc: Luke Diamand, Junio C Hamano, Vitor Antunes This patch makes the client path detection more robust by limiting the valid results from p4 where. The test case is also made more complex, to guarantee that such client views are supported. Signed-off-by: Vitor Antunes <vitor.hda@gmail.com> --- git-p4.py | 4 +++- t/t9801-git-p4-branch.sh | 12 ++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/git-p4.py b/git-p4.py index 262a95b..28d0d90 100755 --- a/git-p4.py +++ b/git-p4.py @@ -507,7 +507,9 @@ def p4Where(depotPath): output = None for entry in outputList: if "depotFile" in entry: - if entry["depotFile"].find(depotPath) >= 0: + # 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: diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh index 4fe4e18..0aafd03 100755 --- a/t/t9801-git-p4-branch.sh +++ b/t/t9801-git-p4-branch.sh @@ -512,23 +512,28 @@ test_expect_success 'restart 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 remove the "base" folder from the workspace +# 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 && - p4 add file1 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" && @@ -561,16 +566,19 @@ test_expect_success 'git p4 clone simple branches with base folder on server sid 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 && -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] git-p4: Improve client path detection when branches are used 2015-04-18 23:24 ` [PATCH] git-p4: Improve client path detection when branches are used Vitor Antunes @ 2015-04-19 0:58 ` Junio C Hamano 2015-04-19 10:59 ` Vitor Antunes 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2015-04-19 0:58 UTC (permalink / raw) To: Vitor Antunes; +Cc: git, Luke Diamand Vitor Antunes <vitor.hda@gmail.com> writes: > This patch makes the client path detection more robust by limiting the valid > results from p4 where. The test case is also made more complex, to guarantee > that such client views are supported. > > Signed-off-by: Vitor Antunes <vitor.hda@gmail.com> > --- Was this designed to be squashed into the previous 2/2 patch? I do not think either 1/2 or 2/2 is in 'next' yet, and if this was to correct mistakes in the 2/2 that was posted earlier, it would be nicer to have a replacement patch with corrected log message. Thanks. > git-p4.py | 4 +++- > t/t9801-git-p4-branch.sh | 12 ++++++++++-- > 2 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/git-p4.py b/git-p4.py > index 262a95b..28d0d90 100755 > --- a/git-p4.py > +++ b/git-p4.py > @@ -507,7 +507,9 @@ def p4Where(depotPath): > output = None > for entry in outputList: > if "depotFile" in entry: > - if entry["depotFile"].find(depotPath) >= 0: > + # 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: > diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh > index 4fe4e18..0aafd03 100755 > --- a/t/t9801-git-p4-branch.sh > +++ b/t/t9801-git-p4-branch.sh > @@ -512,23 +512,28 @@ test_expect_success 'restart 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 remove the "base" folder from the workspace > +# 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 && > - p4 add file1 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" && > @@ -561,16 +566,19 @@ test_expect_success 'git p4 clone simple branches with base folder on server sid > 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 && ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] git-p4: Improve client path detection when branches are used 2015-04-19 0:58 ` Junio C Hamano @ 2015-04-19 10:59 ` Vitor Antunes 2015-04-20 5:32 ` Junio C Hamano 0 siblings, 1 reply; 18+ messages in thread From: Vitor Antunes @ 2015-04-19 10:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Luke Diamand Junio C Hamano <gitster@pobox.com> wrote on Sat, 18 Apr 2015 17:58:11 -0700 > Vitor Antunes <vitor.hda@gmail.com> writes: > >> This patch makes the client path detection more robust by limiting the valid >> results from p4 where. The test case is also made more complex, to guarantee >> that such client views are supported. >> >> Signed-off-by: Vitor Antunes <vitor.hda@gmail.com> >> --- > >Was this designed to be squashed into the previous 2/2 patch? I >do not think either 1/2 or 2/2 is in 'next' yet, and if this was >to correct mistakes in the 2/2 that was posted earlier, it would >be nicer to have a replacement patch with corrected log message. > >Thanks. Hope I got everything right this time :) Thanks for your help, Vitor ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] git-p4: Improve client path detection when branches are used 2015-04-19 10:59 ` Vitor Antunes @ 2015-04-20 5:32 ` Junio C Hamano 0 siblings, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2015-04-20 5:32 UTC (permalink / raw) To: Vitor Antunes; +Cc: git, Luke Diamand Vitor Antunes <vitor.hda@gmail.com> writes: > Hope I got everything right this time :) > > Thanks for your help, Thanks. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-04-20 5:32 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-28 12:28 [PATCH 0/2] git-p4: Improve client path detection Vitor Antunes 2015-03-28 12:28 ` [PATCH 1/2] git-p4: Check branch detection and client view together Vitor Antunes 2015-03-28 12:28 ` [PATCH 2/2] git-p4: Improve client path detection when branches are used Vitor Antunes 2015-03-29 23:31 ` [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests Vitor Antunes 2015-03-30 3:03 ` Junio C Hamano 2015-03-31 13:26 ` Luke Diamand 2015-03-31 23:29 ` [PATCH V2] " Vitor Antunes 2015-04-04 8:31 ` [PATCH] " Luke Diamand 2015-04-04 19:41 ` Junio C Hamano 2015-04-05 23:08 ` [PATCH V3] " Vitor Antunes 2015-04-05 19:27 ` [PATCH 0/2] git-p4: Improve client path detection Luke Diamand 2015-04-05 22:57 ` Vitor Antunes 2015-04-13 3:40 ` Junio C Hamano 2015-04-18 22:40 ` Vitor Antunes 2015-04-18 23:24 ` [PATCH] git-p4: Improve client path detection when branches are used Vitor Antunes 2015-04-19 0:58 ` Junio C Hamano 2015-04-19 10:59 ` Vitor Antunes 2015-04-20 5:32 ` Junio C Hamano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.