Adding .xsession to guile-wm

Message ID CAPWBkuYNeYU_CEFLMXemOBN7W9Du72Q3g-jft7mm53tc2akaqA@mail.gmail.com
State New
Headers

Commit Message

Alex ter Weele Sept. 12, 2016, 3:01 a.m. UTC
  Adding an xsession to guile-wm so that display managers will identify it
as an available wm.
  

Comments

David Thompson Sept. 12, 2016, 3:40 p.m. UTC | #1
On Sun, Sep 11, 2016 at 11:01 PM, Alex ter Weele
<alex.ter.weele@gmail.com> wrote:
> Adding an xsession to guile-wm so that display managers will identify it
> as an available wm.

I think it would be best if this were submitted as a patch to guile-wm
upstream instead, and we can include the patch in the "patches" field
until a new release is made.

- Dave
  
Alex ter Weele Sept. 12, 2016, 3:56 p.m. UTC | #2
Manually making a .desktop is what xmonad and fluxbox do:
http://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/wm.scm#n244
http://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/wm.scm#n359
Hence this approach. I could also work on moving this to a patch.
Which is preferred?

-Alex
  
David Thompson Sept. 12, 2016, 3:59 p.m. UTC | #3
On Mon, Sep 12, 2016 at 11:56 AM, Alex ter Weele
<alex.ter.weele@gmail.com> wrote:
> Manually making a .desktop is what xmonad and fluxbox do:
> http://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/wm.scm#n244
> http://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/wm.scm#n359
> Hence this approach. I could also work on moving this to a patch.
> Which is preferred?
>
> -Alex

Well... if we already have prior art then I suppose it's OK!  I'll let
someone else weigh in here and apply the patch.

Thanks for your contribution!

- Dave
  
John Darrington Sept. 12, 2016, 4:05 p.m. UTC | #4
Most of the .desktop files in my store do not have the /gnu/store/... on their
Exec= line.  Perhaps they should?  What is the policy here?

J'

