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

From: Juan José Santamaría Flecha <juanjo(dot)santamaria(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
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-25 09:43:29
Message-ID: CAC+AXB0+rMmS_=pGss9wrhq+Ld6pAR6X51sAasRE6SuULScOdA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

I will not have much available time for this list in the next few
weeks, so as quick eye review:

On Tue, Jun 25, 2019 at 10:08 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> Thanks for the new patch set! I have been looking at that in depths
> and I have adjusted the whole logic a bit here and there. At the end
> I found the logic changed in AddLibrary more confusing because the
> debugging suffix may change depending on if we use a release-quality
> build or not, so I have kept the original interface, and completed it
> with a logic allowing the scripts to grab all the libraries it
> expects. This has a small gain when using a non-debug installation as
> the library names are the same for Win32 and Win64 for >= 1.1.0.
>

It actually looks clearer and less intrusive (better) to me too.

> Also, your patch was not working with other versions of MSVC as the
> new routine got stuck into the 2017 class, so I had to move it, and I
> found that it was cleaner to just make it return a string made of the
> 3 first digits and to do direct string comparisons.
>

If you are using a string you will need padding, maybe mimic
OPENSSL_VERSION_NUMBER [1].

> Please note that it is not necessary to create versions for the
> back-branches yet. If necessary, I'll do that myself, but first let's
> make sure that we agree about the shape of the patch for HEAD.
> Attached is an updated version which I would be fine to commit. I
> have tested it with compilation linking to OpenSSL 1.0.2 and 1.1.0 on
> Win32 and the build is able to complete. This applies on HEAD only,
> where I have run all my tests. The patch is properly indented.
>

There is a typo:

s/versoin/version/

+# made of the three first digits of the OpenSSL versoin, which is enough

Regards,

Juan José Santamaría Flecha

[1] https://www.openssl.org/docs/man1.1.1/man3/OPENSSL_VERSION_NUMBER.html

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Faran Ali 2019-06-25 09:47:07 Bug
Previous Message Amit Langote 2019-06-25 08:56:00 Re: BUG #15724: Can't create foreign table as partition