GNU bug report logs - #46012
Upgrade Nheko

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46012 in the body.
You can then email your comments to 46012 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Nicolò Balzarotti <anothersms <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: Upgrade Nheko
Date: Thu, 21 Jan 2021 01:37:19 +0100

[Message part 1 (text/plain, inline)]

Hi guix! Today nheko 0.8.0 was released.

This patch series upgrades it and its dependency, mtxclient.
I alsotook some time to unbundle all its dependencies (adding
cpp-httplib, blurhash, and single-applicaiton-qt5).

About this last dependency (single-applicaiton-qt5), I'm unsure on how
to name it.  Also, devs are suggested to include its source directly,
and by default builds a static library.  The main SingleApplication
class inherit from a Qt*Application class which is choosen at build
time, so the library to be useful in the target program must be built
with the correct flag (I'm using the one used by nheko by default).

Nheko builds and run fine.  It should support voice call now, but I
cannot test it (I get `[error] WebRTC: failed to start device monitor',
not sure if the problem is in my setup, in gstreamer or in my package).

[0001-gnu-Add-cpp-httplib.patch (text/x-patch, attachment)]

[0002-gnu-Add-blurhash.patch (text/x-patch, attachment)]

[0003-gnu-mtxclient-Update-to-0.4.0.patch (text/x-patch, attachment)]

[0004-gnu-Add-single-application-qt5.patch (text/x-patch, attachment)]

[0005-gnu-nheko-Update-to-0.8.0.patch (text/x-patch, attachment)]

Message #8 received at 46012 <at> debbugs.gnu.org (full text, mbox):

From: Nicolò Balzarotti <anothersms <at> gmail.com>
To: 46012 <at> debbugs.gnu.org
Subject: Re: bug#46012: Acknowledgement (Upgrade Nheko)
Date: Thu, 28 Jan 2021 00:02:59 +0100

[Message part 1 (text/plain, inline)]

In the meanwhile a new version has been released, so here's the update
patch set.  Discussion happened on bug#46013, v3, compared to v2, just
updates mtxclient to v 0.4.1 (with relative hash) and nheko to 0.8.1.

[v3-0001-gnu-Add-cpp-httplib.patch (text/x-patch, attachment)]

[v3-0002-gnu-Add-blurhash.patch (text/x-patch, attachment)]

[v3-0004-gnu-nheko-Update-to-0.8.1.patch (text/x-patch, attachment)]

[v3-0004-gnu-nheko-Update-to-0.8.1.patch (text/x-patch, attachment)]

Message #13 received at 46012 <at> debbugs.gnu.org (full text, mbox):

From: Nicolò Balzarotti <anothersms <at> gmail.com>
To: 46012 <at> debbugs.gnu.org
Subject: Re: [bug#46012] Acknowledgement (Upgrade Nheko)
Date: Tue, 27 Apr 2021 15:56:32 +0200

[Message part 1 (text/plain, inline)]

Nicolò Balzarotti <anothersms <at> gmail.com> writes:

> In the meanwhile a new version has been released, so here's the update
> patch set.  Discussion happened on bug#46013, v3, compared to v2, just
> updates mtxclient to v 0.4.1 (with relative hash) and nheko to 0.8.1.
A new version has been release.  I updated the patchset accordingly.
This (v4) updates mtxclient to 0.5.1, changes lmdbxx repo to the one
used by nheko (nheko is the only package in guix using lmdbxx) and
upgrades nheko to 0.8.2.  I upgraded them all in the same commit as each
upgrade breaks the other package.

Thanks!

[v4-0001-gnu-Add-cpp-httplib.patch (text/x-patch, attachment)]

[v4-0002-gnu-Add-blurhash.patch (text/x-patch, attachment)]

[v4-0003-gnu-Add-single-application-qt5.patch (text/x-patch, attachment)]

[v4-0004-gnu-nheko-Update-to-0.8.2.patch (text/x-patch, attachment)]

Message #16 received at 46012 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Nicolò Balzarotti <anothersms <at> gmail.com>, 
 46012 <at> debbugs.gnu.org
Subject: Re: [bug#46012] Acknowledgement (Upgrade Nheko)
Date: Tue, 27 Apr 2021 20:09:08 +0200

[Message part 1 (text/plain, inline)]

Nicolò Balzarotti schreef op di 27-04-2021 om 15:56 [+0200]:
> +    (synopsis "C++ header-only HTTP/HTTPS server and client library")
> +    (description "cpp-httplib is a C++11 single-file header-only cross
> +platform blocking HTTP/HTTPS library, easy to setup.  Just include the
> +@file{httplib.h} file in your code!")

This is a little misleading, as shared libraries are build, as BUILD_SHARED_LIBS
is enabled.  Maybe "cpp-http is a single-file header-only library" -->
"cpp-http can be used as a single-file header-only 
library"?

About ‘header-only’: this is true, but ultimately irrelevant to the user
(= C++ developer on a Guix System or using Guix on top of a foreign distro).
But there's also a desirable thing called ‘portability’, the user might be
searching for a single-header web server software to distribute to other
people (not on Guix) in source form ...

I'm conflicted if "single-file header" should be included in the description.
If you decide to remove it, I suggest you add a comment like

  ;; this package is not graftable, as everything is implemented in a single
  ;; header

to prevent trouble in a (admittedly somewhat far-fetched, no insult intended
to its developers) future where cpp-httplib becomes a very popular dependency
in Guix.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (replace 'check
> +           (lambda* (#:key source #:allow-other-keys)
> +             ;; openssl genrsa wants to write a file in the git checkout
> +             (copy-file (string-append source "/test") "test")
> +             (chmod "test" #o744)
> +             (invoke "make"))))))

Tests most likely should not be run when cross-compiling.  I'm not 100% sure,
but you might need to do something like

> +           (lambda* (#:key tests? source #:allow-other-keys)
> +             ;; openssl genrsa wants to write a file in the git checkout
> +             (when tests?
> +               (copy-file
(string-append source "/test") "test")
> +               (chmod "test" #o744)
> +               (invoke "make")))))))

Search for example for "when tests?" in gnu/packages/python-xyz.scm.

> +       ("zlib" ,zlib)))

In <https://github.com/yhirose/cpp-httplib/blob/master/httplib.h> I see
a few lines

  #ifdef CPPHTTPLIB_ZLIB_SUPPORT
  #include <zlib.h>
  #endif

so it seems zlib should be in (inputs ...) instead.

I also saw these lines:

  #ifdef CPPHTTPLIB_BROTLI_SUPPORT
  #include <brotli/decode.h>
  #include <brotli/encode.h>
  #endif

Would it be useful to include brotli?

  #ifdef CPPHTTPLIB_OPENSSL_SUPPORT
  #include <openssl/err.h>
  #include <openssl/md5.h>
  ...

Likewise, for openssl?

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #19 received at 46012 <at> debbugs.gnu.org (full text, mbox):

From: Nicolò Balzarotti <anothersms <at> gmail.com>
To: Maxime Devos <maximedevos <at> telenet.be>, 46012 <at> debbugs.gnu.org
Subject: Re: [bug#46012] Acknowledgement (Upgrade Nheko)
Date: Tue, 27 Apr 2021 23:00:22 +0200

[Message part 1 (text/plain, inline)]

Hi Maxime, many thanks for the review!

Maxime Devos <maximedevos <at> telenet.be> writes:

> Nicolò Balzarotti schreef op di 27-04-2021 om 15:56 [+0200]:
>> +    (synopsis "C++ header-only HTTP/HTTPS server and client library")
>> +    (description "cpp-httplib is a C++11 single-file header-only cross
>> +platform blocking HTTP/HTTPS library, easy to setup.  Just include the
>> +@file{httplib.h} file in your code!")
>
> This is a little misleading, as shared libraries are build, as BUILD_SHARED_LIBS
> is enabled.  Maybe "cpp-http is a single-file header-only library" -->
> "cpp-http can be used as a single-file header-only 
> library"?
>
I removed it from the synopsis, and reworded the description to: 1. make
it sound less like marketing, and keeping the single-header thing as a bonus.

> About ‘header-only’: this is true, but ultimately irrelevant to the user
> (= C++ developer on a Guix System or using Guix on top of a foreign distro).
> But there's also a desirable thing called ‘portability’, the user might be
> searching for a single-header web server software to distribute to other
> people (not on Guix) in source form ...
>
> I'm conflicted if "single-file header" should be included in the description.
> If you decide to remove it, I suggest you add a comment like
>
>   ;; this package is not graftable, as everything is implemented in a single
>   ;; header
>
> to prevent trouble in a (admittedly somewhat far-fetched, no insult intended
> to its developers) future where cpp-httplib becomes a very popular dependency
> in Guix.
>
I don't know enought about grafts, so I trust you on that.  I did not
know where to put the comment, so I added it at the very top.

I also noticed that this was updated since last time, so I updated it to
0.8.8 (latest tagged version).

>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (replace 'check
>> +           (lambda* (#:key source #:allow-other-keys)
>> +             ;; openssl genrsa wants to write a file in the git checkout
>> +             (copy-file (string-append source "/test") "test")
>> +             (chmod "test" #o744)
>> +             (invoke "make"))))))
>
> Tests most likely should not be run when cross-compiling.

> I'm not 100% sure, but you might need to do something like
>
>> +           (lambda* (#:key tests? source #:allow-other-keys)
>> +             ;; openssl genrsa wants to write a file in the git checkout
>> +             (when tests?
>> +               (copy-file
> (string-append source "/test") "test")
>> +               (chmod "test" #o744)
>> +               (invoke "make")))))))
>

Didn't think about that, I wrapped it in a `when tests?` (and added
`tests?` as argument to the lambda) as you suggested.  I also changed it
a bit making it more clear.  There are now tests requiring network
access, so I disabled them.

>
>> +       ("zlib" ,zlib)))
>
> In <https://github.com/yhirose/cpp-httplib/blob/master/httplib.h> I see
> a few lines
>
>   #ifdef CPPHTTPLIB_ZLIB_SUPPORT
>   #include <zlib.h>
>   #endif
>
> so it seems zlib should be in (inputs ...) instead.
>

> I also saw these lines:
>
>   #ifdef CPPHTTPLIB_BROTLI_SUPPORT
>   #include <brotli/decode.h>
>   #include <brotli/encode.h>
>   #endif
>
> Would it be useful to include brotli?
>
>   #ifdef CPPHTTPLIB_OPENSSL_SUPPORT
>   #include <openssl/err.h>
>   #include <openssl/md5.h>
>   ...
>
> Likewise, for openssl?
Sure, added brotli and moved openssl to inputs.  I also aadded the
"HTTPLIB_REQUIRE_" flags just to be sure they are used int the build.
They shouldn't be needed as HTTPLIB_USE_*_IF_AVAILABLE defaults to ON,
but if they change default we are covered.


Regarding why openssl and zlib were in native-inputs, this is (probably)
how it went: I built it without openssl, tests failed to run because the
command openssl was required to generate a certificate, so I added it to
native-inputs.  Then probably I added zlib not noticing I placed it
under native-inputs, at least that's how I think it went.

nheko still builds and run, so here the v5 of the series.

>
> Greetings,
> Maxime.

Thanks, Nicolò

[v5-0001-gnu-Add-cpp-httplib.patch (text/x-patch, attachment)]

[v5-0002-gnu-Add-blurhash.patch (text/x-patch, attachment)]

[v5-0003-gnu-Add-single-application-qt5.patch (text/x-patch, attachment)]

[v5-0004-gnu-nheko-Update-to-0.8.2.patch (text/x-patch, attachment)]

Message #24 received at 46012-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Nicolò Balzarotti <anothersms <at> gmail.com>
Cc: 46012-done <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#46012: Upgrade Nheko
Date: Fri, 03 Sep 2021 22:07:23 +0200

Hi Nicolò,

Nicolò Balzarotti <anothersms <at> gmail.com> skribis:

>>From 5ea66aa57f81976d39c60f59357ddf6027fadabb Mon Sep 17 00:00:00 2001
> From: nixo <nicolo <at> nixo.xyz>
> Date: Wed, 27 Jan 2021 23:44:04 +0100
> Subject: [PATCH v5 1/4] gnu: Add cpp-httplib.
>
> * gnu/packages/cpp.scm (cpp-httplib): New variable.

[...]

>>From f2982d9f3a8f6fcf66950ed78259125f3750ca08 Mon Sep 17 00:00:00 2001
> From: nixo <nicolo <at> nixo.xyz>
> Date: Wed, 20 Jan 2021 17:56:04 +0100
> Subject: [PATCH v5 2/4] gnu: Add blurhash.
>
> * gnu/packages/image.scm (blurhash): New variable.

[...]

>>From a709f28a8dacd890a46eb848bbf6b42efaa6a447 Mon Sep 17 00:00:00 2001
> From: nixo <nicolo <at> nixo.xyz>
> Date: Wed, 20 Jan 2021 19:23:17 +0100
> Subject: [PATCH v5 3/4] gnu: Add single-application-qt5.
> 
> * gnu/packages/qt.scm (single-application-qt5): New variable.

[...]

>>From 2564b19691b0721407ee48f90de18353796278d0 Mon Sep 17 00:00:00 2001
> From: nixo <nicolo <at> nixo.xyz>
> Date: Wed, 27 Jan 2021 23:54:55 +0100
> Subject: [PATCH v5 4/4] gnu: nheko: Update to 0.8.2.
>
> * gnu/packages/messaging.scm (nheko): Update to 0.8.2.
> [source snippet]: Remove third_party folder.
> [arguments]: Remove -fpermissive flag, remove remove-Werror phase, add phase
> wrap-program and unbundle-dependencies.
> [inputs]: Add gst-plugins-base, gst-plugins-bad, libnice, qtkeychain,
> and unbundle blurhash, cpp-httplib and single-application, remove tweeny.
> [native-inputs]: Add doxygen, graphviz used to build documentation.
> [description]: Simplify by removing the long list, add new features.
> (mtxclient): Update to 0.5.1.
> [arguments]: Remove set-home phase.
> * gnu/packages/databases.scm (lmdbxx): Update to 1.0.0.
> [source]: Change repository to a fork required by nheko.
> [home-page]: Update accordingly.

I’m really sorry that it took 7 months (!), but I’ve now applied this
series.  I had to tweak ‘single-application-qt5’ to adjust it to the
qtbase -> qtbase-5 rename, but that’s about it.

Thank you, and thanks Maxime for the review—much appreciated, as always!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.