Bug 61636 - testcrypto tries to test both OpenSSL and NSS, regardless of what it was built with
Summary: testcrypto tries to test both OpenSSL and NSS, regardless of what it was buil...
Status: NEW
Alias: None
Product: APR
Classification: Unclassified
Component: APR test (show other bugs)
Version: HEAD
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache Portable Runtime bugs mailinglist
Keywords: PatchAvailable
Depends on:
Reported: 2017-10-19 09:08 UTC by Michal Karm Babacek
Modified: 2017-10-24 15:08 UTC (History)
1 user (show)

Patch for testcrypto.c (8.40 KB, text/plain)
2017-10-19 11:46 UTC, Michal Karm Babacek

Note You need to log in before you can comment on or make changes to this bug.
Description Michal Karm Babacek 2017-10-19 09:08:40 UTC
Call to action: Make testcrypto configurable

Original post to the APR Dev list:

Hi guys,

I've switched my Windows CI from APR Util 1.5.x
to the 1.6.x branch. Little did I know it would
demand NSS even though I built with OpenSSL.

This is the pertinent excerpt from the log [1],
the full build log (large text) can be found here [2]
and last but not least, this is the build script [3],
note that I tried to set -DAPU_HAVE_NSS=OFF out of good
sport, but it was in vain.

Could you tell me what might be improved in the
CMakeLists.txt so as the test run doesn't look for NSS?

Is there anything else fishy about it? Perhaps the root
cause is entirely different.

Thank you for any pointers



[1] https://www.hastebin.com/umitizokor.lua
[2] https://ci.modcluster.io/job/apr-util-windows/15/label=w2k12r2/consoleText

Michal Karm Babacek

Sent from my Hosaka Ono-Sendai Cyberspace 7
Comment 1 Michal Karm Babacek 2017-10-19 09:24:25 UTC
I'll add a conf option.
Comment 2 Michal Karm Babacek 2017-10-19 11:46:31 UTC
Created attachment 35441 [details]
Patch for testcrypto.c
Comment 3 Michal Karm Babacek 2017-10-19 11:49:21 UTC
Regarding the patch: attachment 35441 [details] (GitHub branch [1])

The default behavior remains the same, see [2].
Three new env props can be optionally set, APU_TEST_NSS=on|off, APU_TEST_OPENSSL=on|off, APU_TEST_COMMONCRYPTO=on|off

Here is an example with my build where I have only OpenSSL crypto available (apr_crypto_openssl dlls):

> C:\Users\karm\source\apr-util\karm-vs\Debug (1.6.x)
> λ set APU_TEST_NSS=off & set APU_TEST_COMMONCRYPTO=off & set "PATH=C:\tmp\apr\bin\;C:\tmp\libexpat-R_2_2_0-64\bin\;C:\tmp\OpenSSL_1_0_2h-64\bin\;%PATH%" & .\testall.exe -v testcrypto
> testcrypto          : SUCCESS
> /Error 730061 occurred attempting to reach memcached on localhost:11211.  Skipping apr_memcache tests...
> /Error 730061 occurred attempting to reach Redis on localhost:6379.  Skipping apr_redis tests...
> All tests passed.

Legit? WDYT?


[1] https://github.com/Karm/apr-util/tree/1.6.x-BZ-61636
[2] http://www.mergely.com/T2Hd0zGd/
Comment 5 William A. Rowe Jr. 2017-10-24 14:13:26 UTC
Comment on attachment 35441 [details]
Patch for testcrypto.c

Reviewing the patch, I'd rather see us inspect APU_HAVE_NSS, APU_HAVE_OPENSSL,
APU_HAVE_COMMONCRYPTO as emitted from the configure phase in our private or public headers. We don't rely on envvars elsewhere in the test framework.
Comment 6 jfclere 2017-10-24 14:54:14 UTC
actually get_nss_driver() and get_openssl_driver() can test what apr-util is capable doing, can't we use those to find what we can test?
May with a flag checking APU_HAVE_NSS, APU_HAVE_OPENSSL,
APU_HAVE_COMMONCRYPTO in case we run the against what we have just compiled...
Comment 7 Michal Karm Babacek 2017-10-24 15:08:34 UTC
I'll post a different patch that won't use env props then...

I'm gonna explore get_*driver first, because APU_HAVE_* smells of #ifdefs, IIUC.