Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Mark H Weaver]

Hi Marius,

mbakke@fastmail.com (Marius Bakke) writes:
> mbakke pushed a commit to branch master
> in repository guix.
>
> commit 4f3dcdd99ba13ab3bdbf1e014afcd076cd95fac7
> Author: Marius Bakke <mbakke@fastmail.com>
> Date:   Mon Mar 13 16:53:27 2017 +0100
>
>     gnu: nss, nss-certs: Update to 3.29.3.
>     
>     * gnu/packages/gnuzilla.scm (nss): Update to 3.29.3.

Hydra has tried to build nss-3.29.3 on x86_64 twice, and the test suite
failed both times.

  https://hydra.gnu.org/build/1905330

The first time, we got this:

[  FAILED  ] 1 test, listed below:
[  FAILED  ] DamageYDatagram/TlsDamageDHYTest.DamageClientY/8, where GetParam() = ("DTLS", 770, 4, true)

and the second time, this:

[  FAILED  ] 1 test, listed below:
[  FAILED  ] ExtensionPre13Datagram/TlsExtensionTestPre13.AlpnReturnedBadNameLength/0, where GetParam() = ("TLS", 770)

This is the first time I've ever seen NSS fail to build on x86_64.  It's
quite serious since it means around 230 dependency failures, including
IceCat, GNOME and Qt/KDE.

  https://hydra.gnu.org/eval/109538#tabs-now-fail

If this problem cannot be fixed very soon, we'll need to revert this
update.  Did it build successfully on your machine?  If so, which
architecture did you test it on?

      Mark

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Mark H Weaver]

I wrote:
> If this problem cannot be fixed very soon, we'll need to revert this
> update.

One more thing: If you decide to revert the nss update (commit
4f3dcdd99b), please also revert commit 87f1c7efc1 (gnu: nss: Use
'modify-phases' syntax), to save Hydra from needlessly rebuilding
several hundred packages.  These two commits should be reverted
together, and then re-applied together when the problem is ultimately
resolved.

      Thanks,
        Mark

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]

Mark H Weaver <mhw@netris.org> writes:

> Hi Marius,
>
> mbakke@fastmail.com (Marius Bakke) writes:
>> mbakke pushed a commit to branch master
>> in repository guix.
>>
>> commit 4f3dcdd99ba13ab3bdbf1e014afcd076cd95fac7
>> Author: Marius Bakke <mbakke@fastmail.com>
>> Date:   Mon Mar 13 16:53:27 2017 +0100
>>
>>     gnu: nss, nss-certs: Update to 3.29.3.
>>     
>>     * gnu/packages/gnuzilla.scm (nss): Update to 3.29.3.
>
> Hydra has tried to build nss-3.29.3 on x86_64 twice, and the test suite
> failed both times.
>
>   https://hydra.gnu.org/build/1905330
>
> The first time, we got this:
>
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] DamageYDatagram/TlsDamageDHYTest.DamageClientY/8, where GetParam() = ("DTLS", 770, 4, true)
>
> and the second time, this:
>
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] ExtensionPre13Datagram/TlsExtensionTestPre13.AlpnReturnedBadNameLength/0, where GetParam() = ("TLS", 770)
>
> This is the first time I've ever seen NSS fail to build on x86_64.  It's
> quite serious since it means around 230 dependency failures, including
> IceCat, GNOME and Qt/KDE.
>
>   https://hydra.gnu.org/eval/109538#tabs-now-fail
>
> If this problem cannot be fixed very soon, we'll need to revert this
> update.  Did it build successfully on your machine?  If so, which
> architecture did you test it on?

Hi Mark,

I have built this without trouble on two different x86_64 systems. The
release notes[0] lists a single entry[1] which looks innocuous[2], so I
doubt the failure is related to the upgrade.

I can't find the build log of the first run, but note how in the second
failure the test took more than five seconds:

