| Message ID | 68b68ce1e4782bba552a016867bfc629f0d5e24f.1554141338.git.amazo@checkvideo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-p4: a few assorted fixes for branches, excludes | expand |
Hi Junio and Andrey, On Mon, Apr 01, 2019 at 06:02:21PM +0000, Mazo, Andrey wrote: > diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh > index 6a86d6996b..c48532e12b 100755 > --- a/t/t9801-git-p4-branch.sh > +++ b/t/t9801-git-p4-branch.sh > @@ -608,10 +608,102 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie > cd branch1 && > grep "client spec" file1 > ) > ' > > +test_expect_success 'restart p4d (case folding enabled)' ' > + kill_p4d && > + start_p4d -C1 > +' There is a semantic conflict between this patch and commit 07353d9042 (git p4 test: clean up the p4d cleanup functions, 2019-03-13) in 'sg/test-atexit' currently cooking in 'pu': this patch adds a new callsite of 'kill_p4d', but that commit renamed that function to 'stop_and_cleanup_p4d'. Consequently, t9801 on 'pu' now fails with: +kill_p4d t9801-git-p4-branch.sh: 4: eval: kill_p4d: not found error: last command exited with $?=127 not ok 28 - restart p4d (case folding enabled) https://travis-ci.org/git/git/jobs/514513463#L5827 I wonder whether it would be worth amending 07353d9042 to keep 'kill_p4d' around as a wrapper around 'stop_and_cleanup_p4d' for the time being. https://public-inbox.org/git/20190313122419.2210-9-szeder.dev@gmail.com/
> On Mon, Apr 01, 2019 at 06:02:21PM +0000, Mazo, Andrey wrote: >> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh >> index 6a86d6996b..c48532e12b 100755 >> --- a/t/t9801-git-p4-branch.sh >> +++ b/t/t9801-git-p4-branch.sh >> @@ -608,10 +608,102 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie >> cd branch1 && >> grep "client spec" file1 >> ) >> ' >> >> +test_expect_success 'restart p4d (case folding enabled)' ' >> + kill_p4d && >> + start_p4d -C1 >> +' > > There is a semantic conflict between this patch and commit 07353d9042 > (git p4 test: clean up the p4d cleanup functions, 2019-03-13) in > 'sg/test-atexit' currently cooking in 'pu': this patch adds a new > callsite of 'kill_p4d', but that commit renamed that function to > 'stop_and_cleanup_p4d'. Consequently, t9801 on 'pu' now fails with: > > +kill_p4d > t9801-git-p4-branch.sh: 4: eval: kill_p4d: not found > error: last command exited with $?=127 > not ok 28 - restart p4d (case folding enabled) > > https://travis-ci.org/git/git/jobs/514513463#L5827 > > I wonder whether it would be worth amending 07353d9042 to keep > 'kill_p4d' around as a wrapper around 'stop_and_cleanup_p4d' for the > time being. > > > https://public-inbox.org/git/20190313122419.2210-9-szeder.dev@gmail.com/ Good catch! Don't know what's the proper workflow here, but I see 2 more options: * Resolve the conflict in t/t9801-git-p4-branch.sh while merging am/p4-branches-excludes commit d15068a650 ("git-p4: respect excluded paths when detecting branches", 2019-04-01) * I can rebase my git-p4 changes on top of sg/test-atexit branch commit 74ec8cf674 ("t9811-git-p4-label-import: fix pipeline negation", 2019-03-13) In case this might be helpful, I did a conflict resolution locally, (by doing `git checkout d15068a650; git merge 74ec8cf674`) and here's the patch of the merge. Basically, * a newly added "kill_p4d" is replaced with "stop_and_cleanup_p4d"; and * "kill_p4d" in the end of the script is removed. diff --cc t/t9801-git-p4-branch.sh index 38d6b9043b,9654362052..67ff2711f5 --- a/t/t9801-git-p4-branch.sh +++ b/t/t9801-git-p4-branch.sh @@@ -610,4 -650,100 +650,96 @@@ test_expect_success 'Update a file in g ) ' + test_expect_success 'restart p4d (case folding enabled)' ' - kill_p4d && ++ stop_and_cleanup_p4d && + start_p4d -C1 + ' + + # + # 1: //depot/main/mf1 + # 2: integrate //depot/main/... -> //depot/branch1/... + # 3: //depot/main/mf2 + # 4: //depot/BRANCH1/B1f3 + # 5: //depot/branch1/b1f4 + # + test_expect_success !CASE_INSENSITIVE_FS 'basic p4 branches for case folding' ' + ( + cd "$cli" && + mkdir -p main && + + echo mf1 >main/mf1 && + p4 add main/mf1 && + p4 submit -d "main/mf1" && + + p4 integrate //depot/main/... //depot/branch1/... && + p4 submit -d "integrate main to branch1" && + + echo mf2 >main/mf2 && + p4 add main/mf2 && + p4 submit -d "main/mf2" && + + mkdir BRANCH1 && + echo B1f3 >BRANCH1/B1f3 && + p4 add BRANCH1/B1f3 && + p4 submit -d "BRANCH1/B1f3" && + + echo b1f4 >branch1/b1f4 && + p4 add branch1/b1f4 && + p4 submit -d "branch1/b1f4" + ) + ' + + # Check that files are properly split across branches when ignorecase is set + test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList main:branch1 && + git config --type=bool core.ignoreCase true && + git p4 clone --dest=. --detect-branches //depot@all && + + git log --all --graph --decorate --stat && + + git reset --hard p4/master && + test_path_is_file mf1 && + test_path_is_file mf2 && + test_path_is_missing B1f3 && + test_path_is_missing b1f4 && + + git reset --hard p4/depot/branch1 && + test_path_is_file mf1 && + test_path_is_missing mf2 && + test_path_is_file B1f3 && + test_path_is_file b1f4 + ) + ' + + # Check that files are properly split across branches when ignorecase is set, use-client-spec case + test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' ' + client_view "//depot/... //client/..." && + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList main:branch1 && + git config --type=bool core.ignoreCase true && + git p4 clone --dest=. --use-client-spec --detect-branches //depot@all && + + git log --all --graph --decorate --stat && + + git reset --hard p4/master && + test_path_is_file mf1 && + test_path_is_file mf2 && + test_path_is_missing B1f3 && + test_path_is_missing b1f4 && + + git reset --hard p4/depot/branch1 && + test_path_is_file mf1 && + test_path_is_missing mf2 && + test_path_is_file B1f3 && + test_path_is_file b1f4 + ) + ' + -test_expect_success 'kill p4d' ' - kill_p4d -' - test_done
SZEDER Gábor <szeder.dev@gmail.com> writes: > I wonder whether it would be worth amending 07353d9042 to keep > 'kill_p4d' around as a wrapper around 'stop_and_cleanup_p4d' for the > time being. I think renaming was the right thing to do; "show --cc" shows that the post-image lives in the new world order where calling kill_p4d is not usually needed clearly with a hunk that replaces kill_p4d (which only existed in the old world) with stop_and_cleanup (which exists only in the new world). Will redo the merge and teach my rerere database. Thanks.
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh index 6a86d6996b..c48532e12b 100755 --- a/t/t9801-git-p4-branch.sh +++ b/t/t9801-git-p4-branch.sh @@ -608,10 +608,102 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie cd branch1 && grep "client spec" file1 ) ' +test_expect_success 'restart p4d (case folding enabled)' ' + kill_p4d && + start_p4d -C1 +' + +# +# 1: //depot/main/mf1 +# 2: integrate //depot/main/... -> //depot/branch1/... +# 3: //depot/main/mf2 +# 4: //depot/BRANCH1/B1f3 +# 5: //depot/branch1/b1f4 +# +test_expect_success !CASE_INSENSITIVE_FS 'basic p4 branches for case folding' ' + ( + cd "$cli" && + mkdir -p main && + + echo mf1 >main/mf1 && + p4 add main/mf1 && + p4 submit -d "main/mf1" && + + p4 integrate //depot/main/... //depot/branch1/... && + p4 submit -d "integrate main to branch1" && + + echo mf2 >main/mf2 && + p4 add main/mf2 && + p4 submit -d "main/mf2" && + + mkdir BRANCH1 && + echo B1f3 >BRANCH1/B1f3 && + p4 add BRANCH1/B1f3 && + p4 submit -d "BRANCH1/B1f3" && + + echo b1f4 >branch1/b1f4 && + p4 add branch1/b1f4 && + p4 submit -d "branch1/b1f4" + ) +' + +# Check that files are properly split across branches when ignorecase is set +test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList main:branch1 && + git config --type=bool core.ignoreCase true && + git p4 clone --dest=. --detect-branches //depot@all && + + git log --all --graph --decorate --stat && + + git reset --hard p4/master && + test_path_is_file mf1 && + test_path_is_file mf2 && + test_path_is_missing B1f3 && + test_path_is_missing b1f4 && + + git reset --hard p4/depot/branch1 && + test_path_is_file mf1 && + test_path_is_missing mf2 && + test_path_is_file B1f3 && + test_path_is_file b1f4 + ) +' + +# Check that files are properly split across branches when ignorecase is set, use-client-spec case +test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' ' + client_view "//depot/... //client/..." && + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList main:branch1 && + git config --type=bool core.ignoreCase true && + git p4 clone --dest=. --use-client-spec --detect-branches //depot@all && + + git log --all --graph --decorate --stat && + + git reset --hard p4/master && + test_path_is_file mf1 && + test_path_is_file mf2 && + test_path_is_missing B1f3 && + test_path_is_missing b1f4 && + + git reset --hard p4/depot/branch1 && + test_path_is_file mf1 && + test_path_is_missing mf2 && + test_path_is_file B1f3 && + test_path_is_file b1f4 + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' test_done
In preparation for a fix, add a failing test case to test that git-p4 doesn't fold the case in file paths when doing branch detection case insensitively. (i.e. when core.ignorecase is set) Signed-off-by: Andrey Mazo <amazo@checkvideo.com> --- t/t9801-git-p4-branch.sh | 92 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+)