Subject: Re: [PATCH v4] gnu: Add remmina.

From: Leo Prikler <[email-masked]>
Date: Tue, 11 May 2021 08:02:49 +0200

Am Montag, den 10.05.2021, 19:33 -0400 schrieb Raghav Gururajan:
> * gnu/packages/remmina.scm: New file.
I don't think an extra file is needed here.  Remmina can fit quite well
to the other VNC clients.
> * gnu/packages/patches/remmina-plugin-path.patch: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES,dist_path_DATA): Adjust.
Adjust how?
> * gnu/packages/remmina.scm (remmina): New variable.
You already have remmina.scm in your ChangeLog.
> ---
>  gnu/local.mk                                  |   2 +
>  .../patches/remmina-plugin-path.patch         |  79 +++++++++
>  gnu/packages/remmina.scm                      | 158
> ++++++++++++++++++
>  3 files changed, 239 insertions(+)
>  create mode 100644 gnu/packages/patches/remmina-plugin-path.patch
>  create mode 100644 gnu/packages/remmina.scm
> 
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 37166bb2fc..3608f6e798 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -485,6 +485,7 @@ GNU_SYSTEM_MODULES =				
> \
>    %D%/packages/readline.scm			\
>    %D%/packages/rednotebook.scm			\
>    %D%/packages/regex.scm				\
> +  %D%/packages/remmina.scm                 \
>    %D%/packages/robotics.scm			\
>    %D%/packages/rpc.scm				\
>    %D%/packages/rrdtool.scm			\
> @@ -1643,6 +1644,7 @@ dist_patch_DATA =				
> 		\
>    %D%/packages/patches/rct-add-missing-headers.patch		\
>    %D%/packages/patches/readline-link-ncurses.patch		\
>    %D%/packages/patches/readline-6.2-CVE-2014-2524.patch		
> \
> +  %D%/packages/patches/remmina-plugin-path.patch                 \
>    %D%/packages/patches/renpy-use-system-fribidi.patch		
> \
>    %D%/packages/patches/reposurgeon-add-missing-docbook-files.patch	
> \
>    %D%/packages/patches/r-httpuv-1.5.5-unvendor-libuv.patch	\
> diff --git a/gnu/packages/patches/remmina-plugin-path.patch
> b/gnu/packages/patches/remmina-plugin-path.patch
> new file mode 100644
> index 0000000000..ce382be57a
> --- /dev/null
> +++ b/gnu/packages/patches/remmina-plugin-path.patch
> @@ -0,0 +1,79 @@
> +From e6e3c454e380bc1e55a719907df43f73f491fca2 Mon Sep 17 00:00:00
> 2001
> +From: Raghav Gururajan <[email-masked]>
> +Date: Mon, 10 May 2021 18:21:02 -0400
> +Subject: [PATCH] [PATCH]: Change path variable for loading plugins.
> +
> +Remmina is coded to load plugins from path
> (REMMINA_RUNTIME_PLUGINDIR)
> +provided during compile-time. This causes cycle between outputs
> 'out'
> +and 'plugins'. Therefore, code it to load plugins from path
> +(REMMINA_PLUGIN_PATH) provided during run-time.
> +---
> + CMakeLists.txt               |  3 ---
> + config.h.in                  |  1 -
> + src/remmina_plugin_manager.c | 10 ++++++----
> + 3 files changed, 6 insertions(+), 8 deletions(-)
> +
> +diff --git a/CMakeLists.txt b/CMakeLists.txt
> +index 17a58b04..acbbe440 100644
> +--- a/CMakeLists.txt
> ++++ b/CMakeLists.txt
> +@@ -313,9 +313,6 @@ if(NOT REMMINA_LOCALEDIR)
> + endif()
> + if(NOT REMMINA_PLUGINDIR)
> +   set(REMMINA_PLUGINDIR
> "${CMAKE_INSTALL_FULL_LIBDIR}/remmina/plugins")
> +-  if(NOT REMMINA_RUNTIME_PLUGINDIR)
> +-    set(REMMINA_RUNTIME_PLUGINDIR "${REMMINA_PLUGINDIR}")
> +-  endif()
> + endif()
> + if(NOT REMMINA_UIDIR)
> +   set(REMMINA_UIDIR "${REMMINA_DATADIR}/remmina/ui")
> +diff --git a/config.h.in b/config.h.in
> +index e59bf78a..8e8da940 100644
> +--- a/config.h.in
> ++++ b/config.h.in
> +@@ -22,7 +22,6 @@
> + 
> + #define REMMINA_RUNTIME_DATADIR			"${REMMINA_RUNT
> IME_DATADIR}"
> + #define REMMINA_RUNTIME_LOCALEDIR		"${REMMINA_RUNTIME_LOCA
> LEDIR}"
> +-#define REMMINA_RUNTIME_PLUGINDIR		"${REMMINA_RUNTIME_PLUG
> INDIR}"
> + #define REMMINA_RUNTIME_UIDIR			"${REMMINA_RUNT
> IME_UIDIR}"
> + #define REMMINA_RUNTIME_THEMEDIR		"${REMMINA_RUNTIME_THEM
> EDIR}"
> + #define REMMINA_RUNTIME_EXTERNAL_TOOLS_DIR	"${REMMINA_RUNTIME_EXTE
> RNAL_TOOLS_DIR}"
> +diff --git a/src/remmina_plugin_manager.c
> b/src/remmina_plugin_manager.c
> +index a1b713a8..74e2c0f0 100644
> +--- a/src/remmina_plugin_manager.c
> ++++ b/src/remmina_plugin_manager.c
> +@@ -294,7 +294,7 @@ void remmina_plugin_manager_init()
> + {
> + 	TRACE_CALL(__func__);
> + 	GDir *dir;
> +-	const gchar *name, *ptr;
> ++	const gchar *remmina_plugin_path, *name, *ptr;
> + 	gchar *fullpath;
> + 	RemminaPlugin *plugin;
> + 	RemminaSecretPlugin *sp;
> +@@ -312,8 +312,10 @@ void remmina_plugin_manager_init()
> + 		return;
> + 	}
> + 
> +-	g_print("Load modules from %s\n", REMMINA_RUNTIME_PLUGINDIR);
> +-	dir = g_dir_open(REMMINA_RUNTIME_PLUGINDIR, 0, NULL);
> ++	remmina_plugin_path = g_getenv("REMMINA_PLUGIN_PATH");
> ++
> ++	g_print("Load modules from %s\n", remmina_plugin_path);
> ++	dir = g_dir_open(remmina_plugin_path, 0, NULL);
I don't think this does, what you think it does.
"REMMINA_PLUGIN_PATH" is here interpreted as a single directory; usage
as a PATH variable is therefore dangerous.  (Consider a setting, where
REMINA_PLUGIN_PATH="guix-profile-a/...:guix-profile-b/...")

