[PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[Todd Zullinger]

Hi,

While reviewing my local build logs, I noticed a number of tests being
skipped unintentionally.  I had not done so with any of the 2.45.0
releases, unfortunately, which is when this began.

92 of the 202 tests in t1016-compatObjectFormat.sh are skipped due to
the GNUPGHOME directory missing, e.g.:

    ok 5 # SKIP create a sha1 signed commit (missing GPG2)
    ok 6 # SKIP create a sha1 signed tag (missing GPG2)
    ok 8 # SKIP create another sha1 signed tag (missing GPG2)
    ok 9 # SKIP merge the sha1 branches together (missing GPG2)

With these changes, they are all run (successfully). :)

I presume that they have been skipped in the Github CI runs as well,
but I don't know that the logs show enough detail to confirm that.

Thanks,
Todd


Todd Zullinger (2):
  t/lib-gpg: add prepare_gnupghome() to create GNUPGHOME dir
  t/lib-gpg: call prepare_gnupghome() in GPG2 prereq

 t/lib-gpg.sh | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
2.45.2

[PATCH 1/2] t/lib-gpg: add prepare_gnupghome() to create GNUPGHOME dir

[Todd Zullinger]

We create the $GNUPGHOME directory in both the GPG and GPGSSH prereqs.
Replace the redundancy with a function.

Use `mkdir -p` to ensure we do not fail if a test includes more than one
of these prereqs.

Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
 t/lib-gpg.sh | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index add11e88fc..4e44f182bb 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -9,6 +9,11 @@
 GNUPGHOME="$PWD/gpghome"
 export GNUPGHOME
 
+prepare_gnupghome () {
+	mkdir -p "$GNUPGHOME" &&
+	chmod 0700 "$GNUPGHOME"
+}
+
 test_lazy_prereq GPG '
 	gpg_version=$(gpg --version 2>&1)
 	test $? != 127 || exit 1
@@ -38,8 +43,7 @@ test_lazy_prereq GPG '
 		# To export ownertrust:
 		#	gpg --homedir /tmp/gpghome --export-ownertrust \
 		#		> lib-gpg/ownertrust
-		mkdir "$GNUPGHOME" &&
-		chmod 0700 "$GNUPGHOME" &&
+		prepare_gnupghome &&
 		(gpgconf --kill all || : ) &&
 		gpg --homedir "${GNUPGHOME}" --import \
 			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
@@ -132,8 +136,7 @@ test_lazy_prereq GPGSSH '
 	test $? = 0 || exit 1;
 
 	# Setup some keys and an allowed signers file
-	mkdir -p "${GNUPGHOME}" &&
-	chmod 0700 "${GNUPGHOME}" &&
+	prepare_gnupghome &&
 	(setfacl -k "${GNUPGHOME}" 2>/dev/null || true) &&
 	ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_PRIMARY}" >/dev/null &&
 	ssh-keygen -t rsa -b 2048 -N "" -C "git rsa2048 key" -f "${GPGSSH_KEY_SECONDARY}" >/dev/null &&
-- 
2.45.2

[PATCH 2/2] t/lib-gpg: call prepare_gnupghome() in GPG2 prereq

[Todd Zullinger]

The GPG2 prereq added in 2f36339fa8 (t/lib-gpg: introduce new prereq
GPG2, 2023-06-04) does not create the $GNUPGHOME directory.

Tests which use the GPG2 prereq without previously using the GPG prereq
fail because of the missing directory.  This currently affects
t1016-compatObjectFormat.

Ensure $GNUPGHOME is created in the GPG2 prereq.

Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
 t/lib-gpg.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 4e44f182bb..21666b2ab0 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -66,6 +66,7 @@ test_lazy_prereq GPG2 '
 		exit 1
 		;;
 	*)
+		prepare_gnupghome &&
 		(gpgconf --kill all || : ) &&
 		gpg --homedir "${GNUPGHOME}" --import \
 			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
-- 
2.45.2

Re: [PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[Todd Zullinger]

I wrote:
> 92 of the 202 tests in t1016-compatObjectFormat.sh are skipped due to
> the GNUPGHOME directory missing, e.g.:
> 
>     ok 5 # SKIP create a sha1 signed commit (missing GPG2)
>     ok 6 # SKIP create a sha1 signed tag (missing GPG2)
>     ok 8 # SKIP create another sha1 signed tag (missing GPG2)
>     ok 9 # SKIP merge the sha1 branches together (missing GPG2)
> 
> With these changes, they are all run (successfully). :)
> 
> I presume that they have been skipped in the Github CI runs as well,
> but I don't know that the logs show enough detail to confirm that.

D'oh!  I spoke too soon.  I'd run the test suite on several
different rpm-based hosts (Fedora 39 and Rocky 9).  Waiting
for the Github actions to run is what I should have done.

A number of these fail, e.g.:

