Improvements for Perforce tests

PATCH Superseded Patrick Steinhardt 4 participants 19 messages Jul 23, 2024 – Jul 31, 2024

lore

Share & tools

lore

Patrick Steinhardt

author

2024-07-23 14:05 UTC #0 lore raw

cover Improvements for Perforce tests

Hi,

I was recently trying to figure out whether the Perforce tests are memory-leak free, but noticed that the tests don't run at all on my machine because I have a newer version of Perforce than our CI. And because the on-disk depot format has changed since r21, tests are broken.

This small series adapts tests to work with both r21 and r23, updates the version of Perforce used in CI and then ultimately marks all of the Perforce tests as memory-leak free.

Thanks!

Patrick

Patrick Steinhardt (3):
t98xx: fix Perforce tests with p4d r23 and newer
ci: update Perforce version to r23.2
t98xx: mark Perforce tests as memory-leak free

 ci/install-dependencies.sh                  |  2 +-
 t/t9800-git-p4-basic.sh                     | 14 ++++++++++++--
 t/t9801-git-p4-branch.sh                    |  1 +
 t/t9802-git-p4-filetype.sh                  | 16 +++++++++++++---
 t/t9803-git-p4-shell-metachars.sh           |  1 +
 t/t9804-git-p4-label.sh                     |  1 +
 t/t9805-git-p4-skip-submit-edit.sh          |  1 +
 t/t9806-git-p4-options.sh                   |  1 +
 t/t9808-git-p4-chdir.sh                     |  1 +
 t/t9809-git-p4-client-view.sh               |  1 +
 t/t9810-git-p4-rcs.sh                       |  1 +
 t/t9811-git-p4-label-import.sh              |  1 +
 t/t9812-git-p4-wildcards.sh                 |  1 +
 t/t9813-git-p4-preserve-users.sh            |  1 +
 t/t9814-git-p4-rename.sh                    |  1 +
 t/t9815-git-p4-submit-fail.sh               |  1 +
 t/t9816-git-p4-locked.sh                    |  1 +
 t/t9817-git-p4-exclude.sh                   |  1 +
 t/t9818-git-p4-block.sh                     |  1 +
 t/t9819-git-p4-case-folding.sh              |  1 +
 t/t9820-git-p4-editor-handling.sh           |  1 +
 t/t9821-git-p4-path-variations.sh           |  1 +
 t/t9822-git-p4-path-encoding.sh             |  1 +
 t/t9823-git-p4-mock-lfs.sh                  |  1 +
 t/t9825-git-p4-handle-utf16-without-bom.sh  | 19 ++++++++++++++++---
 t/t9826-git-p4-keep-empty-commits.sh        |  1 +
 t/t9827-git-p4-change-filetype.sh           |  1 +
 t/t9828-git-p4-map-user.sh                  |  1 +
 t/t9829-git-p4-jobs.sh                      |  1 +
 t/t9830-git-p4-symlink-dir.sh               |  1 +
 t/t9831-git-p4-triggers.sh                  |  1 +
 t/t9832-unshelve.sh                         |  1 +
 t/t9833-errors.sh                           |  1 +
 t/t9834-git-p4-file-dir-bug.sh              |  1 +
 t/t9835-git-p4-metadata-encoding-python2.sh |  1 +
 t/t9836-git-p4-metadata-encoding-python3.sh |  1 +
 36 files changed, 74 insertions(+), 9 deletions(-)

Signature · 1 line

2.46.0.rc1.dirty

Patrick Steinhardt

author

2024-07-23 14:05 UTC #1 lore raw

PATCH 1/3 t98xx: fix Perforce tests with p4d r23 and newer

Some of the tests in t98xx modify the Perforce depot in ways that the tool wouldn't normally allow. This is done to test behaviour of git-p4 in certain edge cases that we have observed in the wild, but which should in theory not be possible.

Naturally, modifying the depot on disk directly is quite intimate with the tool and thus prone to breakage when Perforce updates the way that data is stored. And indeed, those tests are broken nowadays with r23 of Perforce. While a file revision was previously stored as plain file "depot/file,v", it is now stored in a directory "depot/file,d" with compression.

Adapt those tests to handle both old- and new-style depot layouts.

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t9800-git-p4-basic.sh                    | 13 +++++++++++--
 t/t9802-git-p4-filetype.sh                 | 15 ++++++++++++---
 t/t9825-git-p4-handle-utf16-without-bom.sh | 18 +++++++++++++++---
 3 files changed, 38 insertions(+), 8 deletions(-)

t/t9800-git-p4-basic.sh +11 -2

index 53af8e34ac..4e95622670 100755
@@ -297,8 +297,17 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '
 # p4 changes, files, or describe; just in p4 print.  If P4CLIENT is unset, the
 # message will include "Librarian checkout".
 test_expect_success 'exit gracefully for p4 server errors' '
-	test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
-	mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden &&
+	case "$(echo "$db"/depot/file1*)" in
+	*,v)
+		test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
+		mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden;;
+	*,d)
+		path="$(echo "$db"/depot/file1,d/*.gz)" &&
+		test_when_finished "mv \"$path\",hidden \"$path\"" &&
+		mv "$path" "$path",hidden;;
+	*)
+		BUG "unhandled p4d layout";;
+	esac &&
 	test_when_finished cleanup_git &&
 	test_expect_code 1 git p4 clone --dest="$git" //depot@1 >out 2>err &&
 	test_grep "Error from p4 print" err

t/t9802-git-p4-filetype.sh +12 -3

index bb236cd2b5..557e11b16c 100755
@@ -301,9 +301,18 @@ test_expect_success SYMLINKS 'empty symlink target' '
 		#     @@
 		#
 		cd "$db/depot" &&
-		sed "/@target1/{; s/target1/@/; n; d; }" \
-		    empty-symlink,v >empty-symlink,v.tmp &&
-		mv empty-symlink,v.tmp empty-symlink,v
+		case "$(echo empty-symlink*)" in
+		empty-symlink,v)
+			sed "/@target1/{; s/target1/@/; n; d; }" \
+			    empty-symlink,v >empty-symlink,v.tmp &&
+			mv empty-symlink,v.tmp empty-symlink,v;;
+		empty-symlink,d)
+			path="empty-symlink,d/$(ls empty-symlink,d/ | tail -n1)" &&
+			rm "$path" &&
+			gzip </dev/null >"$path";;
+		*)
+			BUG "unhandled p4d layout";;
+		esac
 	) &&
 	(
 		# Make sure symlink really is empty.  Asking

t/t9825-git-p4-handle-utf16-without-bom.sh +15 -3

index f049ff8229..8e34f72198 100755
@@ -22,9 +22,21 @@ test_expect_success 'init depot with UTF-16 encoded file and artificially remove
 		cd db &&
 		p4d -jc &&
 		# P4D automatically adds a BOM. Remove it here to make the file invalid.
-		sed -e "\$d" depot/file1,v >depot/file1,v.new &&
-		mv depot/file1,v.new depot/file1,v &&
-		printf "@$UTF16@" >>depot/file1,v &&
+		case "$(echo depot/file1*)" in
+		depot/file1,v)
+			sed -e "\$d" depot/file1,v >depot/file1,v.new &&
+			mv depot/file1,v.new depot/file1,v &&
+			printf "@$UTF16@" >>depot/file1,v;;
+		depot/file1,d)
+			path="$(echo depot/file1,d/*.gz)" &&
+			gunzip -c "$path" >"$path.unzipped" &&
+			sed -e "\$d" "$path.unzipped" >"$path.new" &&
+			printf "$UTF16" >>"$path.new" &&
+			gzip -c "$path.new" >"$path" &&
+			rm "$path.unzipped" "$path.new";;
+		*)
+			BUG "unhandled p4d layout";;
+		esac &&
 		p4d -jrF checkpoint.1
 	)
 '