Also consider packages like Emacs, where the inclusion of their own
outputs in their path variables was dangerous.  (Although in this case,
it's probably sane, since you're using an extra output, but do verify
Remmina to work as intended without it.)
> + 
> + 	if (dir == NULL)
> + 		return;
> +@@ -323,7 +325,7 @@ void remmina_plugin_manager_init()
> + 		ptr++;
> + 		if (!remmina_plugin_manager_loader_supported(ptr))
> + 			continue;
> +-		fullpath = g_strdup_printf(REMMINA_RUNTIME_PLUGINDIR
> "/%s", name);
> ++		fullpath = g_strconcat(remmina_plugin_path, "/", name);
> + 		remmina_plugin_manager_load_plugin(fullpath);
> + 		g_free(fullpath);
> + 	}
> +-- 
> +2.31.1
> +
> diff --git a/gnu/packages/remmina.scm b/gnu/packages/remmina.scm
> new file mode 100644
> index 0000000000..9611ff2513
> --- /dev/null
> +++ b/gnu/packages/remmina.scm
> @@ -0,0 +1,158 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2021 Raghav Gururajan <[email-masked]>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify
> it
> +;;; under the terms of the GNU General Public License as published
> by
> +;;; the Free Software Foundation; either version 3 of the License,
> or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public
> License
> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>
> ;.
> +
> +(define-module (gnu packages remmina)
> +  #:use-module ((guix licenses) #:prefix license:)
> +  #:use-module (guix packages)
> +  #:use-module (guix git-download)
> +  #:use-module (guix build utils)
> +  #:use-module (guix build-system cmake)
> +  #:use-module (guix build-system glib-or-gtk)
> +  #:use-module (gnu packages)
> +  #:use-module (gnu packages avahi)
> +  #:use-module (gnu packages crypto)
> +  #:use-module (gnu packages cups)
> +  #:use-module (gnu packages freedesktop)
> +  #:use-module (gnu packages gettext)
> +  #:use-module (gnu packages glib)
> +  #:use-module (gnu packages gnome)
> +  #:use-module (gnu packages gnupg)
> +  #:use-module (gnu packages gtk)
> +  #:use-module (gnu packages pcre)
> +  #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages rdesktop)
> +  #:use-module (gnu packages spice)
> +  #:use-module (gnu packages ssh)
> +  #:use-module (gnu packages tls)
> +  #:use-module (gnu packages video)
> +  #:use-module (gnu packages vnc)
> +  #:use-module (gnu packages webkit)
> +  #:use-module (gnu packages xorg))
> +
> +(define-public remmina
> +  (package
> +    (name "remmina")
> +    (version "1.4.13")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri
> +        (git-reference
> +         (url "https://gitlab.com/Remmina/Remmina")
> +         (commit (string-append "v" version))))
> +       (file-name (git-file-name name version))
> +       (patches
> +        (search-patches
> +         ;; To honor REMMINA_PLUGIN_PATH.
> +         "remmina-plugin-path.patch"))
> +       (sha256
> +        (base32
> "1hc2r42hr34ksbxrybglw5caw0zjhwlybvb5c9s99dr761rmzr27"))))
> +    (build-system cmake-build-system)
> +    (outputs '("out" "plugins"))
> +    (arguments
> +     `(#:tests? #f                      ; No target
> +       #:configure-flags
> +       (list
> +        ;; Install plugins in separate output.
> +        (string-append "-DREMMINA_PLUGINDIR="
> +                       (assoc-ref %outputs "plugins")
> +                       "/lib/remmina/plugins")
> +        ;; Disable online version checking.
> +        "-DWITH_NEWS=OFF")
> +       #:imported-modules
> +       (,@%cmake-build-system-modules
> +        (guix build glib-or-gtk-build-system))
> +       #:modules
> +       ((guix build cmake-build-system)
> +        ((guix build glib-or-gtk-build-system)
> +         #:prefix glib-or-gtk:)
> +        (guix build utils))
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'install 'wrap-progs
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let ((out (assoc-ref outputs "out")))
> +               (for-each
> +                (lambda (name)
> +                  (let ((file (string-append out "/bin/" name))
> +                        (gi-typelib-path (getenv
> "GI_TYPELIB_PATH")))
> +                    (wrap-program file
> +                      `("GI_TYPELIB_PATH" ":" prefix (,gi-typelib-
> path)))))
> +                '("remmina" "remmina-file-wrapper")))
> +             #t))
> +         (add-after 'wrap-progs 'glib-or-gtk-compile-schemas
> +           (assoc-ref glib-or-gtk:%standard-phases 'glib-or-gtk-
> compile-schemas))
> +         (add-after 'glib-or-gtk-compile-schemas 'glib-or-gtk-wrap
> +           (assoc-ref glib-or-gtk:%standard-phases 'glib-or-gtk-
> wrap)))))
> +    (native-inputs
> +     `(("gettext" ,gettext-minimal)
> +       ("glib:bin" ,glib "bin")
> +       ("gobject-introspection" ,gobject-introspection)
> +       ("gtk+:bin" ,gtk+ "bin")
> +       ("intl" ,intltool)
> +       ("pkg-config" ,pkg-config)))
> +    (inputs
> +     `(("app-indicator" ,libappindicator)
> +       ("atk" ,atk)
> +       ("avahi" ,avahi)
> +       ("cairo" ,cairo)
> +       ("cups" ,cups)
> +       ("ffmpeg" ,ffmpeg)
> +       ("freerdp" ,freerdp)             ; for rdp plugin
> +       ("gcrypt" ,libgcrypt)
> +       ("gdk-pixbuf" ,gdk-pixbuf+svg)
> +       ("glib" ,glib)
> +       ("gnome-keyring" ,gnome-keyring)
> +       ("gsettings-desktop-schemas" ,gsettings-desktop-schemas)
> +       ("gtk+" ,gtk+)
> +       ("harfbuzz" ,harfbuzz)
> +       ("json-glib" ,json-glib)
> +       ("libsecret" ,libsecret)         ; for secret plugin
> +       ("libsoup" ,libsoup)
> +       ("libssh" ,libssh)               ; for ssh plugin
> +       ("libvnc" ,libvnc)               ; for vnc plugin
> +       ("openssl" ,openssl)
> +       ("pango" ,pango)
> +       ("pcre" ,pcre)
> +       ("shared-mime-info" ,shared-mime-info)
> +       ("sodium" ,libsodium)
> +       ("spice-client-gtk" ,spice-gtk)  ; for spice plugin
> +       ("telepathy" ,telepathy-glib)    ; for telepathy plugin
> +       ("vte" ,vte)
> +       ("wayland" ,wayland)
> +       ("webkitgtk" ,webkitgtk)         ; for www plugin
> +       ("x11" ,libx11)
> +       ("xext" ,libxext)                ; for xdmcp plugin
> +       ("xdg-utils" ,xdg-utils)
> +       ("xkbfile" ,libxkbfile)))        ; for nx plugin
> +    (propagated-inputs
> +     `(("dconf" ,dconf)))
> +    (native-search-paths
> +     (list
> +      ;; To load plugins when installed via 'plugins' output.
> +      (search-path-specification
> +       (variable "REMMINA_PLUGIN_PATH")
> +       (files
> +        (list
> +         "lib/remmina/plugins")))))
> +    (search-paths native-search-paths)
> +    (home-page "https://remmina.org/")
> +    (synopsis "Remote Desktop Client")
> +    (description "Remmina is a client to use other desktops
> remotely.
> +RDP, VNC, SPICE, NX, XDMCP, SSH and EXEC network protocols are
> supported.")
> +    (license license:gpl2+)))