[bug#45721] Telegram Desktop (v9)

[Leo Prikler]

Hi Raghav,

congratulations on getting a working Telegram Desktop.  I haven't yet
built this version on my own, but I want to comment on the patches a
little.

Am Freitag, den 15.01.2021, 11:08 -0500 schrieb Raghav Gururajan:
> * gnu/packages/cpp.scm (gsl): New variable.
> * gnu/packages/patches/gsl-gtest.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
LGTM, but there seem to be whitespace issues.  Any idea?

>  (define-module (gnu packages fcitx)
> -  #:use-module ((guix licenses) #:select (gpl2+))
> +  #:use-module (guix licenses)
Personal nitpick, but those should likely be prefixed with license:
To keep backwards-compatibility, you can (define gpl2+ license:gpl2+)
inside the module.

> * gnu/packages/language.scm (libchewing): New variable.
LGTM.

> +         (add-after 'unpack 'patch-std
> +           (lambda _
> +             (substitute* '("configure" "Makefile")
> +               (("17")
> +                "11"))
> +             #t))
Is there a less broad way of doing this?  E.g. replacing c++-17 by c++-
11?

> +        (git-reference
> +         (url "https://github.com/hamonikr/nimf.git";)
> +         (commit
> +          (string-append "nimf-" version))))
FWIW there seems to also exist an older version over at
https://gitlab.com/nimf-i18n/nimf
Would it be worth packaging that?

> +    (synopsis "Korean input method framework")
> +    (description "Nimf is a lightweight, fast and extensible input
> method
> +framework.")
In my opinion the synopsis should be "Lightweight input method
framework" and the description should mention, that this specific fork
has a special focus on Korean.

> * gnu/packages/cmake.scm (cmake-shared): New variable.
LGTM, albeit admittedly weird.

> +    (synopsis "Material Decoration for Qt")
> +    (description "MaterialDecoration is the client-side decoration
> for Qt
> +applications on Wayland.")
It's not "the", just "a".  Usually such projects describe Material
Design in some way, but apparently it has come to a point where just
throwing around the word "Material" is enough.

> +    (description "Range-v3 is the range library for
> C++14/17/20.  This code was
> +the basis of a formal proposal to add range support to the C++
> standard library.")
I find the following more useful:
> [range-v3 is] an extension of the Standard Template Library that
> makes its iterators and algorithms more powerful by making them
> composable.  Unlike other range-like solutions which, seek to do away
> with iterators, in range-v3 ranges are an abstration layer on top of
> iterators.

> +    (license
> +     (list
> +      ;; Dual-Licensed
> +      license:expat
> +      license:ncsa))))
It appears, that this library carries a few more (free) licenses with
it.  Perhaps this needs to be revised?

> * gnu/packages/cpp.scm (rlottie): New variable.
LGTM.

> * gnu/packages/qt.scm (qt5ct): New variable.
LGTM.

> +(define-public tg_owt
> +  (package
> +    (name "tg_owt")
> +    (version "0.0.0")
Use a proper version.  Packages, that build directly from git without
any tagged versions usually have a preamble of 
  (let ((commit <hash>) 
        (revision <number))
    (package ...))

> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri
> +        (git-reference
> +         (url "https://github.com/desktop-app/tg_owt.git";)
> +         (commit "fa86fcc")
> +         (recursive? #t)))
Is there a way of making this checkout non-recursive?  I know you've
made that change in accordance to an upstream recommendation, but one
ought to look at it a little closer. 

> +    (inputs
> +     `(("abseil-cpp" ,abseil-cpp)
> +       ("alsa" ,alsa-lib)
> +       ("ffmpeg" ,ffmpeg)
> +       ("libjpeg" ,libjpeg-turbo)
> +       ("libsrtp" ,libsrtp)
> +       ("libvpx" ,libvpx)
> +       ;; ("libyuv" ,libyuv)
> +       ("openh264" ,openh264)
> +       ("openssl" ,openssl)
> +       ("opus" ,opus)
> +       ("protobuf" ,protobuf)
> +       ("pulseaudio" ,pulseaudio)
> +       ("rnnoise" ,rnnoise)))
It seems that some of those inputs are also found as third_party/
libraries.  Can you remove their respective sources from the package?

> +    (synopsis "WebRTC build for Telegram")
> +    (description "TG_OWT is the packaged build of WebRTC, for its
> use in
> +Telegram-Desktop application.")
I really don't like that synopsis and description.  Granted, upstream
offers little to work with, but there ought to be a better way of
phrasing this.

By the way, it would appear we already have some WebRTC stuff packaged,
but no direct "webrtc" package (which I guess is normal, given that it
is a protocol and not an individual piece of software).  How tightly is
Telegram coupled to this specific implementation?  Would there be a way
of replacing it with something else (kinda like our udev/eudev
situation)?

> * gnu/packages/telegram.scm: New module.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> * gnu/packages/telegram.scm (tdesktop): New variable.
Would there be a way of moving this into another module, e.g. (gnu
packages messaging)?

Regards,
Leo