Signature · 1 line

2.46.0.rc1.dirty

Patrick Steinhardt

author

2024-07-23 14:05 UTC #4 lore raw

PATCH 2/3 ci: update Perforce version to r23.2

In reply to Patrick Steinhardt

Update our Perforce version from r21.2 to r23.2. Note that the updated version is not the newest version. Instead, it is the last version where the way that Perforce is being distributed remains the same as in r21.2. Newer releases stopped distributing p4 and p4d executablesas well as the macOS archives directly and would thus require more work.

Signed-off-by Patrick Steinhardt <[email protected]>

---
 ci/install-dependencies.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

ci/install-dependencies.sh +1 -1

index 6ec0f85972..b59fd7c1fd 100755
@@ -7,7 +7,7 @@
 
 begin_group "Install dependencies"
 
-P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
+P4WHENCE=https://cdist2.perforce.com/perforce/r23.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 JGITWHENCE=https://repo.eclipse.org/content/groups/releases//org/eclipse/jgit/org.eclipse.jgit.pgm/6.8.0.202311291450-r/org.eclipse.jgit.pgm-6.8.0.202311291450-r.sh

Signature · 1 line

2.46.0.rc1.dirty

Patrick Steinhardt

author

2024-07-23 14:05 UTC #11 lore raw

PATCH 3/3 t98xx: mark Perforce tests as memory-leak free

In reply to Patrick Steinhardt

All the Perforce tests are free of memory leaks. This went unnoticed because most folks do not have p4 and p4d installed on their computers. Consequently, given that the prerequisites for running those tests aren't fulfilled, TEST_PASSES_SANITIZE_LEAK=check won't notice that those tests are indeed memory leak free.

Mark those tests accordingly.

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t9800-git-p4-basic.sh                     | 1 +
 t/t9801-git-p4-branch.sh                    | 1 +
 t/t9802-git-p4-filetype.sh                  | 1 +
 t/t9803-git-p4-shell-metachars.sh           | 1 +
 t/t9804-git-p4-label.sh                     | 1 +
 t/t9805-git-p4-skip-submit-edit.sh          | 1 +
 t/t9806-git-p4-options.sh                   | 1 +
 t/t9808-git-p4-chdir.sh                     | 1 +
 t/t9809-git-p4-client-view.sh               | 1 +
 t/t9810-git-p4-rcs.sh                       | 1 +
 t/t9811-git-p4-label-import.sh              | 1 +
 t/t9812-git-p4-wildcards.sh                 | 1 +
 t/t9813-git-p4-preserve-users.sh            | 1 +
 t/t9814-git-p4-rename.sh                    | 1 +
 t/t9815-git-p4-submit-fail.sh               | 1 +
 t/t9816-git-p4-locked.sh                    | 1 +
 t/t9817-git-p4-exclude.sh                   | 1 +
 t/t9818-git-p4-block.sh                     | 1 +
 t/t9819-git-p4-case-folding.sh              | 1 +
 t/t9820-git-p4-editor-handling.sh           | 1 +
 t/t9821-git-p4-path-variations.sh           | 1 +
 t/t9822-git-p4-path-encoding.sh             | 1 +
 t/t9823-git-p4-mock-lfs.sh                  | 1 +
 t/t9825-git-p4-handle-utf16-without-bom.sh  | 1 +
 t/t9826-git-p4-keep-empty-commits.sh        | 1 +
 t/t9827-git-p4-change-filetype.sh           | 1 +
 t/t9828-git-p4-map-user.sh                  | 1 +
 t/t9829-git-p4-jobs.sh                      | 1 +
 t/t9830-git-p4-symlink-dir.sh               | 1 +
 t/t9831-git-p4-triggers.sh                  | 1 +
 t/t9832-unshelve.sh                         | 1 +
 t/t9833-errors.sh                           | 1 +
 t/t9834-git-p4-file-dir-bug.sh              | 1 +
 t/t9835-git-p4-metadata-encoding-python2.sh | 1 +
 t/t9836-git-p4-metadata-encoding-python3.sh | 1 +
 35 files changed, 35 insertions(+)

t/t9800-git-p4-basic.sh +1 -0

index 4e95622670..00b5bdd727 100755
@@ -5,6 +5,7 @@ test_description='git p4 tests'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9801-git-p4-branch.sh +1 -0

index c598011635..cdbfacc727 100755
@@ -5,6 +5,7 @@ test_description='git p4 tests for p4 branches'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9802-git-p4-filetype.sh +1 -0

index 557e11b16c..4f8cb4a768 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 filetype tests'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9803-git-p4-shell-metachars.sh +1 -0

index 2913277013..ab7fe16266 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 transparency to shell metachars in filenames'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9804-git-p4-label.sh +1 -0

index 3236457106..c8963fd398 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 label tests'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9805-git-p4-skip-submit-edit.sh +1 -0

index 90ef647db7..72dce3d2b4 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 skipSubmitEdit config variables'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9806-git-p4-options.sh +1 -0

index c26d297433..e4ce44ebf3 100755
@@ -5,6 +5,7 @@ test_description='git p4 options'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9808-git-p4-chdir.sh +1 -0

index 58a9b3b71e..342f7f3d4a 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 relative chdir'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9809-git-p4-client-view.sh +1 -0

index 9c9710d8c7..f33fdea889 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 client view'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9810-git-p4-rcs.sh +1 -0

index 5fe83315ec..15e32c9f35 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 rcs keywords'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 CP1252="\223\224"

t/t9811-git-p4-label-import.sh +1 -0

index 5ac5383fb7..52a4b0af81 100755
@@ -5,6 +5,7 @@ test_description='git p4 label tests'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9812-git-p4-wildcards.sh +1 -0

index 254a7c2446..46aa5fd56c 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 wildcards'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9813-git-p4-preserve-users.sh +1 -0

index fd018c87a8..0efea28da2 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 preserve users'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9814-git-p4-rename.sh +1 -0

index 2a9838f37f..00df6ebd3b 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 rename'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9815-git-p4-submit-fail.sh +1 -0

index c766fd159f..92ef9d8c24 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 submit failure handling'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9816-git-p4-locked.sh +1 -0

index 5e904ac80d..e687fbc25f 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 locked file behavior'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9817-git-p4-exclude.sh +1 -0

index ec3d937c6a..3deb334fed 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 tests for excluded paths during clone and sync'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9818-git-p4-block.sh +1 -0

index de591d875c..091bb72bdb 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 fetching changes in multiple blocks'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9819-git-p4-case-folding.sh +1 -0

index b4d93f0c17..985be20357 100755
@@ -2,6 +2,7 @@
 
 test_description='interaction with P4 case-folding'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 if test_have_prereq CASE_INSENSITIVE_FS

t/t9820-git-p4-editor-handling.sh +1 -0

index fa1bba1dd9..48e4dfb95c 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 handling of EDITOR'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9821-git-p4-path-variations.sh +1 -0

index ef80f1690b..49691c53da 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories with path case variations'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d with case folding enabled' '

t/t9822-git-p4-path-encoding.sh +1 -0