[  FAILED  ] ExtensionPre13Datagram/TlsExtensionTestPre13.AlpnReturnedBadNameLength/0, where GetParam() = ("TLS", 770) (5543 ms)
(from https://hydra.gnu.org/build/1905330/nixlog/6/raw )

This is reminiscent of the trouble we had getting 3.29.2 to build on
armhf[3]. Something caused the build to stall for a few seconds, which
in turn makes the test fail due to exceeding time treshold.

Can you try restarting the build once more?

0: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_3.29.3_release_notes
1: https://bugzilla.mozilla.org/show_bug.cgi?id=1342358
2: https://hg.mozilla.org/projects/nss/rev/4e9aee9c5343
3: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25974#17

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Mark H Weaver]

Marius Bakke <mbakke@fastmail.com> writes:

> I have built this without trouble on two different x86_64 systems. The
> release notes[0] lists a single entry[1] which looks innocuous[2], so I
> doubt the failure is related to the upgrade.
>
> I can't find the build log of the first run,

When restarting builds on Hydra, the log for the first build attempt is
lost.  However, I'm in the habit of making private copies of these logs
before restarting jobs, so I have a copy of both.

> but note how in the second failure the test took more than five seconds:
>
> [  FAILED  ] ExtensionPre13Datagram/TlsExtensionTestPre13.AlpnReturnedBadNameLength/0, where GetParam() = ("TLS", 770) (5543 ms)
> (from https://hydra.gnu.org/build/1905330/nixlog/6/raw )
>
> This is reminiscent of the trouble we had getting 3.29.2 to build on
> armhf[3]. Something caused the build to stall for a few seconds, which
> in turn makes the test fail due to exceeding time treshold.

The nss-3.29.3 build on armhf also failed:

  https://hydra.gnu.org/build/1906025

> Can you try restarting the build once more?

This is not really sustainable.  A single build attempt takes 7 hours on
armhf, and about 40 hours on mips.  When the failure occurs, it causes
hundreds of other dependency failures, which must be restarted manually,
one at a time, via the web interface.  (We have a way to restart *all*
dependency failures, but that results in a huge amount of wasted work
for Hydra).

We need test suites to be robust on heavily loaded build machines.

Is there a compelling reason not to revert this update for now?

      Mark

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]

On Tue, Mar 14, 2017 at 05:02:12PM -0400, Mark H Weaver wrote:
> This is not really sustainable.  A single build attempt takes 7 hours on
> armhf, and about 40 hours on mips.  When the failure occurs, it causes
> hundreds of other dependency failures, which must be restarted manually,
> one at a time, via the web interface.  (We have a way to restart *all*
> dependency failures, but that results in a huge amount of wasted work
> for Hydra).
> 
> We need test suites to be robust on heavily loaded build machines.

I agree that this situation is not sustainable. If we are committed to
offering substitutes, we can't have such a critical package not building
reliably.

But, it seems unsatisfactory to not update NSS / nss-certs without
working towards a real solution.

Nss-certs provides the CA certificate store in Guix. It does get updated
along with NSS [0], although not in every NSS release.

I think we should find a way to decouple the certificate store from NSS,
since we can't build NSS reliably.

> Is there a compelling reason not to revert this update for now?

Since there were no changes to the certificates between 3.29.2 and
3.29.3, I think it's fine to revert.

[0]
https://wiki.mozilla.org/NSS:Release_Versions

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1879 bytes --]

Leo Famulari <leo@famulari.name> writes:

> On Tue, Mar 14, 2017 at 05:02:12PM -0400, Mark H Weaver wrote:
>> This is not really sustainable.  A single build attempt takes 7 hours on
>> armhf, and about 40 hours on mips.  When the failure occurs, it causes
>> hundreds of other dependency failures, which must be restarted manually,
>> one at a time, via the web interface.  (We have a way to restart *all*
>> dependency failures, but that results in a huge amount of wasted work
>> for Hydra).
>> 
>> We need test suites to be robust on heavily loaded build machines.
>
> I agree that this situation is not sustainable. If we are committed to
> offering substitutes, we can't have such a critical package not building
> reliably.
>
> But, it seems unsatisfactory to not update NSS / nss-certs without
> working towards a real solution.
>
> Nss-certs provides the CA certificate store in Guix. It does get updated
> along with NSS [0], although not in every NSS release.
>
> I think we should find a way to decouple the certificate store from NSS,
> since we can't build NSS reliably.
>
>> Is there a compelling reason not to revert this update for now?
>
> Since there were no changes to the certificates between 3.29.2 and
> 3.29.3, I think it's fine to revert.

