Re: [PATCH 1/2] gnu: Add freerdp.

[Ricardo Wurmus <rekado@elephly.net>]

Hi Thomas,

> * gnu/packages/rdesktop.scm (freerdp): New variable.
> ---
>  gnu/packages/rdesktop.scm | 71 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/rdesktop.scm b/gnu/packages/rdesktop.scm
> index e40852b..14965e1 100644
> --- a/gnu/packages/rdesktop.scm
> +++ b/gnu/packages/rdesktop.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2015 Ludovic Courtès <ludo@gnu.org>
> +;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -20,9 +21,19 @@
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix packages)
>    #:use-module (guix download)
> +  #:use-module (guix git-download)
> +  #:use-module (guix build-system cmake)
>    #:use-module (guix build-system gnu)
> +  #:use-module (gnu packages compression)
> +  #:use-module (gnu packages cups)
> +  #:use-module (gnu packages gstreamer)
> +  #:use-module (gnu packages linux)
> +  #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages pulseaudio)
>    #:use-module (gnu packages tls)
> -  #:use-module (gnu packages xorg))
> +  #:use-module (gnu packages video)
> +  #:use-module (gnu packages xorg)
> +  #:use-module (gnu packages xml))
>
>  (define-public rdesktop
>    (package
> @@ -57,3 +68,61 @@
>  capable of natively speaking Remote Desktop Protocol (RDP).  It allows users
>  to remotely control a user's Windows desktop.")
>      (license license:gpl3+)))
> +
> +(define-public freerdp
> +  (package
> +    (name "freerdp")
> +    (version "1.2.0-beta1+android9")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "git://github.com/FreeRDP/FreeRDP.git")
> +                    (commit version)))
> +              (file-name (git-file-name name version))
> +              (sha256
> +               (base32 "1m0lzrr7hkxfvc5f9p8snimv0rmin2463zhg25mv36wig8g5k7l3"))))
> +    (build-system cmake-build-system)
> +    (native-inputs
> +     `(("pkg-config" ,pkg-config)
> +       ("xmlto" ,xmlto)))
> +    (inputs
> +     `(("libx11" ,libx11)
> +       ("libxkbfile" ,libxkbfile)
> +       ("libxcursor" ,libxcursor)
> +       ("libxext" ,libxext)
> +       ("libxi" ,libxi)
> +       ("libxv" ,libxv)
> +       ("libxrandr" ,libxrandr)
> +       ("libxrender" ,libxrender)
> +       ("libxinerama" ,libxinerama)
> +       ("libxshmfence" ,libxshmfence)
> +       ("libxml2" ,libxml2)
> +       ("libxslt" ,libxslt)
> +       ("cups" ,cups)
> +       ("ffmpeg" ,ffmpeg)
> +       ("pulseaudio" ,pulseaudio)
> +       ("alsa-lib" ,alsa-lib)
> +       ("gstreamer" ,gstreamer)
> +       ("gst-plugins-base" ,gst-plugins-base)
> +       ("zlib" ,zlib)
> +       ("openssl" ,openssl)))
> +    (arguments
> +     `(#:configure-flags
> +       '("-DCMAKE_INSTALL_LIBDIR=lib"
> +         "-DWITH_PULSE=ON"
> +         "-DWITH_CUPS=ON" "-DWITH_SSE2=ON")

Is WITH_SSE2 used to ensure that this won’t use uncommon CPU features?

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'configure 'patch-cmakelists
> +           (lambda _
> +             (substitute* "channels/client/CMakeLists.txt"
> +               (("list\\(REMOVE_DUPLICATES CHANNEL_STATIC_CLIENT_ENTRIES\\)")
> +                "")))))

Please add a comment that explains why this is needed.  Also make sure
to end the phase with “#t” because “substitute*” does not (yet) return a
truthy value.

> +       #:tests? #f)) ; no 'test' target

Actually, the usual target is “check” :)  The comment should probably
either say “no tests” or “no ’check’ target”.  But this is nitpicking.

> +    (home-page "https://www.freerdp.com")
> +    (synopsis "Remote Desktop Protocol implementation")
> +    (description "FreeRDP implements Microsoft's Remote Desktop Protocol.  It
> +consists of the @code{xfreerdp} client, libraries for client and server
> +functionality, and Windows Portable Runtime (WinPR), a portable implementation
> +of parts of the Windows API.")
> +    (license license:asl2.0)))

Okay with the above changes.  Thanks!

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net