index 572d395498..e62ed49f51 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories with non ASCII paths'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 UTF8_ESCAPED="a-\303\244_o-\303\266_u-\303\274.txt"

t/t9823-git-p4-mock-lfs.sh +1 -0

index 88b76dc4d6..98a40d8af3 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories and store files in Mock LFS'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_file_is_not_in_mock_lfs () {

t/t9825-git-p4-handle-utf16-without-bom.sh +1 -0

index 8e34f72198..4d1d4fa5f3 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 handling of UTF-16 files without BOM'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 UTF16="\227\000\227\000"

t/t9826-git-p4-keep-empty-commits.sh +1 -0

index fd64afe064..54083f842e 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories and keep empty commits'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9827-git-p4-change-filetype.sh +1 -0

index d3670bd7a2..3476ea2fd3 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 support for file type change'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9828-git-p4-map-user.sh +1 -0

index ca6c2942bd..7c8f9e3930 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories and map users'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9829-git-p4-jobs.sh +1 -0

index 88cfb1fcd3..3fc0948d9c 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 retrieve job info'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9830-git-p4-symlink-dir.sh +1 -0

index 3fb6960c18..02561a7f0e 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 symlinked directories'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9831-git-p4-triggers.sh +1 -0

index ff6c0352e6..f287f41e37 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 with server triggers'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9832-unshelve.sh +1 -0

index 6b3cb0414a..a266775408 100755
@@ -6,6 +6,7 @@ last_shelved_change () {
 
 test_description='git p4 unshelve'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9833-errors.sh +1 -0

index e22369ccdf..da1d30c142 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 errors'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9834-git-p4-file-dir-bug.sh +1 -0

index dac67e89d7..565870fc74 100755
@@ -6,6 +6,7 @@ This test creates files and directories with the same name in perforce and
 checks that git-p4 recovers from the error at the same time as the perforce
 repository.'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9835-git-p4-metadata-encoding-python2.sh +1 -0

index 036bf79c66..ad20ffdede 100755
@@ -6,6 +6,7 @@ This test checks that the import process handles inconsistent text
 encoding in p4 metadata (author names, commit messages, etc) without
 failing, and produces maximally sane output in git.'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 python_target_version='2'

t/t9836-git-p4-metadata-encoding-python3.sh +1 -0

index 63350dc4b5..71ae763399 100755
@@ -6,6 +6,7 @@ This test checks that the import process handles inconsistent text
 encoding in p4 metadata (author names, commit messages, etc) without
 failing, and produces maximally sane output in git.'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 python_target_version='3'

Signature · 1 line

2.46.0.rc1.dirty

Johannes Schindelin

2024-07-24 08:39 UTC #5 lore raw

Re: [PATCH 2/3] ci: update Perforce version to r23.2

In reply to Patrick Steinhardt

Hi Patrick,

Quoted 5 lines · Patrick Steinhardt

Update our Perforce version from r21.2 to r23.2. Note that the updated version is not the newest version. Instead, it is the last version where the way that Perforce is being distributed remains the same as in r21.2. Newer releases stopped distributing p4 and p4d executablesas well as the macOS archives directly and would thus require more work.

An alternative would be to simply stop installing p4 in CI. I would actually be in favor of that, for multiple reasons:

- The pace of reviews and integration of git-p4 patches has slowed down over the couple of years. For example,
https://lore.kernel.org/git/20210510183638.156a6b1d@ado-tr/ has not seen any traction in over three years (most likely because we no longer have any active contributor with a vested interest in git-p4), and
https://github.com/gitgitgadget/git/pull/1028 and
https://github.com/gitgitgadget/git/pull/1070 have not even been submitted to the Git mailing list (most likely because of the hurdles to contribute).

- Over the years, it has been made harder and harder to install Perforce in CI. I spent a good deal of time trying to keep the Homebrew taps up to date (which was hard because Perforce kept replacing the archive behind that URL with newer versions, which always broke Homebrew's SHA check until it was adjusted accordingly).

- The git-p4 tests use quite a bit of time and electricity in all those CI builds. Therefore, it seems desirable to me to stop running these tests as part of the CI builds.

Ciao,
Johannes

Patrick Steinhardt

author

2024-07-24 09:01 UTC #6 lore raw

Re: [PATCH 2/3] ci: update Perforce version to r23.2

Quoted 29 lines · Johannes Schindelin

Hi Patrick,
Quoted 5 lines · Patrick Steinhardt
Update our Perforce version from r21.2 to r23.2. Note that the updated version is not the newest version. Instead, it is the last version where the way that Perforce is being distributed remains the same as in r21.2. Newer releases stopped distributing p4 and p4d executablesas well as the macOS archives directly and would thus require more work.
An alternative would be to simply stop installing p4 in CI. I would actually be in favor of that, for multiple reasons:
- The pace of reviews and integration of git-p4 patches has slowed down over the couple of years. For example,
https://lore.kernel.org/git/20210510183638.156a6b1d@ado-tr/ has not seen any traction in over three years (most likely because we no longer have any active contributor with a vested interest in git-p4), and
https://github.com/gitgitgadget/git/pull/1028 and
https://github.com/gitgitgadget/git/pull/1070 have not even been submitted to the Git mailing list (most likely because of the hurdles to contribute).
- Over the years, it has been made harder and harder to install Perforce in CI. I spent a good deal of time trying to keep the Homebrew taps up to date (which was hard because Perforce kept replacing the archive behind that URL with newer versions, which always broke Homebrew's SHA check until it was adjusted accordingly).

Quoted 3 lines

- The git-p4 tests use quite a bit of time and electricity in all those CI builds. Therefore, it seems desirable to me to stop running these tests as part of the CI builds.

I don't think that is a good idea. If we stop installing p4, the result is that _nobody_ will ever run the tests at all. The tests, and by extension git-p4 itself, would start to bitrot and we wouldn't notice any kind of regressions at all anymore.

If we want to consider going down that route, I'd rather say we should do it all or nothing: either we rip out git-p4 and the tests, or we leave both of them in. I couldn't care less about git-p4 itself, so I would not mind ripping it out altogether. But there may be users of this script out there that do care, so I don't want to make that decision unilaterally.

Patrick

Junio C Hamano

2024-07-24 16:10 UTC #7 lore raw

Re: [PATCH 2/3] ci: update Perforce version to r23.2

Quoted 11 lines · Patrick Steinhardt

I don't think that is a good idea. If we stop installing p4, the result is that _nobody_ will ever run the tests at all. The tests, and by extension git-p4 itself, would start to bitrot and we wouldn't notice any kind of regressions at all anymore.
If we want to consider going down that route, I'd rather say we should do it all or nothing: either we rip out git-p4 and the tests, or we leave both of them in. I couldn't care less about git-p4 itself, so I would not mind ripping it out altogether. But there may be users of this script out there that do care, so I don't want to make that decision unilaterally.

Yup, I was actually interpreting Dscho's message as advocating the removal of "git p4". Such a move would certainly force people who do care about it to come out. It is up to them to volunteer to help maintaining "git p4".

This may be a good example to discuss "support policy" not on niche platforms but on niche features (Emily Cc'ed).

Thanks.

Patrick Steinhardt

author

2024-07-30 06:00 UTC #8 lore raw

Re: [PATCH 2/3] ci: update Perforce version to r23.2

Quoted 14 lines · Junio C Hamano

Quoted 11 lines · Patrick Steinhardt
I don't think that is a good idea. If we stop installing p4, the result is that _nobody_ will ever run the tests at all. The tests, and by extension git-p4 itself, would start to bitrot and we wouldn't notice any kind of regressions at all anymore.
If we want to consider going down that route, I'd rather say we should do it all or nothing: either we rip out git-p4 and the tests, or we leave both of them in. I couldn't care less about git-p4 itself, so I would not mind ripping it out altogether. But there may be users of this script out there that do care, so I don't want to make that decision unilaterally.

Quoted 7 lines

Yup, I was actually interpreting Dscho's message as advocating the removal of "git p4". Such a move would certainly force people who do care about it to come out. It is up to them to volunteer to help maintaining "git p4".
This may be a good example to discuss "support policy" not on niche platforms but on niche features (Emily Cc'ed).

As said, I wouldn't mind dropping support for git p4 altogether. That is a much bigger discussion though, and I'm not sure whether we should just drop it at a "random" point in time without something like a grace period where people can chime in and express their wish to help out with the maintenance of it. In other words, we should probably deprecate it properly and announce our intent to deprecate it. Both our release notes and "Documentation/BreakingChanges.txt" could we viable ways to do that.

And while we haven't yet decided to rip out support for Perforce, I think that the proposed patch series is somewhat sensible. I honestly really only care about marking the patches as leak-free to help my bigger goal of making the whole test suite leak-free. The other patches that make the tests compatible with newer versions of Perforce aren't all that important, but at least they help to make those tests a bit more accessible to interested folks.

Patrick

Justin Tobler

2024-07-30 22:42 UTC #2 lore raw

Re: [PATCH 1/3] t98xx: fix Perforce tests with p4d r23 and newer

In reply to Patrick Steinhardt

Quoted 4 lines · Patrick Steinhardt

Some of the tests in t98xx modify the Perforce depot in ways that the tool wouldn't normally allow. This is done to test behaviour of git-p4 in certain edge cases that we have observed in the wild, but which should in theory not be possible.

If in theory these edge cases being tested should not be possible, that sounds like a bug and maybe in newer versions of p4 that is no longer relevant? Does it make sense to even support these rather intimate test cases going forward? Maybe we could instead skip these tests for newer versions?

Quoted 6 lines

Naturally, modifying the depot on disk directly is quite intimate with the tool and thus prone to breakage when Perforce updates the way that data is stored. And indeed, those tests are broken nowadays with r23 of Perforce. While a file revision was previously stored as plain file "depot/file,v", it is now stored in a directory "depot/file,d" with compression.

s/plain/a plain/

This sounds like a bit of a maintenance headache, especially if there are not many eyes on it to begin with. I guess this ties in with other discussion from this thread about whether and of git-p4 should remain in the codebase.

Quoted 20 lines

Adapt those tests to handle both old- and new-style depot layouts.

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t9800-git-p4-basic.sh                    | 13 +++++++++++--
 t/t9802-git-p4-filetype.sh                 | 15 ++++++++++++---
 t/t9825-git-p4-handle-utf16-without-bom.sh | 18 +++++++++++++++---
 3 files changed, 38 insertions(+), 8 deletions(-)

t/t9800-git-p4-basic.sh +1 -2

index 53af8e34ac..4e95622670 100755
@@ -297,8 +297,17 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '
 # p4 changes, files, or describe; just in p4 print.  If P4CLIENT is unset, the
 # message will include "Librarian checkout".
 test_expect_success 'exit gracefully for p4 server errors' '
-	test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
-	mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden &&
+	case "$(echo "$db"/depot/file1*)" in

t/t9800-git-p4-basic.sh test_expect_success 'exit when p4 fails to produce marshaled output' ' · line 310

Quoted 10 lines

t/t9800-git-p4-basic.sh +10 -0

+	*,v)
+		test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
+		mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden;;
+	*,d)
+		path="$(echo "$db"/depot/file1,d/*.gz)" &&
+		test_when_finished "mv \"$path\",hidden \"$path\"" &&
+		mv "$path" "$path",hidden;;
+	*)
+		BUG "unhandled p4d layout";;
+	esac &&

I'm not familiar with Perforce, but the test looks like it is simply appending ",hidden" to the file name. I assume this to trigger some error.

We are simply extending the test to also perform the same rename if, instead of depot/file1,f, a newer version uses depot/file1,d.

Makes sense to me, but without surrounding context its rather difficult to understand that the "case" statement here pertains to different Perforce versions that may be used. It might be nice to have a comment explaining this.

Quoted 16 lines

t/t9800-git-p4-basic.sh +0 -0

 	test_when_finished cleanup_git &&
 	test_expect_code 1 git p4 clone --dest="$git" //depot@1 >out 2>err &&
 	test_grep "Error from p4 print" err

t/t9802-git-p4-filetype.sh +2 -3

index bb236cd2b5..557e11b16c 100755
@@ -301,9 +301,18 @@ test_expect_success SYMLINKS 'empty symlink target' '
 		#     @@
 		#
 		cd "$db/depot" &&
-		sed "/@target1/{; s/target1/@/; n; d; }" \
-		    empty-symlink,v >empty-symlink,v.tmp &&
-		mv empty-symlink,v.tmp empty-symlink,v
+		case "$(echo empty-symlink*)" in
+		empty-symlink,v)

t/t9802-git-p4-filetype.sh test_expect_success SYMLINKS 'empty symlink target' ' · line 315

Quoted 10 lines

t/t9802-git-p4-filetype.sh +10 -0

+			sed "/@target1/{; s/target1/@/; n; d; }" \
+			    empty-symlink,v >empty-symlink,v.tmp &&
+			mv empty-symlink,v.tmp empty-symlink,v;;
+		empty-symlink,d)
+			path="empty-symlink,d/$(ls empty-symlink,d/ | tail -n1)" &&
+			rm "$path" &&
+			gzip </dev/null >"$path";;
+		*)
+			BUG "unhandled p4d layout";;
+		esac