NSS is a crypto library in addition to a certificate store and 3.29.3
resolves a crash when TLS 1.3 is enabled. I don't know how serious this
issue is, but we should try to keep up on both packages.

Nevertheless, I've reverted the commits for now so the old substitutes
should be valid again.

Going forward, I wonder if there could be any unintended side effects by
simply increasing the timeouts in nss/gtests/ssl_gtest/tls_connect.cc
from 5000 ms to something like 20000. If a 0-day is discovered in "nss",
we don't want to wait several days to get a successful build.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

On Tue, Mar 14, 2017 at 10:39:48PM +0100, Marius Bakke wrote:
> Going forward, I wonder if there could be any unintended side effects by
> simply increasing the timeouts in nss/gtests/ssl_gtest/tls_connect.cc
> from 5000 ms to something like 20000. If a 0-day is discovered in "nss",
> we don't want to wait several days to get a successful build.

If these failures seem to be timeout issues, I think we should try this.
We do have other packages that make similar changes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 954 bytes --]

Leo Famulari <leo@famulari.name> writes:

> On Tue, Mar 14, 2017 at 10:39:48PM +0100, Marius Bakke wrote:
>> Going forward, I wonder if there could be any unintended side effects by
>> simply increasing the timeouts in nss/gtests/ssl_gtest/tls_connect.cc
>> from 5000 ms to something like 20000. If a 0-day is discovered in "nss",
>> we don't want to wait several days to get a successful build.
>
> If these failures seem to be timeout issues, I think we should try this.
> We do have other packages that make similar changes.

I'm currently building NSS (on x86_64) with timeouts increased from 5s
to 25s. The recent armhf test failure took 22725ms, and when we
struggled with 3.29.2 it took ~20000ms too.

Patch attached. I *think* the two values are for client and server
respectively, but will study the source and build logs some more to make
sure we're adjusting the right knobs.

I suggest we try this on 'core-updates' if the patch is correct.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-gnu-nss-Increase-test-timeouts.patch --]
[-- Type: text/x-patch, Size: 2940 bytes --]

From c8e0b85953133328e27f0bfcc9a9a0330e36ce5a Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Tue, 14 Mar 2017 22:54:41 +0100
Subject: [PATCH] gnu: nss: Increase test timeouts.

* gnu/packages/patches/nss-increase-test-timeout.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/gnuzilla.scm (nss)[source]: Use it.
---
 gnu/local.mk                                         |  1 +
 gnu/packages/gnuzilla.scm                            |  3 ++-
 gnu/packages/patches/nss-increase-test-timeout.patch | 20 ++++++++++++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/nss-increase-test-timeout.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index c1b076a5f..7fb22ebb5 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -780,6 +780,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/ninja-tests.patch			\
   %D%/packages/patches/ninja-zero-mtime.patch			\
   %D%/packages/patches/node-9077.patch				\
+  %D%/packages/patches/nss-increase-test-timeout.patch		\
   %D%/packages/patches/nss-pkgconfig.patch			\
   %D%/packages/patches/ntfs-3g-CVE-2017-0358.patch		\
   %D%/packages/patches/nvi-assume-preserve-path.patch		\
diff --git a/gnu/packages/gnuzilla.scm b/gnu/packages/gnuzilla.scm
index e6e24f665..1d84e7a9a 100644
--- a/gnu/packages/gnuzilla.scm
+++ b/gnu/packages/gnuzilla.scm
@@ -199,7 +199,8 @@ in the Mozilla clients.")
                (base32
                 "149807rmzb76hnh48rw4m9jw83iw0168njzchz0hmbsgc8mk0i5w"))
               ;; Create nss.pc and nss-config.
