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

[Alex Sassmannshausen <alex.sassmannshausen@gmail.com>]

[-- 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