[bug#42229,8/9] gnu: jami: Fix compilation by providing necessary webchat files.

* gnu/packages/jami.scm (define-module): Add copy-build-system.
(libringclient)[source]: Delete chatview files.
[arguments]: Remove chatview files from CMakeLists.txt.
(jami-chatview): New variable.
(jami)[source]: Delete chatview directory.
[inputs]: Add jami-chatview.
[arguments]: Copu chatview files to destination.
---
 gnu/packages/jami.scm | 80 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

Jan Wielkiewicz <tona_kosmicznego_smiecia@interia.pl> writes:

> * gnu/packages/jami.scm (define-module): Add copy-build-system.
> (libringclient)[source]: Delete chatview files.
> [arguments]: Remove chatview files from CMakeLists.txt.
> (jami-chatview): New variable.
> (jami)[source]: Delete chatview directory.
> [inputs]: Add jami-chatview.
> [arguments]: Copu chatview files to destination.
> ---
>  gnu/packages/jami.scm | 80 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/jami.scm b/gnu/packages/jami.scm
> index ea94efb2f9..58e10ae6d8 100644
> --- a/gnu/packages/jami.scm
> +++ b/gnu/packages/jami.scm
> @@ -56,6 +56,7 @@
>    #:use-module (gnu packages xorg)
>    #:use-module (gnu packages)
>    #:use-module (guix build-system cmake)
> +  #:use-module (guix build-system copy)
>    #:use-module (guix build-system gnu)
>    #:use-module (guix download)
>    #:use-module (guix git-download)
> @@ -526,6 +527,16 @@ functionality.")
>                        (url "https://review.jami.net/ring-lrc")
>                        (commit commit)))
>                  (file-name (git-file-name name version))
> +                (modules '((guix build utils)))
> +                ;; The chatview is a common code among Jami clients.
> +                ;; Since Guix likes functional design, I'm removing
> +                ;; the directory containing it and making it a new
> +                ;; package. The other reason is that there is no
> +                ;; any rule in the build system to check if it is
> +                ;; included.

I'd perhaps just say instead that these files are packaged as
jami-chatview which is used by the jami package, that's probably what's
useful to know here.

> +                (snippet
> +                 '(begin
> +                    (delete-file-recursively "src/web-chatview")))
>                  (sha256
>                   (base32
>                    "1s1wi80drabfjhvxbiyl8k9c2jsafvmx5v84s3wldsmmdg4f6xdf"))))
> @@ -542,6 +553,21 @@ functionality.")
>                                (assoc-ref %build-inputs "libring") "/include"))
>           #:phases
>           (modify-phases %standard-phases
> +           (add-after 'unpack 'do-not-install-chatview
> +             (lambda _
> +               (substitute* "CMakeLists.txt"
> +                 (("src/web-chatview/.eslintrc.json") "")
> +                 (("src/web-chatview/chatview-gnome.css") "")
> +                 (("src/web-chatview/chatview.css") "")
> +                 (("src/web-chatview/chatview.html") "")
> +                 (("src/web-chatview/chatview.js") "")
> +                 (("src/web-chatview/jed.js") "")
> +                 (("src/web-chatview/linkify-html.js") "")
> +                 (("src/web-chatview/linkify-string.js") "")
> +                 (("src/web-chatview/linkify.js") "")
> +                 (("src/web-chatview/qwebchannel.js") "")
> +                 (("src/web-chatview/web.gresource.xml") ""))

Because substitute* uses regular expressions, characters like . should
probably be escaped. However, you might want to try something like the
following:

               (substitute* "CMakeLists.txt"
                 (("src/web-chatview/.*") ""))


> +               #t))
>             (add-before 'configure 'fix-dbus-interfaces-path
>               (lambda* (#:key inputs #:allow-other-keys)
>                 (substitute* "CMakeLists.txt"
> @@ -558,6 +584,40 @@ This package provides a library common to all Jami clients.")
>        (home-page "https://jami.net")
>        (license license:gpl3+))))
>  
> +(define-public jami-chatview
> +  (let ((commit "0a790d86a20ae80e5d2ee0d35452fa77cfb8cc40")
> +        (revision "0"))
> +    (package
> +      (name "jami-chatview")
> +      (version (git-version %jami-version revision commit))
> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference
> +                      (url "https://review.jami.net/jami-chatview")
> +                      (commit commit)))
> +                (file-name (git-file-name name version))
> +                (sha256
> +                 (base32
> +                  "1zb3qa1x37kclbgk4wdg6qp4ivr6513ypj8s4k0qjps7h2clgna8"))))
> +      (build-system copy-build-system)
> +      (arguments
> +       '(#:phases
> +         (modify-phases %standard-phases
> +           (add-after 'unpack 'allow-copying
> +             (lambda* (#:key inputs #:allow-other-keys)
> +               ;; Permissions of this file are not right.
> +               ;; Let's change this to allow installing it properly.
> +               (chmod ".eslintrc.json" #o644)
> +               #t)))
> +         #:install-plan '(("." "include/libringclient/web-chatview")
> +                          (".eslintrc.json"
> +                           "include/libringclient/web-chatview/.eslintrc.json"))))
> +      (synopsis "Chatview for Jami")
> +      (description "Web chatview for Jami - html, css, js files.
> +It provides some common code for Jami clients.")

It would be good if this could be clearer as to what the user gets if
they install this package. I'm guessing because it's an input to Jami,
it's probably not useful to have it in your profile?

Also, if it's going to describe the contents of the package, I'd do it
in a full sentance, so something like: "This package provides the HTML,
CSS and JavaScript files for the web chatview for Jami".

> +      (home-page "https://jami.net")
> +      (license license:gpl3+))))

I think there's some Expat stuff in there too.

> +
>  (define-public jami
>    (let ((commit "85cda2e1fde84230f6b9f5419f9ec1e23867c2a1")
>          (revision "0"))
> @@ -570,6 +630,11 @@ This package provides a library common to all Jami clients.")
>                        (url "https://review.jami.net/ring-client-gnome")
>                        (commit commit)))
>                  (file-name (git-file-name name version))
> +                (modules '((guix build utils)))
> +                ;; The web directory contains links to nowhere
> +                (snippet
> +                 '(begin
> +                    (delete-file-recursively "web")))
>                  (sha256
>                   (base32
>                    "0xzcx8x04yc8m8b2vf7sxfgw79idbm0hifzaw4s2vh26hyy9sp2g"))))
> @@ -584,6 +649,8 @@ This package provides a library common to all Jami clients.")
>           ("clutter-gtk" ,clutter-gtk)
>           ("libcanberra" ,libcanberra)
>           ("libnm" ,network-manager)
> +         ("jami-chatview" ,jami-chatview)
> +         ("qtbase" ,qtbase)
>           ("webkitgtk" ,webkitgtk)
>           ("sqlite" ,sqlite)))
>        (native-inputs
> @@ -596,7 +663,18 @@ This package provides a library common to all Jami clients.")
>           ("adwaita-icon-theme" ,adwaita-icon-theme)
>           ("evolution-data-server" ,evolution-data-server)))
>        (arguments
> -       `(#:tests? #f)) ; There is no testsuite.
> +       `(#:tests? #f ; There is no testsuite.
> +         #:phases
> +         (modify-phases %standard-phases
> +           (add-after 'unpack 'copy-chatview-files
> +             (lambda* (#:key inputs #:allow-other-keys)
> +               (let ((chatview
> +                      (string-append
> +                       (assoc-ref inputs "jami-chatview")
> +                       "/include/libringclient/web-chatview"))
> +                     (destination-dir "web"))
> +                 (copy-recursively chatview destination-dir))

Rather than copying, could this be a symlink? That way you're just using
another store item, rather than copying the files from one to another.

> +               #t)))))
>        (synopsis "Distributed, privacy-respecting communication program")
>        (description "Jami (formerly GNU Ring) is a secure and distributed voice,
>  video and chat communication platform that requires no centralized server and

Hello.

On Mon, 06 Jul 2020 22:19:42 +0100
Christopher Baines <mail@cbaines.net> wrote:

> I'd perhaps just say instead that these files are packaged as
> jami-chatview which is used by the jami package, that's probably
> what's useful to know here.
Done.

> 
> Because substitute* uses regular expressions, characters like . should
> probably be escaped. However, you might want to try something like the
> following:
> 
>                (substitute* "CMakeLists.txt"
>                  (("src/web-chatview/.*") ""))
> 
I use to forget this, don't know why. Testing how it works right now.

> 
> It would be good if this could be clearer as to what the user gets if
> they install this package. I'm guessing because it's an input to Jami,
> it's probably not useful to have it in your profile?
Users don't have to know this. I made it private by changing
define-public to define.

> Also, if it's going to describe the contents of the package, I'd do it
> in a full sentance, so something like: "This package provides the
> HTML, CSS and JavaScript files for the web chatview for Jami".
Done.

> I think there's some Expat stuff in there too.
Yes, there is, added expat to license list. Shouldn't Jami files
contain explicit license statement though?

> 
> Rather than copying, could this be a symlink? That way you're just
> using another store item, rather than copying the files from one to
> another.
Working on this. The build fails on the ".eslintrc.json". Guix for some
reason doesn't like dotfiles.



Jan Wielkiewicz

I need some more time for this one. Some features seem to be broken
(despite dropping the qtcore commit).


Jan Wielkiewicz

Jan Wielkiewicz <tona_kosmicznego_smiecia@interia.pl> writes:

>> I think there's some Expat stuff in there too.
> Yes, there is, added expat to license list. Shouldn't Jami files
> contain explicit license statement though?

I'm not sure what you mean by explicit license statement, adding expat
to the licenses list was all I had in mind.

>> Rather than copying, could this be a symlink? That way you're just
>> using another store item, rather than copying the files from one to
>> another.
>
> Working on this. The build fails on the ".eslintrc.json". Guix for some
> reason doesn't like dotfiles.

Ok, sounds like the .eslintrc.json file can be deleted at build time, as
I think it's configuration for a linter. I'm not sure if that'll help or
not, but it might.

Thanks,

Chris

Hello Jan!

Jan Wielkiewicz <tona_kosmicznego_smiecia@interia.pl> writes:

> * gnu/packages/jami.scm (define-module): Add copy-build-system.
> (libringclient)[source]: Delete chatview files.
> [arguments]: Remove chatview files from CMakeLists.txt.
> (jami-chatview): New variable.
> (jami)[source]: Delete chatview directory.
> [inputs]: Add jami-chatview.
> [arguments]: Copu chatview files to destination.
> ---
>  gnu/packages/jami.scm | 80 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/jami.scm b/gnu/packages/jami.scm
> index ea94efb2f9..58e10ae6d8 100644
> --- a/gnu/packages/jami.scm
> +++ b/gnu/packages/jami.scm
> @@ -56,6 +56,7 @@
>    #:use-module (gnu packages xorg)
>    #:use-module (gnu packages)
>    #:use-module (guix build-system cmake)
> +  #:use-module (guix build-system copy)
>    #:use-module (guix build-system gnu)
>    #:use-module (guix download)
>    #:use-module (guix git-download)
> @@ -526,6 +527,16 @@ functionality.")
>                        (url "https://review.jami.net/ring-lrc")
>                        (commit commit)))
>                  (file-name (git-file-name name version))
> +                (modules '((guix build utils)))
> +                ;; The chatview is a common code among Jami clients.
> +                ;; Since Guix likes functional design, I'm removing
> +                ;; the directory containing it and making it a new
> +                ;; package. The other reason is that there is no
> +                ;; any rule in the build system to check if it is
> +                ;; included.
> +                (snippet
> +                 '(begin
> +                    (delete-file-recursively "src/web-chatview")))
>                  (sha256
>                   (base32
>                    "1s1wi80drabfjhvxbiyl8k9c2jsafvmx5v84s3wldsmmdg4f6xdf"))))
> @@ -542,6 +553,21 @@ functionality.")
>                                (assoc-ref %build-inputs "libring") "/include"))
>           #:phases
>           (modify-phases %standard-phases
> +           (add-after 'unpack 'do-not-install-chatview
> +             (lambda _
> +               (substitute* "CMakeLists.txt"
> +                 (("src/web-chatview/.eslintrc.json") "")
> +                 (("src/web-chatview/chatview-gnome.css") "")
> +                 (("src/web-chatview/chatview.css") "")
> +                 (("src/web-chatview/chatview.html") "")
> +                 (("src/web-chatview/chatview.js") "")
> +                 (("src/web-chatview/jed.js") "")
> +                 (("src/web-chatview/linkify-html.js") "")
> +                 (("src/web-chatview/linkify-string.js") "")
> +                 (("src/web-chatview/linkify.js") "")
> +                 (("src/web-chatview/qwebchannel.js") "")
> +                 (("src/web-chatview/web.gresource.xml") ""))
> +               #t))
>             (add-before 'configure 'fix-dbus-interfaces-path
>               (lambda* (#:key inputs #:allow-other-keys)
>                 (substitute* "CMakeLists.txt"
> @@ -558,6 +584,40 @@ This package provides a library common to all Jami clients.")
>        (home-page "https://jami.net")
>        (license license:gpl3+))))
>  
> +(define-public jami-chatview
> +  (let ((commit "0a790d86a20ae80e5d2ee0d35452fa77cfb8cc40")
> +        (revision "0"))
> +    (package
> +      (name "jami-chatview")
> +      (version (git-version %jami-version revision commit))
> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference
> +                      (url "https://review.jami.net/jami-chatview")
> +                      (commit commit)))

This doesn't seem to be the canonical place to get the latest chatview.
Rather, it lives in the LRC repo [0]. It was copied there sometime in 2019,
and no new commit were made to the repo you used above since.

[0]  https://git.jami.net/savoirfairelinux/ring-lrc/commits/master/src/web-chatview

> +                (file-name (git-file-name name version))
> +                (sha256
> +                 (base32
> +                  "1zb3qa1x37kclbgk4wdg6qp4ivr6513ypj8s4k0qjps7h2clgna8"))))
> +      (build-system copy-build-system)
> +      (arguments
> +       '(#:phases
> +         (modify-phases %standard-phases
> +           (add-after 'unpack 'allow-copying
> +             (lambda* (#:key inputs #:allow-other-keys)
> +               ;; Permissions of this file are not right.
> +               ;; Let's change this to allow installing it properly.
> +               (chmod ".eslintrc.json" #o644)
> +               #t)))
> +         #:install-plan '(("." "include/libringclient/web-chatview")
> +                          (".eslintrc.json"
> +                           "include/libringclient/web-chatview/.eslintrc.json"))))
> +      (synopsis "Chatview for Jami")
> +      (description "Web chatview for Jami - html, css, js files.
> +It provides some common code for Jami clients.")
> +      (home-page "https://jami.net")
> +      (license license:gpl3+))))
> +
>  (define-public jami
>    (let ((commit "85cda2e1fde84230f6b9f5419f9ec1e23867c2a1")
>          (revision "0"))
> @@ -570,6 +630,11 @@ This package provides a library common to all Jami clients.")
>                        (url "https://review.jami.net/ring-client-gnome")
>                        (commit commit)))
>                  (file-name (git-file-name name version))
> +                (modules '((guix build utils)))
> +                ;; The web directory contains links to nowhere
> +                (snippet
> +                 '(begin
> +                    (delete-file-recursively "web")))
>                  (sha256
>                   (base32
>                    "0xzcx8x04yc8m8b2vf7sxfgw79idbm0hifzaw4s2vh26hyy9sp2g"))))
> @@ -584,6 +649,8 @@ This package provides a library common to all Jami clients.")
>           ("clutter-gtk" ,clutter-gtk)
>           ("libcanberra" ,libcanberra)
>           ("libnm" ,network-manager)
> +         ("jami-chatview" ,jami-chatview)
> +         ("qtbase" ,qtbase)
>           ("webkitgtk" ,webkitgtk)
>           ("sqlite" ,sqlite)))
>        (native-inputs
> @@ -596,7 +663,18 @@ This package provides a library common to all Jami clients.")
>           ("adwaita-icon-theme" ,adwaita-icon-theme)
>           ("evolution-data-server" ,evolution-data-server)))
>        (arguments
> -       `(#:tests? #f)) ; There is no testsuite.
> +       `(#:tests? #f ; There is no testsuite.
> +         #:phases
> +         (modify-phases %standard-phases
> +           (add-after 'unpack 'copy-chatview-files
> +             (lambda* (#:key inputs #:allow-other-keys)
> +               (let ((chatview
> +                      (string-append
> +                       (assoc-ref inputs "jami-chatview")
> +                       "/include/libringclient/web-chatview"))
> +                     (destination-dir "web"))
> +                 (copy-recursively chatview destination-dir))
> +               #t)))))
>        (synopsis "Distributed, privacy-respecting communication program")
>        (description "Jami (formerly GNU Ring) is a secure and distributed voice,
>  video and chat communication platform that requires no centralized server and

As I understand it, jami-chatview is *not* maintained as a distinct
component upstream, hence it makes little sense to maintain it as one in
Guix: it'd burden us with maintaining it, with little to no gain.  IIUC,
all we need is to symlink the web-chatview from the lrc package, which
every Jami clients will already depend on.

The suggestion I'd make to upstream is to *install* those files as part
of a LRC installation (somewhere under share/), and then have the build
system check for their presence at configure time and use them directly.

Does that make sense?

Maxim

Dnia 2020-08-04, o godz. 00:40:25
maxim.cournoyer@gmail.com napisał(a):

> Hello Jan!

Hi!

> This doesn't seem to be the canonical place to get the latest
> chatview. Rather, it lives in the LRC repo [0]. It was copied there
> sometime in 2019, and no new commit were made to the repo you used
> above since.
Didn't notice that, thanks. I was too busy looking for the source of
the pjproject bug.

> [0]
> https://git.jami.net/savoirfairelinux/ring-lrc/commits/master/src/web-chatview
> 
> As I understand it, jami-chatview is *not* maintained as a distinct
> component upstream, hence it makes little sense to maintain it as one
> in Guix: it'd burden us with maintaining it, with little to no gain.
> IIUC, all we need is to symlink the web-chatview from the lrc
> package, which every Jami clients will already depend on.
I was hoping Jami developers will make it a separate package or at
least add some CMake rules to install it properly, but they're busy
doing other things. The problem is it is file when installing from
a tarball, but libringclient fetched from git only contains symlinks in
./webchat pointing to a higher directory "../../" - this doesn't make
sense outside of the context of tarball or source fetched to right
directories.

> The suggestion I'd make to upstream is to *install* those files as
> part of a LRC installation (somewhere under share/), and then have
> the build system check for their presence at configure time and use
> them directly.
> 
> Does that make sense?
Yes. I'm lazily refactoring my code allowing fetching from git - I made
a diff of directories of Jami built from tarball and from git and I
exactly know where to place webchat. I'll just fix the libringclient
package.

> Maxim

Thanks
Jan Wielkiewicz