-              (patches (search-patches "nss-pkgconfig.patch"))))
+              (patches (search-patches "nss-pkgconfig.patch"
+                                       "nss-increase-test-timeout.patch"))))
     (build-system gnu-build-system)
     (outputs '("out" "bin"))
     (arguments
diff --git a/gnu/packages/patches/nss-increase-test-timeout.patch b/gnu/packages/patches/nss-increase-test-timeout.patch
new file mode 100644
index 000000000..76d4853ba
--- /dev/null
+++ b/gnu/packages/patches/nss-increase-test-timeout.patch
@@ -0,0 +1,20 @@
+--- a/nss/gtests/ssl_gtest/tls_connect.cc	2017-03-14 22:47:30.855813629 +0100
++++ b/nss/gtests/ssl_gtest/tls_connect.cc	2017-03-14 22:48:49.042335273 +0100
+@@ -245,7 +245,7 @@
+ 
+   ASSERT_TRUE_WAIT((client_->state() != TlsAgent::STATE_CONNECTING) &&
+                        (server_->state() != TlsAgent::STATE_CONNECTING),
+-                   5000);
++                   25000);
+ }
+ 
+ void TlsConnectTestBase::EnableExtendedMasterSecret() {
+@@ -387,7 +387,7 @@
+   } else {
+     fail_agent = server_;
+   }
+-  ASSERT_TRUE_WAIT(fail_agent->state() == TlsAgent::STATE_ERROR, 5000);
++  ASSERT_TRUE_WAIT(fail_agent->state() == TlsAgent::STATE_ERROR, 25000);
+ }
+ 
+ void TlsConnectTestBase::ConfigureVersion(uint16_t version) {
-- 
2.12.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 543 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

> Patch attached. I *think* the two values are for client and server
> respectively, but will study the source and build logs some more to make
> sure we're adjusting the right knobs.
>
> I suggest we try this on 'core-updates' if the patch is correct.

The patch builds fine on x86_64, and I've verified that these are the
correct settings by decreasing the values instead of increasing.

What do you think? Should we check if 25s is high enough on
'core-updates' or push it directly to 'master'?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-gnu-nss-Increase-test-timeouts.patch --]
[-- Type: text/x-patch, Size: 3150 bytes --]

From 33bbf7bc60b222adc6effc7257440fd8222ef04b Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Tue, 14 Mar 2017 22:54:41 +0100
Subject: [PATCH] gnu: nss: Increase test timeouts.

* gnu/packages/patches/nss-increase-test-timeout.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/gnuzilla.scm (nss)[source]: Use it.
---
 gnu/local.mk                                       |  1 +
 gnu/packages/gnuzilla.scm                          |  3 ++-
 .../patches/nss-increase-test-timeout.patch        | 25 ++++++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/nss-increase-test-timeout.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index c1b076a5f..7fb22ebb5 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -780,6 +780,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/ninja-tests.patch			\
   %D%/packages/patches/ninja-zero-mtime.patch			\
   %D%/packages/patches/node-9077.patch				\
+  %D%/packages/patches/nss-increase-test-timeout.patch		\
   %D%/packages/patches/nss-pkgconfig.patch			\
   %D%/packages/patches/ntfs-3g-CVE-2017-0358.patch		\
   %D%/packages/patches/nvi-assume-preserve-path.patch		\
diff --git a/gnu/packages/gnuzilla.scm b/gnu/packages/gnuzilla.scm
index e6e24f665..1d84e7a9a 100644
--- a/gnu/packages/gnuzilla.scm
+++ b/gnu/packages/gnuzilla.scm
@@ -199,7 +199,8 @@ in the Mozilla clients.")
                (base32
                 "149807rmzb76hnh48rw4m9jw83iw0168njzchz0hmbsgc8mk0i5w"))
               ;; Create nss.pc and nss-config.