Looks like for this test, for previous Perforce versions we were making a symlink point to nothing. For newer versions of Perforce, we seem to accomplish the same think by creating an empty compressed file.

Seems reasonable to me.

Quoted 19 lines

t/t9802-git-p4-filetype.sh +0 -0

 	) &&
 	(
 		# Make sure symlink really is empty.  Asking

t/t9825-git-p4-handle-utf16-without-bom.sh +5 -3

index f049ff8229..8e34f72198 100755
@@ -22,9 +22,21 @@ test_expect_success 'init depot with UTF-16 encoded file and artificially remove
 		cd db &&
 		p4d -jc &&
 		# P4D automatically adds a BOM. Remove it here to make the file invalid.
-		sed -e "\$d" depot/file1,v >depot/file1,v.new &&
-		mv depot/file1,v.new depot/file1,v &&
-		printf "@$UTF16@" >>depot/file1,v &&
+		case "$(echo depot/file1*)" in
+		depot/file1,v)
+			sed -e "\$d" depot/file1,v >depot/file1,v.new &&
+			mv depot/file1,v.new depot/file1,v &&
+			printf "@$UTF16@" >>depot/file1,v;;

t/t9825-git-p4-handle-utf16-without-bom.sh test_expect_success 'init depot with UTF-16 encoded file and artificially remove · line 39

Quoted 10 lines

t/t9825-git-p4-handle-utf16-without-bom.sh +10 -0

