* [PATCH 1/3] t98xx: fix Perforce tests with p4d r23 and newer
2024-07-23 14:05 [PATCH 0/3] Improvements for Perforce tests Patrick Steinhardt
@ 2024-07-23 14:05 ` Patrick Steinhardt
2024-07-30 22:41 ` Justin Tobler
2024-07-23 14:05 ` [PATCH 2/3] ci: update Perforce version to r23.2 Patrick Steinhardt
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-23 14:05 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 4144 bytes --]
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 <ps@pks.im>
---
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(-)
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 53af8e34ac..4e95622670 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -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
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index bb236cd2b5..557e11b16c 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -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
diff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh
index f049ff8229..8e34f72198 100755
--- a/t/t9825-git-p4-handle-utf16-without-bom.sh
+++ b/t/t9825-git-p4-handle-utf16-without-bom.sh
@@ -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
)
'
--
2.46.0.rc1.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 1/3] t98xx: fix Perforce tests with p4d r23 and newer
2024-07-23 14:05 ` [PATCH 1/3] t98xx: fix Perforce tests with p4d r23 and newer Patrick Steinhardt
@ 2024-07-30 22:41 ` Justin Tobler
2024-07-31 10:28 ` Patrick Steinhardt
0 siblings, 1 reply; 19+ messages in thread
From: Justin Tobler @ 2024-07-30 22:41 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
On 24/07/23 04:05PM, Patrick Steinhardt wrote:
> 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?
> 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.
> Adapt those tests to handle both old- and new-style depot layouts.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> 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(-)
>
> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
> index 53af8e34ac..4e95622670 100755
> --- a/t/t9800-git-p4-basic.sh
> +++ b/t/t9800-git-p4-basic.sh
> @@ -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 &&
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.
> 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
> diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
> index bb236cd2b5..557e11b16c 100755
> --- a/t/t9802-git-p4-filetype.sh
> +++ b/t/t9802-git-p4-filetype.sh
> @@ -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
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.
> ) &&
> (
> # Make sure symlink really is empty. Asking
> diff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh
> index f049ff8229..8e34f72198 100755
> --- a/t/t9825-git-p4-handle-utf16-without-bom.sh
> +++ b/t/t9825-git-p4-handle-utf16-without-bom.sh
> @@ -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 &&
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
> p4d -jrF checkpoint.1
> )
> '
> --
> 2.46.0.rc1.dirty
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/3] t98xx: fix Perforce tests with p4d r23 and newer
2024-07-30 22:41 ` Justin Tobler
@ 2024-07-31 10:28 ` Patrick Steinhardt
0 siblings, 0 replies; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-31 10:28 UTC (permalink / raw)
To: Justin Tobler; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 4762 bytes --]
On Tue, Jul 30, 2024 at 05:41:18PM -0500, Justin Tobler wrote:
> On 24/07/23 04:05PM, Patrick Steinhardt wrote:
> > 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.
> > 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.
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.
> > Adapt those tests to handle both old- and new-style depot layouts.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > 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(-)
> >
> > diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
> > index 53af8e34ac..4e95622670 100755
> > --- a/t/t9800-git-p4-basic.sh
> > +++ b/t/t9800-git-p4-basic.sh
> > @@ -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 &&
>
> 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.
Fair, I will add a comment to all three sites.
Patric
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/3] ci: update Perforce version to r23.2
2024-07-23 14:05 [PATCH 0/3] Improvements for Perforce tests Patrick Steinhardt
2024-07-23 14:05 ` [PATCH 1/3] t98xx: fix Perforce tests with p4d r23 and newer Patrick Steinhardt
@ 2024-07-23 14:05 ` Patrick Steinhardt
2024-07-24 8:39 ` Johannes Schindelin
2024-07-30 22:48 ` Justin Tobler
2024-07-23 14:05 ` [PATCH 3/3] t98xx: mark Perforce tests as memory-leak free Patrick Steinhardt
2024-07-31 10:37 ` [PATCH v2 0/3] Improvements for Perforce tests Patrick Steinhardt
3 siblings, 2 replies; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-23 14:05 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]
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 <ps@pks.im>
---
ci/install-dependencies.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 6ec0f85972..b59fd7c1fd 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -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
--
2.46.0.rc1.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ci: update Perforce version to r23.2
2024-07-23 14:05 ` [PATCH 2/3] ci: update Perforce version to r23.2 Patrick Steinhardt
@ 2024-07-24 8:39 ` Johannes Schindelin
2024-07-24 9:01 ` Patrick Steinhardt
2024-07-30 22:48 ` Justin Tobler
1 sibling, 1 reply; 19+ messages in thread
From: Johannes Schindelin @ 2024-07-24 8:39 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
Hi Patrick,
On Tue, 23 Jul 2024, Patrick Steinhardt wrote:
> 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ci: update Perforce version to r23.2
2024-07-24 8:39 ` Johannes Schindelin
@ 2024-07-24 9:01 ` Patrick Steinhardt
2024-07-24 16:10 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-24 9:01 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]
On Wed, Jul 24, 2024 at 10:39:54AM +0200, Johannes Schindelin wrote:
> Hi Patrick,
>
> On Tue, 23 Jul 2024, Patrick Steinhardt wrote:
>
> > 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.
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ci: update Perforce version to r23.2
2024-07-24 9:01 ` Patrick Steinhardt
@ 2024-07-24 16:10 ` Junio C Hamano
2024-07-30 6:00 ` Patrick Steinhardt
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2024-07-24 16:10 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Johannes Schindelin, git, Emily Shaffer
Patrick Steinhardt <ps@pks.im> writes:
> 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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ci: update Perforce version to r23.2
2024-07-24 16:10 ` Junio C Hamano
@ 2024-07-30 6:00 ` Patrick Steinhardt
0 siblings, 0 replies; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-30 6:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Emily Shaffer
[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]
On Wed, Jul 24, 2024 at 09:10:54AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > 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).
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ci: update Perforce version to r23.2
2024-07-23 14:05 ` [PATCH 2/3] ci: update Perforce version to r23.2 Patrick Steinhardt
2024-07-24 8:39 ` Johannes Schindelin
@ 2024-07-30 22:48 ` Justin Tobler
2024-07-31 10:15 ` Patrick Steinhardt
1 sibling, 1 reply; 19+ messages in thread
From: Justin Tobler @ 2024-07-30 22:48 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
On 24/07/23 04:05PM, Patrick Steinhardt wrote:
> 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/
> 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.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> ci/install-dependencies.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 6ec0f85972..b59fd7c1fd 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -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
>
> --
> 2.46.0.rc1.dirty
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ci: update Perforce version to r23.2
2024-07-30 22:48 ` Justin Tobler
@ 2024-07-31 10:15 ` Patrick Steinhardt
0 siblings, 0 replies; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-31 10:15 UTC (permalink / raw)
To: Justin Tobler; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]
On Tue, Jul 30, 2024 at 05:48:38PM -0500, Justin Tobler wrote:
> On 24/07/23 04:05PM, Patrick Steinhardt wrote:
> > 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/
>
> > 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/3] t98xx: mark Perforce tests as memory-leak free
2024-07-23 14:05 [PATCH 0/3] Improvements for Perforce tests Patrick Steinhardt
2024-07-23 14:05 ` [PATCH 1/3] t98xx: fix Perforce tests with p4d r23 and newer Patrick Steinhardt
2024-07-23 14:05 ` [PATCH 2/3] ci: update Perforce version to r23.2 Patrick Steinhardt
@ 2024-07-23 14:05 ` Patrick Steinhardt
2024-07-30 22:54 ` Justin Tobler
2024-07-31 10:37 ` [PATCH v2 0/3] Improvements for Perforce tests Patrick Steinhardt
3 siblings, 1 reply; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-23 14:05 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 15308 bytes --]
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 <ps@pks.im>
---
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(+)
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 4e95622670..00b5bdd727 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -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' '
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index c598011635..cdbfacc727 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -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' '
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 557e11b16c..4f8cb4a768 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -2,6 +2,7 @@
test_description='git p4 filetype tests'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh
index 2913277013..ab7fe16266 100755
--- a/t/t9803-git-p4-shell-metachars.sh
+++ b/t/t9803-git-p4-shell-metachars.sh
@@ -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' '
diff --git a/t/t9804-git-p4-label.sh b/t/t9804-git-p4-label.sh
index 3236457106..c8963fd398 100755
--- a/t/t9804-git-p4-label.sh
+++ b/t/t9804-git-p4-label.sh
@@ -2,6 +2,7 @@
test_description='git p4 label tests'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9805-git-p4-skip-submit-edit.sh b/t/t9805-git-p4-skip-submit-edit.sh
index 90ef647db7..72dce3d2b4 100755
--- a/t/t9805-git-p4-skip-submit-edit.sh
+++ b/t/t9805-git-p4-skip-submit-edit.sh
@@ -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' '
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index c26d297433..e4ce44ebf3 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -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' '
diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
index 58a9b3b71e..342f7f3d4a 100755
--- a/t/t9808-git-p4-chdir.sh
+++ b/t/t9808-git-p4-chdir.sh
@@ -2,6 +2,7 @@
test_description='git p4 relative chdir'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 9c9710d8c7..f33fdea889 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -2,6 +2,7 @@
test_description='git p4 client view'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
index 5fe83315ec..15e32c9f35 100755
--- a/t/t9810-git-p4-rcs.sh
+++ b/t/t9810-git-p4-rcs.sh
@@ -2,6 +2,7 @@
test_description='git p4 rcs keywords'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
CP1252="\223\224"
diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
index 5ac5383fb7..52a4b0af81 100755
--- a/t/t9811-git-p4-label-import.sh
+++ b/t/t9811-git-p4-label-import.sh
@@ -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' '
diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh
index 254a7c2446..46aa5fd56c 100755
--- a/t/t9812-git-p4-wildcards.sh
+++ b/t/t9812-git-p4-wildcards.sh
@@ -2,6 +2,7 @@
test_description='git p4 wildcards'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index fd018c87a8..0efea28da2 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -2,6 +2,7 @@
test_description='git p4 preserve users'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 2a9838f37f..00df6ebd3b 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -2,6 +2,7 @@
test_description='git p4 rename'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh
index c766fd159f..92ef9d8c24 100755
--- a/t/t9815-git-p4-submit-fail.sh
+++ b/t/t9815-git-p4-submit-fail.sh
@@ -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' '
diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index 5e904ac80d..e687fbc25f 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -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' '
diff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh
index ec3d937c6a..3deb334fed 100755
--- a/t/t9817-git-p4-exclude.sh
+++ b/t/t9817-git-p4-exclude.sh
@@ -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' '
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index de591d875c..091bb72bdb 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -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' '
diff --git a/t/t9819-git-p4-case-folding.sh b/t/t9819-git-p4-case-folding.sh
index b4d93f0c17..985be20357 100755
--- a/t/t9819-git-p4-case-folding.sh
+++ b/t/t9819-git-p4-case-folding.sh
@@ -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
diff --git a/t/t9820-git-p4-editor-handling.sh b/t/t9820-git-p4-editor-handling.sh
index fa1bba1dd9..48e4dfb95c 100755
--- a/t/t9820-git-p4-editor-handling.sh
+++ b/t/t9820-git-p4-editor-handling.sh
@@ -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' '
diff --git a/t/t9821-git-p4-path-variations.sh b/t/t9821-git-p4-path-variations.sh
index ef80f1690b..49691c53da 100755
--- a/t/t9821-git-p4-path-variations.sh
+++ b/t/t9821-git-p4-path-variations.sh
@@ -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' '
diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
index 572d395498..e62ed49f51 100755
--- a/t/t9822-git-p4-path-encoding.sh
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -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"
diff --git a/t/t9823-git-p4-mock-lfs.sh b/t/t9823-git-p4-mock-lfs.sh
index 88b76dc4d6..98a40d8af3 100755
--- a/t/t9823-git-p4-mock-lfs.sh
+++ b/t/t9823-git-p4-mock-lfs.sh
@@ -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 () {
diff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh
index 8e34f72198..4d1d4fa5f3 100755
--- a/t/t9825-git-p4-handle-utf16-without-bom.sh
+++ b/t/t9825-git-p4-handle-utf16-without-bom.sh
@@ -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"
diff --git a/t/t9826-git-p4-keep-empty-commits.sh b/t/t9826-git-p4-keep-empty-commits.sh
index fd64afe064..54083f842e 100755
--- a/t/t9826-git-p4-keep-empty-commits.sh
+++ b/t/t9826-git-p4-keep-empty-commits.sh
@@ -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' '
diff --git a/t/t9827-git-p4-change-filetype.sh b/t/t9827-git-p4-change-filetype.sh
index d3670bd7a2..3476ea2fd3 100755
--- a/t/t9827-git-p4-change-filetype.sh
+++ b/t/t9827-git-p4-change-filetype.sh
@@ -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' '
diff --git a/t/t9828-git-p4-map-user.sh b/t/t9828-git-p4-map-user.sh
index ca6c2942bd..7c8f9e3930 100755
--- a/t/t9828-git-p4-map-user.sh
+++ b/t/t9828-git-p4-map-user.sh
@@ -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' '
diff --git a/t/t9829-git-p4-jobs.sh b/t/t9829-git-p4-jobs.sh
index 88cfb1fcd3..3fc0948d9c 100755
--- a/t/t9829-git-p4-jobs.sh
+++ b/t/t9829-git-p4-jobs.sh
@@ -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' '
diff --git a/t/t9830-git-p4-symlink-dir.sh b/t/t9830-git-p4-symlink-dir.sh
index 3fb6960c18..02561a7f0e 100755
--- a/t/t9830-git-p4-symlink-dir.sh
+++ b/t/t9830-git-p4-symlink-dir.sh
@@ -2,6 +2,7 @@
test_description='git p4 symlinked directories'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9831-git-p4-triggers.sh b/t/t9831-git-p4-triggers.sh
index ff6c0352e6..f287f41e37 100755
--- a/t/t9831-git-p4-triggers.sh
+++ b/t/t9831-git-p4-triggers.sh
@@ -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' '
diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
index 6b3cb0414a..a266775408 100755
--- a/t/t9832-unshelve.sh
+++ b/t/t9832-unshelve.sh
@@ -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' '
diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh
index e22369ccdf..da1d30c142 100755
--- a/t/t9833-errors.sh
+++ b/t/t9833-errors.sh
@@ -2,6 +2,7 @@
test_description='git p4 errors'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9834-git-p4-file-dir-bug.sh b/t/t9834-git-p4-file-dir-bug.sh
index dac67e89d7..565870fc74 100755
--- a/t/t9834-git-p4-file-dir-bug.sh
+++ b/t/t9834-git-p4-file-dir-bug.sh
@@ -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' '
diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
index 036bf79c66..ad20ffdede 100755
--- a/t/t9835-git-p4-metadata-encoding-python2.sh
+++ b/t/t9835-git-p4-metadata-encoding-python2.sh
@@ -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'
diff --git a/t/t9836-git-p4-metadata-encoding-python3.sh b/t/t9836-git-p4-metadata-encoding-python3.sh
index 63350dc4b5..71ae763399 100755
--- a/t/t9836-git-p4-metadata-encoding-python3.sh
+++ b/t/t9836-git-p4-metadata-encoding-python3.sh
@@ -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'
--
2.46.0.rc1.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 3/3] t98xx: mark Perforce tests as memory-leak free
2024-07-23 14:05 ` [PATCH 3/3] t98xx: mark Perforce tests as memory-leak free Patrick Steinhardt
@ 2024-07-30 22:54 ` Justin Tobler
0 siblings, 0 replies; 19+ messages in thread
From: Justin Tobler @ 2024-07-30 22:54 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
On 24/07/23 04:05PM, Patrick Steinhardt wrote:
> 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
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 0/3] Improvements for Perforce tests
2024-07-23 14:05 [PATCH 0/3] Improvements for Perforce tests Patrick Steinhardt
` (2 preceding siblings ...)
2024-07-23 14:05 ` [PATCH 3/3] t98xx: mark Perforce tests as memory-leak free Patrick Steinhardt
@ 2024-07-31 10:37 ` Patrick Steinhardt
2024-07-31 10:37 ` [PATCH v2 1/3] t98xx: fix Perforce tests with p4d r23 and newer Patrick Steinhardt
` (3 more replies)
3 siblings, 4 replies; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-31 10:37 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Johannes Schindelin, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 6293 bytes --]
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 <ps@pks.im>
3: d0a80ba403 = 3: bf03405db9 t98xx: mark Perforce tests as memory-leak free
base-commit: d19b6cd2dd72dc811f19df4b32c7ed223256c3ee
--
2.46.0.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v2 1/3] t98xx: fix Perforce tests with p4d r23 and newer
2024-07-31 10:37 ` [PATCH v2 0/3] Improvements for Perforce tests Patrick Steinhardt
@ 2024-07-31 10:37 ` Patrick Steinhardt
2024-07-31 10:37 ` [PATCH v2 2/3] ci: update Perforce version to r23.2 Patrick Steinhardt
` (2 subsequent siblings)
3 siblings, 0 replies; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-31 10:37 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Johannes Schindelin, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 4633 bytes --]
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 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 <ps@pks.im>
---
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(-)
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 53af8e34ac..0816763e46 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -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
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index bb236cd2b5..df01a5d338 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -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
diff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh
index f049ff8229..6a60b32349 100755
--- a/t/t9825-git-p4-handle-utf16-without-bom.sh
+++ b/t/t9825-git-p4-handle-utf16-without-bom.sh
@@ -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
)
'
--
2.46.0.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 2/3] ci: update Perforce version to r23.2
2024-07-31 10:37 ` [PATCH v2 0/3] Improvements for Perforce tests Patrick Steinhardt
2024-07-31 10:37 ` [PATCH v2 1/3] t98xx: fix Perforce tests with p4d r23 and newer Patrick Steinhardt
@ 2024-07-31 10:37 ` Patrick Steinhardt
2024-07-31 10:37 ` [PATCH v2 3/3] t98xx: mark Perforce tests as memory-leak free Patrick Steinhardt
2024-07-31 20:50 ` [PATCH v2 0/3] Improvements for Perforce tests Justin Tobler
3 siblings, 0 replies; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-31 10:37 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Johannes Schindelin, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]
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 <ps@pks.im>
---
ci/install-dependencies.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 6ec0f85972..b59fd7c1fd 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -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
--
2.46.0.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 3/3] t98xx: mark Perforce tests as memory-leak free
2024-07-31 10:37 ` [PATCH v2 0/3] Improvements for Perforce tests Patrick Steinhardt
2024-07-31 10:37 ` [PATCH v2 1/3] t98xx: fix Perforce tests with p4d r23 and newer Patrick Steinhardt
2024-07-31 10:37 ` [PATCH v2 2/3] ci: update Perforce version to r23.2 Patrick Steinhardt
@ 2024-07-31 10:37 ` Patrick Steinhardt
2024-07-31 20:50 ` [PATCH v2 0/3] Improvements for Perforce tests Justin Tobler
3 siblings, 0 replies; 19+ messages in thread
From: Patrick Steinhardt @ 2024-07-31 10:37 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Johannes Schindelin, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 15304 bytes --]
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 <ps@pks.im>
---
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(+)
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 0816763e46..3e6dfce248 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -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' '
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index c598011635..cdbfacc727 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -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' '
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index df01a5d338..1bc48305b0 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -2,6 +2,7 @@
test_description='git p4 filetype tests'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh
index 2913277013..ab7fe16266 100755
--- a/t/t9803-git-p4-shell-metachars.sh
+++ b/t/t9803-git-p4-shell-metachars.sh
@@ -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' '
diff --git a/t/t9804-git-p4-label.sh b/t/t9804-git-p4-label.sh
index 3236457106..c8963fd398 100755
--- a/t/t9804-git-p4-label.sh
+++ b/t/t9804-git-p4-label.sh
@@ -2,6 +2,7 @@
test_description='git p4 label tests'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9805-git-p4-skip-submit-edit.sh b/t/t9805-git-p4-skip-submit-edit.sh
index 90ef647db7..72dce3d2b4 100755
--- a/t/t9805-git-p4-skip-submit-edit.sh
+++ b/t/t9805-git-p4-skip-submit-edit.sh
@@ -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' '
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index c26d297433..e4ce44ebf3 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -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' '
diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
index 58a9b3b71e..342f7f3d4a 100755
--- a/t/t9808-git-p4-chdir.sh
+++ b/t/t9808-git-p4-chdir.sh
@@ -2,6 +2,7 @@
test_description='git p4 relative chdir'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 9c9710d8c7..f33fdea889 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -2,6 +2,7 @@
test_description='git p4 client view'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
index 5fe83315ec..15e32c9f35 100755
--- a/t/t9810-git-p4-rcs.sh
+++ b/t/t9810-git-p4-rcs.sh
@@ -2,6 +2,7 @@
test_description='git p4 rcs keywords'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
CP1252="\223\224"
diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
index 5ac5383fb7..52a4b0af81 100755
--- a/t/t9811-git-p4-label-import.sh
+++ b/t/t9811-git-p4-label-import.sh
@@ -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' '
diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh
index 254a7c2446..46aa5fd56c 100755
--- a/t/t9812-git-p4-wildcards.sh
+++ b/t/t9812-git-p4-wildcards.sh
@@ -2,6 +2,7 @@
test_description='git p4 wildcards'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index fd018c87a8..0efea28da2 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -2,6 +2,7 @@
test_description='git p4 preserve users'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 2a9838f37f..00df6ebd3b 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -2,6 +2,7 @@
test_description='git p4 rename'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh
index c766fd159f..92ef9d8c24 100755
--- a/t/t9815-git-p4-submit-fail.sh
+++ b/t/t9815-git-p4-submit-fail.sh
@@ -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' '
diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index 5e904ac80d..e687fbc25f 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -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' '
diff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh
index ec3d937c6a..3deb334fed 100755
--- a/t/t9817-git-p4-exclude.sh
+++ b/t/t9817-git-p4-exclude.sh
@@ -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' '
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index de591d875c..091bb72bdb 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -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' '
diff --git a/t/t9819-git-p4-case-folding.sh b/t/t9819-git-p4-case-folding.sh
index b4d93f0c17..985be20357 100755
--- a/t/t9819-git-p4-case-folding.sh
+++ b/t/t9819-git-p4-case-folding.sh
@@ -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
diff --git a/t/t9820-git-p4-editor-handling.sh b/t/t9820-git-p4-editor-handling.sh
index fa1bba1dd9..48e4dfb95c 100755
--- a/t/t9820-git-p4-editor-handling.sh
+++ b/t/t9820-git-p4-editor-handling.sh
@@ -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' '
diff --git a/t/t9821-git-p4-path-variations.sh b/t/t9821-git-p4-path-variations.sh
index ef80f1690b..49691c53da 100755
--- a/t/t9821-git-p4-path-variations.sh
+++ b/t/t9821-git-p4-path-variations.sh
@@ -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' '
diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
index 572d395498..e62ed49f51 100755
--- a/t/t9822-git-p4-path-encoding.sh
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -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"
diff --git a/t/t9823-git-p4-mock-lfs.sh b/t/t9823-git-p4-mock-lfs.sh
index 88b76dc4d6..98a40d8af3 100755
--- a/t/t9823-git-p4-mock-lfs.sh
+++ b/t/t9823-git-p4-mock-lfs.sh
@@ -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 () {
diff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh
index 6a60b32349..d0b86537dd 100755
--- a/t/t9825-git-p4-handle-utf16-without-bom.sh
+++ b/t/t9825-git-p4-handle-utf16-without-bom.sh
@@ -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"
diff --git a/t/t9826-git-p4-keep-empty-commits.sh b/t/t9826-git-p4-keep-empty-commits.sh
index fd64afe064..54083f842e 100755
--- a/t/t9826-git-p4-keep-empty-commits.sh
+++ b/t/t9826-git-p4-keep-empty-commits.sh
@@ -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' '
diff --git a/t/t9827-git-p4-change-filetype.sh b/t/t9827-git-p4-change-filetype.sh
index d3670bd7a2..3476ea2fd3 100755
--- a/t/t9827-git-p4-change-filetype.sh
+++ b/t/t9827-git-p4-change-filetype.sh
@@ -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' '
diff --git a/t/t9828-git-p4-map-user.sh b/t/t9828-git-p4-map-user.sh
index ca6c2942bd..7c8f9e3930 100755
--- a/t/t9828-git-p4-map-user.sh
+++ b/t/t9828-git-p4-map-user.sh
@@ -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' '
diff --git a/t/t9829-git-p4-jobs.sh b/t/t9829-git-p4-jobs.sh
index 88cfb1fcd3..3fc0948d9c 100755
--- a/t/t9829-git-p4-jobs.sh
+++ b/t/t9829-git-p4-jobs.sh
@@ -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' '
diff --git a/t/t9830-git-p4-symlink-dir.sh b/t/t9830-git-p4-symlink-dir.sh
index 3fb6960c18..02561a7f0e 100755
--- a/t/t9830-git-p4-symlink-dir.sh
+++ b/t/t9830-git-p4-symlink-dir.sh
@@ -2,6 +2,7 @@
test_description='git p4 symlinked directories'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9831-git-p4-triggers.sh b/t/t9831-git-p4-triggers.sh
index ff6c0352e6..f287f41e37 100755
--- a/t/t9831-git-p4-triggers.sh
+++ b/t/t9831-git-p4-triggers.sh
@@ -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' '
diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
index 6b3cb0414a..a266775408 100755
--- a/t/t9832-unshelve.sh
+++ b/t/t9832-unshelve.sh
@@ -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' '
diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh
index e22369ccdf..da1d30c142 100755
--- a/t/t9833-errors.sh
+++ b/t/t9833-errors.sh
@@ -2,6 +2,7 @@
test_description='git p4 errors'
+TEST_PASSES_SANITIZE_LEAK=true
. ./lib-git-p4.sh
test_expect_success 'start p4d' '
diff --git a/t/t9834-git-p4-file-dir-bug.sh b/t/t9834-git-p4-file-dir-bug.sh
index dac67e89d7..565870fc74 100755
--- a/t/t9834-git-p4-file-dir-bug.sh
+++ b/t/t9834-git-p4-file-dir-bug.sh
@@ -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' '
diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
index 036bf79c66..ad20ffdede 100755
--- a/t/t9835-git-p4-metadata-encoding-python2.sh
+++ b/t/t9835-git-p4-metadata-encoding-python2.sh
@@ -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'
diff --git a/t/t9836-git-p4-metadata-encoding-python3.sh b/t/t9836-git-p4-metadata-encoding-python3.sh
index 63350dc4b5..71ae763399 100755
--- a/t/t9836-git-p4-metadata-encoding-python3.sh
+++ b/t/t9836-git-p4-metadata-encoding-python3.sh
@@ -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'
--
2.46.0.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 0/3] Improvements for Perforce tests
2024-07-31 10:37 ` [PATCH v2 0/3] Improvements for Perforce tests Patrick Steinhardt
` (2 preceding siblings ...)
2024-07-31 10:37 ` [PATCH v2 3/3] t98xx: mark Perforce tests as memory-leak free Patrick Steinhardt
@ 2024-07-31 20:50 ` Justin Tobler
2024-07-31 22:32 ` Junio C Hamano
3 siblings, 1 reply; 19+ messages in thread
From: Justin Tobler @ 2024-07-31 20:50 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Johannes Schindelin, Junio C Hamano
On 24/07/31 12:37PM, Patrick Steinhardt wrote:
> 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.
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. :)
>
> 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
^ permalink raw reply [flat|nested] 19+ messages in thread