dmd: make user-mode easier to use.

dmd: make user-mode easier to use.

[Alex Sassmannshausen]

Hello,

This patch series allows dmd to work out of the box in 'user-mode'
after installation. To achieve this it ensures all dmd related files
are stored in ~/.dmd.d/ when run in user-mode, and it creates a
bare-bones configuration file for the user to start building upon if
no configuration file exists yet.

I decided to implement the above minimising any changes to the actual
logic of existing dmd. The result is a slight mixing up of side-effect
folder/file creation and side-effect free checks.

The alternative would have been to separate the folder creation from
the procedures carrying out the checks for the existence of the
configuration folder and files.

I wasn't sure whether it was better to minimise the changes in the
patches or better to 'just do the right thing', even if it entails
larger and more complex patches. Let me know your thoughts if you
think I should have gone the other way.

Best wishes,

Alex

=?y?q?=5BPATCH=201/2=5D=20dmd=3A=20Use=20=7E/=2Edmd=2Ed/=20by=20default=20when=20not=20run=20as=20root=2E?=

[Alex Sassmannshausen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 2625 bytes --]

* modules/dmd/support.scm: Add copyright.
  (user-dmddir): New variable.
  (default-logfile): Use it instead of user-homedir.
  (default-persistency-state-file): Use it instead of user-homedir.
  (default-configfile): Use it instead of user-homedir, use init.scm
  as default name for configfile, ensure .dmd.d exists.
  (default-socket-dir): Add check for root, if #f, use user-dmddir as
  base for socket.
---
 modules/dmd/support.scm |   21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/modules/dmd/support.scm b/modules/dmd/support.scm
index 413e65b..f9c9989 100644
--- a/modules/dmd/support.scm
+++ b/modules/dmd/support.scm
@@ -1,4 +1,5 @@
-;; support.scm -- Various general support facilities, shared by deco and dmd.
+;; support.scm -- Various support facilities, used by deco and dmd.
+;; Copyright (C) 2014 A.Sassmannshausen <alex.sassmannshausen@gmail.com>
 ;; Copyright (C) 2013 Ludovic Courtès <ludo@gnu.org>
 ;; Copyright (C) 2002, 2003 Wolfgang Jährling <wolfgang@pro-linux.de>
 ;;
@@ -158,21 +159,31 @@ There is NO WARRANTY, to the extent permitted by law.")))
       (getenv "HOME")
       "/"))
 
