Re: remove internal support in pgcrypto? - Mailing list pgsql-hackers

> On 3 Nov 2021, at 16:06, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
>
> On 03.11.21 11:16, Daniel Gustafsson wrote:
>>> On 30 Oct 2021, at 14:11, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
>>>
>>> On 24.08.21 11:13, Peter Eisentraut wrote:
>>>> So I'm tempted to suggest that we remove the built-in, non-OpenSSL cipher and hash implementations in pgcrypto
(basicallyINT_SRCS in pgcrypto/Makefile), and then also pursue the simplifications in the OpenSSL code paths described
in[0]. 
>>>
>>> Here is a patch for this.
>> This patch doesn't work on Windows, which I think is because it pulls in
>> pgcrypto even in builds without OpenSSL.  Poking at that led me to realize that
>> we can simplify even more with this.  The conditonal source includes can go
>> away and be replaced with a simple OBJS clause, and with that the special hacks
>> in Mkvcbuild.pm to overcome that.
>> Attached is a diff on top of your patch to do the above.  I haven't tested it
>> on Windows yet, but if you think it's in the right direction we'll take it for
>> a spin in a CI with/without OpenSSL.
>
> Here is a consolidated patch.  I have tested it locally, so it should be okay on Windows.

I don't think this bit is correct, as OSSL_TESTS have been removed from the Makefie:

-              $config->{openssl}
-              ? GetTests("OSSL_TESTS", $m)
-              : GetTests("INT_TESTS",  $m);
+              GetTests("OSSL_TESTS", $m);

I think we need something like the (untested) diff below:

diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index e3a323b8bf..fc2406b2be 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -729,13 +729,10 @@ sub fetchTests
                        # pgcrypto is special since the tests depend on the
                        # configuration of the build

-                       my $cftests =
-                         GetTests("OSSL_TESTS", $m);
                        my $pgptests =
                          $config->{zlib}
                          ? GetTests("ZLIB_TST",     $m)
                          : GetTests("ZLIB_OFF_TST", $m);
-                       $t =~ s/\$\(CF_TESTS\)/$cftests/;
                        $t =~ s/\$\(CF_PGP_TESTS\)/$pgptests/;
                }
        }

>> Now, *if* we merge the NSS patch this does introduce special cases again which
>> this rips out.  I prefer to try and fix them in that patch to keep avoiding the
>> need for them rather than keep them on speculation for a patch which hasn't
>> been decided on.
>
> Okay, I wasn't sure about the preferred way forward here.  I'm content with the approach you have chosen.

I'm honestly not sure either; but as the NSS patch author, if I break it I get
to keep both pieces =)

--
Daniel Gustafsson        https://vmware.com/