On Thu, Jun 20, 2019 at 4:50 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> - 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.
>
Great.
> 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.
>
Since the third parameter is only currently used with OpenSSL
libraries, I thought it would be ok to touch it in this patch. The
reasson behind doing so was to keep the logic for the new fancy
library naming tidy. Documenting better is not a thing I can argue
against, xlib stands for expanded library name as used in
GetAdditionalLinkerDependencies and maybe a comment like this can make
things clearer:
# The parameter $slib is for sufixed run-time library.
# When available it will be added, if not it silently defaults to $lib.
sub AddLibrary
> +sub GetOpenSSLVersion
> [...]
> 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.
No problem with that. Even more so, it can return 4 values: the 3
digits and the letter. For example, 1.1.1b would be (1, 1, 1, b).
Regards,
Juan José Santamaría Flecha