+;; dmd default subdirectory if dmd is run as a normal user.
+(define user-dmddir (string-append user-homedir "/.dmd.d"))
+
 ;; Logfile.
 (define default-logfile
   (if (zero? (getuid))
       (string-append %localstatedir "/log/dmd.log")
-      (string-append user-homedir "/.dmd.log")))
+      (string-append user-dmddir "/dmd.log")))
 
 ;; Configuration file.
 (define default-config-file
   (if (zero? (getuid))
       (string-append Prefix-dir "/etc/dmdconf.scm")
-    (string-append user-homedir "/.dmdconf.scm")))
+      (begin
+        (let ((config-file (string-append user-dmddir "/init.scm")))
+          (cond ((not (file-exists? user-dmddir))
+                 (mkdir user-dmddir)
+                 config-file)
+                (else config-file))))))
 
 ;; The directory where the socket resides.
 (define default-socket-dir
-  (string-append %localstatedir "/run/dmd"))
+  (if (zero? (getuid))
+      (string-append %localstatedir "/run/dmd")
+      (string-append user-dmddir "/run")))
 
 ;; Unix domain socket for receiving commands in dmd.
 (define default-socket-file
@@ -182,7 +193,7 @@ There is NO WARRANTY, to the extent permitted by law.")))
 (define default-persistency-state-file
   (if (zero? (getuid))
       (string-append %localstatedir "/lib/misc/dmd-state")
-      (string-append user-homedir "/.dmd-state")))
+      (string-append user-dmddir "/dmd-state")))
 
 ;; Global variables set from (dmd).
 (define persistency #f)
-- 
1.7.9.5

[PATCH 2/2] dmd: Make config file if necessary when not run as root.

[Alex Sassmannshausen]

* modules/dmd/support.scm (make-bare-init-file): new procedure to
  generate basic init file.
  (default-logfile): Check for init file, create it if necessary.
---
 modules/dmd/support.scm |   42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/modules/dmd/support.scm b/modules/dmd/support.scm
index f9c9989..f1b6248 100644
--- a/modules/dmd/support.scm
+++ b/modules/dmd/support.scm
@@ -162,6 +162,45 @@ There is NO WARRANTY, to the extent permitted by law.")))
 ;; dmd default subdirectory if dmd is run as a normal user.
 (define user-dmddir (string-append user-homedir "/.dmd.d"))
 
+(define (make-bare-init-file target)
+  "Return #t if a bare init file was created at TARGET; #f otherwise.
+
+TARGET should be a string representing a filepath + name."
+  (with-output-to-file target
+    (lambda ()
+      (format #t
+";; init.scm -- default dmd configuration file.
+;; Copyright (C) 2014 A. Sassmannshausen <alex.sassmannshausen@gmail.com>
+;;
+;; This file is part of GNU dmd.
+;;
+;; GNU dmd is free software; you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation; either version 3 of the License, or (at
+;; your option) any later version.
+;;
+;; GNU dmd is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with GNU dmd.  If not, see <http://www.gnu.org/licenses/>.
+
+;; Services known to dmd:
+;; Add new services to dmd here by providing them through the
+;; MAKE-SERVICE procedure to REGISTER SERVICES.
+ (register-services)
+
+;; Send dmd into the background
+ (action 'dmd 'daemonize)
+
+;; Services to start when dmd starts:
+;; Add the name of each service known to dmd that should be started to
+;;the list passed to FOR-EACH.
+ (for-each start '())
+"))))
+
 ;; Logfile.
 (define default-logfile
   (if (zero? (getuid))
@@ -176,7 +215,10 @@ There is NO WARRANTY, to the extent permitted by law.")))
         (let ((config-file (string-append user-dmddir "/init.scm")))
           (cond ((not (file-exists? user-dmddir))
                  (mkdir user-dmddir)
+                 (make-bare-init-file config-file)
                  config-file)
+                ((not (file-exists? config-file))
+                 (make-bare-init-file config-file))
                 (else config-file))))))
 
 ;; The directory where the socket resides.
-- 
1.7.9.5

Re: [PATCH 1/2] dmd: Use ~/.dmd.d/ by default when not run as root.

[Ludovic Courtès]

Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

> * modules/dmd/support.scm: Add copyright.
>   (user-dmddir): New variable.
>   (default-logfile): Use it instead of user-homedir.
>   (default-persistency-state-file): Use it instead of user-homedir.
>   (default-configfile): Use it instead of user-homedir, use init.scm
>   as default name for configfile, ensure .dmd.d exists.
>   (default-socket-dir): Add check for root, if #f, use user-dmddir as
>   base for socket.

Applied, thanks!

>  ;; Configuration file.
>  (define default-config-file
>    (if (zero? (getuid))
>        (string-append Prefix-dir "/etc/dmdconf.scm")
> -    (string-append user-homedir "/.dmdconf.scm")))
> +      (begin
> +        (let ((config-file (string-append user-dmddir "/init.scm")))
> +          (cond ((not (file-exists? user-dmddir))
> +                 (mkdir user-dmddir)
> +                 config-file)
> +                (else config-file))))))

The file-exists?/mkdir pattern above introduces a TOCTTOU problem: when
running ‘make check -j’ the first time, one of the tests failed because
‘mkdir’ failed with EEXIST.

So I changed it to (catch-system-error (mkdir user-dmddir)).

Ludo’.

Re: dmd: make user-mode easier to use.

[Ludovic Courtès]

Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

> This patch series allows dmd to work out of the box in 'user-mode'
> after installation. To achieve this it ensures all dmd related files
> are stored in ~/.dmd.d/ when run in user-mode, and it creates a
> bare-bones configuration file for the user to start building upon if
> no configuration file exists yet.

Sounds like the right thing to do.

Ludo’.

Re: [PATCH 2/2] dmd: Make config file if necessary when not run as root.

[Ludovic Courtès]

Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

> * modules/dmd/support.scm (make-bare-init-file): new procedure to
>   generate basic init file.
>   (default-logfile): Check for init file, create it if necessary.

A good idea to help people get started!

A couple of comments:

> +(define (make-bare-init-file target)
> +  "Return #t if a bare init file was created at TARGET; #f otherwise.
> +
> +TARGET should be a string representing a filepath + name."
> +  (with-output-to-file target
> +    (lambda ()
> +      (format #t
> +";; init.scm -- default dmd configuration file.
> +;; Copyright (C) 2014 A. Sassmannshausen <alex.sassmannshausen@gmail.com>
> +;;
> +;; This file is part of GNU dmd.
> +;;
> +;; GNU dmd is free software; you can redistribute it and/or modify it
> +;; under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation; either version 3 of the License, or (at
> +;; your option) any later version.
> +;;
> +;; GNU dmd is distributed in the hope that it will be useful, but
> +;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;; GNU General Public License for more details.
> +;;
> +;; You should have received a copy of the GNU General Public License
> +;; along with GNU dmd.  If not, see <http://www.gnu.org/licenses/>.

I think the skeleton file should not include a copyright notice.

> +;; Services known to dmd:
> +;; Add new services to dmd here by providing them through the
> +;; MAKE-SERVICE procedure to REGISTER SERVICES.

IMO this should be ‘make-service’ and ‘register-services’ (lower-case,
quoted.)

> + (register-services)
   ^

Extra space here.

> +;; Send dmd into the background
> + (action 'dmd 'daemonize)

Ditto.

> +;; Services to start when dmd starts:
> +;; Add the name of each service known to dmd that should be started to
> +;;the list passed to FOR-EACH.
> + (for-each start '())

Ditto.

>          (let ((config-file (string-append user-dmddir "/init.scm")))
>            (cond ((not (file-exists? user-dmddir))
>                   (mkdir user-dmddir)
> +                 (make-bare-init-file config-file)
>                   config-file)
> +                ((not (file-exists? config-file))
> +                 (make-bare-init-file config-file))
>                  (else config-file))))))

So I guess this should be changed to:

  (catch 'system-error
    (lambda ()
      (mkdir user-dmddir)
      (make-bare-init-file config-file))
    (const #f))

Thanks!

Ludo’.

Re: [PATCH 2/2] dmd: Make config file if necessary when not run as root.

[Alex Sassmannshausen]

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

Hello,

Thanks for the review.

Ludovic Courtès writes:

>> +(define (make-bare-init-file target)
>> […]
>
> I think the skeleton file should not include a copyright notice.

I've now removed this.

>> +;; Services known to dmd:
>> +;; Add new services to dmd here by providing them through the
>> +;; MAKE-SERVICE procedure to REGISTER SERVICES.
>
> IMO this should be ‘make-service’ and ‘register-services’ (lower-case,
> quoted.)

Done.

>> + (register-services)
>    ^
>> +;; Send dmd into the background
>> + (action 'dmd 'daemonize)
>    ^
>> + (for-each start '())
>    ^
>
> Extra space here.

The extra spaces were there to stop emacs (maybe Scheme mode?) from
going nutty because of the open parens within a string at column 0.

I've removed the spacing, but you may find when editing the file in
Emacs that syntax highlighting doesn't work properly after the above
definition.

Don't know if there is a solution to this or whether this is a bug in
Scheme mode.

>>          (let ((config-file (string-append user-dmddir "/init.scm")))
>>            (cond ((not (file-exists? user-dmddir))
>>                   (mkdir user-dmddir)
>> +                 (make-bare-init-file config-file)
>>                   config-file)
>> +                ((not (file-exists? config-file))
>> +                 (make-bare-init-file config-file))
>>                  (else config-file))))))
>
> So I guess this should be changed to:
>
>   (catch 'system-error
>     (lambda ()
>       (mkdir user-dmddir)
>       (make-bare-init-file config-file))
>     (const #f))

I've changed this to :

         (catch-system-error (mkdir user-dmddir))
         (if (not (file-exists? config-file))
             (make-bare-init-file config-file))
         config-file)))

I think you may have a situation where your configuration directory
exists, but where no config file exists. In this case I think we should
simply create the new file.

I also think the race condition should not be a problem for the config
file: if the config file is added after the check, then we simply
override it. If the config file is removed after the check then guix
crashes when it tries to load it, as it always would do before this
patch.

Hope this works now. You should find the patch inline.

Best wishes,

Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Revised patch 2/2 --]
[-- Type: text/x-diff, Size: 2229 bytes --]

From 860f97179991fde8fa0f8a0a2e7400c0cd14ec6e Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 2 Feb 2014 17:07:50 +0100
Subject: [PATCH] dmd: Make config file if necessary when not run as root.

* modules/dmd/support.scm (make-bare-init-file): new procedure to
  generate basic init file.
  (default-logfile): Check for init file, create it if necessary.
---
 modules/dmd/support.scm |   26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/modules/dmd/support.scm b/modules/dmd/support.scm
index 16be9a7..f95e8e8 100644
--- a/modules/dmd/support.scm
+++ b/modules/dmd/support.scm
@@ -162,6 +162,29 @@ There is NO WARRANTY, to the extent permitted by law.")))
 ;; dmd default subdirectory if dmd is run as a normal user.
 (define user-dmddir (string-append user-homedir "/.dmd.d"))
 
+(define (make-bare-init-file target)
+  "Return #t if a bare init file was created at TARGET; #f otherwise.
+
+TARGET should be a string representing a filepath + name."
+  (with-output-to-file target
+    (lambda ()
+      (format #t
+              ";; init.scm -- default dmd configuration file.
+
+;; Services known to dmd:
+;; Add new services (defined using 'make <service>') to dmd here by
+;; providing them as arguments to 'register-services'.
+(register-services)
+
+;; Send dmd into the background
+(action 'dmd 'daemonize)
+
+;; Services to start when dmd starts:
+;; Add the name of each service that should be started to the list
+;; below passed to 'for-each'.
+(for-each start '())
+"))))
+
 ;; Logfile.
 (define default-logfile
   (if (zero? (getuid))
@@ -174,6 +197,8 @@ There is NO WARRANTY, to the extent permitted by law.")))
       (string-append Prefix-dir "/etc/dmdconf.scm")
       (let ((config-file (string-append user-dmddir "/init.scm")))
         (catch-system-error (mkdir user-dmddir))
+        (if (not (file-exists? config-file))
+            (make-bare-init-file config-file))
         config-file)))
 
 ;; The directory where the socket resides.
@@ -231,4 +256,3 @@ which has essential bindings pulled in."
 	     (begin
 	       (local-output "Socket directory setup is insecure.")
 	       (quit 1))))))
-
-- 
1.7.9.5

Re: [PATCH 2/2] dmd: Make config file if necessary when not run as root.

[Ludovic Courtès]

Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

> Ludovic Courtès writes:

[...]

>>> + (register-services)
>>    ^
>>> +;; Send dmd into the background
>>> + (action 'dmd 'daemonize)
>>    ^
>>> + (for-each start '())
>>    ^
>>
>> Extra space here.
>
> The extra spaces were there to stop emacs (maybe Scheme mode?) from
> going nutty because of the open parens within a string at column 0.

Oh, right, I forgot that.

[...]

>>>          (let ((config-file (string-append user-dmddir "/init.scm")))
>>>            (cond ((not (file-exists? user-dmddir))
>>>                   (mkdir user-dmddir)
>>> +                 (make-bare-init-file config-file)
>>>                   config-file)
>>> +                ((not (file-exists? config-file))
>>> +                 (make-bare-init-file config-file))
>>>                  (else config-file))))))
>>
>> So I guess this should be changed to:
>>
>>   (catch 'system-error
>>     (lambda ()
>>       (mkdir user-dmddir)
>>       (make-bare-init-file config-file))
>>     (const #f))
>
> I've changed this to :
>
>          (catch-system-error (mkdir user-dmddir))
>          (if (not (file-exists? config-file))
>              (make-bare-init-file config-file))
>          config-file)))

[...]

> I also think the race condition should not be a problem for the config
> file: if the config file is added after the check, then we simply
> override it. If the config file is removed after the check then guix
> crashes when it tries to load it, as it always would do before this
> patch.

Right.  Hopefully it’s not a problem with the test suite because tests
use ‘-c’ anyway.

> Hope this works now. You should find the patch inline.

Yep.  Pushed, thanks!

Ludo’.