Ups, wrong PR ;-)

Pauli, AppVeyer CI is not ready yet, but my personal AppVeyer CI notified me of a build failure, which I a can reproduce on my windows machine:

With my change from shared to static, the windows build now fails as follows:

perl Configure VC-WIN64A shared --prefix=%USERPROFILE%\install\openssl --openssldir=%USERPROFILE%\install\openssl\ssl
...

C:\src\openssl>nmake

Microsoft (R) Program Maintenance Utility Version 14.00.24210.0
Copyright (C) Microsoft Corporation.  All rights reserved.

NMAKE : fatal error U1073: don't know how to make 'crypto\include\internal\bn_conf.h'
Stop.

Resetting to HEAD~, the build succeeds because the file is generated by perl:

C:\src\openssl>nmake

Microsoft (R) Program Maintenance Utility Version 14.00.24210.0
Copyright (C) Microsoft Corporation.  All rights reserved.

        "C:\Perl64\bin\perl.exe" "-I." -Mconfigdata "util\dofile.pl"  "-omakefile" "crypto\include\internal\bn_conf.h.in" > crypto\include\internal\bn_conf.h
        "C:\Perl64\bin\perl.exe" "-I." -Mconfigdata "util\dofile.pl"  "-omakefile" "crypto\include\internal\dso_conf.h.in" > crypto\include\internal\dso_conf.h
        "C:\Perl64\bin\perl.exe" "-I." -Mconfigdata "util\dofile.pl"  "-omakefile" "include\openssl\opensslconf.h.in" > include\openssl\opensslconf.h
        "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\nmake.exe" /                   depend && "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\nmake.exe" /                   _all

...

Can you please tell me where and how I have to enter the missing dependency?

See https://ci.appveyor.com/project/mspncp/openssl/builds/19693883 (I hope this link is accessible without login)

Outside my areas of experience unfortunately. @levitte any idea?
I can see the AppVeyor build failures you linked to but don't know what's wrong.

Yeah, you're trying with a trick that isn't quite supported. It so happens to work on Unix platforms, but that's luck more than anything else.

Also, on Windows, you can't have static libraries when building with configuration option shared (which is the default), and Configure said so:

Linking with static variants of shared libraries is not supported in this configuration

The .a suffix in build.info should be used throughout when used, i.e. it's an indicator that we're building that library as static only. That assumption can be relaxed, of course, but you'll still end up with the same problem on Windows.

An alternative would be to have all static libraries for installation renamed libfoo.a => libfoo_static.a. This has been mentioned a rare few times over a couple of years, maybe it's time to get through with it. Note that this breaks earlier patterns, so people who want to link with our static libraries will have to link with -lcrypto_static rather than -lcrypto.

Richard, there are a lot of other tests which currently link against libcrypto.a. Are you saying these don't work on all platforms?

