[bug#41083] gnu: xfe: Fix hard-coded fhs directories.

["Ludovic Courtès" <ludo@gnu.org>]

Hello!

Raghav Gururajan <raghavgururajan@disroot.org> skribis:

>>From 660f134e15438e7ee7aec1c076dca93c68e4edc6 Mon Sep 17 00:00:00 2001
> From: Raghav Gururajan <raghavgururajan@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’.