+		depot/file1,d)
+			path="$(echo depot/file1,d/*.gz)" &&
+			gunzip -c "$path" >"$path.unzipped" &&
+			sed -e "\$d" "$path.unzipped" >"$path.new" &&
+			printf "$UTF16" >>"$path.new" &&
+			gzip -c "$path.new" >"$path" &&
+			rm "$path.unzipped" "$path.new";;
+		*)
+			BUG "unhandled p4d layout";;
+		esac &&

Looks like the same deal here. For the new version we are modifying some file, but first have to uncompress it and then recompress it after it is modified. Again seems reasonable.

Thanks
-Justin

Quoted 6 lines

t/t9825-git-p4-handle-utf16-without-bom.sh +0 -0
 		p4d -jrF checkpoint.1
 	)
 '
Signature · 1 line
2.46.0.rc1.dirty

Justin Tobler

2024-07-30 22:49 UTC #9 lore raw

Re: [PATCH 2/3] ci: update Perforce version to r23.2

In reply to Patrick Steinhardt

Quoted 4 lines · Patrick Steinhardt

Update our Perforce version from r21.2 to r23.2. Note that the updated version is not the newest version. Instead, it is the last version where the way that Perforce is being distributed remains the same as in r21.2. Newer releases stopped distributing p4 and p4d executablesas well as the

s/executablesas/executables as/

Quoted 1 line

macOS archives directly and would thus require more work.

Out of curiousity, for Perforce is there a defined range of versions that the Git project supports? I guess I'm trying to figure if it even makes sense to support older version of Perforce in our tests.

Quoted 22 lines

Signed-off-by Patrick Steinhardt <[email protected]>

---
 ci/install-dependencies.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

ci/install-dependencies.sh +1 -1

index 6ec0f85972..b59fd7c1fd 100755
@@ -7,7 +7,7 @@
 
 begin_group "Install dependencies"
 
-P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
+P4WHENCE=https://cdist2.perforce.com/perforce/r23.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 JGITWHENCE=https://repo.eclipse.org/content/groups/releases//org/eclipse/jgit/org.eclipse.jgit.pgm/6.8.0.202311291450-r/org.eclipse.jgit.pgm-6.8.0.202311291450-r.sh

Signature · 1 line

2.46.0.rc1.dirty

Justin Tobler

2024-07-30 22:55 UTC #12 lore raw

Re: [PATCH 3/3] t98xx: mark Perforce tests as memory-leak free

In reply to Patrick Steinhardt

Quoted 7 lines · Patrick Steinhardt

All the Perforce tests are free of memory leaks. This went unnoticed because most folks do not have p4 and p4d installed on their computers. Consequently, given that the prerequisites for running those tests aren't fulfilled, TEST_PASSES_SANITIZE_LEAK=check won't notice that those tests are indeed memory leak free.
Mark those tests accordingly.

Nice to have more tests marked off the list! To me, it makes sense to go ahead and mark these tests as leak free even if we end up getting rid of them in the future. Thanks

Quoted 2 lines

Signed-off-by Patrick Steinhardt <[email protected]>

Patrick Steinhardt

author

2024-07-31 10:15 UTC #10 lore raw

Re: [PATCH 2/3] ci: update Perforce version to r23.2

In reply to Justin Tobler

Quoted 6 lines · Justin Tobler

Quoted 4 lines · Patrick Steinhardt
Update our Perforce version from r21.2 to r23.2. Note that the updated version is not the newest version. Instead, it is the last version where the way that Perforce is being distributed remains the same as in r21.2. Newer releases stopped distributing p4 and p4d executablesas well as the

Quoted 7 lines

s/executablesas/executables as/
Quoted 1 line
macOS archives directly and would thus require more work.
Out of curiousity, for Perforce is there a defined range of versions that the Git project supports? I guess I'm trying to figure if it even makes sense to support older version of Perforce in our tests.

Not that I'd know of. Which is why I'm being doubly cautious to deprecate support for older versions :) Not that many people would care, but still, I don't want to make such decisions without having any clue at all about the surrounding Perforce ecosystem.

Patrick

Patrick Steinhardt

author

2024-07-31 10:28 UTC #3 lore raw

Re: [PATCH 1/3] t98xx: fix Perforce tests with p4d r23 and newer

In reply to Justin Tobler

Quoted 11 lines · Justin Tobler

Quoted 4 lines · Patrick Steinhardt
Some of the tests in t98xx modify the Perforce depot in ways that the tool wouldn't normally allow. This is done to test behaviour of git-p4 in certain edge cases that we have observed in the wild, but which should in theory not be possible.
If in theory these edge cases being tested should not be possible, that sounds like a bug and maybe in newer versions of p4 that is no longer relevant? Does it make sense to even support these rather intimate test cases going forward? Maybe we could instead skip these tests for newer versions?

I wouldn't feel comfortable skipping this without knowing anything about the underlying bug that had been tested in the past. If we removed it, then we'd be second guessing whether or not that bug is still possible. Seemingly, it wasn't possible to trigger it without bugs in the past either, so it feels like we'd be assuming too much.

Quoted 7 lines

Quoted 6 lines
Naturally, modifying the depot on disk directly is quite intimate with the tool and thus prone to breakage when Perforce updates the way that data is stored. And indeed, those tests are broken nowadays with r23 of Perforce. While a file revision was previously stored as plain file "depot/file,v", it is now stored in a directory "depot/file,d" with compression.

Quoted 6 lines

s/plain/a plain/
This sounds like a bit of a maintenance headache, especially if there are not many eyes on it to begin with. I guess this ties in with other discussion from this thread about whether and of git-p4 should remain in the codebase.

Oh, it certainly isn't great. Whether it is a headache remains to be seen. If we had to pile onto hacks like this in the future then I'd also vote to remove or skip such tests. But we didn't really have to do a lot of maintenance for the Perforce tests in the past either, so I rather assume that it's going to be okayish in the future, too.

Part of the reason is likely that git-p4 simply isn't well-maintained, and the less maintained something is the less churn you have. But for now I only see two ways out of it:

- Either we get somebody willing to adopt it, and in that case it will be their responsibility to handle such potential headaches. In other words, I'll leave it to them to decide what to do with those tests, also because they are going to be better equipped with knowledge around Perforce itself.

- Or we remove it, and then it doesn't matter much anyway.

All to say that I'm merely being pragmatic and punt the issue to the future.

Quoted 35 lines

Quoted 20 lines

Adapt those tests to handle both old- and new-style depot layouts.

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t9800-git-p4-basic.sh                    | 13 +++++++++++--
 t/t9802-git-p4-filetype.sh                 | 15 ++++++++++++---
 t/t9825-git-p4-handle-utf16-without-bom.sh | 18 +++++++++++++++---
 3 files changed, 38 insertions(+), 8 deletions(-)

t/t9800-git-p4-basic.sh +1 -2

index 53af8e34ac..4e95622670 100755
@@ -297,8 +297,17 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '
 # p4 changes, files, or describe; just in p4 print.  If P4CLIENT is unset, the
 # message will include "Librarian checkout".
 test_expect_success 'exit gracefully for p4 server errors' '
-	test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
-	mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden &&
+	case "$(echo "$db"/depot/file1*)" in

t/t9800-git-p4-basic.sh test_expect_success 'exit when p4 fails to produce marshaled output' ' · line 310

Quoted 10 lines

t/t9800-git-p4-basic.sh +10 -0

+	*,v)
+		test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
+		mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden;;
+	*,d)
+		path="$(echo "$db"/depot/file1,d/*.gz)" &&
+		test_when_finished "mv \"$path\",hidden \"$path\"" &&
+		mv "$path" "$path",hidden;;
+	*)
+		BUG "unhandled p4d layout";;
+	esac &&