https://github.com/tmzullinger/git/actions/runs/9780387020/job/27001952643#step:4:1871

    Error: failed: t1016.173 Verify commit signedcommit4's sha1 oid
    failure: t1016.173 Verify commit signedcommit4's sha1 oid 
	    git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
	    test_cmp ${name}_sha1 ${name}_sha1_expected
      
      + git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 5d70155cc40e4c16515c89ad0b11d8c691436fc4a4d3ca246669a4c21f07e454
      + test_cmp signedcommit4_sha1 signedcommit4_sha1_expected
      + test 2 -ne 2
      + eval diff -u "$@"
      + diff -u signedcommit4_sha1 signedcommit4_sha1_expected
      --- signedcommit4_sha1	2024-07-03 15:11:05.597537579 +0000
      +++ signedcommit4_sha1_expected	2024-07-03 15:11:05.553537766 +0000
      @@ -1 +1 @@
      -9179ccc5b15588bc3a45c5cc75bdec380f8ccb86
      +c6c46f92bc2cfda57ad6bf7981fa654825376b24
      error: last command exited with $?=1
      not ok 173 - Verify commit signedcommit4's sha1 oid
      #	
      #		git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
      #		test_cmp ${name}_sha1 ${name}_sha1_expected
      #	

This seems like it's just exposing a pre-existing failure,
as I can't imagine how creating GNUPGHOME would cause the
actual and expected SHA's to differ. :)

Perhaps the intended gpg wrapper script which sets
`--faked-system-time` isn't being used?

I'm not sure why that would differ in the Github actions
from my local builds, but I don't know what else differs in
the Ubuntu images and/or environment used by the actions.

-- 
Todd

Re: [PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[Todd Zullinger]

Hi,

I'm following up to an old thread because this test breakage
remains.

I've intended to dig into it further over the past few
months but have not managed to spend enough time to work out
the root of the problem.

I hope that someone more familiar with these tests (or
perhaps someone with fresh eyes) will spot the problem.

I wrote:
> I wrote:
>> 92 of the 202 tests in t1016-compatObjectFormat.sh are skipped due to
>> the GNUPGHOME directory missing, e.g.:
>> 
>>     ok 5 # SKIP create a sha1 signed commit (missing GPG2)
>>     ok 6 # SKIP create a sha1 signed tag (missing GPG2)
>>     ok 8 # SKIP create another sha1 signed tag (missing GPG2)
>>     ok 9 # SKIP merge the sha1 branches together (missing GPG2)
>> 
>> With these changes, they are all run (successfully). :)
>> 
>> I presume that they have been skipped in the Github CI runs as well,
>> but I don't know that the logs show enough detail to confirm that.
> 
> D'oh!  I spoke too soon.  I'd run the test suite on several
> different rpm-based hosts (Fedora 39 and Rocky 9).  Waiting
> for the Github actions to run is what I should have done.
> 
> A number of these fail, e.g.:
> 
> https://github.com/tmzullinger/git/actions/runs/9780387020/job/27001952643#step:4:1871
> 
>     Error: failed: t1016.173 Verify commit signedcommit4's sha1 oid
>     failure: t1016.173 Verify commit signedcommit4's sha1 oid 
> 	    git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
> 	    test_cmp ${name}_sha1 ${name}_sha1_expected
>       
>       + git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 5d70155cc40e4c16515c89ad0b11d8c691436fc4a4d3ca246669a4c21f07e454
>       + test_cmp signedcommit4_sha1 signedcommit4_sha1_expected
>       + test 2 -ne 2
>       + eval diff -u "$@"
>       + diff -u signedcommit4_sha1 signedcommit4_sha1_expected
>       --- signedcommit4_sha1	2024-07-03 15:11:05.597537579 +0000
>       +++ signedcommit4_sha1_expected	2024-07-03 15:11:05.553537766 +0000
>       @@ -1 +1 @@
>       -9179ccc5b15588bc3a45c5cc75bdec380f8ccb86
>       +c6c46f92bc2cfda57ad6bf7981fa654825376b24
>       error: last command exited with $?=1
>       not ok 173 - Verify commit signedcommit4's sha1 oid
>       #	
>       #		git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
>       #		test_cmp ${name}_sha1 ${name}_sha1_expected
>       #	
> 
> This seems like it's just exposing a pre-existing failure,
> as I can't imagine how creating GNUPGHOME would cause the
> actual and expected SHA's to differ. :)
> 
> Perhaps the intended gpg wrapper script which sets
> `--faked-system-time` isn't being used?
> 
> I'm not sure why that would differ in the Github actions
> from my local builds, but I don't know what else differs in
> the Ubuntu images and/or environment used by the actions.

I have run a good number of builds with the patches applied
and t1016-compatObjectFormat regularly fails for all of the
tests which use the GPG2 prereq.  A recent Github CI run is
here:

    https://github.com/tmzullinger/git/actions/runs/13570544425