~/src/openssl$ grep 'DEPEND.*libcrypto.a' test/build.info
    DEPEND[drbgtest]=../libcrypto.a libtestutil.a
    DEPEND[poly1305_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[chacha_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[asn1_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[modes_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[x509_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[ctype_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[siphash_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[sm2_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[sm4_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[curve448_internal_test]=../libcrypto.a libtestutil.a
    DEPEND[rdrand_sanitytest]=../libcrypto.a libtestutil.a

Anyhow, I don't need drbgtest linked statically per-se. This pull request intended to solve the problem of duplicated code in #7456: I needed to access the function rand_drbg_seedlen(), implemented in drbg_lib.c, from drbgtest.c. Since I didn't want to export the private symbol, I copypasted the function to drbgtest.c, but added a warning comment to both.

https://github.com/openssl/openssl/blob/ef40152c5e8cb32ed373e3eae3b32ed5cb7b610f/crypto/rand/drbg_lib.c#L1034-L1044

It seemed to be the least evil solution to me, but @paulidale suggested to link the test statically instead, see the outdated comment. Maybe you can propose an alternative solution? Refactoring the static builds is not an option, because I would like to cherry-pick the tests to 1.1.1.

Why I answer things just before going to bed, I wonder...

You're right, we're already utilizing this "trick". However, that is under some explicit conditions... those instances that you grepped for, they're all within this condition (cited with comment):

  # Internal test programs.  These are essentially a collection of internal
  # test routines.  Some of them need to reach internal symbols that aren't
  # available through the shared library (at least on Linux, Solaris, Windows
  # and VMS, where the exported symbols are those listed in util/*.num), these
  # programs are forcibly linked with the static libraries, where all symbols
  # are always available.  This excludes linking these programs natively on
  # Windows when building shared libraries, since the static libraries share
  # names with the DLL import libraries.
  IF[{- $disabled{shared} || $target{build_scheme}->[1] ne 'windows' -}]
    ...
  ENDIF

So if you want to link a program with libcrypto.a, you have to move it inside that block, and you also have to avoid running the corresponding test recipe when building shared on Windows, such as:

plan skip_all => "This test is unsupported in a shared library build on Windows"
    if $^O eq 'MSWin32' && !disabled("shared");

(ref: https://github.com/openssl/openssl/blob/master/test/recipes/03-test_internal_poly1305.t#L16-L17)

Ok, that makes sense. Thank you for the explanation!

(ref: https://github.com/openssl/openssl/blob/master/test/recipes/03-test_internal_poly1305.t#L16-L17)

BTW: if you ever wondered how to get these nice little inline code windows instead of just a link: The trick is to use links which contain no branch references, but only commit hashes instead. GitHub has a cool keyboard shortcut for converting the former into the latter: To try it out, just go to the above link and then press the 'y' key. (To see all shortcuts, type '?' instead of 'y'.) This will convert the symbolic references in the url by a commit hash (like git rev-parse) and you obtain

Note: an important advantage of the latter type is that it's target is immutable and doesn't evolve in time.

Another thought comes to my mind:

An alternative would be to have all static libraries for installation renamed libfoo.a => libfoo_static.a. This has been mentioned a rare few times over a couple of years, maybe it's time to get through with it. Note that this breaks earlier patterns, so people who want to link with our static libraries will have to link with -lcrypto_static rather than -lcrypto.

Instead of renaming the installed libraries, which would be a rather intrusive ABI change, you could build additional static libraries (on windows only) to make the "static tests" available on windows even if shared builds are configured. You don't even have to rename the static libraries, just place them in a separate location (e.g. static\libcrypto.lib, static\libssl.lib), to distinguish them from the import libraries. These static libraries will only be used for the static tests and not installed later.

If building two library versions is undesired on this notoriously slow platform, then one could add a --static-tests option for windows, which is disabled by default.

... and then press the 'y' key.

... iiiinteresting.

As for having static libcrypto and libssl in build that won't be installed is an idea, of course... I'll consider it.

Postponed until issue #7492 is resolved.

Update: fix for #7492 is in preparation, see #7496.

Rebased this pull request on top of #7496.

Rebased onto master.

Shouldn't the Hold label be removed?

As soon as I tested. Windows is still building ;-).

BTW: Why was there no CI running on your #7496?

Wasn't there? Hard to see now...

Ok, now it gets interesting... all internal tests were still disabled in the recipes.

Correction: s/enabled/enable/ in commit message

Wasn't there? Hard to see now...

Ok... never noticed before that the ci box disappears eventually. Does this happen when the pr gets closed?

@levitte I rebased without squashing and added a fixup commit which removes the duplicate rand_drbg_seedlen() implementation again. (The original reason for fixing the internal tests.)

Please re-review.

Side note: The test_rand_add and test_rand_seed tests succeed --with-rand-seed=none, but most of the others drbgtests (and a lot more) fail in that configuration. I have an idea how this can be fixed... (wip)

ping

Merged, thank you!

master

1901516 Test: enable internal tests for shared Windows builds
1c615e4 Test: link drbgtest statically against libcrypto

1.1.1

cdf3350 Test: enable internal tests for shared Windows builds
c39df74 Test: link drbgtest statically against libcrypto