diff mbox series

[bug#46012] Acknowledgement (Upgrade Nheko)

Message ID 87eeevqw7t.fsf@guixSD.i-did-not-set--mail-host-address--so-tickle-me
State Accepted
Headers show
Series [bug#46012] Acknowledgement (Upgrade Nheko) | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Nicolò Balzarotti April 27, 2021, 9 p.m. UTC
Hi Maxime, many thanks for the review!

Maxime Devos <maximedevos@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ò

Comments

Ludovic Courtès Sept. 3, 2021, 8:07 p.m. UTC | #1
Hi Nicolò,

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

>>From 5ea66aa57f81976d39c60f59357ddf6027fadabb Mon Sep 17 00:00:00 2001
> From: nixo <nicolo@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@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@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@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’.
diff mbox series

Patch

From 2564b19691b0721407ee48f90de18353796278d0 Mon Sep 17 00:00:00 2001
From: nixo <nicolo@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.
---
 gnu/packages/databases.scm | 10 ++---
 gnu/packages/messaging.scm | 91 ++++++++++++++++++++++----------------
 2 files changed, 57 insertions(+), 44 deletions(-)

diff --git a/gnu/packages/databases.scm b/gnu/packages/databases.scm
index f9e63f3722..88d5120f2e 100644
--- a/gnu/packages/databases.scm
+++ b/gnu/packages/databases.scm
@@ -39,7 +39,7 @@ 
 ;;; Copyright © 2019 Pierre Langlois <pierre.langlois@gmx.com>
 ;;; Copyright © 2019, 2021 Guillaume Le Vaillant <glv@posteo.net>
 ;;; Copyright © 2020 Pierre Neidhardt <mail@ambrevar.xyz>
-;;; Copyright © 2020 Nicolò Balzarotti <nicolo@nixo.xyz>
+;;; Copyright © 2020, 2021 Nicolò Balzarotti <nicolo@nixo.xyz>
 ;;; Copyright © 2020 Tanguy Le Carrour <tanguy@bioneland.org>
 ;;; Copyright © 2020 Lars-Dominik Braun <ldb@leibniz-psychology.org>
 ;;; Copyright © 2020 Guy Fleury Iteriteka <gfleury@disroot.org>
@@ -2437,16 +2437,16 @@  virtual address space — not physical RAM.")
 (define-public lmdbxx
   (package
     (name "lmdbxx")
-    (version "0.9.14.0")
+    (version "1.0.0")
     (source
      (origin
        (method git-fetch)
        (uri (git-reference
-             (url "https://github.com/drycpp/lmdbxx")
+             (url "https://github.com/hoytech/lmdbxx")
              (commit version)))
        (file-name (git-file-name name version))
        (sha256
-        (base32 "1jmb9wg2iqag6ps3z71bh72ymbcjrb6clwlkgrqf1sy80qwvlsn6"))))
+        (base32 "12k5rz74d1l0skcks9apry1svkl96g9lf5dcgylgjmh7v1jm0b7c"))))
     (arguments
      `(#:make-flags
        (list (string-append "PREFIX=" (assoc-ref %outputs "out")))
@@ -2455,7 +2455,7 @@  virtual address space — not physical RAM.")
          (delete 'configure))))
     (build-system gnu-build-system)
     (inputs `(("lmdb" ,lmdb)))
-    (home-page "http://lmdbxx.sourceforge.net")
+    (home-page "https://github.com/hoytech/lmdbxx")
     (synopsis "C++11 wrapper for the LMDB embedded B+ tree database library")
     (description "@code{lmdbxx} is a comprehensive @code{C++} wrapper for the
 @code{LMDB} embedded database library, offering both an error-checked
diff --git a/gnu/packages/messaging.scm b/gnu/packages/messaging.scm
index c356498fd2..b8bb172020 100644
--- a/gnu/packages/messaging.scm
+++ b/gnu/packages/messaging.scm
@@ -18,7 +18,7 @@ 
 ;;; Copyright © 2019 Tanguy Le Carrour <tanguy@bioneland.org>
 ;;; Copyright © 2019, 2020 Brett Gilio <brettg@gnu.org>
 ;;; Copyright © 2019, 2020 Timotej Lazar <timotej.lazar@araneo.si>
-;;; Copyright © 2020 Nicolò Balzarotti <nicolo@nixo.xyz>
+;;; Copyright © 2020, 2021 Nicolò Balzarotti <nicolo@nixo.xyz>
 ;;; Copyright © 2020 Vincent Legoll <vincent.legoll@gmail.com>
 ;;; Copyright © 2020 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2020 Reza Alizadeh Majd <r.majd@pantherx.org>
@@ -2250,7 +2250,7 @@  QMatrixClient project.")
 (define-public mtxclient
   (package
     (name "mtxclient")
-    (version "0.3.1")
+    (version "0.5.1")
     (source
      (origin
        (method git-fetch)
@@ -2259,7 +2259,7 @@  QMatrixClient project.")
              (commit (string-append "v" version))))
        (file-name (git-file-name name version))
        (sha256
-        (base32 "1dg4dq20g0ah62j5s3gpsxqq4ny7lxkxdxa9q6g54hdwkrb9ms7x"))))
+        (base32 "1xznfx2bhw0ahwmkxm0rs05vz05ijk5k4190rj6qp3bvb9byiajh"))))
     (arguments
      `(#:configure-flags
        (list
@@ -2272,12 +2272,6 @@  QMatrixClient project.")
              (substitute* "CMakeLists.txt"
                (("add_test\\((BasicConnectivity|ClientAPI|MediaAPI|Encryption|Pushrules)")
                 "# add_test"))
-             #t))
-         (add-before 'configure 'set-home
-           (lambda _
-             ;; Tries to create package registry file
-             ;; So, set HOME.
-             (setenv "HOME" "/tmp")
              #t)))))
     (build-system cmake-build-system)
     (inputs
@@ -2300,7 +2294,7 @@  for the Matrix protocol.  It is built on to of @code{Boost.Asio}.")
 (define-public nheko
   (package
     (name "nheko")
-    (version "0.7.2")
+    (version "0.8.2")
     (source
      (origin
        (method git-fetch)
@@ -2309,30 +2303,60 @@  for the Matrix protocol.  It is built on to of @code{Boost.Asio}.")
              (commit (string-append "v" version))))
        (file-name (git-file-name name version))
        (sha256
-        (base32 "1cbhgaf9klgxdirrxj571fqwspm0byl75c1xc40l727a6qswvp7s"))))
+        (base32 "0362hkbprc6jqlgmvzwxyvify4b1ldjakyqdz55m25xsypbpv2f3"))
+       (modules '((guix build utils)))
+       (snippet
+        '(begin
+           (delete-file-recursively "third_party")))))
     (arguments
      `(#:tests? #f                      ;no test target
        #:configure-flags
-       (list
-        "-DCMAKE_BUILD_TYPE=Release"
-        "-DCMAKE_CXX_FLAGS=-fpermissive")
+       '("-DCMAKE_BUILD_TYPE=Release"
+         "-DBUILD_DOCS=ON"
+         ;; Fix required because we are using a static SingleApplication
+         "-DCMAKE_CXX_FLAGS= \"-DQAPPLICATION_CLASS=QApplication\" "
+         ;; Compile Qml will make Nheko faster, but you will need to recompile
+         ;; it, when you update Qt.  That's fine for us.
+         "-DCOMPILE_QML=ON")
        #:phases
        (modify-phases %standard-phases
-         (add-after 'unpack 'remove-Werror
-           (lambda _
-             (substitute* "CMakeLists.txt"
-               (("-Werror") ""))
-             #t))
+         (add-after 'unpack 'unbundle-dependencies
+           (lambda* (#:key inputs #:allow-other-keys)
+             (let ((single-app (assoc-ref inputs "single-application")))
+               (substitute* "CMakeLists.txt"
+                 ;; Remove include and source dirs,replace with the correct one
+                 (("third_party/blurhash/blurhash.cpp") "")
+                 (("third_party/cpp-httplib-0.5.12")
+                  (string-append "\"" single-app "/include\""))
+                 (("add_subdirectory.*third_party/SingleApplication.*") "")
+                 ;; Link using the correct static/shared libs
+                 (("SingleApplication::SingleApplication")
+                  (string-append
+                   ;; Dynamic libraries
+                   "httplib" "\n" "blurhash" "\n"
+                   ;; Static library
+                   single-app "/lib/libSingleApplication.a"))))))
          (add-after 'unpack 'fix-determinism
            (lambda _
              ;; Make Qt deterministic.
-             (setenv "QT_RCC_SOURCE_DATE_OVERRIDE" "1")
-             #t)))))
+             (setenv "QT_RCC_SOURCE_DATE_OVERRIDE" "1")))
+         (add-after 'install 'wrap-program
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let ((out (assoc-ref outputs "out"))
+                   (gst-plugin-path (getenv "GST_PLUGIN_SYSTEM_PATH")))
+               (wrap-program (string-append out "/bin/nheko")
+                 `("GST_PLUGIN_SYSTEM_PATH" ":" prefix (,gst-plugin-path)))))))))
     (build-system qt-build-system)
     (inputs
      `(("boost" ,boost)
+       ("blurhash" ,blurhash)
+       ("cpp-httplib" ,cpp-httplib)
        ("cmark" ,cmark)
+       ("gst-plugins-base" ,gst-plugins-base)
+       ("gst-plugins-bad" ,gst-plugins-bad)   ; sdp & webrtc for voip
+       ("gst-plugins-good" ,gst-plugins-good) ; rtpmanager for voip
        ("json-modern-cxx" ,json-modern-cxx)
+       ("libnice" ,libnice)                   ; for voip
        ("libolm" ,libolm)
        ("lmdb" ,lmdb)
        ("lmdbxx" ,lmdbxx)
@@ -2340,15 +2364,18 @@  for the Matrix protocol.  It is built on to of @code{Boost.Asio}.")
        ("openssl" ,openssl)
        ("qtbase" ,qtbase)
        ("qtdeclarative" ,qtdeclarative)
+       ("qtkeychain" ,qtkeychain)
        ("qtgraphicaleffects" ,qtgraphicaleffects)
        ("qtmultimedia" ,qtmultimedia)
        ("qtquickcontrols2" ,qtquickcontrols2)
        ("qtsvg" ,qtsvg)
        ("spdlog" ,spdlog)
-       ("tweeny" ,tweeny)
+       ("single-application" ,single-application-qt5)
        ("zlib" ,zlib)))
     (native-inputs
-     `(("pkg-config" ,pkg-config)
+     `(("doxygen" ,doxygen)
+       ("graphviz" ,graphviz)
+       ("pkg-config" ,pkg-config)
        ("qtlinguist" ,qttools)))
     (home-page "https://github.com/Nheko-Reborn/nheko")
     (synopsis "Desktop client for Matrix using Qt and C++14")
@@ -2356,22 +2383,8 @@  for the Matrix protocol.  It is built on to of @code{Boost.Asio}.")
 Matrix protocol that feels more like a mainstream chat app and less like an IRC
 client.
 
-There is support for:
-@itemize
-@item E2E encryption (text messages only: attachments are currently sent unencrypted).
-@item User registration.
-@item Creating, joining & leaving rooms.
-@item Sending & receiving invites.
-@item Sending & receiving files and emoji.
-@item Typing notifications.
-@item Username auto-completion.
-@item Message & mention notifications.
-@item Redacting messages.
-@item Read receipts.
-@item Basic communities support.
-@item Room switcher (@key{ctrl-K}).
-@item Light, Dark & System themes.
-@end itemize")
+Many matrix features are supported, including user registration, rooms, typing
+notification, emojis, E2E encryption, and voip calls.")
     (license license:gpl3+)))
 
 (define-public quaternion
-- 
2.31.1