unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
@ 2024-01-06 15:07 Tomas Volf
  2024-04-16 18:29 ` Fabio Natali via Guix-patches via
                   ` (5 more replies)
  0 siblings, 6 replies; 23+ messages in thread
From: Tomas Volf @ 2024-01-06 15:07 UTC (permalink / raw)
  To: 68289; +Cc: Tomas Volf

When user does not use any desktop environment, the typical sequence is to log
in and then type `startx' into the tty to get a window manager running.  Most
distributions do provide startx by default, but Guix has only
xorg-start-command, that is not suitable for this type of task.

This commit adds second procedure, xorg-start-command-xinit, that correctly
picks virtual terminal to use, sets up XAUTHORITY and starts xinit with
correct arguments.  That should make running Guix without any desktop
environment more approachable.

* gnu/services/xorg.scm (xorg-start-command-xinit): New procedure.
(define-module): Export it.
* doc/guix.texi (X Window): Document it.

Change-Id: I17cb16093d16a5c6550b1766754700d4fe014ae9
---
 doc/guix.texi         | 18 ++++++++++
 gnu/services/xorg.scm | 82 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index a648a106b3..72c5527270 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -23177,6 +23177,24 @@ X Window
 Usually the X server is started by a login manager.
 @end deffn
 
+@deffn {Procedure} xorg-start-command-xinit [config]
+Return a @code{startx} script in which the modules, fonts,
+etc. specified in @var{config}, are available.  The result should be
+used in place of @code{startx}.  Compared to the
+@code{xorg-start-command} it calls xinit, therefore it works well when
+executed from tty.  If you are using a desktop environment, you are
+unlikely to have a need for this procedure.
+
+The resulting file should be invoked by user from the tty after login,
+common name for the program would be @code{startx}.  Convenience link
+can be created by (for example) this home service:
+
+@lisp
+(simple-service 'home-files home-files-service-type
+                `(("bin/startx" ,(xorg-start-command-xinit))))
+@end lisp
+@end deffn
+
 
 @defvar screen-locker-service-type
 Type for a service that adds a package for a screen locker or screen
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index 1ee15ea90c..2f5aa3b4f3 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -53,6 +53,7 @@ (define-module (gnu services xorg)
   #:use-module (gnu packages gnome)
   #:use-module (gnu packages admin)
   #:use-module (gnu packages bash)
+  #:use-module (gnu packages linux)
   #:use-module (gnu system shadow)
   #:use-module (guix build-system glib-or-gtk)
   #:use-module (guix build-system trivial)
@@ -84,6 +85,7 @@ (define-module (gnu services xorg)
 
             xorg-wrapper
             xorg-start-command
+            xorg-start-command-xinit
             xinitrc
             xorg-server-service-type
 
@@ -414,6 +416,86 @@ (define* (xorg-start-command #:optional (config (xorg-configuration)))
 
   (program-file "startx" exp))
 
+(define* (xorg-start-command-xinit #:optional (config (xorg-configuration)))
+  "Return a @code{startx} script in which the modules, fonts, etc. specified in
+@var{config}, are available.  The result should be used in place of
+@code{startx}.  Compared to the @code{xorg-start-command} it calls xinit,
+therefore it works well when executed from tty."
+  (define X
+    (xorg-wrapper config))
+
+  (define exp
+    ;; Small wrapper providing subset of functionality of typical startx script
+    ;; from distributions like alpine.
+    #~(begin
+        (use-modules (ice-9 popen)
+                     (ice-9 textual-ports))
+
+        (define (checked-system* . args)
+          (if (= 0 (status:exit-val (apply system* args)))
+              #t
+              (error "command failed")))
+
+        (define (capture-stdout . prog+args)
+          (let* ((port (apply open-pipe* OPEN_READ prog+args))
+                 (data (get-string-all port)))
+            (if (= 0 (status:exit-val (close-pipe port)))
+                (string-trim-right data #\newline)
+                (error "command failed"))))
+
+        (define (determine-unused-display n)
+          (let ((lock-file (format #f "/tmp/.X~a-lock" n))
+                (sock-file (format #f "/tmp/.X11-unix/X~a" n)))
+            (if (or (file-exists? lock-file)
+                    (false-if-exception
+                     (eq? 'socket (stat:type (stat sock-file)))))
+                (determine-unused-display (+ n 1))
+                (format #f ":~a" n))))
+        (define (determine-vty)
+          (let ((fd0 (readlink "/proc/self/fd/0"))
+                (pref "/dev/tty"))
+            (if (string-prefix? pref fd0)
+                (string-append "vt" (substring fd0 (string-length pref)))
+                (error (format #f "Cannot determine VT from: ~a" fd0)))))
+
+        (define (enable-xauth server-auth-file display)
+          ;; Configure and enable X authority
+          (or (getenv "XAUTHORITY")
+              (setenv "XAUTHORITY" (string-append (getenv "HOME") "/.Xauthority")))
+
+          (let* ((bin/xauth (string-append #$xauth "/bin/xauth"))
+                 (bin/mcookie (string-append #$util-linux "/bin/mcookie"))
+
+                 (mcookie (capture-stdout bin/mcookie)))
+            (checked-system* bin/xauth "-qf" server-auth-file
+                             "add" display "." mcookie)
+            (checked-system* bin/xauth "-q"
+                             "add" display "." mcookie)))
+
+        (let* ((xinit (string-append #$xinit "/bin/xinit"))
+               (display (determine-unused-display 0))
+               (vty (determine-vty))
+               (server-auth-port (mkstemp "/tmp/serverauth.XXXXXX"))
+               (server-auth-file (port-filename server-auth-port)))
+          (close-port server-auth-port)
+          (enable-xauth server-auth-file display)
+          (apply execl
+                 xinit
+                 xinit
+                 "--"
+                 #$X
+                 display
+                 vty
+                 "-keeptty"
+                 "-auth" server-auth-file
+                 ;; These are set by xorg-start-command, so do the same to keep
+                 ;; it consistent.
+                 "-logverbose" "-verbose" "-terminate"
+                 #$@(xorg-configuration-server-arguments config)
+                 (cdr (command-line))))))
+
+  (program-file "startx" exp))
+
 (define* (xinitrc #:key fallback-session)
   "Return a system-wide xinitrc script that starts the specified X session,
 which should be passed to this script as the first argument.  If not, the

base-commit: e994bc0abf39db228fa61f1aaf24840c19c47647
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-01-06 15:07 [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Tomas Volf
@ 2024-04-16 18:29 ` Fabio Natali via Guix-patches via
  2024-04-17  9:30   ` Fabio Natali via Guix-patches via
  2024-05-11 13:26 ` [bug#68289] [PATCH v2 1/3] " Tomas Volf
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 23+ messages in thread
From: Fabio Natali via Guix-patches via @ 2024-04-16 18:29 UTC (permalink / raw)
  To: 68289, ~

Hi Tomas,

Thanks for patch 68289 re `xorg-start-command-xinit'. I think it'd be
great to have a command like that in Guix.

In a clumsy attempt to review the patch, I've compared it with the code
for `startx' that I found here⁰. My comments, including some general
observations that might help other reviewers, follow.

tl;dr:

- I hope someone more Xorg savvy than me can have a look.
- Other than a couple of questions (below), things look alright to me.
- I haven't tested the patch on my system yet, but I plan to do it soon.

Thanks, have a great day, Fabio.

⁰ https://gitlab.freedesktop.org/xorg/app/xinit/-/blob/master/startx.cpp


`(determine-unused-display n)' maps closely to this code block:

,----
| XCOMM Automatically determine an unused $DISPLAY
| d=0
| while true ; do
|     [ -e "/tmp/.X$d-lock" -o -S "/tmp/.X11-unix/X$d" ] || break
|     d=$(($d + 1))
| done
| defaultdisplay=":$d"
| unset d
`----

`(determine-vty)' is similar to the block below, but `startx' relies on
the `tty' command from Coreutils. Do you think there might be any
advantage in using it in `(determine-vty)'? A slight simplification
perhaps?

,----
| #ifdef __linux__
|     XCOMM When starting the defaultserver start X on the current tty to avoid
|     XCOMM the startx session being seen as inactive:
|     XCOMM "https://bugzilla.redhat.com/show_bug.cgi?id=806491"
|     tty=$(tty)
|     if expr "$tty" : '/dev/tty[0-9][0-9]*$' > /dev/null; then
|         tty_num=$(echo "$tty" | grep -oE '[0-9]+$')
|         vtarg="vt$tty_num -keeptty"
|     fi
| #endif
`----

`(enable-xauth server-auth-file display)' maps closely to:

,----
|     XCOMM create a file with auth information for the server. ':0' is a dummy.
|     xserverauthfile=$HOME/.serverauth.$$
|     trap "rm -f '$xserverauthfile'" HUP INT QUIT ILL TRAP KILL BUS TERM
|     xauth -q -f "$xserverauthfile" << EOF
| add :$dummy . $mcookie
| EOF
| #if defined(__APPLE__) || defined(__CYGWIN__)
|     xserverauthfilequoted=$(echo ${xserverauthfile} | sed "s/'/'\\\\''/g")
|     serverargs=${serverargs}" -auth '"${xserverauthfilequoted}"'"
| #else
|     serverargs=${serverargs}" -auth "${xserverauthfile}
| #endif
|
|     XCOMM now add the same credentials to the client authority file
|     XCOMM if '$displayname' already exists do not overwrite it as another
|     XCOMM server may need it. Add them to the '$xserverauthfile' instead.
|     for displayname in $authdisplay $hostname$authdisplay; do
|         authcookie=`XAUTH list "$displayname" @@
|         | sed -n "s/.*$displayname[[:space:]*].*[[:space:]*]//p"` 2>/dev/null;
|         if [ "z${authcookie}" = "z" ] ; then
|             XAUTH -q << EOF
| add $displayname . $mcookie
| EOF
`----

The patch saves the server's auth file in `/tmp' whereas `startx' uses
the home directory. I wonder if this might make any difference in terms
of security. Related, how can we be sure that `(mkstemp
"/tmp/serverauth.XXXXXX")' will be setting the right file permissions?

Here's the two relevant bits:

,----
| (server-auth-port (mkstemp "/tmp/serverauth.XXXXXX"))
| (server-auth-file (port-filename server-auth-port))
`----

,----
|     xserverauthfile=$HOME/.serverauth.$$
|     trap "rm -f '$xserverauthfile'" HUP INT QUIT ILL TRAP KILL BUS TERM
`----

Finally, on a purely cosmetic side, any reason to have `(define X
(xorg-wrapper config))' outside the G-expression, while the other
definitions are inside?


-- 
Fabio Natali
https://fabionatali.com




^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-04-16 18:29 ` Fabio Natali via Guix-patches via
@ 2024-04-17  9:30   ` Fabio Natali via Guix-patches via
  2024-04-18 18:43     ` Fabio Natali via Guix-patches via
  2024-04-18 21:09     ` Tomas Volf
  0 siblings, 2 replies; 23+ messages in thread
From: Fabio Natali via Guix-patches via @ 2024-04-17  9:30 UTC (permalink / raw)
  To: 68289, ~

Hi, a quick follow-up on a couple of points.

On 2024-04-16, 19:29 +0100, Fabio Natali <me@fabionatali.com> wrote:
> - I haven't tested the patch on my system yet, but I plan to do it
> soon.

I've tested the patch and it works as expected on my system.

> `(determine-vty)' is similar to the block below, but `startx' relies
> on the `tty' command from Coreutils. Do you think there might be any
> advantage in using it in `(determine-vty)'? A slight simplification
> perhaps?

Looking into this more closely, the `tty' command wouldn't be a
simplification. It might be a bit more consistent with other parts of
the patch and it'd abstract away the hardcoded `/proc/self/fd/0', but
probably not worth the change?

> The patch saves the server's auth file in `/tmp' whereas `startx' uses
> the home directory. I wonder if this might make any difference in
> terms of security. Related, how can we be sure that `(mkstemp
> "/tmp/serverauth.XXXXXX")' will be setting the right file permissions?

I see the reason why we want to use `/tmp', as otherwise the number of
stale `serverauth.XXXXXX' files would grow indefinitely. Using `/tmp',
at least we know they'll be garbage collected at every reboot. Any way
to emulate `startx' and use some sort of `trap' to remove the file on
exit?

> Finally, on a purely cosmetic side, any reason to have `(define X
> (xorg-wrapper config))' outside the G-expression, while the other
> definitions are inside?

Oh yes, the `(define X ...)' has to be outside the G-expression, of
course.

The security aspect (in relation to the server auth file, its
permissions and location) is the only remaining point where I'd like an
extra pair of eyes. The rest of the patch LGTM.

There's a couple of microscopic formatting issues (e.g. an occurrence of
tty where I'd write TTY instead), I'll list them all in a follow-up.

Thanks, best wishes, Fabio.




^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-04-17  9:30   ` Fabio Natali via Guix-patches via
@ 2024-04-18 18:43     ` Fabio Natali via Guix-patches via
  2024-04-18 21:17       ` Tomas Volf
  2024-04-18 21:09     ` Tomas Volf
  1 sibling, 1 reply; 23+ messages in thread
From: Fabio Natali via Guix-patches via @ 2024-04-18 18:43 UTC (permalink / raw)
  To: 68289, ~

On 2024-04-17, 10:30 +0100, Fabio Natali <me@fabionatali.com> wrote:
> Hi, a quick follow-up on a couple of points.

Also, I suppose one could use Guix's 'invoke' instead of a custom
'checked-system*'?

https://issues.guix.gnu.org/issue/68289/#0-lineno88

Cheers, F.




^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-04-17  9:30   ` Fabio Natali via Guix-patches via
  2024-04-18 18:43     ` Fabio Natali via Guix-patches via
@ 2024-04-18 21:09     ` Tomas Volf
  2024-04-19 12:25       ` Fabio Natali via Guix-patches via
  1 sibling, 1 reply; 23+ messages in thread
From: Tomas Volf @ 2024-04-18 21:09 UTC (permalink / raw)
  To: Fabio Natali; +Cc: 68289

[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]

Hello Fabio,

first, let me thank you for the review, and apologize for somewhat late
response, sadly I have been busy.

On 2024-04-17 10:30:12 +0100, Fabio Natali wrote:
> Hi, a quick follow-up on a couple of points.
>
> On 2024-04-16, 19:29 +0100, Fabio Natali <me@fabionatali.com> wrote:
> > - I haven't tested the patch on my system yet, but I plan to do it
> > soon.
>
> I've tested the patch and it works as expected on my system.

Great! :)

>
> > `(determine-vty)' is similar to the block below, but `startx' relies
> > on the `tty' command from Coreutils. Do you think there might be any
> > advantage in using it in `(determine-vty)'? A slight simplification
> > perhaps?
>
> Looking into this more closely, the `tty' command wouldn't be a
> simplification. It might be a bit more consistent with other parts of
> the patch and it'd abstract away the hardcoded `/proc/self/fd/0', but
> probably not worth the change?

I think the current way is fine, since this is Guix specific code, so it does
not have to be extremely portable.  But that is just my opinion.  Would be nice
to know if it works on Hurd.

>
> > The patch saves the server's auth file in `/tmp' whereas `startx' uses
> > the home directory. I wonder if this might make any difference in
> > terms of security. Related, how can we be sure that `(mkstemp
> > "/tmp/serverauth.XXXXXX")' will be setting the right file permissions?

While POSIX does not seem to specify the permissions of the created file, the
Guile's manual is pretty clear regarding it:

     POSIX doesn’t specify the permissions mode of the file.  On GNU and
     most systems it’s ‘#o600’; an application can use ‘chmod’ to relax
     that if desired.

In my understanding that makes this usage safe.

>
> I see the reason why we want to use `/tmp', as otherwise the number of
> stale `serverauth.XXXXXX' files would grow indefinitely. Using `/tmp',
> at least we know they'll be garbage collected at every reboot. Any way
> to emulate `startx' and use some sort of `trap' to remove the file on
> exit?

Yes, the clean up was the main motivator.  The script could *try* to clean up,
but even then it would leave garbage in the $HOME in situations like power
failure and kernel crashes.  So using /tmp seems like simple yet reliable
solution.

>
> > Finally, on a purely cosmetic side, any reason to have `(define X
> > (xorg-wrapper config))' outside the G-expression, while the other
> > definitions are inside?
>
> Oh yes, the `(define X ...)' has to be outside the G-expression, of
> course.
>
> The security aspect (in relation to the server auth file, its
> permissions and location) is the only remaining point where I'd like an
> extra pair of eyes. The rest of the patch LGTM.
>
> There's a couple of microscopic formatting issues (e.g. an occurrence of
> tty where I'd write TTY instead), I'll list them all in a follow-up.
>
> Thanks, best wishes, Fabio.

Have a nice day,
Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-04-18 18:43     ` Fabio Natali via Guix-patches via
@ 2024-04-18 21:17       ` Tomas Volf
  0 siblings, 0 replies; 23+ messages in thread
From: Tomas Volf @ 2024-04-18 21:17 UTC (permalink / raw)
  To: Fabio Natali; +Cc: 68289

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

On 2024-04-18 19:43:41 +0100, Fabio Natali wrote:
> On 2024-04-17, 10:30 +0100, Fabio Natali <me@fabionatali.com> wrote:
> > Hi, a quick follow-up on a couple of points.
>
> Also, I suppose one could use Guix's 'invoke' instead of a custom
> 'checked-system*'?
>
> https://issues.guix.gnu.org/issue/68289/#0-lineno88
>
> Cheers, F.

Yes, that would be an option.  I do not remember why I wrote it like this, it is
possible I just did not know about `invoke' at that time (the code is over a
year old).  However I am not sure whether it is fine to depend on code in the
(guix build utils) module for non-build purposes.  Assuming it is fine, I have
no problem sending a v2.

Cheers,
T.

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-04-18 21:09     ` Tomas Volf
@ 2024-04-19 12:25       ` Fabio Natali via Guix-patches via
  2024-04-24 11:59         ` Fabio Natali via Guix-patches via
  0 siblings, 1 reply; 23+ messages in thread
From: Fabio Natali via Guix-patches via @ 2024-04-19 12:25 UTC (permalink / raw)
  To: Tomas Volf; +Cc: 68289

On 2024-04-18, 23:09 +0200, Tomas Volf <~@wolfsden.cz> wrote:
> first, let me thank you for the review, and apologize for somewhat
> late response, sadly I have been busy.

Contrarily, thank you for getting back to my points. 🙏

It all sounds good. I'll try and bring this up on IRC or during one of
the patch review sessions organised by Futurile - in case there's a
committer who's willing to merge it.

Have a nice day.

Best, Fabio.


-- 
Fabio Natali
https://fabionatali.com




^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-04-19 12:25       ` Fabio Natali via Guix-patches via
@ 2024-04-24 11:59         ` Fabio Natali via Guix-patches via
  2024-04-24 17:43           ` Tomas Volf
  0 siblings, 1 reply; 23+ messages in thread
From: Fabio Natali via Guix-patches via @ 2024-04-24 11:59 UTC (permalink / raw)
  To: Tomas Volf; +Cc: 68289

On 2024-04-19, 13:25 +0100, Fabio Natali <me@fabionatali.com> wrote:
> On 2024-04-18, 23:09 +0200, Tomas Volf <~@wolfsden.cz> wrote:
>> first, let me thank you for the review, and apologize for somewhat
>> late response, sadly I have been busy.

Hi Tomas,

Sorry for the slow follow-up. After some further testing and some input
from other Guix friends, this is my humble feedback on what I'd put in a
v2 patch.

- Use Guix's 'invoke' instead of a custom 'checked-system*' procedure.

- Where possible, use '#$(file-append foobar "/bin/foo")' instead of
  '(string-append #$foobar "/bin/foo")', so that as much computation as
  possible happens at build time as opposed to run time. It's a
  microscopic difference, but still worth the change I think.

What do you think? Not urgent, but do you think this is something you
might be interested to include in a v2? No problem if you're busy, but
let me know if there's anything I can help with.

Tangentally, with regard to 'capture-stdout', I'm exploring if this is
something that could be added to '(guix build utils)'⁰ or perhaps
addressed in Guile¹ instead of Guix. This can be left as it is in the
patch, and potentially refactored away once a similar procedure is
available from Guix or Guile.

Thanks, best wishes, Fabio.


⁰ https://lists.gnu.org/archive/html/guix-devel/2024-04/msg00199.html
¹ https://lists.gnu.org/archive/html/bug-guile/2024-04/msg00015.html


-- 
Fabio Natali
https://fabionatali.com




^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-04-24 11:59         ` Fabio Natali via Guix-patches via
@ 2024-04-24 17:43           ` Tomas Volf
  2024-05-02  9:55             ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Tomas Volf @ 2024-04-24 17:43 UTC (permalink / raw)
  To: Fabio Natali; +Cc: 68289

[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]

On 2024-04-24 12:59:55 +0100, Fabio Natali wrote:
> On 2024-04-19, 13:25 +0100, Fabio Natali <me@fabionatali.com> wrote:
> > On 2024-04-18, 23:09 +0200, Tomas Volf <~@wolfsden.cz> wrote:
> >> first, let me thank you for the review, and apologize for somewhat
> >> late response, sadly I have been busy.
>
> Hi Tomas,
>
> Sorry for the slow follow-up. After some further testing and some input
> from other Guix friends, this is my humble feedback on what I'd put in a
> v2 patch.
>
> - Use Guix's 'invoke' instead of a custom 'checked-system*' procedure.
>
> - Where possible, use '#$(file-append foobar "/bin/foo")' instead of
>   '(string-append #$foobar "/bin/foo")', so that as much computation as
>   possible happens at build time as opposed to run time. It's a
>   microscopic difference, but still worth the change I think.
>
> What do you think? Not urgent, but do you think this is something you
> might be interested to include in a v2? No problem if you're busy, but
> let me know if there's anything I can help with.

All sounds reasonable, will send v2, cannot guarantee when, hopefully this week.

>
> Tangentally, with regard to 'capture-stdout', I'm exploring if this is
> something that could be added to '(guix build utils)'⁰ or perhaps
> addressed in Guile¹ instead of Guix. This can be left as it is in the
> patch, and potentially refactored away once a similar procedure is
> available from Guix or Guile.
>
> Thanks, best wishes, Fabio.

Yes, I noticed the thread.  Having the option of doing basically

  (with-output-to-string (λ _ (invoke "date")))

would be amazing.  I hope someone will take it up and implement. :)



Have a nice day,
Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-04-24 17:43           ` Tomas Volf
@ 2024-05-02  9:55             ` Ludovic Courtès
  2024-05-02 14:58               ` Tomas Volf
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2024-05-02  9:55 UTC (permalink / raw)
  To: Tomas Volf; +Cc: 68289, Fabio Natali

Hi!

Tomas Volf <~@wolfsden.cz> skribis:

> On 2024-04-24 12:59:55 +0100, Fabio Natali wrote:
>> On 2024-04-19, 13:25 +0100, Fabio Natali <me@fabionatali.com> wrote:
>> > On 2024-04-18, 23:09 +0200, Tomas Volf <~@wolfsden.cz> wrote:
>> >> first, let me thank you for the review, and apologize for somewhat
>> >> late response, sadly I have been busy.
>>
>> Hi Tomas,
>>
>> Sorry for the slow follow-up. After some further testing and some input
>> from other Guix friends, this is my humble feedback on what I'd put in a
>> v2 patch.
>>
>> - Use Guix's 'invoke' instead of a custom 'checked-system*' procedure.
>>
>> - Where possible, use '#$(file-append foobar "/bin/foo")' instead of
>>   '(string-append #$foobar "/bin/foo")', so that as much computation as
>>   possible happens at build time as opposed to run time. It's a
>>   microscopic difference, but still worth the change I think.
>>
>> What do you think? Not urgent, but do you think this is something you
>> might be interested to include in a v2? No problem if you're busy, but
>> let me know if there's anything I can help with.
>
> All sounds reasonable, will send v2, cannot guarantee when, hopefully this week.

I hadn’t seen this patch; having a ‘startx’ command is something that
has often been asked, so I’m glad you’re fixing it!

The patch and the suggestions Fabio made look great to me.  Perhaps a
useful addition would be to add a service so one can write nothing more
than:

  (service startx-command-service-type)

to get a ‘startx’ command?  (Not a blocker though.)

Ludo’.




^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-05-02  9:55             ` Ludovic Courtès
@ 2024-05-02 14:58               ` Tomas Volf
  2024-05-03  9:57                 ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Tomas Volf @ 2024-05-02 14:58 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 68289, Fabio Natali

[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]

On 2024-05-02 11:55:28 +0200, Ludovic Courtès wrote:
> Hi!

Hi :)

> Tomas Volf <~@wolfsden.cz> skribis:
> > [..]
>
> I hadn’t seen this patch; having a ‘startx’ command is something that
> has often been asked, so I’m glad you’re fixing it!
>
> The patch and the suggestions Fabio made look great to me.  Perhaps a
> useful addition would be to add a service so one can write nothing more
> than:
>
>   (service startx-command-service-type)
>
> to get a ‘startx’ command?  (Not a blocker though.)

That sounds useful and I have no issue adding it into the v2.  Could I just ask
you for a guidance regarding how to achieve that?  Currently I just place the
script into ~/bin (which I have in the $PATH).  I assume the service will have
to place the file... somewhere? on the $PATH to achieve the same.

Is there already existing service I could use as an inspiration?

Thanks,
Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-05-02 14:58               ` Tomas Volf
@ 2024-05-03  9:57                 ` Ludovic Courtès
  2024-05-11 13:29                   ` Tomas Volf
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2024-05-03  9:57 UTC (permalink / raw)
  To: Tomas Volf; +Cc: 68289, Fabio Natali

Hi!

Tomas Volf <~@wolfsden.cz> skribis:

> On 2024-05-02 11:55:28 +0200, Ludovic Courtès wrote:

[...]

>> The patch and the suggestions Fabio made look great to me.  Perhaps a
>> useful addition would be to add a service so one can write nothing more
>> than:
>>
>>   (service startx-command-service-type)
>>
>> to get a ‘startx’ command?  (Not a blocker though.)
>
> That sounds useful and I have no issue adding it into the v2.  Could I just ask
> you for a guidance regarding how to achieve that?  Currently I just place the
> script into ~/bin (which I have in the $PATH).  I assume the service will have
> to place the file... somewhere? on the $PATH to achieve the same.

I would extend ‘profile-service-type’ such that ‘startx’ appears in
/run/current-system/profile/bin.

It does mean that you need to create a computed-file that produces
/gnu/store/…/bin/startx (‘startx’ must be in a bin/ sub-directory).

How does that sound?

Ludo’.




^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH v2 1/3] services: xorg: Add xorg-start-command-xinit procedure.
  2024-01-06 15:07 [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Tomas Volf
  2024-04-16 18:29 ` Fabio Natali via Guix-patches via
@ 2024-05-11 13:26 ` Tomas Volf
  2024-05-11 13:26   ` [bug#68289] [PATCH v2 2/3] services: xorg: Add startx-command-service-type Tomas Volf
  2024-05-11 13:26   ` [bug#68289] [PATCH v2 3/3] home: services: Add home-startx-command-service-type Tomas Volf
  2024-05-30 17:33 ` [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Arun Isaac
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 23+ messages in thread
From: Tomas Volf @ 2024-05-11 13:26 UTC (permalink / raw)
  To: 68289; +Cc: Tomas Volf, Florian Pelz, Ludovic Courtès

When user does not use any desktop environment, the typical sequence is to log
in and then type `startx' into the tty to get a window manager running.  Most
distributions do provide startx by default, but Guix has only
xorg-start-command, that is not suitable for this type of task.

This commit adds second procedure, xorg-start-command-xinit, that correctly
picks virtual terminal to use, sets up XAUTHORITY and starts xinit with
correct arguments.  That should make running Guix without any desktop
environment more approachable.

* gnu/services/xorg.scm (xorg-start-command-xinit): New procedure.
(define-module): Export it.
* doc/guix.texi (X Window): Document it.

Change-Id: I17cb16093d16a5c6550b1766754700d4fe014ae9
---
v2:
* Use invoke instead of checked-system*.
* Use file-append instead of string-append.

 doc/guix.texi         | 18 ++++++++++
 gnu/services/xorg.scm | 80 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index f20208f94f..c47b6fdd9c 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -23561,6 +23561,24 @@ X Window
 Usually the X server is started by a login manager.
 @end deffn

+@deffn {Procedure} xorg-start-command-xinit [config]
+Return a @code{startx} script in which the modules, fonts,
+etc. specified in @var{config}, are available.  The result should be
+used in place of @code{startx}.  Compared to the
+@code{xorg-start-command} it calls xinit, therefore it works well when
+executed from tty.  If you are using a desktop environment, you are
+unlikely to have a need for this procedure.
+
+The resulting file should be invoked by user from the tty after login,
+common name for the program would be @code{startx}.  Convenience link
+can be created by (for example) this home service:
+
+@lisp
+(simple-service 'home-files home-files-service-type
+                `(("bin/startx" ,(xorg-start-command-xinit))))
+@end lisp
+@end deffn
+

 @defvar screen-locker-service-type
 Type for a service that adds a package for a screen locker or screen
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index 51d704439e..11b9c36995 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -54,11 +54,13 @@ (define-module (gnu services xorg)
   #:use-module (gnu packages gnome)
   #:use-module (gnu packages admin)
   #:use-module (gnu packages bash)
+  #:use-module (gnu packages linux)
   #:use-module (gnu system shadow)
   #:use-module (guix build-system glib-or-gtk)
   #:use-module (guix build-system trivial)
   #:use-module (guix gexp)
   #:use-module (guix store)
+  #:use-module ((guix modules) #:select (source-module-closure))
   #:use-module (guix packages)
   #:use-module (guix derivations)
   #:use-module (guix records)
@@ -86,6 +88,7 @@ (define-module (gnu services xorg)

             xorg-wrapper
             xorg-start-command
+            xorg-start-command-xinit
             xinitrc
             xorg-server-service-type

@@ -416,6 +419,83 @@ (define* (xorg-start-command #:optional (config (xorg-configuration)))

   (program-file "startx" exp))

+(define* (xorg-start-command-xinit #:optional (config (xorg-configuration)))
+  "Return a @code{startx} script in which the modules, fonts, etc. specified
+in @var{config}, are available.  The result should be used in place of
+@code{startx}.  Compared to the @code{xorg-start-command} it calls xinit,
+therefore it works well when executed from tty."
+  (define X
+    (xorg-wrapper config))
+
+  (define exp
+    ;; Small wrapper providing subset of functionality of typical startx
+    ;; script from distributions like alpine.
+    (with-imported-modules (source-module-closure '((guix build utils)))
+      #~(begin
+          (use-modules (guix build utils)
+                       (ice-9 popen)
+                       (ice-9 textual-ports))
+
+          (define (capture-stdout . prog+args)
+            (let* ((port (apply open-pipe* OPEN_READ prog+args))
+                   (data (get-string-all port)))
+              (if (= 0 (status:exit-val (close-pipe port)))
+                  (string-trim-right data #\newline)
+                  (error "command failed"))))
+
+          (define (determine-unused-display n)
+            (let ((lock-file (format #f "/tmp/.X~a-lock" n))
+                  (sock-file (format #f "/tmp/.X11-unix/X~a" n)))
+              (if (or (file-exists? lock-file)
+                      (false-if-exception
+                       (eq? 'socket (stat:type (stat sock-file)))))
+                  (determine-unused-display (+ n 1))
+                  (format #f ":~a" n))))
+          (define (determine-vty)
+            (let ((fd0 (readlink "/proc/self/fd/0"))
+                  (pref "/dev/tty"))
+              (if (string-prefix? pref fd0)
+                  (string-append "vt" (substring fd0 (string-length pref)))
+                  (error (format #f "Cannot determine VT from: ~a" fd0)))))
+
+          (define (enable-xauth server-auth-file display)
+            ;; Configure and enable X authority
+            (or (getenv "XAUTHORITY")
+                (setenv "XAUTHORITY" (string-append (getenv "HOME") "/.Xauthority")))
+
+            (let* ((bin/xauth #$(file-append xauth "/bin/xauth"))
+                   (bin/mcookie #$(file-append util-linux "/bin/mcookie"))
+
+                   (mcookie (capture-stdout bin/mcookie)))
+              (invoke bin/xauth "-qf" server-auth-file
+                      "add" display "." mcookie)
+              (invoke bin/xauth "-q"
+                      "add" display "." mcookie)))
+
+          (let* ((xinit #$(file-append xinit "/bin/xinit"))
+                 (display (determine-unused-display 0))
+                 (vty (determine-vty))
+                 (server-auth-port (mkstemp "/tmp/serverauth.XXXXXX"))
+                 (server-auth-file (port-filename server-auth-port)))
+            (close-port server-auth-port)
+            (enable-xauth server-auth-file display)
+            (apply execl
+                   xinit
+                   xinit
+                   "--"
+                   #$X
+                   display
+                   vty
+                   "-keeptty"
+                   "-auth" server-auth-file
+                   ;; These are set by xorg-start-command, so do the same to keep
+                   ;; it consistent.
+                   "-logverbose" "-verbose" "-terminate"
+                   #$@(xorg-configuration-server-arguments config)
+                   (cdr (command-line)))))))
+
+  (program-file "startx" exp))
+
 (define* (xinitrc #:key fallback-session)
   "Return a system-wide xinitrc script that starts the specified X session,
 which should be passed to this script as the first argument.  If not, the
--
2.41.0




^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH v2 2/3] services: xorg: Add startx-command-service-type.
  2024-05-11 13:26 ` [bug#68289] [PATCH v2 1/3] " Tomas Volf
@ 2024-05-11 13:26   ` Tomas Volf
  2024-05-11 13:26   ` [bug#68289] [PATCH v2 3/3] home: services: Add home-startx-command-service-type Tomas Volf
  1 sibling, 0 replies; 23+ messages in thread
From: Tomas Volf @ 2024-05-11 13:26 UTC (permalink / raw)
  To: 68289; +Cc: Tomas Volf

* gnu/services/xorg.scm (startx-command-profile-service),
(startx-command-service-type): New variables.
(define-module): Export startx-command-service-type.

Change-Id: Ia2a7c3b2d5ebf6bcfff40cb2640b17d3baf6eba0
---
v2: New patch in v2.

 gnu/services/xorg.scm | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index 11b9c36995..51fa6f619c 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -91,6 +91,7 @@ (define-module (gnu services xorg)
             xorg-start-command-xinit
             xinitrc
             xorg-server-service-type
+            startx-command-service-type

             %default-slim-theme
             %default-slim-theme-name
@@ -496,6 +497,39 @@ (define* (xorg-start-command-xinit #:optional (config (xorg-configuration)))

   (program-file "startx" exp))

+(define (startx-command-profile-service config)
+  ;; XXX: profile-service-type only accepts <package> objects.
+  (list
+   (package
+     (name "startx-profile-package")
+     (version "0")
+     (source (xorg-start-command-xinit config))
+     (build-system trivial-build-system)
+     (arguments
+      (list
+       #:modules '((guix build utils))
+       #:builder
+       #~(begin
+           (use-modules (guix build utils))
+           (let ((bin (string-append #$output "/bin")))
+             (mkdir-p bin)
+             (symlink #$source (string-append bin "/startx"))))))
+     (home-page #f)
+     (synopsis #f)
+     (description #f)
+     (license #f))))
+
+(define startx-command-service-type
+  (service-type
+   (name 'startx-command)
+   (extensions
+    (list (service-extension profile-service-type
+                             startx-command-profile-service)))
+   (default-value (xorg-configuration))
+   (description "Add @command{startx} to the system profile.")))
+
+\f
+
 (define* (xinitrc #:key fallback-session)
   "Return a system-wide xinitrc script that starts the specified X session,
 which should be passed to this script as the first argument.  If not, the
--
2.41.0




^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH v2 3/3] home: services: Add home-startx-command-service-type.
  2024-05-11 13:26 ` [bug#68289] [PATCH v2 1/3] " Tomas Volf
  2024-05-11 13:26   ` [bug#68289] [PATCH v2 2/3] services: xorg: Add startx-command-service-type Tomas Volf
@ 2024-05-11 13:26   ` Tomas Volf
  1 sibling, 0 replies; 23+ messages in thread
From: Tomas Volf @ 2024-05-11 13:26 UTC (permalink / raw)
  To: 68289; +Cc: Tomas Volf

* gnu/home/services/desktop.scm (home-startx-command-service-type): New
variable.
(define-module): Export it.
(startx-command-service-type): New service-type mapping.

Change-Id: Id38b5dc7b9235e04e3a9a1b70a35b02e8fae95f0
---
v2: New patch in v2.

gnu/home/services/desktop.scm | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/gnu/home/services/desktop.scm b/gnu/home/services/desktop.scm
index 91465bf168..679ba31c0f 100644
--- a/gnu/home/services/desktop.scm
+++ b/gnu/home/services/desktop.scm
@@ -23,6 +23,7 @@ (define-module (gnu home services desktop)
   #:use-module (gnu home services)
   #:use-module (gnu home services shepherd)
   #:use-module (gnu services configuration)
+  #:use-module (gnu services xorg)
   #:autoload   (gnu packages glib)    (dbus)
   #:autoload   (gnu packages xdisorg) (redshift unclutter)
   #:autoload   (gnu packages xorg) (setxkbmap xmodmap)
@@ -43,7 +44,9 @@ (define-module (gnu home services desktop)
             home-unclutter-service-type

             home-xmodmap-configuration
-            home-xmodmap-service-type))
+            home-xmodmap-service-type
+
+            home-startx-command-service-type))

 \f
 ;;;
@@ -429,3 +432,12 @@ (define home-xmodmap-service-type
    (default-value (home-xmodmap-configuration))
    (description "Run the @code{xmodmap} utility to modify keymaps and pointer
 buttons under the Xorg display server via user-defined expressions.")))
+
+\f
+(define home-startx-command-service-type
+  (service-type
+   (inherit (system->home-service-type startx-command-service-type))
+   (default-value (for-home (xorg-configuration)))))
+
+(define-service-type-mapping
+  startx-command-service-type => home-startx-command-service-type)
--
2.41.0




^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-05-03  9:57                 ` Ludovic Courtès
@ 2024-05-11 13:29                   ` Tomas Volf
  0 siblings, 0 replies; 23+ messages in thread
From: Tomas Volf @ 2024-05-11 13:29 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 68289, Fabio Natali

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

Hello :)

On 2024-05-03 11:57:07 +0200, Ludovic Courtès wrote:
> I would extend ‘profile-service-type’ such that ‘startx’ appears in
> /run/current-system/profile/bin.
>
> It does mean that you need to create a computed-file that produces
> /gnu/store/…/bin/startx (‘startx’ must be in a bin/ sub-directory).
>
> How does that sound?

Right, so I finally found some time to look into it and produced a v2.  I took
inspiration from already existing services in (gnu services xorg), and in the
end it was not that hard to do.

Let me know what you think.

Have a nice day,
Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-01-06 15:07 [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Tomas Volf
  2024-04-16 18:29 ` Fabio Natali via Guix-patches via
  2024-05-11 13:26 ` [bug#68289] [PATCH v2 1/3] " Tomas Volf
@ 2024-05-30 17:33 ` Arun Isaac
  2024-05-30 18:27   ` Tomas Volf
  2024-05-30 18:18 ` [bug#68289] [PATCH v3 1/2] services: xorg: Add startx-command-service-type Tomas Volf
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 23+ messages in thread
From: Arun Isaac @ 2024-05-30 17:33 UTC (permalink / raw)
  To: 68289; +Cc: Tomas Volf, Ludovic Courtès, Fabio Natali


Hi all,

Thanks, Tomas for the patchset! And, thanks, Fabio and Ludo for
reviewing it!

I have pushed patch 1 with a few minor improvements. Could you please
add some documentation to the manual in patches 2 and 3 for the services
added therein? Thanks! Once that's done, please send me an updated
patchset and I'll push.

The changes I made to patch 1 are as follows:
- I tweaked capture-stdout a little to use zero? and print out the
  failing command.
- In the documentation, I removed the part about the convenience
  link. Since we will have dedicated system and home services for this,
  it would be better to refer to those instead.

Regards,
Arun




^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH v3 1/2] services: xorg: Add startx-command-service-type.
  2024-01-06 15:07 [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Tomas Volf
                   ` (2 preceding siblings ...)
  2024-05-30 17:33 ` [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Arun Isaac
@ 2024-05-30 18:18 ` Tomas Volf
  2024-05-30 18:18   ` [bug#68289] [PATCH v3 2/2] home: services: Add home-startx-command-service-type Tomas Volf
  2024-05-30 18:29 ` [bug#68289] [PATCH v4 1/2] services: xorg: Add startx-command-service-type Tomas Volf
  2024-05-30 21:43 ` bug#68289: [PATCH] services: xorg: Add xorg-start-command-xinit procedure Arun Isaac
  5 siblings, 1 reply; 23+ messages in thread
From: Tomas Volf @ 2024-05-30 18:18 UTC (permalink / raw)
  To: 68289
  Cc: arunisaac, Tomas Volf, Florian Pelz, Ludovic Courtès,
	Matthew Trzcinski, Maxim Cournoyer

* gnu/services/xorg.scm (startx-command-profile-service),
(startx-command-service-type): New variables.
(define-module): Export startx-command-service-type.

Change-Id: Ia2a7c3b2d5ebf6bcfff40cb2640b17d3baf6eba0
---
 doc/guix.texi         | 10 ++++++++++
 gnu/services/xorg.scm | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index b07b8dce6f..2cf1e58b5f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -23803,6 +23803,16 @@ X Window
 
 @end deftp
 
+@defvar startx-command-service-type
+Add @command{startx} to the system profile putting it onto the
+@env{PATH}.
+
+The value for this service is a @code{<xorg-configuration>} object which
+is passed to the @code{xorg-start-command-xinit} procedure producing the
+@command{startx} used.  Default value is @code{(xorg-configuration)}.
+@end defvar
+
+
 
 @node Printing Services
 @subsection Printing Services
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index 0b9803c425..d29daa59a8 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -92,6 +92,7 @@ (define-module (gnu services xorg)
             xorg-start-command-xinit
             xinitrc
             xorg-server-service-type
+            startx-command-service-type
 
             %default-slim-theme
             %default-slim-theme-name
@@ -496,6 +497,39 @@ (define* (xorg-start-command-xinit #:optional (config (xorg-configuration)))
 
   (program-file "startx" exp))
 
+(define (startx-command-profile-service config)
+  ;; XXX: profile-service-type only accepts <package> objects.
+  (list
+   (package
+     (name "startx-profile-package")
+     (version "0")
+     (source (xorg-start-command-xinit config))
+     (build-system trivial-build-system)
+     (arguments
+      (list
+       #:modules '((guix build utils))
+       #:builder
+       #~(begin
+           (use-modules (guix build utils))
+           (let ((bin (string-append #$output "/bin")))
+             (mkdir-p bin)
+             (symlink #$source (string-append bin "/startx"))))))
+     (home-page #f)
+     (synopsis #f)
+     (description #f)
+     (license #f))))
+
+(define startx-command-service-type
+  (service-type
+   (name 'startx-command)
+   (extensions
+    (list (service-extension profile-service-type
+                             startx-command-profile-service)))
+   (default-value (xorg-configuration))
+   (description "Add @command{startx} to the system profile.")))
+
+\f
+
 (define* (xinitrc #:key fallback-session)
   "Return a system-wide xinitrc script that starts the specified X session,
 which should be passed to this script as the first argument.  If not, the
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH v3 2/2] home: services: Add home-startx-command-service-type.
  2024-05-30 18:18 ` [bug#68289] [PATCH v3 1/2] services: xorg: Add startx-command-service-type Tomas Volf
@ 2024-05-30 18:18   ` Tomas Volf
  0 siblings, 0 replies; 23+ messages in thread
From: Tomas Volf @ 2024-05-30 18:18 UTC (permalink / raw)
  To: 68289; +Cc: arunisaac, Tomas Volf

* gnu/home/services/desktop.scm (home-startx-command-service-type): New
variable.
(define-module): Export it.
(startx-command-service-type): New service-type mapping.

Change-Id: Id38b5dc7b9235e04e3a9a1b70a35b02e8fae95f0
---
 doc/guix.texi                 |  9 +++++++++
 gnu/home/services/desktop.scm | 14 +++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 2cf1e58b5f..1f2e45f525 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -46376,6 +46376,15 @@ Desktop Home Services
 @end table
 @end deftp
 
+@defvar home-startx-command-service-type
+Add @command{startx} to the home profile putting it onto the @env{PATH}.
+
+The value for this service is a @code{<xorg-configuration>} object which
+is passed to the @code{xorg-start-command-xinit} procedure producing the
+@command{startx} used.  Default value is @code{(xorg-configuration)}.
+@end defvar
+
+
 @node Guix Home Services
 @subsection Guix Home Services
 
diff --git a/gnu/home/services/desktop.scm b/gnu/home/services/desktop.scm
index 91465bf168..679ba31c0f 100644
--- a/gnu/home/services/desktop.scm
+++ b/gnu/home/services/desktop.scm
@@ -23,6 +23,7 @@ (define-module (gnu home services desktop)
   #:use-module (gnu home services)
   #:use-module (gnu home services shepherd)
   #:use-module (gnu services configuration)
+  #:use-module (gnu services xorg)
   #:autoload   (gnu packages glib)    (dbus)
   #:autoload   (gnu packages xdisorg) (redshift unclutter)
   #:autoload   (gnu packages xorg) (setxkbmap xmodmap)
@@ -43,7 +44,9 @@ (define-module (gnu home services desktop)
             home-unclutter-service-type
 
             home-xmodmap-configuration
-            home-xmodmap-service-type))
+            home-xmodmap-service-type
+
+            home-startx-command-service-type))
 
 \f
 ;;;
@@ -429,3 +432,12 @@ (define home-xmodmap-service-type
    (default-value (home-xmodmap-configuration))
    (description "Run the @code{xmodmap} utility to modify keymaps and pointer
 buttons under the Xorg display server via user-defined expressions.")))
+
+\f
+(define home-startx-command-service-type
+  (service-type
+   (inherit (system->home-service-type startx-command-service-type))
+   (default-value (for-home (xorg-configuration)))))
+
+(define-service-type-mapping
+  startx-command-service-type => home-startx-command-service-type)
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-05-30 17:33 ` [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Arun Isaac
@ 2024-05-30 18:27   ` Tomas Volf
  0 siblings, 0 replies; 23+ messages in thread
From: Tomas Volf @ 2024-05-30 18:27 UTC (permalink / raw)
  To: Arun Isaac; +Cc: 68289, Ludovic Courtès, Fabio Natali

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

Hello,

On 2024-05-30 18:33:35 +0100, Arun Isaac wrote:
>
> Hi all,
>
> Thanks, Tomas for the patchset! And, thanks, Fabio and Ludo for
> reviewing it!
>
> I have pushed patch 1 with a few minor improvements.

Thank you for taking the time to merge this, all your changes look good :)

> Could you please add some documentation to the manual in patches 2 and 3 for
> the services added therein? Thanks! Once that's done, please send me an
> updated patchset and I'll push.

I have sent v3 containing just the two later patches.  I sent it to this issue
and also took the liberty of CC-ing you directly per the text above.  Hope it is
what you have imagined.

Uff, only now I realize I forgot to update the commit messages :/.  I will sent
v4.

>
> The changes I made to patch 1 are as follows:
> - I tweaked capture-stdout a little to use zero? and print out the
>   failing command.
> - In the documentation, I removed the part about the convenience
>   link. Since we will have dedicated system and home services for this,
>   it would be better to refer to those instead.
>
> Regards,
> Arun

Have a nice day,
Tomas Volf

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH v4 1/2] services: xorg: Add startx-command-service-type.
  2024-01-06 15:07 [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Tomas Volf
                   ` (3 preceding siblings ...)
  2024-05-30 18:18 ` [bug#68289] [PATCH v3 1/2] services: xorg: Add startx-command-service-type Tomas Volf
@ 2024-05-30 18:29 ` Tomas Volf
  2024-05-30 18:29   ` [bug#68289] [PATCH v4 2/2] home: services: Add home-startx-command-service-type Tomas Volf
  2024-05-30 21:43 ` bug#68289: [PATCH] services: xorg: Add xorg-start-command-xinit procedure Arun Isaac
  5 siblings, 1 reply; 23+ messages in thread
From: Tomas Volf @ 2024-05-30 18:29 UTC (permalink / raw)
  To: 68289; +Cc: arunisaac, Tomas Volf

* gnu/services/xorg.scm (startx-command-profile-service),
(startx-command-service-type): New variables.
(define-module): Export startx-command-service-type.
* doc/guix.texi (X Window): Document it.

Change-Id: Ia2a7c3b2d5ebf6bcfff40cb2640b17d3baf6eba0
---
 doc/guix.texi         | 10 ++++++++++
 gnu/services/xorg.scm | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index b07b8dce6f..2cf1e58b5f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -23803,6 +23803,16 @@ X Window
 
 @end deftp
 
+@defvar startx-command-service-type
+Add @command{startx} to the system profile putting it onto the
+@env{PATH}.
+
+The value for this service is a @code{<xorg-configuration>} object which
+is passed to the @code{xorg-start-command-xinit} procedure producing the
+@command{startx} used.  Default value is @code{(xorg-configuration)}.
+@end defvar
+
+
 
 @node Printing Services
 @subsection Printing Services
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index 0b9803c425..d29daa59a8 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -92,6 +92,7 @@ (define-module (gnu services xorg)
             xorg-start-command-xinit
             xinitrc
             xorg-server-service-type
+            startx-command-service-type
 
             %default-slim-theme
             %default-slim-theme-name
@@ -496,6 +497,39 @@ (define* (xorg-start-command-xinit #:optional (config (xorg-configuration)))
 
   (program-file "startx" exp))
 
+(define (startx-command-profile-service config)
+  ;; XXX: profile-service-type only accepts <package> objects.
+  (list
+   (package
+     (name "startx-profile-package")
+     (version "0")
+     (source (xorg-start-command-xinit config))
+     (build-system trivial-build-system)
+     (arguments
+      (list
+       #:modules '((guix build utils))
+       #:builder
+       #~(begin
+           (use-modules (guix build utils))
+           (let ((bin (string-append #$output "/bin")))
+             (mkdir-p bin)
+             (symlink #$source (string-append bin "/startx"))))))
+     (home-page #f)
+     (synopsis #f)
+     (description #f)
+     (license #f))))
+
+(define startx-command-service-type
+  (service-type
+   (name 'startx-command)
+   (extensions
+    (list (service-extension profile-service-type
+                             startx-command-profile-service)))
+   (default-value (xorg-configuration))
+   (description "Add @command{startx} to the system profile.")))
+
+\f
+
 (define* (xinitrc #:key fallback-session)
   "Return a system-wide xinitrc script that starts the specified X session,
 which should be passed to this script as the first argument.  If not, the
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [bug#68289] [PATCH v4 2/2] home: services: Add home-startx-command-service-type.
  2024-05-30 18:29 ` [bug#68289] [PATCH v4 1/2] services: xorg: Add startx-command-service-type Tomas Volf
@ 2024-05-30 18:29   ` Tomas Volf
  0 siblings, 0 replies; 23+ messages in thread
From: Tomas Volf @ 2024-05-30 18:29 UTC (permalink / raw)
  To: 68289; +Cc: arunisaac, Tomas Volf

* gnu/home/services/desktop.scm (home-startx-command-service-type): New
variable.
(define-module): Export it.
(startx-command-service-type): New service-type mapping.
* doc/guix.texi (Guix Home Services): Document it.

Change-Id: Id38b5dc7b9235e04e3a9a1b70a35b02e8fae95f0
---
 doc/guix.texi                 |  9 +++++++++
 gnu/home/services/desktop.scm | 14 +++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 2cf1e58b5f..1f2e45f525 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -46376,6 +46376,15 @@ Desktop Home Services
 @end table
 @end deftp
 
+@defvar home-startx-command-service-type
+Add @command{startx} to the home profile putting it onto the @env{PATH}.
+
+The value for this service is a @code{<xorg-configuration>} object which
+is passed to the @code{xorg-start-command-xinit} procedure producing the
+@command{startx} used.  Default value is @code{(xorg-configuration)}.
+@end defvar
+
+
 @node Guix Home Services
 @subsection Guix Home Services
 
diff --git a/gnu/home/services/desktop.scm b/gnu/home/services/desktop.scm
index 91465bf168..679ba31c0f 100644
--- a/gnu/home/services/desktop.scm
+++ b/gnu/home/services/desktop.scm
@@ -23,6 +23,7 @@ (define-module (gnu home services desktop)
   #:use-module (gnu home services)
   #:use-module (gnu home services shepherd)
   #:use-module (gnu services configuration)
+  #:use-module (gnu services xorg)
   #:autoload   (gnu packages glib)    (dbus)
   #:autoload   (gnu packages xdisorg) (redshift unclutter)
   #:autoload   (gnu packages xorg) (setxkbmap xmodmap)
@@ -43,7 +44,9 @@ (define-module (gnu home services desktop)
             home-unclutter-service-type
 
             home-xmodmap-configuration
-            home-xmodmap-service-type))
+            home-xmodmap-service-type
+
+            home-startx-command-service-type))
 
 \f
 ;;;
@@ -429,3 +432,12 @@ (define home-xmodmap-service-type
    (default-value (home-xmodmap-configuration))
    (description "Run the @code{xmodmap} utility to modify keymaps and pointer
 buttons under the Xorg display server via user-defined expressions.")))
+
+\f
+(define home-startx-command-service-type
+  (service-type
+   (inherit (system->home-service-type startx-command-service-type))
+   (default-value (for-home (xorg-configuration)))))
+
+(define-service-type-mapping
+  startx-command-service-type => home-startx-command-service-type)
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#68289: [PATCH] services: xorg: Add xorg-start-command-xinit procedure.
  2024-01-06 15:07 [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Tomas Volf
                   ` (4 preceding siblings ...)
  2024-05-30 18:29 ` [bug#68289] [PATCH v4 1/2] services: xorg: Add startx-command-service-type Tomas Volf
@ 2024-05-30 21:43 ` Arun Isaac
  5 siblings, 0 replies; 23+ messages in thread
From: Arun Isaac @ 2024-05-30 21:43 UTC (permalink / raw)
  To: 68289-done; +Cc: Arun Isaac, Tomas Volf, Ludovic Courtès, Fabio Natali


Pushed with minor tweaks. Thank you!




^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2024-05-30 21:43 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-06 15:07 [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Tomas Volf
2024-04-16 18:29 ` Fabio Natali via Guix-patches via
2024-04-17  9:30   ` Fabio Natali via Guix-patches via
2024-04-18 18:43     ` Fabio Natali via Guix-patches via
2024-04-18 21:17       ` Tomas Volf
2024-04-18 21:09     ` Tomas Volf
2024-04-19 12:25       ` Fabio Natali via Guix-patches via
2024-04-24 11:59         ` Fabio Natali via Guix-patches via
2024-04-24 17:43           ` Tomas Volf
2024-05-02  9:55             ` Ludovic Courtès
2024-05-02 14:58               ` Tomas Volf
2024-05-03  9:57                 ` Ludovic Courtès
2024-05-11 13:29                   ` Tomas Volf
2024-05-11 13:26 ` [bug#68289] [PATCH v2 1/3] " Tomas Volf
2024-05-11 13:26   ` [bug#68289] [PATCH v2 2/3] services: xorg: Add startx-command-service-type Tomas Volf
2024-05-11 13:26   ` [bug#68289] [PATCH v2 3/3] home: services: Add home-startx-command-service-type Tomas Volf
2024-05-30 17:33 ` [bug#68289] [PATCH] services: xorg: Add xorg-start-command-xinit procedure Arun Isaac
2024-05-30 18:27   ` Tomas Volf
2024-05-30 18:18 ` [bug#68289] [PATCH v3 1/2] services: xorg: Add startx-command-service-type Tomas Volf
2024-05-30 18:18   ` [bug#68289] [PATCH v3 2/2] home: services: Add home-startx-command-service-type Tomas Volf
2024-05-30 18:29 ` [bug#68289] [PATCH v4 1/2] services: xorg: Add startx-command-service-type Tomas Volf
2024-05-30 18:29   ` [bug#68289] [PATCH v4 2/2] home: services: Add home-startx-command-service-type Tomas Volf
2024-05-30 21:43 ` bug#68289: [PATCH] services: xorg: Add xorg-start-command-xinit procedure Arun Isaac

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).