diff mbox series

[bug#68716,gnome-team,RFC,1/3] services: Modularise gnome-desktop-configuration.

Message ID f75a3f173bb2db1723ee813cde74b6466f6e950a.1706199146.git.liliana.prikler@gmail.com
State New
Headers show
Series *** SUBJECT HERE *** | expand

Commit Message

Liliana Marie Prikler Jan. 25, 2024, 3:35 p.m. UTC 
* gnu/services/gnome.scm (%gnome-core-services, %gnome-core-shell)
(%gnome-core-utilities, %gnome-essential-extras): New variables.
(gnome-desktop-configuration): Add ‘core-services’, ‘shell’, ‘utilities’,
and ‘extra-packages’.  Deprecate ‘gnome’.
(gnome-desktop-configuration-core-services, gnome-desktop-configuration-shell)
(gnome-desktop-configuration-utilities)
(gnome-desktop-configuration-extra-packages): Export publicly.
(gnome-udev-rules, gnome-polkit-settings): Adjust accordingly.
(gnome-profile): New variable.
(gnome-desktop-service-type): Adjust accordingly.
---
 gnu/services/desktop.scm | 172 +++++++++++++++++++++++++++++++++++----
 1 file changed, 158 insertions(+), 14 deletions(-)

Comments

Vivien Kraus Jan. 25, 2024, 5:12 p.m. UTC | #1 
Hello!

Thank you for this series.

Le jeudi 25 janvier 2024 à 16:35 +0100, Liliana Marie Prikler a écrit :
>  (define (gnome-udev-rules config)
>    "Return the list of GNOME dependencies that provide udev rules."
> -  (let ((gnome (gnome-desktop-configuration-gnome config)))
> -    (gnome-packages gnome '("gnome-settings-daemon"))))
> +  (let* ((gnome (gnome-desktop-configuration-gnome config))
> +         (shell (gnome-desktop-configuration-shell config)))
> +    (or (any (match-lambda
> +               ((and pkg (= package-name "gnome-settings-daemon"))
> +                (list pkg))
> +               (_ #f))
> +             shell)
> +        (and (maybe-value-set? gnome)
> +             (gnome-packages gnome '("gnome-settings-daemon")))
> +        (raise
> +         (condition
> +          (&error-location
> +           (location (gnome-desktop-configuration-source-location
> config)))
> +          (&message (message (G_ "Missing gnome-settings-
> daemon"))))))))
>  
Is there a reason why we can’t just add everything (like what your new
gnome-profile function does)?

As far as I understand, the user doesn’t have a say over which
individual rules will be used, among the rules that come from the same
package. So if we want the user to have full control over the rules
that are picked, then the current configuration record does not allow
sufficient precision.

Also, we now have udev hardware files. I’m not sure we have any GNOME
packages that install hardware files, but the user might have some. The
gnome-udev-rules function name is misleading now (sorry, my bad, I
should have renamed it when the udev service changed), because every
package listed here will also have its hardware files installed, not
just rules. If you want to pick rules separately from hardware files,
you have to use file->udev-rule, file->udev-hardware (or udev-rule and
udev-hardware) from (gnu services base).

> @@ -1422,7 +1566,7 @@ (define gnome-desktop-service-type
>            (service-extension polkit-service-type
>                               gnome-polkit-settings)
>            (service-extension profile-service-type
> -                             (compose list gnome-desktop-
> configuration-gnome))))
> +                             gnome-profile)))
>     (default-value (gnome-desktop-configuration))
>     (description "Run the GNOME desktop environment.")))
I think the gnome-desktop-service-type could be easily made extensible,
so that e.g a gnome-circle-service-type could add all the circles
applications in their respective sections.

Maybe it could be extended with other instances of gnome-desktop-
configuration? It looks to me that merging multiple instances is quite
easy, as you could just merge the lists for each category, and the
"gnome" field of the extension could simply be ignored. There are still
issues, such as: how can a service extension replace a package with a
conflicting one. Maybe that case is too far out of scope.

That being said, the extensibility of the gnome-desktop-service-type is
not very much needed, because we can also extend the udev, polkit and
profile services directly.

Other than that, the series looks great to me!

Vivien
Liliana Marie Prikler Jan. 25, 2024, 5:38 p.m. UTC | #2 
Am Donnerstag, dem 25.01.2024 um 18:12 +0100 schrieb Vivien Kraus:
> Hello!
> 
> Thank you for this series.
> 
> Le jeudi 25 janvier 2024 à 16:35 +0100, Liliana Marie Prikler a
> écrit :
> >  (define (gnome-udev-rules config)
> >    "Return the list of GNOME dependencies that provide udev rules."
> > -  (let ((gnome (gnome-desktop-configuration-gnome config)))
> > -    (gnome-packages gnome '("gnome-settings-daemon"))))
> > +  (let* ((gnome (gnome-desktop-configuration-gnome config))
> > +         (shell (gnome-desktop-configuration-shell config)))
> > +    (or (any (match-lambda
> > +               ((and pkg (= package-name "gnome-settings-daemon"))
> > +                (list pkg))
> > +               (_ #f))
> > +             shell)
> > +        (and (maybe-value-set? gnome)
> > +             (gnome-packages gnome '("gnome-settings-daemon")))
> > +        (raise
> > +         (condition
> > +          (&error-location
> > +           (location (gnome-desktop-configuration-source-location
> > config)))
> > +          (&message (message (G_ "Missing gnome-settings-
> > daemon"))))))))
> >  
> Is there a reason why we can’t just add everything (like what your
> new gnome-profile function does)?
> 
> As far as I understand, the user doesn’t have a say over which
> individual rules will be used, among the rules that come from the
> same package. So if we want the user to have full control over the
> rules that are picked, then the current configuration record does not
> allow sufficient precision.
> 
> Also, we now have udev hardware files. I’m not sure we have any GNOME
> packages that install hardware files, but the user might have some.
> The gnome-udev-rules function name is misleading now (sorry, my bad,
> I should have renamed it when the udev service changed), because
> every package listed here will also have its hardware files
> installed, not just rules. If you want to pick rules separately from
> hardware files, you have to use file->udev-rule, file->udev-hardware
> (or udev-rule and udev-hardware) from (gnu services base).
That is a good point that I've been debating with myself.  For v1, I
simply aimed at service equivalence, but I have yet to decide whether
that's desirable or useful.

I don't want to simply drop all packages into udev-service-type,
though, unless there's some clever filtering going on behind the back.
I'm not sure at which stage we could efficiently check for the presence
of these magic directories and add them.

> > @@ -1422,7 +1566,7 @@ (define gnome-desktop-service-type
> >            (service-extension polkit-service-type
> >                               gnome-polkit-settings)
> >            (service-extension profile-service-type
> > -                             (compose list gnome-desktop-
> > configuration-gnome))))
> > +                             gnome-profile)))
> >     (default-value (gnome-desktop-configuration))
> >     (description "Run the GNOME desktop environment.")))
> I think the gnome-desktop-service-type could be easily made
> extensible, so that e.g a gnome-circle-service-type could add all the
> circles applications in their respective sections.
> 
> Maybe it could be extended with other instances of gnome-desktop-
> configuration? It looks to me that merging multiple instances is
> quite easy, as you could just merge the lists for each category, and
> the "gnome" field of the extension could simply be ignored. There are
> still issues, such as: how can a service extension replace a package
> with a conflicting one. Maybe that case is too far out of scope.
In my humble opinion, instantiating services twice and merging them
doesn't make sense for a number of services, with gnome being one of
them.  We already have an escape hatch for further packages and
configuration records support extension via inheritance (+ public
accessors).

> That being said, the extensibility of the gnome-desktop-service-type
> is not very much needed, because we can also extend the udev, polkit
> and profile services directly.
That too.

Cheers
diff mbox series

Patch

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 5b79fbcda1..86be99252d 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -37,6 +37,7 @@  (define-module (gnu services desktop)
   #:use-module (gnu services)
   #:use-module (gnu services shepherd)
   #:use-module (gnu services base)
+  #:use-module (gnu services configuration)
   #:use-module (gnu services dbus)
   #:use-module (gnu services avahi)
   #:use-module (gnu services xorg)
@@ -56,10 +57,16 @@  (define-module (gnu services desktop)
   #:use-module (gnu packages admin)
   #:use-module (gnu packages cups)
   #:use-module (gnu packages freedesktop)
+  #:use-module (gnu packages fonts)
+  #:use-module (gnu packages geo)
+  #:use-module (gnu packages gstreamer)
+  #:use-module (gnu packages gtk)
   #:use-module (gnu packages gnome)
+  #:use-module (gnu packages gnupg)
   #:use-module (gnu packages kde)
   #:use-module (gnu packages kde-frameworks)
   #:use-module (gnu packages kde-plasma)
+  #:use-module (gnu packages pulseaudio)
   #:use-module (gnu packages xfce)
   #:use-module (gnu packages avahi)
   #:use-module (gnu packages xdisorg)
@@ -73,6 +80,7 @@  (define-module (gnu services desktop)
   #:use-module (gnu packages nfs)
   #:use-module (gnu packages enlightenment)
   #:use-module (guix deprecation)
+  #:use-module (guix diagnostics)
   #:use-module (guix records)
   #:use-module (guix packages)
   #:use-module (guix store)
@@ -81,6 +89,7 @@  (define-module (gnu services desktop)
   #:use-module (guix gexp)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-35)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (<upower-configuration>
@@ -137,8 +146,16 @@  (define-module (gnu services desktop)
             cups-pk-helper-service-type
             sane-service-type
 
+            %gnome-core-services
+            %gnome-core-shell
+            %gnome-core-utilities
+            %gnome-essential-extras
             gnome-desktop-configuration
             gnome-desktop-configuration?
+            gnome-desktop-configuration-core-services
+            gnome-desktop-configuration-shell
+            gnome-desktop-configuration-utilities
+            gnome-desktop-configuration-extra-packages
             gnome-desktop-service
             gnome-desktop-service-type
 
@@ -1382,11 +1399,100 @@  (define sane-service-type
 ;;; GNOME desktop service.
 ;;;
 
-(define-record-type* <gnome-desktop-configuration> gnome-desktop-configuration
-  make-gnome-desktop-configuration
-  gnome-desktop-configuration?
-  (gnome gnome-desktop-configuration-gnome
-         (default gnome)))
+(define %gnome-core-services
+  (list accountsservice network-manager packagekit upower))
+
+(define %gnome-core-shell
+  (list gdm
+        gnome-backgrounds
+        gnome-bluetooth
+        gnome-color-manager
+        gnome-desktop
+        gnome-initial-setup
+        gnome-keyring
+        gnome-menus
+        gnome-session
+        gnome-settings-daemon
+        gnome-shell-extensions
+        gnome-shell
+        gnome-user-docs
+        gnome-user-share
+        gvfs
+        mutter
+        orca
+        rygel
+        sushi
+        adwaita-icon-theme
+        glib-networking
+        gsettings-desktop-schemas))
+
+(define %gnome-core-utilities
+  (list baobab
+        cheese
+        eog
+        epiphany
+        evince
+        file-roller
+        gnome-calculator
+        gnome-calendar
+        gnome-characters
+        gnome-clocks
+        gnome-console
+        gnome-contacts
+        gnome-disk-utility
+        gnome-font-viewer
+        gnome-maps
+        gnome-music
+        gnome-photos
+        gnome-screenshot
+        gnome-system-monitor
+        gnome-text-editor
+        gnome-weather
+        nautilus
+        simple-scan
+        totem
+        tracker-miners
+        xdg-desktop-portal-gnome
+        yelp))
+
+(define %gnome-essential-extras
+  (list at-spi2-core
+        dbus
+        dconf
+        desktop-file-utils
+        font-abattis-cantarell
+        font-dejavu
+        gnome-default-applications      ; XXX: Allow customization by records
+        gst-plugins-base
+        gst-plugins-good
+        gucharmap
+        pinentry-gnome3
+        pulseaudio                      ; XXX: Replace with pipewire
+        shared-mime-info
+        system-config-printer
+        xdg-user-dirs
+        zenity))
+
+(define-maybe/no-serialization package)
+
+(define-configuration/no-serialization gnome-desktop-configuration
+  (core-services
+   (list-of-packages %gnome-core-services)
+   "A list of packages that the GNOME Shell and applications may rely on.")
+  (shell
+   (list-of-packages %gnome-core-shell)
+   "A list of packages that constitute the GNOME Shell, without applications.")
+  (utilities
+   (list-of-packages %gnome-core-utilities)
+   "A list of packages that serve as applications to use on top of the \
+GNOME Shell.")
+  (gnome (maybe-package) "Deprecated.  Do not use.")
+  (extra-packages
+   (list-of-packages %gnome-essential-extras)
+   "A list of GNOME-adjacent packages to also include.  This field is intended
+for users to add their own packages to their GNOME experience.  Note, that it
+already includes some packages that are considered essential by some (most?)
+GNOME users."))
 
 (define (gnome-package gnome name)
   "Return the package NAME among the GNOME package inputs.  NAME can be a
@@ -1400,18 +1506,56 @@  (define (gnome-packages gnome names)
 
 (define (gnome-udev-rules config)
   "Return the list of GNOME dependencies that provide udev rules."
-  (let ((gnome (gnome-desktop-configuration-gnome config)))
-    (gnome-packages gnome '("gnome-settings-daemon"))))
+  (let* ((gnome (gnome-desktop-configuration-gnome config))
+         (shell (gnome-desktop-configuration-shell config)))
+    (or (any (match-lambda
+               ((and pkg (= package-name "gnome-settings-daemon"))
+                (list pkg))
+               (_ #f))
+             shell)
+        (and (maybe-value-set? gnome)
+             (gnome-packages gnome '("gnome-settings-daemon")))
+        (raise
+         (condition
+          (&error-location
+           (location (gnome-desktop-configuration-source-location config)))
+          (&message (message (G_ "Missing gnome-settings-daemon"))))))))
 
 (define (gnome-polkit-settings config)
   "Return the list of GNOME dependencies that provide polkit actions and
 rules."
-  (let ((gnome (gnome-desktop-configuration-gnome config)))
-    (gnome-packages gnome
-                    '("gnome-settings-daemon"
-                      "gnome-control-center"
-                      "gnome-system-monitor"
-                      "gvfs"))))
+  (let ((gnome (gnome-desktop-configuration-gnome config))
+        (shell (gnome-desktop-configuration-shell config)))
+    (or (any (match-lambda ((and pkg (= package-name "gvfs")) (list pkg))
+                           (_ #f))
+              shell)
+        (and (maybe-value-set? gnome)
+             (gnome-packages gnome
+                             '("gnome-settings-daemon"
+                               "gnome-control-center"
+                               "gnome-system-monitor"
+                               "gvfs")))
+        (raise
+         (condition
+          (&error-location
+           (location (gnome-desktop-configuration-source-location config)))
+          (&message (message (G_ "Missing gvfs"))))))))
+
+(define (gnome-profile config)
+  "Return a list of packages propagated through CONFIG."
+  (append
+   (gnome-desktop-configuration-core-services config)
+   (gnome-desktop-configuration-shell config)
+   (gnome-desktop-configuration-utilities config)
+   (let ((gnome-meta (gnome-desktop-configuration-gnome config)))
+     (if (maybe-value-set? gnome-meta)
+         (begin
+           (warning
+            (gnome-desktop-configuration-source-location config)
+            (G_ "Using a meta-package for gnome-desktop is discouraged.~%"))
+           (list gnome-meta))
+         (list)))
+   (gnome-desktop-configuration-extra-packages config)))
 
 (define gnome-desktop-service-type
   (service-type
@@ -1422,7 +1566,7 @@  (define gnome-desktop-service-type
           (service-extension polkit-service-type
                              gnome-polkit-settings)
           (service-extension profile-service-type
-                             (compose list gnome-desktop-configuration-gnome))))
+                             gnome-profile)))
    (default-value (gnome-desktop-configuration))
    (description "Run the GNOME desktop environment.")))