I'm not familiar with Perforce, but the test looks like it is simply appending ",hidden" to the file name. I assume this to trigger some error.

Quoted 7 lines

We are simply extending the test to also perform the same rename if, instead of depot/file1,f, a newer version uses depot/file1,d.
Makes sense to me, but without surrounding context its rather difficult to understand that the "case" statement here pertains to different Perforce versions that may be used. It might be nice to have a comment explaining this.

Fair, I will add a comment to all three sites.

Patric

Patrick Steinhardt

author

2024-07-31 10:37 UTC #13 lore raw

v2 cover Improvements for Perforce tests

In reply to Patrick Steinhardt

Hi,

this is the second version of my patch series that improves Perforce tests a bit. As with the first version, the main intent is to mark all tests as leak free. The other two patches polish the Perforce tests such that they work on my machine, which has a more recent version of Perforce than what we have been testing with in CI.

As discussed, despite these patches I consider Perforce to be basically unmaintained, and my patches do not change that. But given that I don't think we can rip out git-p4(1) just so without any kind of announcement up front, I still think that these patches are worth it to help my own cause, namely plugging all memory leaks hit by tests in Git.

Changes compared to v1:

- Fix some typos.

- Add in-code comments explaining what the compatibility case statements are about.

Thanks!

Patrick

Patrick Steinhardt (3):
t98xx: fix Perforce tests with p4d r23 and newer
ci: update Perforce version to r23.2
t98xx: mark Perforce tests as memory-leak free

 ci/install-dependencies.sh                  |  2 +-
 t/t9800-git-p4-basic.sh                     | 17 +++++++++++++--
 t/t9801-git-p4-branch.sh                    |  1 +
 t/t9802-git-p4-filetype.sh                  | 19 ++++++++++++++---
 t/t9803-git-p4-shell-metachars.sh           |  1 +
 t/t9804-git-p4-label.sh                     |  1 +
 t/t9805-git-p4-skip-submit-edit.sh          |  1 +
 t/t9806-git-p4-options.sh                   |  1 +
 t/t9808-git-p4-chdir.sh                     |  1 +
 t/t9809-git-p4-client-view.sh               |  1 +
 t/t9810-git-p4-rcs.sh                       |  1 +
 t/t9811-git-p4-label-import.sh              |  1 +
 t/t9812-git-p4-wildcards.sh                 |  1 +
 t/t9813-git-p4-preserve-users.sh            |  1 +
 t/t9814-git-p4-rename.sh                    |  1 +
 t/t9815-git-p4-submit-fail.sh               |  1 +
 t/t9816-git-p4-locked.sh                    |  1 +
 t/t9817-git-p4-exclude.sh                   |  1 +
 t/t9818-git-p4-block.sh                     |  1 +
 t/t9819-git-p4-case-folding.sh              |  1 +
 t/t9820-git-p4-editor-handling.sh           |  1 +
 t/t9821-git-p4-path-variations.sh           |  1 +
 t/t9822-git-p4-path-encoding.sh             |  1 +
 t/t9823-git-p4-mock-lfs.sh                  |  1 +
 t/t9825-git-p4-handle-utf16-without-bom.sh  | 23 ++++++++++++++++++---
 t/t9826-git-p4-keep-empty-commits.sh        |  1 +
 t/t9827-git-p4-change-filetype.sh           |  1 +
 t/t9828-git-p4-map-user.sh                  |  1 +
 t/t9829-git-p4-jobs.sh                      |  1 +
 t/t9830-git-p4-symlink-dir.sh               |  1 +
 t/t9831-git-p4-triggers.sh                  |  1 +
 t/t9832-unshelve.sh                         |  1 +
 t/t9833-errors.sh                           |  1 +
 t/t9834-git-p4-file-dir-bug.sh              |  1 +
 t/t9835-git-p4-metadata-encoding-python2.sh |  1 +
 t/t9836-git-p4-metadata-encoding-python3.sh |  1 +
 36 files changed, 84 insertions(+), 9 deletions(-)

Range-diff against v1:
1: f40f62f257 ! 1: 7f06de7e6b t98xx: fix Perforce tests with p4d r23 and newer
@@ Commit message
Naturally, modifying the depot on disk directly is quite intimate with the tool and thus prone to breakage when Perforce updates the way that data is stored. And indeed, those tests are broken nowadays with r23 of
- Perforce. While a file revision was previously stored as plain file
+ Perforce. While a file revision was previously stored as a plain file
"depot/file,v", it is now stored in a directory "depot/file,d" with compression.

@@ t/t9800-git-p4-basic.sh: test_expect_success 'exit when p4 fails to produce mars
test_expect_success 'exit gracefully for p4 server errors' '
- test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
- mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden &&
++ # Note that newer Perforce versions started to store files
++ # compressed in directories. The case statement handles both
++ # old and new layout.
+ case "$(echo "$db"/depot/file1*)" in
+ *,v)
+ test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
@@ t/t9800-git-p4-basic.sh: test_expect_success 'exit when p4 fails to produce mars

## t/t9802-git-p4-filetype.sh ##
@@ t/t9802-git-p4-filetype.sh: test_expect_success SYMLINKS 'empty symlink target' '
+ # text
# @@
#
++ # Note that newer Perforce versions started to store files
++ # compressed in directories. The case statement handles both
++ # old and new layout.
cd "$db/depot" &&
- sed "/@target1/{; s/target1/@/; n; d; }" \
- empty-symlink,v >empty-symlink,v.tmp &&
@@ t/t9825-git-p4-handle-utf16-without-bom.sh: test_expect_success 'init depot with
- sed -e "\$d" depot/file1,v >depot/file1,v.new &&
- mv depot/file1,v.new depot/file1,v &&
- printf "@$UTF16@" >>depot/file1,v &&
++ #
++ # Note that newer Perforce versions started to store files
++ # compressed in directories. The case statement handles both
++ # old and new layout.
+ case "$(echo depot/file1*)" in
+ depot/file1,v)
+ sed -e "\$d" depot/file1,v >depot/file1,v.new &&
2: ee5d836b77 ! 2: f41473d991 ci: update Perforce version to r23.2
@@ Commit message
Update our Perforce version from r21.2 to r23.2. Note that the updated version is not the newest version. Instead, it is the last version where the way that Perforce is being distributed remains the same as in r21.2.
- Newer releases stopped distributing p4 and p4d executablesas well as the
- macOS archives directly and would thus require more work.
+ Newer releases stopped distributing p4 and p4d executables as well as
+ the macOS archives directly and would thus require more work.

Signed-off-by Patrick Steinhardt <[email protected]>

3: d0a80ba403 = 3: bf03405db9 t98xx: mark Perforce tests as memory-leak free

base-commit: d19b6cd2dd72dc811f19df4b32c7ed223256c3ee

Signature · 1 line

2.46.0.dirty

Patrick Steinhardt

author

2024-07-31 10:37 UTC #14 lore raw

PATCH v2 1/3 t98xx: fix Perforce tests with p4d r23 and newer

Naturally, modifying the depot on disk directly is quite intimate with the tool and thus prone to breakage when Perforce updates the way that data is stored. And indeed, those tests are broken nowadays with r23 of Perforce. While a file revision was previously stored as a plain file "depot/file,v", it is now stored in a directory "depot/file,d" with compression.

