diff mbox series

[bug#38509] gnu: libuv: Update to 1.34.0

Message ID 20191206085713.27096-1-andrew@interpretmath.pw
State Accepted
Headers show
Series [bug#38509] gnu: libuv: Update to 1.34.0 | expand

Commit Message

Andrew Miloradovsky Dec. 6, 2019, 8:57 a.m. UTC
* gnu/packages/libevent.scm (libuv): Update to 1.34.0 (from 1.30.1)
---
 gnu/packages/libevent.scm | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Brett Gilio Dec. 7, 2019, 1:27 a.m. UTC | #1
Andrew Miloradovsky <andrew@interpretmath.pw> writes:

> -    (arguments
> -     '(;; XXX: Some tests want /dev/tty, attempt to make connections, etc.
> -       #:tests? #f))
> +    (arguments '(#:tests? #f))
> +    ;; tests 122-124 (getnameinfo_basic_ip*) fail
> +    ;; https://github.com/libuv/libuv/issues/2531

Hi Andrew,

Thank you for your submission. I have some questions before we go
forward with this patch. First, I'd like to note that this change would
trigger a huge rebuild of thousands of dependent packages.

--8<---------------cut here---------------start------------->8---
brettg@oryx ~/Repos/guix$ ./pre-inst-env guix refresh -l libuv
Building the following 2067 packages would ensure 5397 dependent packages are rebuilt:
--8<---------------cut here---------------end--------------->8---

That said, is there some particular functionality or security that is
provided with this patch? If so, it would be great if you could
elaborate on that in the Git sub-header of the commit message.

Since this patch would trigger such a massive rebuild it will need to go
to the `core-updates` branch to rest before it sees master. So having
that detail in the commit message will make it easier for us to see what
we are working with.

Lastly, just curious if there is a way to work around the issues with
tests 122-124 as shown in your above snippet. I know that the tests were
blanket disabled before, so I am just curious if there is a _better_ way
to do this, maybe i'm wrong. If the issue is that the tests require
network functionality that can usually be spoofed with some
effort. Also, stylistically, comments not on the same line as the code
they are commenting usually go before the code in question.

I hope that makes sense! Let me know if you have any questions, I am
happy to help.
Brett Gilio Dec. 7, 2019, 1:34 a.m. UTC | #2
Brett Gilio <brettg@posteo.net> writes:

> Andrew Miloradovsky <andrew@interpretmath.pw> writes:
>
>> -    (arguments
>> -     '(;; XXX: Some tests want /dev/tty, attempt to make connections, etc.
>> -       #:tests? #f))
>> +    (arguments '(#:tests? #f))
>> +    ;; tests 122-124 (getnameinfo_basic_ip*) fail
>> +    ;; https://github.com/libuv/libuv/issues/2531
>
> Hi Andrew,
>
> Thank you for your submission. I have some questions before we go
> forward with this patch. First, I'd like to note that this change would
> trigger a huge rebuild of thousands of dependent packages.
>
> brettg@oryx ~/Repos/guix$ ./pre-inst-env guix refresh -l libuv
> Building the following 2067 packages would ensure 5397 dependent packages are rebuilt:
>
> That said, is there some particular functionality or security that is
> provided with this patch? If so, it would be great if you could
> elaborate on that in the Git sub-header of the commit message.
>
> Since this patch would trigger such a massive rebuild it will need to go
> to the `core-updates` branch to rest before it sees master. So having
> that detail in the commit message will make it easier for us to see what
> we are working with.
>
> Lastly, just curious if there is a way to work around the issues with
> tests 122-124 as shown in your above snippet. I know that the tests were
> blanket disabled before, so I am just curious if there is a _better_ way
> to do this, maybe i'm wrong. If the issue is that the tests require
> network functionality that can usually be spoofed with some
> effort. Also, stylistically, comments not on the same line as the code
> they are commenting usually go before the code in question.
>
> I hope that makes sense! Let me know if you have any questions, I am
> happy to help.

Actually, on further exploration of the `core-updates` branch, libuv is
already at 1.34 there.
Andrew Miloradovsky Dec. 7, 2019, 1:31 p.m. UTC | #3
Hi Brett,

WRT the differences, it seems to be mostly fixes and refactoring:

- https://github.com/libuv/libuv/tree/v1.34.0

WRT the failed tests, it is due to treating EAGAIN as the failure:

- https://github.com/libuv/libuv/issues/2531

Not sure how to properly fix it, fixing the tests themselves is
non-trivial, and retrying several more times likely won't work either.

On 12/7/19 1:34 AM, Brett Gilio wrote:
> Brett Gilio <brettg@posteo.net> writes:
>
>> Andrew Miloradovsky <andrew@interpretmath.pw> writes:
>>
>>> -    (arguments
>>> -     '(;; XXX: Some tests want /dev/tty, attempt to make connections, etc.
>>> -       #:tests? #f))
>>> +    (arguments '(#:tests? #f))
>>> +    ;; tests 122-124 (getnameinfo_basic_ip*) fail
>>> +    ;; https://github.com/libuv/libuv/issues/2531
>> Hi Andrew,
>>
>> Thank you for your submission. I have some questions before we go
>> forward with this patch. First, I'd like to note that this change would
>> trigger a huge rebuild of thousands of dependent packages.
>>
>> brettg@oryx ~/Repos/guix$ ./pre-inst-env guix refresh -l libuv
>> Building the following 2067 packages would ensure 5397 dependent packages are rebuilt:
>>
>> That said, is there some particular functionality or security that is
>> provided with this patch? If so, it would be great if you could
>> elaborate on that in the Git sub-header of the commit message.
>>
>> Since this patch would trigger such a massive rebuild it will need to go
>> to the `core-updates` branch to rest before it sees master. So having
>> that detail in the commit message will make it easier for us to see what
>> we are working with.
>>
>> Lastly, just curious if there is a way to work around the issues with
>> tests 122-124 as shown in your above snippet. I know that the tests were
>> blanket disabled before, so I am just curious if there is a _better_ way
>> to do this, maybe i'm wrong. If the issue is that the tests require
>> network functionality that can usually be spoofed with some
>> effort. Also, stylistically, comments not on the same line as the code
>> they are commenting usually go before the code in question.
>>
>> I hope that makes sense! Let me know if you have any questions, I am
>> happy to help.
> Actually, on further exploration of the `core-updates` branch, libuv is
> already at 1.34 there.
>
Marius Bakke Jan. 27, 2020, 11:20 p.m. UTC | #4
Andrew Miloradovsky <andrew@interpretmath.pw> writes:

> Hi Brett,
>
> WRT the differences, it seems to be mostly fixes and refactoring:
>
> - https://github.com/libuv/libuv/tree/v1.34.0
>
> WRT the failed tests, it is due to treating EAGAIN as the failure:
>
> - https://github.com/libuv/libuv/issues/2531
>
> Not sure how to properly fix it, fixing the tests themselves is
> non-trivial, and retrying several more times likely won't work either.

/etc/nsswitch.conf does not exist in the build container, which is
probably why the getnameinfo tests fail.  Could you submit a patch for
Guix that disables those, and enables the rest?

Meanwhile I'm closing this issue, as we have the latest LibUV on the
'core-updates' branch.

By the way, if you need a newer libuv on 'master', you can create a
separate variable as in e.g. ffec356a29a58d97ec34e1152aa3136e78471dc6.

Thanks!
diff mbox series

Patch

diff --git a/gnu/packages/libevent.scm b/gnu/packages/libevent.scm
index 465ed95eb2..aabc780bfe 100644
--- a/gnu/packages/libevent.scm
+++ b/gnu/packages/libevent.scm
@@ -99,22 +99,21 @@  limited support for fork events.")
 (define-public libuv
   (package
     (name "libuv")
-    (version "1.30.1")
+    (version "1.34.0")
     (source (origin
               (method url-fetch)
               (uri (string-append "https://dist.libuv.org/dist/v" version
                                   "/libuv-v" version ".tar.gz"))
               (sha256
                (base32
-                "12s7ifwgbfxblhv46inqa8c2lsnl8cgmvd37y4a4248xhkx1d0s6"))))
+                "0j416x38cp6gh5isn3fwv331aw6bpfmrk8xgm07rq5py47kyqg52"))))
     (build-system gnu-build-system)
-    (arguments
-     '(;; XXX: Some tests want /dev/tty, attempt to make connections, etc.
-       #:tests? #f))
+    (arguments '(#:tests? #f))
+    ;; tests 122-124 (getnameinfo_basic_ip*) fail
+    ;; https://github.com/libuv/libuv/issues/2531
     (native-inputs `(("autoconf" ,autoconf-wrapper)
                      ("automake" ,automake)
                      ("libtool" ,libtool)
-
                      ;; libuv.pc is installed only when pkg-config is found.
                      ("pkg-config" ,pkg-config)))
     (home-page "https://github.com/libuv/libuv")