From: Brice Waegeneire <brice@waegenei.re>
To: guix@lprndn.info
Cc: 35305@debbugs.gnu.org
Subject: [bug#35305] LightDM service
Date: Tue, 07 Apr 2020 17:06:34 +0000 [thread overview]
Message-ID: <e0dd6ca666986ea18597f95873e3b1c1@waegenei.re> (raw)
In-Reply-To: <87zhooso9g.fsf@lprndn.info>
Hello L p R n d n,
I never did a review before but I would like to see this patch merged,
so
bear with me.
The indentation of lightdm's origin should probably be done in the
commit
01 not 03.
> `("XDG_DATA_DIRS" ":" prefix (,(string-append (assoc-ref inputs
> "hicolor-icon-theme")
> "/share")
> ,(string-append (assoc-ref inputs "glib")
> "/share")
> ,(string-append (assoc-ref inputs
> "shared-mime-info")
> "/share")
> ,(string-append (assoc-ref inputs "gtk+")
> "/share")
> ,(string-append (assoc-ref inputs "exo")
> "/share")
> ,(string-append (assoc-ref outputs "out")
> "/share")
> "/run/current-system/profile/share"))
This part can use a map procedure.
It would be nice if “lightdm-service-type” support
“set-xorg-configuration”
like the other login manager now does by using
“handle-xorg-configuration”
see 50be0da7bfd5c108697679effeb2a893d2f37598 for how it's done in GDM,
SLIM
and co.
> + (comment "LighDM user")
^ a “t” is missing here
> +(define (lightdm-shepherd-service config)
> + "Return a <lightdm-service> for LightDM with CONFIG."
> +
> + (define lightdm-command
> + #~(list (string-append #$(lightdm-configuration-lightdm config)
> "/sbin/lightdm")))
[…]
> + (fork+exec-command
> + (list #$(file-append
> + (lightdm-configuration-lightdm config)
> + "/sbin/lightdm"))
“lightdm-command” isn't used, I get the hint it ought to be the argument
of
“fork+exec-command.”
> +(define (lightdm-etc-service config)
> + (list `("xdg/lightdm/lightdm.conf.d/lightdm.conf"
> + ,(lightdm-configuration-file config))
> + `(,(string-append "xdg/lightdm/"
> + (computed-file-name
> +
> (lightdm-configuration-greeter-configuration config)))
> + ,(lightdm-configuration-greeter-configuration config))))
I've been told, in Guix, it's better to avoid putting configuration in
“/etc” since it cause edge case during rollback and such, specifying the
configuration by passing the “--config” argument to “lightdm” would be
more
appropriate.
> + (define %user
> + (getpw "lightdm"))
> + (let ((directory "/var/lib/lightdm-data"))
> + (mkdir-p directory)
> + (chown directory (passwd:uid %user) (passwd:gid %user))))))
“%user” could go in the “let”. BTW can't lightdm use its user home
directory instead of “/var/lib/lightdm-data” or the reverse; IOW does it
need to have to own two directories in “/var/lib”?
Several lines in “gnu/services/lightdm.scm” exceed the maximal line
length.
Thank you very much for this patch series. I'm impatient seeing it in
Guix
proper.
- Brice
next prev parent reply other threads:[~2020-04-07 17:07 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-17 14:24 [bug#35305] [WIP] LightDM service L p R n d n
2019-04-18 11:20 ` Jonathan Brielmaier
2019-05-23 11:04 ` [bug#35305] [PATCH] " L p R n d n
[not found] ` <handler.35305.B.155550391014002.ack@debbugs.gnu.org>
2019-04-18 13:20 ` [bug#35305] Acknowledgement ([WIP] LightDM service) L p R n d n
2019-04-18 16:03 ` L p R n d n
2019-08-26 15:58 ` L p R n d n
2020-03-15 21:50 ` Nicolò Balzarotti
2020-03-16 7:34 ` Efraim Flashner
2020-03-16 8:36 ` L p R n d n
2020-03-19 11:54 ` [bug#35305] LightDM service L p R n d n
2020-04-07 17:06 ` Brice Waegeneire [this message]
2020-04-09 16:02 ` L p R n d n
2020-04-12 9:53 ` Brice Waegeneire
2020-04-14 9:38 ` L p R n d n
2020-04-14 13:17 ` L p R n d n
2020-04-22 15:26 ` L p R n d n
2020-05-06 14:05 ` L p R n d n
2020-05-08 22:18 ` Ricardo Wurmus
2020-05-09 15:09 ` L p R n d n
2020-05-10 19:21 ` Ricardo Wurmus
2020-05-11 10:14 ` L p R n d n
2020-05-12 9:59 ` L p R n d n
2020-05-20 20:51 ` Ricardo Wurmus
2020-05-21 8:28 ` L p R n d n
2020-05-21 9:23 ` Ricardo Wurmus
2020-06-08 15:35 ` L p R n d n
2022-08-04 5:09 ` [bug#35305] [WIP] " Maxim Cournoyer
2020-06-19 14:47 ` [bug#35305] " L p R n d n
2022-08-04 2:19 ` [bug#35305] [WIP] " Maxim Cournoyer
2022-08-31 7:13 ` bug#35305: " Ricardo Wurmus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e0dd6ca666986ea18597f95873e3b1c1@waegenei.re \
--to=brice@waegenei.re \
--cc=35305@debbugs.gnu.org \
--cc=guix@lprndn.info \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.