I think this test flakiness should be fixed so that we can
apply the patch to fix the GPG2 prereq.  As it is, we're
skipping _all_ of the tests which require GPG2.

Cheers,

-- 
Todd

Re: [PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[Junio C Hamano]

Todd Zullinger <tmz@pobox.com> writes:

> I've intended to dig into it further over the past few
> months but have not managed to spend enough time to work out
> the root of the problem.
>
> I hope that someone more familiar with these tests (or
> perhaps someone with fresh eyes) will spot the problem.
>> 
>> A number of these fail, e.g.:
>> 
>> https://github.com/tmzullinger/git/actions/runs/9780387020/job/27001952643#step:4:1871
>> 
>>     Error: failed: t1016.173 Verify commit signedcommit4's sha1 oid
>>     failure: t1016.173 Verify commit signedcommit4's sha1 oid 
>> 	    git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
>> 	    test_cmp ${name}_sha1 ${name}_sha1_expected
>>       
>>       + git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 5d70155cc40e4c16515c89ad0b11d8c691436fc4a4d3ca246669a4c21f07e454
>>       + test_cmp signedcommit4_sha1 signedcommit4_sha1_expected
>>       + test 2 -ne 2
>>       + eval diff -u "$@"
>>       + diff -u signedcommit4_sha1 signedcommit4_sha1_expected
>>       --- signedcommit4_sha1	2024-07-03 15:11:05.597537579 +0000
>>       +++ signedcommit4_sha1_expected	2024-07-03 15:11:05.553537766 +0000
>>       @@ -1 +1 @@
>>       -9179ccc5b15588bc3a45c5cc75bdec380f8ccb86
>>       +c6c46f92bc2cfda57ad6bf7981fa654825376b24
>>       error: last command exited with $?=1
>>       not ok 173 - Verify commit signedcommit4's sha1 oid
>>       #	
>>       #		git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
>>       #		test_cmp ${name}_sha1 ${name}_sha1_expected
>>       #	
>> 
>> This seems like it's just exposing a pre-existing failure,
>> as I can't imagine how creating GNUPGHOME would cause the
>> actual and expected SHA's to differ. :)
>> 
>> Perhaps the intended gpg wrapper script which sets
>> `--faked-system-time` isn't being used?
>> 
>> I'm not sure why that would differ in the Github actions
>> from my local builds, but I don't know what else differs in
>> the Ubuntu images and/or environment used by the actions.
>
> I have run a good number of builds with the patches applied
> and t1016-compatObjectFormat regularly fails for all of the
> tests which use the GPG2 prereq.  A recent Github CI run is
> here:
>
>     https://github.com/tmzullinger/git/actions/runs/13570544425
>
> I think this test flakiness should be fixed so that we can
> apply the patch to fix the GPG2 prereq.  As it is, we're
> skipping _all_ of the tests which require GPG2.

Any progress or responses?  All of these tests, that nobody seemed
to have caught breakage of because they weren't being run anyway,
seem to be flakey with the new GNUPGHOME set-up.

I am tempted to do this in the meantime, but I'd really prefer not
to have to do so, assuming that these tests, when fixed, would be
materially contributing to the health of our codebase.

Thanks.

 t/t1016-compatObjectFormat.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh
index be3206a16f..0968962f1d 100755
--- a/t/t1016-compatObjectFormat.sh
+++ b/t/t1016-compatObjectFormat.sh
@@ -261,21 +261,21 @@ compare_oids () {
 compare_oids 'blob' hello "$hello_sha1_oid" "$hello_sha256_oid"
 compare_oids 'tree' tree "$tree_sha1_oid" "$tree_sha256_oid"
 compare_oids 'commit' commit "$commit_sha1_oid" "$commit_sha256_oid"
-compare_oids GPG2 'commit' signedcommit "$signedcommit_sha1_oid" "$signedcommit_sha256_oid"
+compare_oids GPG2,FLAKEY 'commit' signedcommit "$signedcommit_sha1_oid" "$signedcommit_sha256_oid"
 compare_oids 'tag' hellotag "$hellotag_sha1_oid" "$hellotag_sha256_oid"
 compare_oids 'tag' treetag "$treetag_sha1_oid" "$treetag_sha256_oid"
 compare_oids 'tag' committag "$committag_sha1_oid" "$committag_sha256_oid"
-compare_oids GPG2 'tag' signedtag "$signedtag_sha1_oid" "$signedtag_sha256_oid"
+compare_oids GPG2,FLAKEY 'tag' signedtag "$signedtag_sha1_oid" "$signedtag_sha256_oid"
 
 compare_oids 'blob' more "$more_sha1_oid" "$more_sha256_oid"
 compare_oids 'blob' another "$another_sha1_oid" "$another_sha256_oid"
 compare_oids 'tree' tree2 "$tree2_sha1_oid" "$tree2_sha256_oid"
 compare_oids 'commit' commit2 "$commit2_sha1_oid" "$commit2_sha256_oid"
-compare_oids GPG2 'tag' signedtag2 "$signedtag2_sha1_oid" "$signedtag2_sha256_oid"
-compare_oids GPG2 'commit' signedcommit2 "$signedcommit2_sha1_oid" "$signedcommit2_sha256_oid"
-compare_oids GPG2 'commit' signedcommit3 "$signedcommit3_sha1_oid" "$signedcommit3_sha256_oid"
-compare_oids GPG2 'commit' signedcommit4 "$signedcommit4_sha1_oid" "$signedcommit4_sha256_oid"
-compare_oids GPG2 'tag' signedtag3 "$signedtag3_sha1_oid" "$signedtag3_sha256_oid"
-compare_oids GPG2 'tag' signedtag4 "$signedtag4_sha1_oid" "$signedtag4_sha256_oid"
+compare_oids GPG2,FLAKEY 'tag' signedtag2 "$signedtag2_sha1_oid" "$signedtag2_sha256_oid"
+compare_oids GPG2,FLAKEY 'commit' signedcommit2 "$signedcommit2_sha1_oid" "$signedcommit2_sha256_oid"
+compare_oids GPG2,FLAKEY 'commit' signedcommit3 "$signedcommit3_sha1_oid" "$signedcommit3_sha256_oid"
+compare_oids GPG2,FLAKEY 'commit' signedcommit4 "$signedcommit4_sha1_oid" "$signedcommit4_sha256_oid"
+compare_oids GPG2,FLAKEY 'tag' signedtag3 "$signedtag3_sha1_oid" "$signedtag3_sha256_oid"
+compare_oids GPG2,FLAKEY 'tag' signedtag4 "$signedtag4_sha1_oid" "$signedtag4_sha256_oid"
 
 test_done
-- 
2.51.1-691-gd530f589c3

Re: [PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[Eric W. Biederman]

Junio C Hamano <gitster@pobox.com> writes:

> Todd Zullinger <tmz@pobox.com> writes:
>
>> I've intended to dig into it further over the past few
>> months but have not managed to spend enough time to work out
>> the root of the problem.
>>
>> I hope that someone more familiar with these tests (or
>> perhaps someone with fresh eyes) will spot the problem.
>>> 
>>> A number of these fail, e.g.:
>>> 
>>> https://github.com/tmzullinger/git/actions/runs/9780387020/job/27001952643#step:4:1871
>>> 
>>>     Error: failed: t1016.173 Verify commit signedcommit4's sha1 oid
>>>     failure: t1016.173 Verify commit signedcommit4's sha1 oid 
>>> 	    git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
>>> 	    test_cmp ${name}_sha1 ${name}_sha1_expected
>>>       
>>>       + git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 5d70155cc40e4c16515c89ad0b11d8c691436fc4a4d3ca246669a4c21f07e454
>>>       + test_cmp signedcommit4_sha1 signedcommit4_sha1_expected
>>>       + test 2 -ne 2
>>>       + eval diff -u "$@"
>>>       + diff -u signedcommit4_sha1 signedcommit4_sha1_expected
>>>       --- signedcommit4_sha1	2024-07-03 15:11:05.597537579 +0000
>>>       +++ signedcommit4_sha1_expected	2024-07-03 15:11:05.553537766 +0000
>>>       @@ -1 +1 @@
>>>       -9179ccc5b15588bc3a45c5cc75bdec380f8ccb86
>>>       +c6c46f92bc2cfda57ad6bf7981fa654825376b24
>>>       error: last command exited with $?=1
>>>       not ok 173 - Verify commit signedcommit4's sha1 oid
>>>       #	
>>>       #		git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
>>>       #		test_cmp ${name}_sha1 ${name}_sha1_expected
>>>       #	
>>> 
>>> This seems like it's just exposing a pre-existing failure,
>>> as I can't imagine how creating GNUPGHOME would cause the
>>> actual and expected SHA's to differ. :)

Hmm. Let me see.

The test goes through and creates 2 repositories, one sha1 and the other
sha256.

In those repositories a gpg signed commit is created.
That gpg signed commit should contain both the signed sha1
and the signed sha256 gpg signatures.  As both object-format
and compat-object-format are set in the config.

Then the commit is extracted and one of the signatures removed.

Then the oid of the signed commit with only a single signed gpg
signature is computed.

In the sha256 repository the sha256 oid is given to get the sha1 oid.

That sha1 oid is of the signed gpg commit is compared with the
previously computed sha1 oid.

So I think a messed up GNUPGHOME could result in the wrong signing
key being used.  Though how that signing key could be inconsistent
from one part of the test to the other I don't know.

Not seeing t/t1016/gpg as the gpg program does look like a more
likely cause of the error there.

>>> 
>>> Perhaps the intended gpg wrapper script which sets
>>> `--faked-system-time` isn't being used?
>>> 
>>> I'm not sure why that would differ in the Github actions
>>> from my local builds, but I don't know what else differs in
>>> the Ubuntu images and/or environment used by the actions.
>>
>> I have run a good number of builds with the patches applied
>> and t1016-compatObjectFormat regularly fails for all of the
>> tests which use the GPG2 prereq.  A recent Github CI run is
>> here:
>>
>>     https://github.com/tmzullinger/git/actions/runs/13570544425
>>
>> I think this test flakiness should be fixed so that we can
>> apply the patch to fix the GPG2 prereq.  As it is, we're
>> skipping _all_ of the tests which require GPG2.
>
> Any progress or responses?  All of these tests, that nobody seemed
> to have caught breakage of because they weren't being run anyway,
> seem to be flakey with the new GNUPGHOME set-up.
>
> I am tempted to do this in the meantime, but I'd really prefer not
> to have to do so, assuming that these tests, when fixed, would be
> materially contributing to the health of our codebase.

I just dug into this a little and hopefully I have paged enough
state back to understand this.

In my testing a missing GNUPGHOME appears enough to prevent the
prerequisite from succeeding. So let's fix that. Todd Zullinger's sent
some nice patches to do that (up-thread), or you can take use my minimal
version.

The only possible source of flakiness in the tests I can see is the
possibility of t/t1016/gpg not getting called (which uses a fixed
timestamp).  It appears you just fixed that problem in commit
516bf45749bb ("t1016: make sure to use specified GPG").

With that commit reverted I can reproduce the flakiness locally
by just running the test manually a few times.

I believe I used the GPG2 prereq because I don't have the older version
of GPG to test with.  So I don't know if t1016 would work on the older
version of GPG or not.


diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 937b876bd052..c4bbedfe081e 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -62,6 +62,8 @@ test_lazy_prereq GPG2 '
 		exit 1
 		;;
 	*)
+		mkdir "$GNUPGHOME" &&
+		chmod 0700 "$GNUPGHOME" &&
 		(gpgconf --kill all || : ) &&
 		gpg --homedir "${GNUPGHOME}" --import \
 			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&


Eric

Re: [PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[Junio C Hamano]

"Eric W. Biederman" <ebiederm@xmission.com> writes:

>> I am tempted to do this in the meantime, but I'd really prefer not
>> to have to do so, assuming that these tests, when fixed, would be
>> materially contributing to the health of our codebase.
>
> I just dug into this a little and hopefully I have paged enough
> state back to understand this.
>
> In my testing a missing GNUPGHOME appears enough to prevent the
> prerequisite from succeeding. So let's fix that. Todd Zullinger's sent
> some nice patches to do that (up-thread), or you can take use my minimal
> version.

Sorry, but I am confused.  The above "tempted to do this" was meant
to come on top of Todd's patches.  IOW, I was seeing flakyness with
Todd's patches that fixed the missing GNUPGHOME.

> The only possible source of flakiness in the tests I can see is the
> possibility of t/t1016/gpg not getting called (which uses a fixed
> timestamp).  It appears you just fixed that problem in commit
> 516bf45749bb ("t1016: make sure to use specified GPG").

I think that one also is in 'seen', and yet we saw t1016 flaky X-<.

Let me isolate the relevant topics and test them again, i.e.

    $ git checkout --detach v2.51.0
    $ git merge --no-ff jc/t1016-setup-fix ;# 516bf45749
    $ git merge --no-ff tz/test-prepare-gnupghome~1 ;# 6cd8369ef3
    $ git log --no-merges --oneline v2.51.0..
    516bf45749 (jc/t1016-setup-fix) t1016: make sure to use specified GPG
    6cd8369ef3 t/lib-gpg: call prepare_gnupghome() in GPG2 prereq
    a35952b493 t/lib-gpg: add prepare_gnupghome() to create GNUPGHOME dir
    $ make
    $ cd t && ./t1016-*.sh --stress
    FAIL 10.1
    FAIL  5.1
    FAIL 34.1
    ...
    ++ eval 'diff -u' '"$@"'
    +++ diff -u signedcommit3_sha1 signedcommit3_sha1_expected
    --- signedcommit3_sha1	2025-10-27 17:34:58.237496945 +0000
    +++ signedcommit3_sha1_expected	2025-10-27 17:34:58.145497051 +0000
    @@ -1 +1 @@
    -de9cabc2419f97eb665452c198ed93e890a7ef87
    +c87cd5157461a81b60ef6d3c47562c12b328ef54
    error: last command exited with $?=1
    not ok 163 - Verify commit signedcommit3's sha1 oid
    #	
    #			git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} >${name}_sha1 &&
    #			test_cmp ${name}_sha1 ${name}_sha1_expected
    #		
    1..163

> With that commit reverted I can reproduce the flakiness locally
> by just running the test manually a few times.

The above is with all three patches mentioned.
FWIW, "gpg --version | head -2" says

    gpg (GnuPG) 2.4.8
    libgcrypt 1.11.2

Hmmmm.....

> I believe I used the GPG2 prereq because I don't have the older version
> of GPG to test with.  So I don't know if t1016 would work on the older
> version of GPG or not.
>
>
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index 937b876bd052..c4bbedfe081e 100644
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -62,6 +62,8 @@ test_lazy_prereq GPG2 '
>  		exit 1
>  		;;
>  	*)
> +		mkdir "$GNUPGHOME" &&
> +		chmod 0700 "$GNUPGHOME" &&
>  		(gpgconf --kill all || : ) &&
>  		gpg --homedir "${GNUPGHOME}" --import \
>  			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
>
>
> Eric

Re: [PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[Eric W. Biederman]

Junio C Hamano <gitster@pobox.com> writes:

> "Eric W. Biederman" <ebiederm@xmission.com> writes:
>
>> The only possible source of flakiness in the tests I can see is the
>> possibility of t/t1016/gpg not getting called (which uses a fixed
>> timestamp).  It appears you just fixed that problem in commit
>> 516bf45749bb ("t1016: make sure to use specified GPG").
>
> I think that one also is in 'seen', and yet we saw t1016 flaky X-<.
>
> Let me isolate the relevant topics and test them again, i.e.
>
>     $ git checkout --detach v2.51.0
>     $ git merge --no-ff jc/t1016-setup-fix ;# 516bf45749
>     $ git merge --no-ff tz/test-prepare-gnupghome~1 ;# 6cd8369ef3
>     $ git log --no-merges --oneline v2.51.0..
>     516bf45749 (jc/t1016-setup-fix) t1016: make sure to use specified GPG
>     6cd8369ef3 t/lib-gpg: call prepare_gnupghome() in GPG2 prereq
>     a35952b493 t/lib-gpg: add prepare_gnupghome() to create GNUPGHOME dir
>     $ make
>     $ cd t && ./t1016-*.sh --stress
>     FAIL 10.1
>     FAIL  5.1
>     FAIL 34.1
>     ...
>     ++ eval 'diff -u' '"$@"'
>     +++ diff -u signedcommit3_sha1 signedcommit3_sha1_expected
>     --- signedcommit3_sha1	2025-10-27 17:34:58.237496945 +0000
>     +++ signedcommit3_sha1_expected	2025-10-27 17:34:58.145497051 +0000
>     @@ -1 +1 @@
>     -de9cabc2419f97eb665452c198ed93e890a7ef87
>     +c87cd5157461a81b60ef6d3c47562c12b328ef54
>     error: last command exited with $?=1
>     not ok 163 - Verify commit signedcommit3's sha1 oid
>     #	
>     #			git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} >${name}_sha1 &&
>     #			test_cmp ${name}_sha1 ${name}_sha1_expected
>     #		
>     1..163

Interesting.  With --stress I can reproduce the flakiness locally as
well.

I am starting to dig any but I haven't found any smoking guns yet.  So
far manually running the commands that resulted in the failure are
giving me the same output, but I have several more to run.

>> With that commit reverted I can reproduce the flakiness locally
>> by just running the test manually a few times.
>
> The above is with all three patches mentioned.
> FWIW, "gpg --version | head -2" says
>
>     gpg (GnuPG) 2.4.8
>     libgcrypt 1.11.2
>
> Hmmmm.....

I have gpg 2.4.7 but otherwise things are identical.

Eric

Re: [PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[Eric W. Biederman]

"Eric W. Biederman" <ebiederm@xmission.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> "Eric W. Biederman" <ebiederm@xmission.com> writes:
>>
>>> The only possible source of flakiness in the tests I can see is the
>>> possibility of t/t1016/gpg not getting called (which uses a fixed
>>> timestamp).  It appears you just fixed that problem in commit
>>> 516bf45749bb ("t1016: make sure to use specified GPG").
>>
>> I think that one also is in 'seen', and yet we saw t1016 flaky X-<.
>>
>> Let me isolate the relevant topics and test them again, i.e.
>>
>>     $ git checkout --detach v2.51.0
>>     $ git merge --no-ff jc/t1016-setup-fix ;# 516bf45749
>>     $ git merge --no-ff tz/test-prepare-gnupghome~1 ;# 6cd8369ef3
>>     $ git log --no-merges --oneline v2.51.0..
>>     516bf45749 (jc/t1016-setup-fix) t1016: make sure to use specified GPG
>>     6cd8369ef3 t/lib-gpg: call prepare_gnupghome() in GPG2 prereq
>>     a35952b493 t/lib-gpg: add prepare_gnupghome() to create GNUPGHOME dir
>>     $ make
>>     $ cd t && ./t1016-*.sh --stress
>>     FAIL 10.1
>>     FAIL  5.1
>>     FAIL 34.1
>>     ...
>>     ++ eval 'diff -u' '"$@"'
>>     +++ diff -u signedcommit3_sha1 signedcommit3_sha1_expected
>>     --- signedcommit3_sha1	2025-10-27 17:34:58.237496945 +0000
>>     +++ signedcommit3_sha1_expected	2025-10-27 17:34:58.145497051 +0000
>>     @@ -1 +1 @@
>>     -de9cabc2419f97eb665452c198ed93e890a7ef87
>>     +c87cd5157461a81b60ef6d3c47562c12b328ef54
>>     error: last command exited with $?=1
>>     not ok 163 - Verify commit signedcommit3's sha1 oid
>>     #	
>>     #			git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} >${name}_sha1 &&
>>     #			test_cmp ${name}_sha1 ${name}_sha1_expected
>>     #		
>>     1..163
>
> Interesting.  With --stress I can reproduce the flakiness locally as
> well.
>
> I am starting to dig any but I haven't found any smoking guns yet.  So
> far manually running the commands that resulted in the failure are
> giving me the same output, but I have several more to run.

So far in the two should be identical sha1 and sha256 repositories
I can confirm the failure is because the repositories are out of sync.

The sha256 gpg signatures match
The sha1 gpg signatures do not match

Which is very weird.  If they both didn't match it would be easy to
explain.

This is starting to look like this is a case of the test doing it's job
and finding a problem, rather than a problem in the test infrastructure.

I will keep digging.

git/t/trash directory.t1016-compatObjectFormat.stress-failed$ ../../git --git-dir=repo-sha256/.git cat-file tag signedtag34
object 94ee57ed028bc464ec9f9dc1d9c4b8c09fd89ac00e34b2bae3105803a995a6cd
type commit
tag signedtag34
tagger C O Mitter <committer@example.com> 1112354055 +0200
gpgsig -----BEGIN PGP SIGNATURE-----
 
 iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCZQhxPBYcY29tbWl0dGVy
 QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN3wIAoLYbVnmMIQnKqAfCDEtLGKDgH+M4
 AKDNi19wI7o7yWzThiujYZ422iMRGA==
 =lsWm
 -----END PGP SIGNATURE-----
 
This is an additional signed tag
-----BEGIN PGP SIGNATURE-----
 
iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCZQhxPBYcY29tbWl0dGVy
QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN21sAn2RYjMjcngN6AqBeo9RmIUn7NnWY
AJ97WUStWCcHXMkxU+HVPeuA/CvPYw==
=7Jpz
-----END PGP SIGNATURE-----
git/t/trash directory.t1016-compatObjectFormat.stress-failed$ ../../git --git-dir=repo-sha1/.git cat-file tag signedtag34
object 9ea30d18399b9957ce40766318510dab211d747b
type commit
tag signedtag34
tagger C O Mitter <committer@example.com> 1112354055 +0200
gpgsig-sha256 -----BEGIN PGP SIGNATURE-----
 
 iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCZQhxPBYcY29tbWl0dGVy
 QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN21sAn2RYjMjcngN6AqBeo9RmIUn7NnWY
 AJ97WUStWCcHXMkxU+HVPeuA/CvPYw==
 =7Jpz
 -----END PGP SIGNATURE-----
 
This is an additional signed tag
-----BEGIN PGP SIGNATURE-----
 
iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCZQhxPRYcY29tbWl0dGVy
QGV4YW1wbGUuY29tAAoJEBO29R7N3kMNvn4AmwRHkPsmDmKgUB6r1XP4dSzXWw+G
AKCEzEgk2bHuKv6d2L/M0bzseGlOfA==
=G+Gp
-----END PGP SIGNATURE-----

Eric

Re: [PATCH 0/2] t/lib-gpg: ensure GNUPGHOME is created as needed

[Junio C Hamano]

"Eric W. Biederman" <ebiederm@xmission.com> writes:

> So far in the two should be identical sha1 and sha256 repositories
> I can confirm the failure is because the repositories are out of sync.
>
> The sha256 gpg signatures match
> The sha1 gpg signatures do not match
>
> Which is very weird.  If they both didn't match it would be easy to
> explain.
>
> This is starting to look like this is a case of the test doing it's job
> and finding a problem, rather than a problem in the test infrastructure.
>
> I will keep digging.

Thanks.

[PATCH] t1016-compatObjectFormat: Really freeze time for reproduciblity

[Eric W. Biederman]

The strategy in t1016-compatObjectFormat is to build two trees with
identical commits, one tree encoded in sha1 the other tree encoded
in sha256 and to use the compatibility code to test and see if
the two trees are identical.

GPG signatures include the current time as part of the signature.

To make gpg deterministic I forced the use of gpg --faked-system-time.
Unfortunately I did not look closely enough.

By default gpg still allows time to move forward with --faked-system-time.
So in those rare instances when the system is heavily loaded an gpg runs
slower than other times, signatures over the exact same data differ
due to timestamps with a minuscule difference.

Reading through the gpg documentation with a close eye, time can be
frozen by including an exclamation point at the end of the argument to
--faked-system-time.

Add the exclamation point so gpg really runs with a fixed notion of time,
resulting in the exact same data having identical gpg signatures.

That is enough that I can run "t1016-compatObjectFormat.sh --stress"
and I don't see any failures.

It is possible a future change to gpg will make replay protection more
robust and not provide a way to allow two separate runs of gpg to
produce exactly the same signature for exactly the same data.  If that
happens a deeper comparison of the two repositories will need to be
performed.  A comparison that simply verifies the signatures and
compares the data for equality.  For now that is a lot of work
for no gain so I am just documenting the possibility.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 t/t1016-compatObjectFormat.sh | 6 ++++++
 t/t1016/gpg                   | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh
index a9af8b239626..0efce53f3aad 100755
--- a/t/t1016-compatObjectFormat.sh
+++ b/t/t1016-compatObjectFormat.sh
@@ -21,6 +21,12 @@ test_description='Test how well compatObjectFormat works'
 # different hash functions result in the same content in the commits.
 # This means that when the commit is translated between hash functions
 # the commit is identical to the commit in the other repository.
+#
+# Similarly this test relies on:
+#	gpg --faked-system-time '20230918T154812!
+# freezing the system time from gpg perspective so that two different
+# runs of gpg applied to the same data result in identical signatures.
+#
 
 compat_hash () {
 	case "$1" in
diff --git a/t/t1016/gpg b/t/t1016/gpg
index 2601cb18a5b3..34d6e055fc9e 100755
--- a/t/t1016/gpg
+++ b/t/t1016/gpg
@@ -1,2 +1,2 @@
 #!/bin/sh
-exec gpg --faked-system-time "20230918T154812" "$@"
+exec gpg --faked-system-time '20230918T154812!' "$@"
-- 
2.41.0

Re: [PATCH] t1016-compatObjectFormat: Really freeze time for reproduciblity

[Junio C Hamano]

"Eric W. Biederman" <ebiederm@xmission.com> writes:

> By default gpg still allows time to move forward with --faked-system-time.
> So in those rare instances when the system is heavily loaded an gpg runs
> slower than other times, signatures over the exact same data differ
> due to timestamps with a minuscule difference.
>
> Reading through the gpg documentation with a close eye, time can be
> frozen by including an exclamation point at the end of the argument to
> --faked-system-time.
> ...
>  t/t1016-compatObjectFormat.sh | 6 ++++++
>  t/t1016/gpg                   | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)

Geez, how are we expected to find the need for '!' ourselves X-<.

Thanks for root causing the issue so quickly once it was raised.

And let me drop the "let's disable flakey ones" band-aid patch from
the queue.

Re: [PATCH] t1016-compatObjectFormat: Really freeze time for reproduciblity

[Todd Zullinger]

Junio C Hamano wrote:
> "Eric W. Biederman" <ebiederm@xmission.com> writes:
> 
>> By default gpg still allows time to move forward with --faked-system-time.
>> So in those rare instances when the system is heavily loaded an gpg runs

s/an/&d/

>> slower than other times, signatures over the exact same data differ
>> due to timestamps with a minuscule difference.
>>
>> Reading through the gpg documentation with a close eye, time can be
>> frozen by including an exclamation point at the end of the argument to
>> --faked-system-time.
>> ...
>>  t/t1016-compatObjectFormat.sh | 6 ++++++
>>  t/t1016/gpg                   | 2 +-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> Geez, how are we expected to find the need for '!' ourselves X-<.
> 
> Thanks for root causing the issue so quickly once it was raised.

I'll second that.  Nicely sleuthed and explained.

It explains why I had trouble that I thought looked like gpg
wasn't setting the time as expected, long before the code
change which caused the custom gpg wrapper to not be used by
all the tests.

Back then, I went so far as to run the whole test suite with
the gpg wrapper setting --faked-system-time, but I didn't
notice the crucial lack of an exclamation point on the time
either.

I applied this and Junio's previous patch to ensure the
wrapper is always used on top of 2.51.2¹ and ran it through
the Fedora build system where I consistently saw failures
before.  With this patch it all worked as expected.

¹ It's much easier for me to test a released tarball with
  the existing Fedora packaging than a snapshot of next;
  even though I know it's of *slightly* less value than
  testing the tip of next or seen.

Thanks!

-- 
Todd