Adapt those tests to handle both old- and new-style depot layouts.

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t9800-git-p4-basic.sh                    | 16 ++++++++++++++--
 t/t9802-git-p4-filetype.sh                 | 18 +++++++++++++++---
 t/t9825-git-p4-handle-utf16-without-bom.sh | 22 +++++++++++++++++++---
 3 files changed, 48 insertions(+), 8 deletions(-)

t/t9800-git-p4-basic.sh +14 -2

index 53af8e34ac..0816763e46 100755
@@ -297,8 +297,20 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '
 # p4 changes, files, or describe; just in p4 print.  If P4CLIENT is unset, the
 # message will include "Librarian checkout".
 test_expect_success 'exit gracefully for p4 server errors' '
-	test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
-	mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden &&
+	# Note that newer Perforce versions started to store files
+	# compressed in directories. The case statement handles both
+	# old and new layout.
+	case "$(echo "$db"/depot/file1*)" in
+	*,v)
+		test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
+		mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden;;
+	*,d)
+		path="$(echo "$db"/depot/file1,d/*.gz)" &&
+		test_when_finished "mv \"$path\",hidden \"$path\"" &&
+		mv "$path" "$path",hidden;;
+	*)
+		BUG "unhandled p4d layout";;
+	esac &&
 	test_when_finished cleanup_git &&
 	test_expect_code 1 git p4 clone --dest="$git" //depot@1 >out 2>err &&
 	test_grep "Error from p4 print" err

t/t9802-git-p4-filetype.sh +15 -3

index bb236cd2b5..df01a5d338 100755
@@ -300,10 +300,22 @@ test_expect_success SYMLINKS 'empty symlink target' '
 		#     text
 		#     @@
 		#
+		# Note that newer Perforce versions started to store files
+		# compressed in directories. The case statement handles both
+		# old and new layout.
 		cd "$db/depot" &&
-		sed "/@target1/{; s/target1/@/; n; d; }" \
-		    empty-symlink,v >empty-symlink,v.tmp &&
-		mv empty-symlink,v.tmp empty-symlink,v
+		case "$(echo empty-symlink*)" in
+		empty-symlink,v)
+			sed "/@target1/{; s/target1/@/; n; d; }" \
+			    empty-symlink,v >empty-symlink,v.tmp &&
+			mv empty-symlink,v.tmp empty-symlink,v;;
+		empty-symlink,d)
+			path="empty-symlink,d/$(ls empty-symlink,d/ | tail -n1)" &&
+			rm "$path" &&
+			gzip </dev/null >"$path";;
+		*)
+			BUG "unhandled p4d layout";;
+		esac
 	) &&
 	(
 		# Make sure symlink really is empty.  Asking

t/t9825-git-p4-handle-utf16-without-bom.sh +19 -3

index f049ff8229..6a60b32349 100755
@@ -22,9 +22,25 @@ test_expect_success 'init depot with UTF-16 encoded file and artificially remove
 		cd db &&
 		p4d -jc &&
 		# P4D automatically adds a BOM. Remove it here to make the file invalid.
-		sed -e "\$d" depot/file1,v >depot/file1,v.new &&
-		mv depot/file1,v.new depot/file1,v &&
-		printf "@$UTF16@" >>depot/file1,v &&
+		#
+		# Note that newer Perforce versions started to store files
+		# compressed in directories. The case statement handles both
+		# old and new layout.
+		case "$(echo depot/file1*)" in
+		depot/file1,v)
+			sed -e "\$d" depot/file1,v >depot/file1,v.new &&
+			mv depot/file1,v.new depot/file1,v &&
+			printf "@$UTF16@" >>depot/file1,v;;
+		depot/file1,d)
+			path="$(echo depot/file1,d/*.gz)" &&
+			gunzip -c "$path" >"$path.unzipped" &&
+			sed -e "\$d" "$path.unzipped" >"$path.new" &&
+			printf "$UTF16" >>"$path.new" &&
+			gzip -c "$path.new" >"$path" &&
+			rm "$path.unzipped" "$path.new";;
+		*)
+			BUG "unhandled p4d layout";;
+		esac &&
 		p4d -jrF checkpoint.1
 	)
 '

Signature · 1 line

2.46.0.dirty

Patrick Steinhardt

author

2024-07-31 10:37 UTC #15 lore raw

PATCH v2 2/3 ci: update Perforce version to r23.2

In reply to Patrick Steinhardt

Update our Perforce version from r21.2 to r23.2. Note that the updated version is not the newest version. Instead, it is the last version where the way that Perforce is being distributed remains the same as in r21.2. Newer releases stopped distributing p4 and p4d executables as well as the macOS archives directly and would thus require more work.

Signed-off-by Patrick Steinhardt <[email protected]>

---
 ci/install-dependencies.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

ci/install-dependencies.sh +1 -1

index 6ec0f85972..b59fd7c1fd 100755
@@ -7,7 +7,7 @@
 
 begin_group "Install dependencies"
 
-P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
+P4WHENCE=https://cdist2.perforce.com/perforce/r23.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 JGITWHENCE=https://repo.eclipse.org/content/groups/releases//org/eclipse/jgit/org.eclipse.jgit.pgm/6.8.0.202311291450-r/org.eclipse.jgit.pgm-6.8.0.202311291450-r.sh

Signature · 1 line

2.46.0.dirty

Patrick Steinhardt

author

2024-07-31 10:38 UTC #16 lore raw

PATCH v2 3/3 t98xx: mark Perforce tests as memory-leak free

In reply to Patrick Steinhardt

Mark those tests accordingly.

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t9800-git-p4-basic.sh                     | 1 +
 t/t9801-git-p4-branch.sh                    | 1 +
 t/t9802-git-p4-filetype.sh                  | 1 +
 t/t9803-git-p4-shell-metachars.sh           | 1 +
 t/t9804-git-p4-label.sh                     | 1 +
 t/t9805-git-p4-skip-submit-edit.sh          | 1 +
 t/t9806-git-p4-options.sh                   | 1 +
 t/t9808-git-p4-chdir.sh                     | 1 +
 t/t9809-git-p4-client-view.sh               | 1 +
 t/t9810-git-p4-rcs.sh                       | 1 +
 t/t9811-git-p4-label-import.sh              | 1 +
 t/t9812-git-p4-wildcards.sh                 | 1 +
 t/t9813-git-p4-preserve-users.sh            | 1 +
 t/t9814-git-p4-rename.sh                    | 1 +
 t/t9815-git-p4-submit-fail.sh               | 1 +
 t/t9816-git-p4-locked.sh                    | 1 +
 t/t9817-git-p4-exclude.sh                   | 1 +
 t/t9818-git-p4-block.sh                     | 1 +
 t/t9819-git-p4-case-folding.sh              | 1 +
 t/t9820-git-p4-editor-handling.sh           | 1 +
 t/t9821-git-p4-path-variations.sh           | 1 +
 t/t9822-git-p4-path-encoding.sh             | 1 +
 t/t9823-git-p4-mock-lfs.sh                  | 1 +
 t/t9825-git-p4-handle-utf16-without-bom.sh  | 1 +
 t/t9826-git-p4-keep-empty-commits.sh        | 1 +
 t/t9827-git-p4-change-filetype.sh           | 1 +
 t/t9828-git-p4-map-user.sh                  | 1 +
 t/t9829-git-p4-jobs.sh                      | 1 +
 t/t9830-git-p4-symlink-dir.sh               | 1 +
 t/t9831-git-p4-triggers.sh                  | 1 +
 t/t9832-unshelve.sh                         | 1 +
 t/t9833-errors.sh                           | 1 +
 t/t9834-git-p4-file-dir-bug.sh              | 1 +
 t/t9835-git-p4-metadata-encoding-python2.sh | 1 +
 t/t9836-git-p4-metadata-encoding-python3.sh | 1 +
 35 files changed, 35 insertions(+)

