GNU bug report logs - #41083
gnu: xfe: Fix hard-coded fhs directories.

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Raghav Gururajan <raghavgururajan <at> disroot.org>
Cc: 41083 <at> debbugs.gnu.org
Subject: [bug#41083] gnu: xfe: Fix hard-coded fhs directories.
Date: Mon, 04 May 2020 23:04:48 +0200

Hello!

Raghav Gururajan <raghavgururajan <at> disroot.org> skribis:

>>From 660f134e15438e7ee7aec1c076dca93c68e4edc6 Mon Sep 17 00:00:00 2001
> From: Raghav Gururajan <raghavgururajan <at> disroot.org>
> Date: Mon, 4 May 2020 13:07:02 -0400
> Subject: [PATCH] gnu: xfe: Fix hard-coded fhs directories.
>
> * gnu/packages/disk.scm (xfe): Fix hard-coded fhs directories.
> [arguments]<#:phases>['patch-xfe-paths]: Delete phase.
> [arguments]<#:phases>['patch-bin-dirs]: New phase.
> [arguments]<#:phases>['patch-share-dirs]: New phase.
> [inputs]<bash,coreutils,file,findutils>: New inputs.

Nitpick: You don’t need to mention #:phases above, and the angle
brackets are inappropriate for inputs.  See ‘git log’ for examples.

> -    (native-inputs
> -     `(("intltool" ,intltool)
> -       ("pkg-config" ,pkg-config)))
> -    (inputs
> -     `(("fox" ,fox)
> -       ("freetype" ,freetype)
> -       ("x11" ,libx11)
> -       ("xcb" ,libxcb)
> -       ("xcb-util" ,xcb-util)
> -       ("xft" ,libxft)
> -       ("xrandr" ,libxrandr)))

To reduce review time :-), it’s a good idea to avoid unnecessary
changes.  In this case, you should avoid moving things around because
that makes the patch harder to read.


> +               (substitute* "src/FilePanel.cpp"
> +                 (("/bin/sh") sh)
> +                 (("/usr/bin/du") du)
> +                 (("/usr/bin/sort") sort)
> +                 (("/usr/bin/cut") cut)
> +                 (("/usr/bin/xargs") xargs))
> +               (substitute* "src/help.h"
> +                 (("/bin/sh") sh)
> +                 (("/bin/ls") ls))
> +               (substitute* "src/SearchPanel.cpp"
> +                 (("/usr/bin/du") du)
> +                 (("/usr/bin/sort") sort)
> +                 (("/usr/bin/cut") cut)
> +                 (("/usr/bin/xargs") xargs))
> +               (substitute* "src/startupnotification.cpp"
> +                 (("/bin/sh") sh))
> +               (substitute* "src/xfeutils.cpp"
> +                 (("/usr/bin/file") file))

I think you can just define a variable like:

  (coreutils (assoc-ref inputs "coreutils"))

and then use (string-append coreutils "/bin/sort") and so on, instead of
defining many variables that have a single user.

>               (let*
> -                 ((out     (assoc-ref outputs "out"))
> -                  (share   (string-append out "/share"))
> -                  (xferc   (string-append out "/share/xfe/xferc"))
> -                  (xfe-theme   (string-append out "/share/xfe/icons/xfe-theme")))
> -               ;; Correct path for xfe registry.
> +                 ((out
> +                   (assoc-ref outputs "out"))
> +                  (share
> +                   (string-append out "/share"))
> +                  (xfe
> +                   (string-append out "/share/xfe"))

Avoid the indentation changes (previous version was fine, although we
usually start the list of bindings on the same line as ‘let*’).

> -               ;; Correct path for xfe icons.
> -               (substitute* "src/xfedefs.h"
> -                 (((string-append
> -                    "~/.config/xfe/icons/xfe-theme:"
> -                    "/usr/local/share/xfe/icons/xfe-theme:"
> -                    "/usr/share/xfe/icons/xfe-theme"))
> -                  xfe-theme))

The ~/.config/xfe bit is going away, right?

Could you send an updated patch?

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.