Re: [PATCH] gnu: awesome: Update to 3.5.9.

[ng0]

Hi,

Thanks for your patches.
It works, I'm currently using this patchset.

Some minor improvements can be made (comments style) which in my opinion
the person commiting your patch can apply.

awesome v3.5.9 (Mighty Ravendark)
• Build: Sun Mar 06 14:05:54Z 2016 for x86_64 by gcc version 4.9.3 (@)
• Compiled against Lua 5.2.4 (running with Lua 5.2)
• D-Bus support: ✔
   
doncatnip <address@hidden> writes:

> Hey Guix !
>
> Let's try this again.
> From 99ac37565b7a4826c9ca3c9f0545825924effc3d Mon Sep 17 00:00:00 2001
> From: doncatnip <address@hidden>
> Date: Thu, 1 Sep 2016 23:08:28 +0200
> Subject: [PATCH 1/3] gnu: lua: Remove conflicting flag, pass MYCFLAGS
>
> * gnu/packages/lua.scm (lua)[arguments]: Use MYCFLAGS instead of
> CFLAGS and remove conflicting -DLUA_USE_POSIX since -DLUA_USE_LINUX
> is passed implicitly for build target "linux".
> ---
>  gnu/packages/lua.scm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gnu/packages/lua.scm b/gnu/packages/lua.scm
> index 8bd67c5..9f19cc9 100644
> --- a/gnu/packages/lua.scm
> +++ b/gnu/packages/lua.scm
> @@ -51,7 +51,7 @@
>                    (srfi srfi-1))
>         #:test-target "test"
>         #:make-flags
> -       '("CFLAGS=-fPIC -DLUA_DL_DLOPEN -DLUA_USE_POSIX"
> +       '("MYCFLAGS=-fPIC -DLUA_DL_DLOPEN"
>           "linux")
>         #:phases
>         (modify-phases %standard-phases
> -- 
> 2.9.3

Looks good to me.

> From 57a998f52e2d77e758ff339d03f26556b3b9c63f Mon Sep 17 00:00:00 2001
> From: doncatnip <address@hidden>
> Date: Thu, 1 Sep 2016 23:53:32 +0200
> Subject: [PATCH 2/3] gnu: lua: Add lua-lgi.
>
> * gnu/packages/lua.scm (lua-lgi): New variable.
> ---
>  gnu/packages/lua.scm | 77 
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 76 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/lua.scm b/gnu/packages/lua.scm
> index 9f19cc9..cdc28b0 100644
> --- a/gnu/packages/lua.scm
> +++ b/gnu/packages/lua.scm
> @@ -5,6 +5,7 @@
>  ;;; Copyright © 2014 Andreas Enge <address@hidden>
>  ;;; Copyright © 2016 Efraim Flashner <address@hidden>
>  ;;; Copyright © 2016 Ricardo Wurmus <address@hidden>
> +;;; Copyright © 2016 doncatnip <address@hidden>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -29,7 +30,12 @@
>    #:use-module (gnu packages)
>    #:use-module (gnu packages readline)
>    #:use-module (gnu packages tls)
> -  #:use-module (gnu packages xml))
> +  #:use-module (gnu packages xml)
> +  #:use-module (gnu packages glib)
> +  #:use-module (gnu packages libffi)
> +  #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages xorg)
> +  #:use-module (gnu packages gtk))
>  
>  (define-public lua
>    (package
> @@ -259,3 +265,72 @@ directory structure and file attributes.")
>  communication.  It takes an already established TCP connection and creates a
>  secure session between the peers.")
>      (license (package-license lua-5.1))))
> +
> +(define-public lua-lgi
> +  (package
> +    (name "lua-lgi")
> +    (version "0.9.1")
> +    (source
> +      (origin
> +        (method url-fetch)
> +        (uri (string-append
> +              "https://github.com/pavouk/lgi/archive/";
> +              version ".tar.gz"))
> +        (file-name (string-append name "-" version ".tar.gz"))
> +        (sha256
> +          (base32
> +            "1fmgdl5y4ph3yc6ycg865s3vai1rjkyda61cgqxk6zd13hmznw0c"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     '(#:phases
> +      (modify-phases %standard-phases
> +        (delete 'configure) ; no configure script
> +        (add-before 'build 'set-env
> +          (lambda* (#:key inputs #:allow-other-keys)
> +            ; we need to load cairo dynamically

I think you can find more info about this either in the Guile
documentation or by looking at other packages or in the GNU Coding
Standards (I'm not sure where in those 3):
For one line comments you should use ";;", for comments which are
appended in the same line as some code you should use ";".
This would become:

;; We need to load cairo dynamically

> +            (let* ((cairo (string-append
> +                            (assoc-ref inputs "cairo") "/lib" )))

I'm not sure about the correction here, I think it should be:

(let
  ((cairo (string-append (assoc-ref inputs "cairo") "/lib")))

As you only use it in the small setenv, what about...

> +              (setenv "LD_LIBRARY_PATH" cairo ))))

(add-before 'build 'set-env
  (lambda* (#:key inputs #:allow-other-keys)
    ;; We need to load cairo dynamically
    (setenv "LD_LIBRARY_PATH" (string-append (assoc-ref inputs "cairo") 
"/lib"))))

Of course it's up to you, and what I suggested could contain an error
too. The only thing which really needs to be fixed is the comment ; -> ;;

> +        (add-before 'build 'set-lua-version
> +          (lambda _
> +            ; lua version and therefore install directories are hardcoded
> +            ; FIXME: This breaks when we update lua to >=5.3

Same as above, ; -> ;;

> +            (substitute* "./lgi/Makefile"
> +                       (("LUA_VERSION=5.1") "LUA_VERSION=5.2"))))
> +        (add-before 'check 'skip-test-gtk
> +          (lambda _
> +            ; FIXME: Skip GTK tests:
> +            ;   gtk3 - can't get it to run with the xorg-server config below
> +            ;          and some non-gtk tests will also fail
> +            ;   gtk2 - lots of functions aren't implemented
> +            ; We choose gtk2 as the lesser evil and simply skip the test.
> +            ; as of 01/09/2016 awesome is the only package dependent on lgi 
> but
> +            ; it doesn't need or interact with gtk using lgi.
> +            (substitute* "./tests/test.lua"
> +                       (("'gtk.lua',") "-- 'gtk.lua',"))))
> +        (add-before 'check 'start-xserver-instance
> +          (lambda* (#:key inputs #:allow-other-keys)
> +            ;; There must be a running X server during tests.
> +            (system (format #f "~a/bin/Xvfb :1 &"
> +                            (assoc-ref inputs "xorg-server")))
> +            (setenv "DISPLAY" ":1"))))
> +       #:make-flags (list "CC=gcc"
> +             (string-append "PREFIX=" (assoc-ref %outputs "out")))))
> +    (inputs
> +     `(("gobject-introspection" ,gobject-introspection)
> +       ("glib" ,glib)
> +       ("pango", pango)
> +       ("gtk", gtk+-2)
> +       ("lua" ,lua)
> +       ("cairo" ,cairo)
> +       ("libffi" ,libffi)
> +       ("xorg-server", xorg-server)))
> +    (native-inputs
> +     `(("pkg-config" ,pkg-config)))
> +    (home-page "https://github.com/pavouk/lgi/";)
> +    (synopsis "Lua bridge to GObject based libraries")
> +    (description
> +     "LGI is gobject-introspection based dynamic Lua binding to GObject
> +based libraries.  It allows using GObject-based libraries directly from Lua.
> +Notable examples are GTK+, GStreamer and Webkit.")
> +    (license license:expat)))
> -- 
> 2.9.3

Otherwise looks good to me, the only "problem" is stylistic, which is
the way comments are made.

> From 60b06fd4bf04fc9b172cd2d557be9237ffc6fd46 Mon Sep 17 00:00:00 2001
> From: doncatnip <address@hidden>
> Date: Fri, 2 Sep 2016 00:41:22 +0200
> Subject: [PATCH 3/3] gnu: awesome: Update to 3.5.9.
>
> * gnu/packages/wm.scm (awesome): Update to 3.5.9.
> (awesome)[inputs]: Add gobject-introspection, lua-lgi, cairo. Use
> latest available lua.
> (awesome)[arguments]: Set lua search paths. Add cairo to
> LD_LIBRARY_PATH. Wrap binary in respect to those paths plus
> GI_TYPELIB_PATH.
> ---
>  gnu/packages/wm.scm | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/gnu/packages/wm.scm b/gnu/packages/wm.scm
> index 2cc9f44..ee67c6d 100644
> --- a/gnu/packages/wm.scm
> +++ b/gnu/packages/wm.scm
> @@ -9,6 +9,7 @@
>  ;;; Copyright © 2016 Al McElrath <address@hidden>
>  ;;; Copyright © 2016 Carlo Zancanaro <address@hidden>
>  ;;; Copyright © 2016 Ludovic Courtès <address@hidden>
> +;;; Copyright © 2016 doncatnip <address@hidden>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -382,7 +383,7 @@ experience.")
>  (define-public awesome
>    (package
>      (name "awesome")
> -    (version "3.4.15")
> +    (version "3.5.9")
>      (source
>       (origin (method url-fetch)
>               (uri (string-append
> @@ -390,7 +391,7 @@ experience.")
>                     version ".tar.xz"))
>               (sha256
>                (base32
> -               "1m910lr7wkw2dgzmirfvz7dasfswhhccdf65l21iiciv24c3w1bb"))
> +               "0kynair1ykr74b39a4gcm2y24viial64337cf26nhlc7azjbby67"))
>               (modules '((guix build utils)
>                          (srfi srfi-19)))
>               (snippet
> @@ -413,7 +414,9 @@ experience.")
>                       ("libxml2" ,libxml2)         ;for XML_CATALOG_FILES
>                       ("pkg-config" ,pkg-config)
>                       ("xmlto" ,xmlto)))
> -    (inputs `(("cairo" ,cairo)
> +    (inputs `(("gobject-introspection" ,gobject-introspection)
> +              ("lua-lgi",lua-lgi)
> +              ("cairo" ,cairo)
>                ("dbus" ,dbus)
>                ("gdk-pixbuf" ,gdk-pixbuf)
>                ("glib" ,glib)
> @@ -422,7 +425,7 @@ experience.")
>                ("libxcb" ,libxcb)
>                ("libxcursor" ,libxcursor)
>                ("libxdg-basedir" ,libxdg-basedir)
> -              ("lua" ,lua-5.1)
> +              ("lua" ,lua)
>                ("pango" ,pango)
>                ("startup-notification" ,startup-notification)
>                ("xcb-util" ,xcb-util)
> @@ -445,11 +448,35 @@ experience.")
>                          (("/xmlto")
>                           (string-append "/xmlto --skip-validation")))
>                        #t))
> +                  (add-before 'configure 'set-lua-paths
> +                    (lambda* (#:key inputs #:allow-other-keys)
> +                      ; needs to load cairo dynamically

Same as above.

> +                      (let* ((cairo (string-append
> +                                      (assoc-ref inputs "cairo") "/lib" ))
> +                             (lua-lgi (assoc-ref inputs "lua-lgi") ))
> +                        (setenv "LD_LIBRARY_PATH" cairo )
> +                        (setenv "LUA_PATH" (string-append
> +                                    lua-lgi "/share/lua/5.2/?.lua"))
> +                        (setenv "LUA_CPATH" (string-append
> +                                    lua-lgi "/lib/lua/5.2/?.so")))))
>                    (replace 'check
>                      (lambda _
>                        ;; There aren't any tests, so just make sure the binary
>                        ;; gets built and can be run successfully.
> -                      (zero? (system* "../build/awesome" "-v")))))))
> +                      (zero? (system* "../build/awesome" "-v"))))
> +                  (add-after 'install 'wrap
> +                    (lambda* (#:key inputs outputs #:allow-other-keys)
> +                      (let* ((awesome (assoc-ref outputs "out"))
> +                             (cairo (string-append
> +                                      (assoc-ref inputs "cairo") "/lib" ))
> +                             (lua-lgi (assoc-ref inputs "lua-lgi") ))
> +                        (wrap-program (string-append awesome "/bin/awesome")
> +                          `("GI_TYPELIB_PATH" ":" prefix (,(getenv 
> "GI_TYPELIB_PATH")))
> +                          `("LD_LIBRARY_PATH" suffix (, cairo))
> +                          `("LUA_PATH" suffix (,(string-append
> +                                lua-lgi "/share/lua/5.2/?.lua")))
> +                          `("LUA_CPATH" suffix (,(string-append
> +                                lua-lgi "/lib/lua/5.2/?.so"))))))))))
>      (synopsis "Highly configurable window manager")
>      (description
>       "awesome is a window manager for X.  It manages windows in different
> -- 
> 2.9.3
>

Looks good to me.
-- 
ng0
For non-prism friendly talk find me on http://www.psyced.org