Package: guix-patches;
Message #105 received at 75688 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: iyzsong <at> envs.net Cc: 75688 <at> debbugs.gnu.org, 宋文武 <iyzsong <at> member.fsf.org>, Vivien Kraus <vivien <at> planete-kraus.eu>, Liliana Marie Prikler <liliana.prikler <at> gmail.com> Subject: Re: bug#75688: Replace wrapper scripts with search path value files in search-paths.d Date: Tue, 28 Jan 2025 22:10:50 +0900
Hi,
iyzsong <at> envs.net writes:
> From: 宋文武 <iyzsong <at> member.fsf.org>
>
> * guix/build/glib-or-gtk-build-system.scm (write-search-path-file):
> New procedure.
>
> (gtk-module-directories): Add version to arguments.
> (gsettings-schema-directories): New procedure.
> (data-directories): Don't check for "/glib-2.0/schemas".
Why not? Because it's handled by the new procedure above?
> (conf-directories): New procedure.
> (wrap-all-programs): Rewrite in terms of 'write-search-path-file'.
OK!
[...]
> +++ b/guix/build/glib-or-gtk-build-system.scm
> @@ -4,6 +4,7 @@
[...]
> +(define* (write-search-path-file output variable value)
> + "Write VALUE to @file{etc/search-paths.d/VARIABLE} under OUTPUT."
> + (let ((search-paths.d (string-append output "/etc/search-paths.d")))
> + (mkdir-p search-paths.d)
> + (with-output-to-file (string-append search-paths.d "/" variable)
> + (lambda () (display value)))))
I think (call-with-output-port (lambda (port) (format port "...")))
would be a bit cleaner (as in more explicit/less magical).
> (define (subdirectory-exists? parent sub-directory)
> (directory-exists? (string-append parent sub-directory)))
>
> @@ -47,32 +55,12 @@ (define (directory-included? directory directories-list)
> (fold (lambda (s p) (or (string-ci=? s directory) p))
> #f directories-list))
>
> -;; We do not include $HOME/.guix-profile/gtk-v.0 (v=2 or 3) because we do not
> -;; want to mix gtk+-2 and gtk+-3 modules. See
> -;; https://developer.gnome.org/gtk3/stable/gtk-running.html
> -(define (gtk-module-directories inputs)
> - "Check for the existence of \"libdir/gtk-v.0\" in INPUTS. Return a list
> +;; We load GTK modules via the GUIX_GTK2_PATH, GUIX_GTK3_PATH and GUIX_GTK4_PATH
> +;; search paths.
> +(define (gtk-module-directories inputs version)
> + "Check for the existence of \"libdir/gtk-VERSION\" in INPUTS. Return a list
> with all found directories."
> - (let* ((version
> - (cond
> - ((string-match "gtk-4"
> - (or (assoc-ref inputs "gtk")
> - (assoc-ref inputs "source")
> - ""))
> - "4.0")
> - ((string-match "gtk\\+-3"
> - (or (assoc-ref inputs "gtk+")
> - (assoc-ref inputs "source")
> - ""))
> - "3.0")
> - ((string-match "gtk\\+-2"
> - (or (assoc-ref inputs "gtk+")
> - (assoc-ref inputs "source")
> - ""))
> - "2.0")
> - (else
> - "4.0"))) ; We default to version 4.0.
> - (gtk-module
> + (let ((gtk-module
> (lambda (input prev)
> (let* ((in (match input
> ((_ . dir) dir)
> @@ -85,27 +73,22 @@ (define (gtk-module-directories inputs)
> prev)))))
> (fold gtk-module '() inputs)))
>
> -;; See
> +;; XDG data files include themes, sounds, icons, etc. See:
> ;; http://www.freedesktop.org/wiki/DesktopThemeSpec
> ;; http://freedesktop.org/wiki/Specifications/sound-theme-spec
> ;; http://freedesktop.org/wiki/Specifications/icon-theme-spec
> ;;
> -;; Currently desktop themes are not well supported and do not honor
> -;; XDG_DATA_DIRS. One example is evince which only looks for desktop themes
> -;; in $HOME/.themes (for backward compatibility) and in XDG_DATA_HOME (which
> -;; defaults to $HOME/.local/share). One way to handle these applications
> -;; appears to be by making $HOME/.themes a symlink to
> -;; $HOME/.guix-profile/share/themes.
> +;; We load them via XDG_DATA_DIRS (from profile, has higher priority) and
> +;; GUIX_XDG_DATA_DIRS (application specified) search paths.
I'd reword to "The data files are searched from the XDG_DATA_DIRS and
GUIX_XDG_DATA_DIRS search paths, with the former having precedence over
the later.
> (define (data-directories inputs)
> - "Check for the existence of \"$datadir/glib-2.0/schemas\" or XDG themes data
> -in INPUTS. Return a list with all found directories."
> + "Check for the existence of XDG data files in INPUTS. Return a list with all found
> +directories."
> (define (data-directory input previous)
> (let* ((in (match input
> ((_ . dir) dir)
> (_ "")))
> (datadir (string-append in "/share")))
> - (if (and (or (subdirectory-exists? datadir "/glib-2.0/schemas")
> - (subdirectory-exists? datadir "/sounds")
OK, I see that's handled in gsettings-schema-directories below, as I
expected.
> + (if (and (or (subdirectory-exists? datadir "/sounds")
> (subdirectory-exists? datadir "/themes")
> (subdirectory-exists? datadir "/cursors")
> (subdirectory-exists? datadir "/wallpapers")
> @@ -117,15 +100,45 @@ (define (data-directories inputs)
>
> (fold data-directory '() inputs))
>
> +;;; XDG configuration files are expected to be installed in etc/xdg directory.
> +;;; We load them via XDG_CONFIG_DIRS (from profile, has higher priority) and
> +;;; GUIX_XDG_CONFIG_DIRS (application specified) search paths.
Likewise, I'd use something like "The configuration files are searched
from the XDG_CONFIG_DIRS and GUIX_XDG_CONFIG_DIRS search paths, with the
former having precedence over the later."
> +(define (conf-directories inputs)
> + "Check for the existence of XDG configuration files in INPUTS. Return a list with
> +all found directories."
80 chars limit (sorry I sound like a broken record; maybe turn on
auto-fill-mode with the fill-column variable set at 78 or the
equivalent in your editor of choice :-)).
> + (define (conf-directory input previous)
> + (let* ((in (match input
> + ((_ . dir) dir)
> + (_ "")))
> + (conf-dir (string-append in "/etc/xdg")))
> + (if (and (directory-exists? conf-dir)
> + (not (directory-included? conf-dir previous)))
> + (cons conf-dir previous)
> + previous)))
> +
> + (fold conf-directory '() inputs))
> +
> +;;; GIO GSettings schemas are expected to be installed in $datadir/glib-2.0/schemas
> +;;; directory. We load them via the GUIX_GSETTINGS_SCHEMA_DIR search path.
in *the* $datadir/glib-2.0/schemas directory
Maybe reword to "The schemas are searched from the GSETTINGS_SCHEMA_DIR
search path."
> +(define (gsettings-schema-directories inputs)
> + "Check for the existence of \"$datadir/glib-2.0/schemas\" in INPUTS.
> +Return a list with all found directories."
> + (define (gsettings-schema-directory input previous)
> + (let* ((in (match input
> + ((_ . dir) dir)
> + (_ "")))
> + (schema-dir (string-append in "/share/glib-2.0/schemas")))
> + (if (and (directory-exists? schema-dir)
> + (not (directory-included? schema-dir previous)))
> + (cons schema-dir previous)
> + previous)))
> +
> + (fold gsettings-schema-directory '() inputs))
I see most of the code of the above procedure gets repeated twice; you
could extract some procedure this out with a prefix argument returning a
specialized procedure.
> ;; All GIO modules are expected to be installed in GLib's $libdir/gio/modules
> ;; directory. That directory has to include a file called giomodule.cache
> -;; listing all available modules. GIO can be made aware of modules in other
> -;; directories with the help of the environment variable GIO_EXTRA_MODULES.
> -;; The official GIO documentation states that this environment variable should
> -;; only be used for testing and not in a production environment. However, it
> -;; appears that there is no other way of specifying multiple modules
> -;; directories (NIXOS also does use this variable). See
> -;; https://developer.gnome.org/gio/stable/running-gio-apps.html
> +;; listing all available modules. We load them via the GUIX_GIO_EXTRA_MODULES
> +;; search path.
Is there a reason why GIO_EXTRA_MODULES (the natively supported
environment variable) is not mentioned like in the other cases above?
Now that I think about it, since it's not a Guix's responsibility to
process these, perhaps we can simply avoid mentioning these native
variants in the above comments as well.
> (define (gio-module-directories inputs)
> "Check for the existence of \"$libdir/gio/modules\" in the INPUTS and
> returns a list with all found directories."
> @@ -141,50 +154,61 @@ (define (gio-module-directories inputs)
>
> (fold gio-module-directory '() inputs))
>
> +
> +;;; XXX: Only works for ELF executables and python3 scripts.
Could we fallback to what is currently done (classic wrapping) for the
other cases? We would have to detect such situation and conditionalize
what happens based on that. Or is the plan to extend to also other
scripting languages and cover every known package from the start? That
could work also, but it sounds a bit more tedious/risky.
> (define* (wrap-all-programs #:key inputs outputs
> (glib-or-gtk-wrap-excluded-outputs '())
> #:allow-other-keys)
In its current form, this phase should be renamed
'write-search-path-files' or similar; we could introduce a deprecated
symbol for wrap-all-programs for backward compatibility.
> "Implement phase \"glib-or-gtk-wrap\": look for GSettings schemas and
> -gtk+-v.0 libraries and create wrappers with suitably set environment variables
> +GTK libraries and create @file{etc/search-paths.d} with suitably set of files
> if found.
Maybe rewrite to: [...] create the @file{etc/search-paths.d} directory
containing one file per search path variable for which there were
matches found.
> Wrapping is not applied to outputs whose name is listed in
> GLIB-OR-GTK-WRAP-EXCLUDED-OUTPUTS. This is useful when an output is known not
> to contain any GLib or GTK+ binaries, and where wrapping would gratuitously
> -add a dependency of that output on GLib and GTK+."
> - ;; Do not require bash to be present in the package inputs
> - ;; even when there is nothing to wrap.
> - ;; Also, calculate (sh) only once to prevent some I/O.
> - (define %sh (delay (search-input-file inputs "bin/bash")))
> - (define (sh) (force %sh))
> +add a dependency of that output on GLib and GTK."
> (define handle-output
> (match-lambda
> ((output . directory)
> (unless (member output glib-or-gtk-wrap-excluded-outputs)
> - (let* ((bindir (string-append directory "/bin"))
> - (libexecdir (string-append directory "/libexec"))
> - (bin-list (filter (negate wrapped-program?)
> - (append (find-files bindir ".*")
> - (find-files libexecdir ".*"))))
> - (datadirs (data-directories
> + (let* ((datadirs (data-directories
> (alist-cons output directory inputs)))
> - (gtk-mod-dirs (gtk-module-directories
> + (confdirs (conf-directories
> (alist-cons output directory inputs)))
> - (gio-mod-dirs (gio-module-directories
> + (schemadirs (gsettings-schema-directories
> (alist-cons output directory inputs)))
> - (env-vars `(,@(if (not (null? datadirs))
> - (list `("XDG_DATA_DIRS" ":" prefix ,datadirs))
> - '())
> - ,@(if (not (null? gtk-mod-dirs))
> - (list `("GTK_PATH" ":" prefix ,gtk-mod-dirs))
> - '())
> - ,@(if (not (null? gio-mod-dirs))
> - (list `("GIO_EXTRA_MODULES" ":"
> - prefix ,gio-mod-dirs))
> - '()))))
> - (for-each (lambda (program)
> - (apply wrap-program program #:sh (sh) env-vars))
> - bin-list))))))
> + (gtk2-mod-dirs (gtk-module-directories
> + (alist-cons output directory inputs)
> + "2.0"))
> + (gtk3-mod-dirs (gtk-module-directories
> + (alist-cons output directory inputs)
> + "3.0"))
> + (gtk4-mod-dirs (gtk-module-directories
> + (alist-cons output directory inputs)
> + "4.0"))
> + (gio-mod-dirs (gio-module-directories
> + (alist-cons output directory inputs))))
> + (when (not (null? datadirs))
> + (write-search-path-file directory "GUIX_XDG_DATA_DIRS"
> + (string-join datadirs ":")))
> + (when (not (null? confdirs))
> + (write-search-path-file directory "GUIX_XDG_CONFIG_DIRS"
> + (string-join confdirs ":")))
> + (when (not (null? schemadirs))
> + (write-search-path-file directory "GUIX_GSETTINGS_SCHEMA_DIR"
> + (string-join schemadirs ":")))
> + (when (not (null? gtk2-mod-dirs))
> + (write-search-path-file directory "GUIX_GTK2_PATH"
> + (string-join gtk2-mod-dirs ":")))
> + (when (not (null? gtk3-mod-dirs))
> + (write-search-path-file directory "GUIX_GTK3_PATH"
> + (string-join gtk3-mod-dirs ":")))
> + (when (not (null? gtk4-mod-dirs))
> + (write-search-path-file directory "GUIX_GTK4_PATH"
> + (string-join gtk4-mod-dirs ":")))
> + (when (not (null? gio-mod-dirs))
> + (write-search-path-file directory "GUIX_GIO_EXTRA_MODULES"
> + (string-join gio-mod-dirs ":"))))))))
Perhaps the check for null could be done in write-search-path-file to
avoid the repetitive checks here. Also, instead of (when (not ...)) you
can use (unless ...).
Other than that, it looks reasonable to me; it'll need a feature branch;
I'm not so sure it requires a GCD anymore since if it works correctly,
it won't impact the API nor the usage of the system. What do you think,
Liliana?
--
Thanks,
Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.