-              (patches (search-patches "nss-pkgconfig.patch"))))
+              (patches (search-patches "nss-pkgconfig.patch"
+                                       "nss-increase-test-timeout.patch"))))
     (build-system gnu-build-system)
     (outputs '("out" "bin"))
     (arguments
diff --git a/gnu/packages/patches/nss-increase-test-timeout.patch b/gnu/packages/patches/nss-increase-test-timeout.patch
new file mode 100644
index 000000000..c6aac6ac0
--- /dev/null
+++ b/gnu/packages/patches/nss-increase-test-timeout.patch
@@ -0,0 +1,25 @@
+We've seen some tests take more than 20s to complete on a busy armhf
+machine. Even a busy x86_64 machine can use more than 5s on some tests.
+
+Increase timeouts to increase chances of a successful build.
+
+--- a/nss/gtests/ssl_gtest/tls_connect.cc	2017-03-14 22:47:30.855813629 +0100
++++ b/nss/gtests/ssl_gtest/tls_connect.cc	2017-03-14 22:48:49.042335273 +0100
+@@ -245,7 +245,7 @@
+ 
+   ASSERT_TRUE_WAIT((client_->state() != TlsAgent::STATE_CONNECTING) &&
+                        (server_->state() != TlsAgent::STATE_CONNECTING),
+-                   5000);
++                   25000);
+ }
+ 
+ void TlsConnectTestBase::EnableExtendedMasterSecret() {
+@@ -387,7 +387,7 @@
+   } else {
+     fail_agent = server_;
+   }
+-  ASSERT_TRUE_WAIT(fail_agent->state() == TlsAgent::STATE_ERROR, 5000);
++  ASSERT_TRUE_WAIT(fail_agent->state() == TlsAgent::STATE_ERROR, 25000);
+ }
+ 
+ void TlsConnectTestBase::ConfigureVersion(uint16_t version) {
-- 
2.12.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Marius Bakke <mbakke@fastmail.com> writes:
>
>> Patch attached. I *think* the two values are for client and server
>> respectively, but will study the source and build logs some more to make
>> sure we're adjusting the right knobs.
>>
>> I suggest we try this on 'core-updates' if the patch is correct.
>
> The patch builds fine on x86_64, and I've verified that these are the
> correct settings by decreasing the values instead of increasing.
>
> What do you think? Should we check if 25s is high enough on
> 'core-updates' or push it directly to 'master'?

Good catch.

It might be best to push to ‘core-updates’ to focus our build
resources.  No strong opinion though.

Thanks!

Ludo’.

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

> Marius Bakke <mbakke@fastmail.com> skribis:
>
>> Marius Bakke <mbakke@fastmail.com> writes:
>>
>>> Patch attached. I *think* the two values are for client and server
>>> respectively, but will study the source and build logs some more to make
>>> sure we're adjusting the right knobs.
>>>
>>> I suggest we try this on 'core-updates' if the patch is correct.
>>
>> The patch builds fine on x86_64, and I've verified that these are the
>> correct settings by decreasing the values instead of increasing.
>>
>> What do you think? Should we check if 25s is high enough on
>> 'core-updates' or push it directly to 'master'?
>
> Good catch.
>
> It might be best to push to ‘core-updates’ to focus our build
> resources.  No strong opinion though.

I pushed it to 'core-updates'.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: 02/05: gnu: nss, nss-certs: Update to 3.29.3.

[Mark H Weaver]

Marius Bakke <mbakke@fastmail.com> writes:

> Marius Bakke <mbakke@fastmail.com> writes:
>
>> Patch attached. I *think* the two values are for client and server
>> respectively, but will study the source and build logs some more to make
>> sure we're adjusting the right knobs.
>>
>> I suggest we try this on 'core-updates' if the patch is correct.
>
> The patch builds fine on x86_64, and I've verified that these are the
> correct settings by decreasing the values instead of increasing.

This seems like the right approach, thanks!

> What do you think? Should we check if 25s is high enough on
> 'core-updates' or push it directly to 'master'?

I'd be okay with pushing this to master.  Thank you!

     Mark