On Sun, Sep 11, 2016 at 11:01:56PM -0400, Alex ter Weele wrote:
     Adding an xsession to guile-wm so that display managers will identify it
     as an available wm.

     From 91f74b1b762596527dfa9412c33e39981932fa70 Mon Sep 17 00:00:00 2001
     From: Alex ter Weele <alex.ter.weele@gmail.com>
     Date: Sun, 11 Sep 2016 22:18:53 -0400
     Subject: [PATCH] Adding .xsession to guile-wm
     
     ---
      gnu/packages/guile-wm.scm | 25 +++++++++++++++++++++++--
      1 file changed, 23 insertions(+), 2 deletions(-)
     
     diff --git a/gnu/packages/guile-wm.scm b/gnu/packages/guile-wm.scm
     index 98e657f..606a792 100644
     --- a/gnu/packages/guile-wm.scm
     +++ b/gnu/packages/guile-wm.scm
     @@ -64,6 +64,7 @@ dependencies.")
        (package
          (name "guile-wm")
          (version "1.0")
     +    (synopsis "Guile window manager")
          (source (origin
                    (method url-fetch)
                    (uri (string-append "http://www.markwitmer.com/dist/guile-wm-"
     @@ -72,7 +73,7 @@ dependencies.")
                     (base32
                      "1l9qcz236jxvryndimjy62cf8zxf8i3f8vg3zpqqjhw15j9mdk3r"))))
          (build-system gnu-build-system)
     -    (arguments '(;; The '.scm' files go to $(datadir), so set that to the
     +    (arguments `(;; The '.scm' files go to $(datadir), so set that to the
                       ;; standard value.
                       #:configure-flags (list (string-append "--datadir="
                                                (assoc-ref %outputs "out")
     @@ -103,7 +104,27 @@ dependencies.")
                                       `("GUILE_LOAD_PATH" ":" prefix (,mods ,xcb))
                                       `("GUILE_LOAD_COMPILED_PATH" ":" prefix
                                         (,mods ,xcb)))))
     -                            %standard-phases))))
     +                            (alist-cons-after
     +                             'install 'install-xsession
     +                             (lambda* (#:key outputs #:allow-other-keys)
     +                               ;; add a .desktop file to xsessions
     +                               (let ((xsessions (string-append
     +                                                 %output "/share/xsessions")))
     +                                 (mkdir-p xsessions)
     +                                 (call-with-output-file
     +                                     (string-append
     +                                      xsessions "/guile-wm.desktop")
     +                                   (lambda (port)
     +                                     (format
     +                                      port
     +                                      "~
     +                                       [Desktop Entry]~@
     +                                       Name=~a~@
     +                                       Comment=~a~@
     +                                       Exec=~a/bin/guile-wm~@
     +                                       Type=Application~%"
     +                                      ,name ,synopsis %output)))))
     +                             %standard-phases)))))
          (native-inputs `(("pkg-config" ,pkg-config)))
          (inputs `(("guile" ,guile-2.0)
                    ("guile-xcb" ,guile-xcb)))
     -- 
     2.10.0
  
Efraim Flashner Sept. 12, 2016, 6:07 p.m. UTC | #5
On Mon, Sep 12, 2016 at 06:05:27PM +0200, John Darrington wrote:
> Most of the .desktop files in my store do not have the /gnu/store/... on their
> Exec= line.  Perhaps they should?  What is the policy here?
> 
> J'
> 

I can comment that enlightenment does, and slim finds it:
Icon=/gnu/store/z9zb4dhqdb6dgl44x5b3czj0bdwg48p6-enlightenment-0.21.2/share/enlightenment/data/images/enlightenment.png
TryExec=/gnu/store/z9zb4dhqdb6dgl44x5b3czj0bdwg48p6-enlightenment-0.21.2/bin/enlightenment_start
Exec=/gnu/store/z9zb4dhqdb6dgl44x5b3czj0bdwg48p6-enlightenment-0.21.2/bin/enlightenment_start

as does openbox:
Exec=/gnu/store/xyx046dfs7lzps4q7ljj9mvkdv935wcl-openbox-3.6.1/bin/openbox-session
TryExec=/gnu/store/xyx046dfs7lzps4q7ljj9mvkdv935wcl-openbox-3.6.1/bin/openbox-session
  
John Darrington Sept. 12, 2016, 6:13 p.m. UTC | #6
On Mon, Sep 12, 2016 at 09:07:10PM +0300, Efraim Flashner wrote:
     On Mon, Sep 12, 2016 at 06:05:27PM +0200, John Darrington wrote:
     > Most of the .desktop files in my store do not have the /gnu/store/... on their
     > Exec= line.  Perhaps they should?  What is the policy here?
     > 
     > J'
     > 
     
     I can comment that enlightenment does, and slim finds it:
     Icon=/gnu/store/z9zb4dhqdb6dgl44x5b3czj0bdwg48p6-enlightenment-0.21.2/share/enlightenment/data/images/enlightenment.png
     TryExec=/gnu/store/z9zb4dhqdb6dgl44x5b3czj0bdwg48p6-enlightenment-0.21.2/bin/enlightenment_start
     Exec=/gnu/store/z9zb4dhqdb6dgl44x5b3czj0bdwg48p6-enlightenment-0.21.2/bin/enlightenment_start
     
     as does openbox:
     Exec=/gnu/store/xyx046dfs7lzps4q7ljj9mvkdv935wcl-openbox-3.6.1/bin/openbox-session
     TryExec=/gnu/store/xyx046dfs7lzps4q7ljj9mvkdv935wcl-openbox-3.6.1/bin/openbox-session
     
     
Many others don't.  And slim also finds them.

J'
  
Alex ter Weele Sept. 13, 2016, 1:23 a.m. UTC | #7
I don't believe there is a standard, but different packages do it
different ways. Under my patch, guile-wm will regenerate its .desktop as
part of the build process, so its Exec= should be kept up-to-date.

-Alex
  
David Thompson Sept. 13, 2016, 1:45 a.m. UTC | #8
On Mon, Sep 12, 2016 at 9:23 PM, Alex ter Weele
<alex.ter.weele@gmail.com> wrote:
> I don't believe there is a standard, but different packages do it
> different ways. Under my patch, guile-wm will regenerate its .desktop as
> part of the build process, so its Exec= should be kept up-to-date.

We should always refer to the absolute path to binaries so that we
don't rely on $PATH, which would be nondeterministic.

- Dave
  
John Darrington Sept. 13, 2016, 4:27 a.m. UTC | #9
On Mon, Sep 12, 2016 at 09:45:22PM -0400, Thompson, David wrote:
     On Mon, Sep 12, 2016 at 9:23 PM, Alex ter Weele
     <alex.ter.weele@gmail.com> wrote:
     > I don't believe there is a standard, but different packages do it
     > different ways. Under my patch, guile-wm will regenerate its .desktop as
     > part of the build process, so its Exec= should be kept up-to-date.
     
     We should always refer to the absolute path to binaries so that we
     don't rely on $PATH, which would be nondeterministic.
     

In that case, below is a (non-exhaustive) list of .desktop files from
various packages which do it "wrong".    Is there not some way we can
automatically flag such cases at build time?

J'

bluetooth-sendto.desktop
emacs.desktop
eog.desktop
epiphany.desktop
evince.desktop
evince-previewer.desktop
evolution-calendar.desktop
exo-file-manager.desktop
exo-mail-reader.desktop
exo-preferred-applications.desktop
exo-terminal-emulator.desktop
exo-web-browser.desktop
gimp.desktop
gnome-background-panel.desktop
gnome-bluetooth-panel.desktop
gnome-color-panel.desktop
gnome-control-center.desktop
gnome-datetime-panel.desktop
gnome.desktop
gnome-display-panel.desktop
gnome-info-panel.desktop
gnome-keyboard-panel.desktop
gnome-mouse-panel.desktop
gnome-network-panel.desktop
gnome-notifications-panel.desktop
gnome-online-accounts-panel.desktop
gnome-power-panel.desktop
gnome-printers-panel.desktop
gnome-privacy-panel.desktop
gnome-region-panel.desktop
gnome-search-panel.desktop
gnome-sharing-panel.desktop
gnome-sound-panel.desktop
gnome-universal-access-panel.desktop
gnome-user-accounts-panel.desktop
gnome-wacom-panel.desktop
gnome-wayland.desktop
gnubik.desktop
gsettings-data-convert.desktop
gtk3-demo.desktop
gtk3-icon-browser.desktop
gtk3-widget-factory.desktop
ibus-setup.desktop
inkscape.desktop
irexec.desktop
mime-dummy-handler.desktop
mutter.desktop
nautilus-autorun-software.desktop
nautilus-autostart.desktop
nautilus-classic.desktop
nm-applet.desktop
nm-connection-editor.desktop
orca-autostart.desktop
org.gnome.baobab.desktop
org.gnome.Devhelp.desktop
org.gnome.FileRoller.desktop
org.gnome.gedit.desktop
org.gnome.Nautilus.desktop
org.gnome.Shell.PortalHelper.desktop
org.gnome.Terminal.desktop
org.gnome.Totem.desktop
panel-desktop-handler.desktop
panel-preferences.desktop
pspp.desktop
ratpoison.desktop
ristretto.desktop
sniff.desktop
Thunar.desktop
Thunar-folder-handler.desktop
thunar-settings.desktop
thunar-volman-settings.desktop
wicd.desktop
wicd-tray.desktop
xfce4-about.desktop
xfce4-accessibility-settings.desktop
xfce4-appfinder.desktop
xfce4-clipman.desktop
xfce4-clipman-plugin-autostart.desktop
xfce4-mime-settings.desktop
xfce4-power-manager.desktop
xfce4-power-manager-settings.desktop
xfce4-run.desktop
xfce4-session-logout.desktop
xfce4-settings-editor.desktop
xfce4-terminal.desktop
xfce-backdrop-settings.desktop
xfce.desktop
xfce-display-settings.desktop
xfce-keyboard-settings.desktop
xfce-mouse-settings.desktop
xfce-session-settings.desktop
xfce-settings-manager.desktop
xfce-ui-settings.desktop
xfce-wm-settings.desktop
xfce-wmtweaks-settings.desktop
xfce-workspaces-settings.desktop
xfsettingsd.desktop
xscreensaver.desktop
yelp.desktop
  
Ludovic Courtès Sept. 13, 2016, 11:51 a.m. UTC | #10
John Darrington <john@darrington.wattle.id.au> skribis:

> Most of the .desktop files in my store do not have the /gnu/store/... on their
> Exec= line.  Perhaps they should?  What is the policy here?

I think it’s best to use absolute file names.

Ludo’.
  
Ludovic Courtès Sept. 13, 2016, 11:52 a.m. UTC | #11
John Darrington <john@darrington.wattle.id.au> skribis:

> On Mon, Sep 12, 2016 at 09:45:22PM -0400, Thompson, David wrote:
>      On Mon, Sep 12, 2016 at 9:23 PM, Alex ter Weele
>      <alex.ter.weele@gmail.com> wrote:
>      > I don't believe there is a standard, but different packages do it
>      > different ways. Under my patch, guile-wm will regenerate its .desktop as
>      > part of the build process, so its Exec= should be kept up-to-date.
>      
>      We should always refer to the absolute path to binaries so that we
>      don't rely on $PATH, which would be nondeterministic.
>      
>
> In that case, below is a (non-exhaustive) list of .desktop files from
> various packages which do it "wrong".    Is there not some way we can
> automatically flag such cases at build time?

Maybe we could have a ‘patch-desktop-files’ phase that automatically
expands relative file names in .desktop files?

Ludo’.
  
Ludovic Courtès Sept. 13, 2016, 12:04 p.m. UTC | #12
Alex ter Weele <alex.ter.weele@gmail.com> skribis:

> From 91f74b1b762596527dfa9412c33e39981932fa70 Mon Sep 17 00:00:00 2001
> From: Alex ter Weele <alex.ter.weele@gmail.com>
> Date: Sun, 11 Sep 2016 22:18:53 -0400
> Subject: [PATCH] Adding .xsession to guile-wm
>
> ---
>  gnu/packages/guile-wm.scm | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)

I agree with David that it would be good to talk to the upstream
developer so that the .desktop file is distributed by upstream
eventually.  Could you try to reach them?

In the meantime, it’s fine to add such a file in Guix.

I pushed the patch as 23de5cbd1487747a2d690ee3ac7630142a7eaeb8.  I added
a copyright line for you, added a ChangeLog-style commit log, and
reindented ‘arguments’ for clarity.

Thanks to your patch, I discovered this:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(ice-9 format)
scheme@(guile-user)> (format #f "foo~@\nbar")
$2 = "foo\nbar"
scheme@(guile-user)> (format #f "foo~@\n      bar")
$3 = "foo\nbar"
--8<---------------cut here---------------end--------------->8---

:-)

Thanks!

Ludo’.
  
Alex ter Weele Sept. 14, 2016, 1:40 a.m. UTC | #13
Ludovic Courtès <ludo@gnu.org> writes:

> I agree with David that it would be good to talk to the upstream
> developer so that the .desktop file is distributed by upstream
> eventually.  Could you try to reach them?

There are improvements I want to make to guile-wm itself, so I plan on
talking to upstream about this and other things.

> In the meantime, it’s fine to add such a file in Guix.
>
> I pushed the patch as 23de5cbd1487747a2d690ee3ac7630142a7eaeb8.  I added
> a copyright line for you, added a ChangeLog-style commit log, and
> reindented ‘arguments’ for clarity.

Thank you, I will note your changes and try to do likewise in my next
patch.

> Thanks to your patch, I discovered this:
>
> --8<---------------cut here---------------start------------->8---
> scheme@(guile-user)> ,use(ice-9 format)
> scheme@(guile-user)> (format #f "foo~@\nbar")
> $2 = "foo\nbar"
> scheme@(guile-user)> (format #f "foo~@\n      bar")
> $3 = "foo\nbar"
> --8<---------------cut here---------------end--------------->8---
>
> :-)

I discovered this from wm.scm, which is where I adapted the patch from ☺.

-Alex
  
Ludovic Courtès Sept. 14, 2016, 2:45 p.m. UTC | #14
Alex ter Weele <alex.ter.weele@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> I agree with David that it would be good to talk to the upstream
>> developer so that the .desktop file is distributed by upstream
>> eventually.  Could you try to reach them?
>
> There are improvements I want to make to guile-wm itself, so I plan on
> talking to upstream about this and other things.

Good to hear!  There were a few glitches that I didn’t take the time to
fix, but otherwise I’d have happily switched to Guile-WM.

Ludo’.
  

Patch

From 91f74b1b762596527dfa9412c33e39981932fa70 Mon Sep 17 00:00:00 2001
From: Alex ter Weele <alex.ter.weele@gmail.com>
Date: Sun, 11 Sep 2016 22:18:53 -0400
Subject: [PATCH] Adding .xsession to guile-wm

---
 gnu/packages/guile-wm.scm | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/guile-wm.scm b/gnu/packages/guile-wm.scm
index 98e657f..606a792 100644
--- a/gnu/packages/guile-wm.scm
+++ b/gnu/packages/guile-wm.scm
@@ -64,6 +64,7 @@  dependencies.")
   (package
     (name "guile-wm")
     (version "1.0")
+    (synopsis "Guile window manager")
     (source (origin
               (method url-fetch)
               (uri (string-append "http://www.markwitmer.com/dist/guile-wm-"
@@ -72,7 +73,7 @@  dependencies.")
                (base32
                 "1l9qcz236jxvryndimjy62cf8zxf8i3f8vg3zpqqjhw15j9mdk3r"))))
     (build-system gnu-build-system)
-    (arguments '(;; The '.scm' files go to $(datadir), so set that to the
+    (arguments `(;; The '.scm' files go to $(datadir), so set that to the
                  ;; standard value.
                  #:configure-flags (list (string-append "--datadir="
                                           (assoc-ref %outputs "out")
@@ -103,7 +104,27 @@  dependencies.")
                                  `("GUILE_LOAD_PATH" ":" prefix (,mods ,xcb))
                                  `("GUILE_LOAD_COMPILED_PATH" ":" prefix
                                    (,mods ,xcb)))))
-                            %standard-phases))))
+                            (alist-cons-after
+                             'install 'install-xsession
+                             (lambda* (#:key outputs #:allow-other-keys)
+                               ;; add a .desktop file to xsessions
+                               (let ((xsessions (string-append
+                                                 %output "/share/xsessions")))
+                                 (mkdir-p xsessions)
+                                 (call-with-output-file
+                                     (string-append
+                                      xsessions "/guile-wm.desktop")
+                                   (lambda (port)
+                                     (format
+                                      port
+                                      "~
+                                       [Desktop Entry]~@
+                                       Name=~a~@
+                                       Comment=~a~@
+                                       Exec=~a/bin/guile-wm~@
+                                       Type=Application~%"
+                                      ,name ,synopsis %output)))))
+                             %standard-phases)))))
     (native-inputs `(("pkg-config" ,pkg-config)))
     (inputs `(("guile" ,guile-2.0)
               ("guile-xcb" ,guile-xcb)))
-- 
2.10.0