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@paquier.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