1293944 - Make EC test programs only depend on softoken libs

Status: RESOLVED FIXED

(NSS :: Test, defect)

Description

[Daiki Ueno [:ueno]]

Attachment: ectest-softoken.patch

The EC test programs (ecperf and ectest) are currently using functions
from NSS libs, while they substantially rely on the softoken libs
only.  The attached patch elliminates the extra dependencies and allows those
programs to build with NSS_BUILD_SOFTOKEN_ONLY=1.  That also makes
downstream packaging simpler, if they split the upstream tarball into
multiple stages, like in Fedora.

To test, I did the following:
- remove external_tests from the toplevel manifest.mn
- make nss_build_all NSS_BUILD_SOFTOKEN_ONLY=1 BUILD_OPT=1 USE_64=1
- cd tests
- NSS_CYCLES=standard NSS_TESTS=ec NSS_SSL_TESTS=' ' NSS_SSL_RUN=' ' USE_64=1 FREEBL_NO_DEPEND=1 BUILD_OPT=1 NSS_BUILD_SOFTOKEN_ONLY=1 NSS_ENABLE_ECC=1 ./all.sh

Comment 1

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8779644 [details] [diff] [review]
ectest-softoken.patch

Review of attachment 8779644 [details] [diff] [review]:
-----------------------------------------------------------------

While I'm in general ok with this change it doesn't work as is.
You probably didn't notice that due to a faulty test script (which is my fault).

To fix it you have to initialise the PRNG for the ecperf tests. That can be achieved by calling RNG_RNGInit and RNG_SystemInfoForRNG.
In order to fix the test script and catch the assertion error that's thrown with your patch (due to the uninitialised prng) you could change ecperf.sh to |ECPERF_OUT=$(ecperf 2>& 1)| and then parse for an assertion failure in addition to failure |PARSED_ECPERF_OUT+=`echo $ECPERF_OUT | grep -i "Assertion failure"`|. (I'm not sure why I get a 0 exit code in the script when the assertion is thrown.)

Comment 2

[Daiki Ueno [:ueno]]

Attachment: Make EC test programs only depend on softoken libs

The EC test programs (ecperf and ectest) are currently using functions
from NSS libs, while they substantially rely on the softoken libs
only.  This patch elliminates the extra dependencies and allows those
programs to build with NSS_BUILD_SOFTOKEN_ONLY=1.  That also makes
downstream packaging simpler, if they split the upstream tarball into
multiple stages, like in Fedora.

Comment 3

[Daiki Ueno [:ueno]]

Thank you for the review.  I have confirmed that the modified test script fails if the PRNG is not properly initialized.  The v2 patch should fix the issue.

Comment 4

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8780000 [details] [diff] [review]
Make EC test programs only depend on softoken libs

Review of attachment 8780000 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with those two things fixed.

A try run https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=479b09bf5ebedc4947fc5f0c8ca12444d8037158

::: cmd/ecperf/ecperf.c
@@ +774,4 @@
>  #endif
>  
>  cleanup:
> +    rv |= SECOID_Shutdown();

We should shutdown the the RNG as well (RNG_RNGShutdown).

::: tests/ec/ectest.sh
@@ +40,5 @@
>  }
>  
>  ectest_init
> +ECTEST_OUT=$(ectest 2>&1)
> +ECTEST_OUT=`echo $ECTEST_OUT | grep -i 'not okay\|Assertion failure'"`

stray "

Comment 5

[Daiki Ueno [:ueno]]

Attachment: Make EC test programs only depend on softoken libs

The EC test programs (ecperf and ectest) are currently using functions
from NSS libs, while they substantially rely on the softoken libs
only.  This patch elliminates the extra dependencies and allows those
programs to build with NSS_BUILD_SOFTOKEN_ONLY=1.  That also makes
downstream packaging simpler, if they split the upstream tarball into
multiple stages, like in Fedora.

Comment 6

[Daiki Ueno [:ueno]]

Thank you for pointing those out.  I am attaching the fixed version for reference, which has been incorporated into Fedora builds now:
http://koji.fedoraproject.org/koji/buildinfo?buildID=790901

Comment 7

[Kai Engert (:KaiE:)]

Looks like Daiki has made the changes that Franziskus asked for, and this is good to be checked in.
Franziskus, do you want to check and land, or should I?

Comment 8

[Franziskus Kiefer [:franziskus]]

https://hg.mozilla.org/projects/nss/rev/df61b15b2cca