t/t9800-git-p4-basic.sh +1 -0

index 0816763e46..3e6dfce248 100755
@@ -5,6 +5,7 @@ test_description='git p4 tests'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9801-git-p4-branch.sh +1 -0

index c598011635..cdbfacc727 100755
@@ -5,6 +5,7 @@ test_description='git p4 tests for p4 branches'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9802-git-p4-filetype.sh +1 -0

index df01a5d338..1bc48305b0 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 filetype tests'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9803-git-p4-shell-metachars.sh +1 -0

index 2913277013..ab7fe16266 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 transparency to shell metachars in filenames'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9804-git-p4-label.sh +1 -0

index 3236457106..c8963fd398 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 label tests'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9805-git-p4-skip-submit-edit.sh +1 -0

index 90ef647db7..72dce3d2b4 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 skipSubmitEdit config variables'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9806-git-p4-options.sh +1 -0

index c26d297433..e4ce44ebf3 100755
@@ -5,6 +5,7 @@ test_description='git p4 options'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9808-git-p4-chdir.sh +1 -0

index 58a9b3b71e..342f7f3d4a 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 relative chdir'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9809-git-p4-client-view.sh +1 -0

index 9c9710d8c7..f33fdea889 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 client view'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9810-git-p4-rcs.sh +1 -0

index 5fe83315ec..15e32c9f35 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 rcs keywords'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 CP1252="\223\224"

t/t9811-git-p4-label-import.sh +1 -0

index 5ac5383fb7..52a4b0af81 100755
@@ -5,6 +5,7 @@ test_description='git p4 label tests'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9812-git-p4-wildcards.sh +1 -0

index 254a7c2446..46aa5fd56c 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 wildcards'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9813-git-p4-preserve-users.sh +1 -0

index fd018c87a8..0efea28da2 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 preserve users'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9814-git-p4-rename.sh +1 -0

index 2a9838f37f..00df6ebd3b 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 rename'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9815-git-p4-submit-fail.sh +1 -0

index c766fd159f..92ef9d8c24 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 submit failure handling'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9816-git-p4-locked.sh +1 -0

index 5e904ac80d..e687fbc25f 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 locked file behavior'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9817-git-p4-exclude.sh +1 -0

index ec3d937c6a..3deb334fed 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 tests for excluded paths during clone and sync'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9818-git-p4-block.sh +1 -0

index de591d875c..091bb72bdb 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 fetching changes in multiple blocks'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9819-git-p4-case-folding.sh +1 -0

index b4d93f0c17..985be20357 100755
@@ -2,6 +2,7 @@
 
 test_description='interaction with P4 case-folding'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 if test_have_prereq CASE_INSENSITIVE_FS

t/t9820-git-p4-editor-handling.sh +1 -0

index fa1bba1dd9..48e4dfb95c 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 handling of EDITOR'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9821-git-p4-path-variations.sh +1 -0

index ef80f1690b..49691c53da 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories with path case variations'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d with case folding enabled' '

t/t9822-git-p4-path-encoding.sh +1 -0

index 572d395498..e62ed49f51 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories with non ASCII paths'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 UTF8_ESCAPED="a-\303\244_o-\303\266_u-\303\274.txt"

t/t9823-git-p4-mock-lfs.sh +1 -0

index 88b76dc4d6..98a40d8af3 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories and store files in Mock LFS'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_file_is_not_in_mock_lfs () {

t/t9825-git-p4-handle-utf16-without-bom.sh +1 -0

index 6a60b32349..d0b86537dd 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 handling of UTF-16 files without BOM'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 UTF16="\227\000\227\000"

t/t9826-git-p4-keep-empty-commits.sh +1 -0

index fd64afe064..54083f842e 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories and keep empty commits'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9827-git-p4-change-filetype.sh +1 -0

index d3670bd7a2..3476ea2fd3 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 support for file type change'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9828-git-p4-map-user.sh +1 -0

index ca6c2942bd..7c8f9e3930 100755
@@ -2,6 +2,7 @@
 
 test_description='Clone repositories and map users'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9829-git-p4-jobs.sh +1 -0

index 88cfb1fcd3..3fc0948d9c 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 retrieve job info'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9830-git-p4-symlink-dir.sh +1 -0

index 3fb6960c18..02561a7f0e 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 symlinked directories'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9831-git-p4-triggers.sh +1 -0

index ff6c0352e6..f287f41e37 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 with server triggers'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9832-unshelve.sh +1 -0

index 6b3cb0414a..a266775408 100755
@@ -6,6 +6,7 @@ last_shelved_change () {
 
 test_description='git p4 unshelve'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9833-errors.sh +1 -0

index e22369ccdf..da1d30c142 100755
@@ -2,6 +2,7 @@
 
 test_description='git p4 errors'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9834-git-p4-file-dir-bug.sh +1 -0

index dac67e89d7..565870fc74 100755
@@ -6,6 +6,7 @@ This test creates files and directories with the same name in perforce and
 checks that git-p4 recovers from the error at the same time as the perforce
 repository.'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '

t/t9835-git-p4-metadata-encoding-python2.sh +1 -0

index 036bf79c66..ad20ffdede 100755
@@ -6,6 +6,7 @@ This test checks that the import process handles inconsistent text
 encoding in p4 metadata (author names, commit messages, etc) without
 failing, and produces maximally sane output in git.'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 python_target_version='2'

t/t9836-git-p4-metadata-encoding-python3.sh +1 -0

index 63350dc4b5..71ae763399 100755
@@ -6,6 +6,7 @@ This test checks that the import process handles inconsistent text
 encoding in p4 metadata (author names, commit messages, etc) without
 failing, and produces maximally sane output in git.'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-git-p4.sh
 
 python_target_version='3'

Signature · 1 line

2.46.0.dirty

Justin Tobler

2024-07-31 20:51 UTC #17 lore raw

In reply to Patrick Steinhardt

Quoted 8 lines · Patrick Steinhardt

Hi,
this is the second version of my patch series that improves Perforce tests a bit. As with the first version, the main intent is to mark all tests as leak free. The other two patches polish the Perforce tests such that they work on my machine, which has a more recent version of Perforce than what we have been testing with in CI.

Quoted 5 lines

As discussed, despite these patches I consider Perforce to be basically unmaintained, and my patches do not change that. But given that I don't think we can rip out git-p4(1) just so without any kind of announcement up front, I still think that these patches are worth it to help my own cause, namely plugging all memory leaks hit by tests in Git.

Agreed that proposing a removal of git-p4(1) is more of a separate issue here. For now, these patches make sense so we can get closer to making the tests memory leak free. :)

Quoted 11 lines

Changes compared to v1:
- Fix some typos.
- Add in-code comments explaining what the compatibility case statements are about.
Thanks!
Patrick

From looking at the range diff, this version looks good to me. Thanks

-Justin

Junio C Hamano

2024-07-31 22:32 UTC #18 lore raw

Quoted 1 line · Justin Tobler

From looking at the range diff, this version looks good to me. Thanks

Thanks, both. Perhaps I should mark this for 'next' by now.