Re: [PATCH] gnu: awesome: Update to 3.5.9.
On Fri, Sep 02, 2016 at 12:59:53AM +0200, doncatnip wrote: > Hey Guix ! > > Let's try this again. Okay! > 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". I made the commit title into a complete sentence and pushed. > Subject: [PATCH 2/3] gnu: lua: Add lua-lgi. > > * gnu/packages/lua.scm (lua-lgi): New variable. I took ng0's advice about the comment styles: https://www.gnu.org/software/guile/manual/html_node/Comments.html I also made the phases 'set-env', 'set-lua-version', 'skip-test-gtk', and 'start-xserver-instance' return #t. We want successful phases to return #t, but things like setenv and substitute* do not specify a return value, so we explicity return #t. I moved #:make-flags to the top of the arguments block, which kept it shorter than 80 columns. And I corrected the indentation of #:phases by one column. Pushed! > 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. The commit message only needs to refer to the (awesome) variable once in this case. Referring to the fields [inputs] and [arguments] later does not require the variable to be mentioned again. Fixed before pushing. I sorted the new inputs alphabetically, since the pre-existing inputs were already sorted. I made the comment about loading Cairo dynamically into a complete sentence. I made the phase 'set-lua-paths' return #t. Finally, since the new 'wrap' phase had a too-long line that was ugly to break up, I shifted the indentation of #:phases down one line and to the left, giving some more room to breathe on the right. Pushed as 22037a327a0340341df7ae71a9c1d3551c28c705 !
Re: [PATCH] gnu: awesome: Update to 3.5.9.
Hi, you possibly need to rebase the patch which adds awesome, I shortened the description of the package in master: https://lists.gnu.org/archive/html/guix-devel/2016-09/msg00317.html I have not tested your patch after this, just a reminder that this was changed recently. ng0 writes: > gno writes: > >> On 9/2/2016 2:53 PM, ng0 wrote: >> >>> For some reason xrandr started to kill awesome (X11) session when I >>> apply my layout now. I checked with arandr and also with --dryrun if my >>> layout is the problem, but there's nothing faulty. >>> Maybe there are changes which can't just be installed in my user profile >>> and need to in system. >> >> Hmm, that's not good. No errors or other logs ? > > Could be unrelated. Could be because I did not launch it in a vm but in > my actual live system. This is not a criteria to reject patches, if > anything this is a bug on my end, and if it's a bug in the package, it > is unlikely you introduced it (I would even say impossible, looking at > the patches), there's nothing which could cause this. > >> Also, since I yet again didn't include the list in my previous response: >> >> On 9/2/2016 12:43 PM, ng0 wrote: >> >>> 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. >> >> Please, I'd appreciate any hints as to how I could improve my comment >> style. Don't hold back >> >> Thanks for testing. >> > > -- > ng0 > For non-prism friendly talk find me on http://www.psyced.org > -- ng0 For non-prism friendly talk find me on http://www.psyced.org
Re: [PATCH] gnu: awesome: Update to 3.5.9.
gno writes: > On 9/2/2016 2:53 PM, ng0 wrote: > >> For some reason xrandr started to kill awesome (X11) session when I >> apply my layout now. I checked with arandr and also with --dryrun if my >> layout is the problem, but there's nothing faulty. >> Maybe there are changes which can't just be installed in my user profile >> and need to in system. > > Hmm, that's not good. No errors or other logs ? Could be unrelated. Could be because I did not launch it in a vm but in my actual live system. This is not a criteria to reject patches, if anything this is a bug on my end, and if it's a bug in the package, it is unlikely you introduced it (I would even say impossible, looking at the patches), there's nothing which could cause this. > Also, since I yet again didn't include the list in my previous response: > > On 9/2/2016 12:43 PM, ng0 wrote: > >> 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. > > Please, I'd appreciate any hints as to how I could improve my comment > style. Don't hold back > > Thanks for testing. > -- ng0 For non-prism friendly talk find me on http://www.psyced.org
Re: [PATCH] gnu: awesome: Update to 3.5.9.
On 9/2/2016 2:53 PM, ng0 wrote: For some reason xrandr started to kill awesome (X11) session when I apply my layout now. I checked with arandr and also with --dryrun if my layout is the problem, but there's nothing faulty. Maybe there are changes which can't just be installed in my user profile and need to in system. Hmm, that's not good. No errors or other logs ? Also, since I yet again didn't include the list in my previous response: On 9/2/2016 12:43 PM, ng0 wrote: 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. Please, I'd appreciate any hints as to how I could improve my comment style. Don't hold back Thanks for testing.
Re: [PATCH] gnu: awesome: Update to 3.5.9.
For some reason xrandr started to kill awesome (X11) session when I apply my layout now. I checked with arandr and also with --dryrun if my layout is the problem, but there's nothing faulty. Maybe there are changes which can't just be installed in my user profile and need to in system. Overall, it works and looks good to me. -- ng0 For non-prism friendly talk find me on http://www.psyced.org
Re: [PATCH] gnu: awesome: Update to 3.5.9.
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 writes:
> Hey Guix !
>
> Let's try this again.
> From 99ac37565b7a4826c9ca3c9f0545825924effc3d Mon Sep 17 00:00:00 2001
> From: doncatnip
> 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
> 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
> ;;; Copyright © 2016 Efraim Flashner
> ;;; Copyright © 2016 Ricardo Wurmus
> +;;; Copyright © 2016 doncatnip
> ;;;
> ;;; 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"
> +
