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
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 |