Re: BUG #15789: libpq compilation with OpenSSL 1.1.1b fails on Windows with Visual Studio 2017

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Juan José Santamaría Flecha <juanjo(dot)santamaria(at)gmail(dot)com>
Cc: Sandeep Thakkar <sandeep(dot)thakkar(at)enterprisedb(dot)com>, serpashk(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #15789: libpq compilation with OpenSSL 1.1.1b fails on Windows with Visual Studio 2017
Date: 2019-06-20 02:49:41
Message-ID: 20190620024941.GB2660@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Wed, Jun 05, 2019 at 09:29:21AM +0200, Juan José Santamaría Flecha wrote:
> The patches are intended to support OpenSSL 1.1.x, including 1.1.1x.
>
> The attached patches are meant for all supported versions and HEAD.

I have been looking at this patch, and here are some comments. First,
I have looked at the differences in what gets installed between 1.0.2
and 1.1.0 and things are rather messy:
- On the Win64 and Win32 installers for 1.0.2, we have both
libssl32.lib and libeay.lib, with the same naming of libraries in
lib/VC/. This keeps the Postgres scripts more simple, but it causes
conflicts with the installers when trying to manage both Win64 and
Win32, so I cannot blame them for the changes done..
- In 1.1.0~, the situation gets fancy:
-- For Win64 and Win32, we have both now libssl.lib and
libcrypto.lib, which is still consistent.
-- lib/VC/ has been heavily reworked so as we have now for example
libssl64MD.lib & co for Win64 and libssl32MD.lib & co for Win32.

Your patch catches the differences nicely.

sub AddLibrary
{
- my ($self, $lib, $dbgsuffix) = @_;
+ my ($self, $lib, $slib) = @_;
The only reason why we have the debugging option in the original
interface is for OpenSSL, where Solution.pm checks for the presence of
lib/VC/ssleay32MD.lib before deciding if we should use the debug libs
or not. I am confused by what you are doing though: what do $slib and
$xlib mean? For simplicity and compatibility, I am not sure that we
should change the current interface, and just check for the debug libs
that we expect, as done previously. If the new interface is more
advantageous, the patch needs an effort of documentation, but I would
keep that as a separate improvement.

+sub GetOpenSSLVersion
[...]
+ if ($sslout =~ /(\d+)\.(\d+)\.(\d+)/m)
+ {
+ return ($1, $2)
I really like this interface, but I think that we should return the
three digits instead to help with the decision making so as we don't
need to rework it later on.

+ my ($major, $minor) = $self->GetOpenSSLVersion();
+ if ($major == 1 && $minor == 1)
If one day OpenSSL bumps to 1.2.X, this code would fail. I think that
we should check that the major and minor digits are at least what we
expect them to be. The same applies to the third digit.
--
Michael

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Michael Paquier 2019-06-20 03:12:18 Re: BUG #15827: Unable to connect on Windows using pg_services.conf using Python psycopg2
Previous Message PG Bug reporting form 2019-06-20 01:49:58 BUG #15861: jsonb exists query retuning inconsistent results