diff mbox series

[bug#55659,1/2] services: ddclient: Fix extra-options serialization.

Message ID 20220526130912.29483-1-attila@lendvai.name
State Accepted
Headers show
Series [bug#55659,1/2] services: ddclient: Fix extra-options serialization. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Attila Lendvai May 26, 2022, 1:09 p.m. UTC 
Prior to this fix it has serialized "extra-options=..." into the ddclient.conf
file.

* gnu/services/dns.scm (serialize-field): Special case for extra-options.
(serialize-extra-options): Deleted unused function.
(ddclient-configuration): Change the type of the extra-options field to be a
string, and emit it verbatime into the ddclient.conf file. Change
docstring accordingly.
---

please note that even though this does fix an issue, i have abandoned
further work on ddclient. so, this is an improvement indeed, but i
suspect that this does not fully fix ddclient. looks like the conf
file format is different than what the current code emits.

the next patch updates ddclient to an RC2 of the next version,
which will be a major upgrade.

 gnu/services/dns.scm | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Ludovic Courtès June 16, 2022, 9:11 p.m. UTC | #1 
Hi,

Attila Lendvai <attila@lendvai.name> skribis:

> Prior to this fix it has serialized "extra-options=..." into the ddclient.conf
> file.

[...]

>  (define (serialize-field field-name val)
> -  (when (not (member field-name '(group secret-file user)))
> -    (format #t "~a=~a\n" (uglify-field-name field-name) val)))
> +  (cond
> +   ((eq? field-name 'extra-options)
> +    (display val))
> +   ((not (member field-name '(group secret-file user)))
> +    (format #t "~a=~a\n" (uglify-field-name field-name) val))))
>  
>  (define (serialize-boolean field-name val)
>    (serialize-field field-name (if val "yes" "no")))
> @@ -921,9 +924,6 @@ (define (serialize-string field-name val)
>  (define (serialize-list field-name val)
>    (if (null? val) "" (serialize-field field-name (string-join val))))
>  
> -(define (serialize-extra-options extra-options)
> -  (string-join extra-options "\n" 'suffix))
> -
>  (define-configuration ddclient-configuration
>    (ddclient
>     (file-like ddclient)
> @@ -959,8 +959,8 @@ (define-configuration ddclient-configuration
>  file contains credentials for use by ddclient.  You are expected to create it
>  manually.")
>    (extra-options
> -   (list '())
> -   "Extra options will be appended to @file{ddclient.conf} file."))
> +   (string "")
> +   "Extra options will be appended verbatim to the @file{ddclient.conf} file."))

In other places, such as (gnu home services desktop), I’ve used an extra
‘raw-configuration-string’ field type to handle this cases.  This might
be clearer than adding the ‘cond’ above in ‘serialize-field’.  WDYT?

The change above also changes the interface (from list to string), but I
think that’s OK.

Thanks,
Ludo’.
Attila Lendvai June 17, 2022, 8:53 a.m. UTC | #2 
> In other places, such as (gnu home services desktop), I’ve used an extra
> ‘raw-configuration-string’ field type to handle this cases. This might
> be clearer than adding the ‘cond’ above in ‘serialize-field’. WDYT?


i agree, it's a much better, more intuitive name. i'll use it from now on.

(note that i'm not planning to work on this more. i hope someone will pick this up who actually uses ddclient, and therefore has the ability to test it with little effort. feel free to close it, too.)

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“There is no leader, there is no teacher, there is nobody to tell you what to do. You are alone in this mad, brutal world.”
	— Jiddu Krishnamurti (1895–1986)
diff mbox series

Patch

diff --git a/gnu/services/dns.scm b/gnu/services/dns.scm
index 50753b7ab6..c2ede312e8 100644
--- a/gnu/services/dns.scm
+++ b/gnu/services/dns.scm
@@ -904,8 +904,11 @@  (define (uglify-field-name field-name)
   (string-delete #\? (symbol->string field-name)))
 
 (define (serialize-field field-name val)
-  (when (not (member field-name '(group secret-file user)))
-    (format #t "~a=~a\n" (uglify-field-name field-name) val)))
+  (cond
+   ((eq? field-name 'extra-options)
+    (display val))
+   ((not (member field-name '(group secret-file user)))
+    (format #t "~a=~a\n" (uglify-field-name field-name) val))))
 
 (define (serialize-boolean field-name val)
   (serialize-field field-name (if val "yes" "no")))
@@ -921,9 +924,6 @@  (define (serialize-string field-name val)
 (define (serialize-list field-name val)
   (if (null? val) "" (serialize-field field-name (string-join val))))
 
-(define (serialize-extra-options extra-options)
-  (string-join extra-options "\n" 'suffix))
-
 (define-configuration ddclient-configuration
   (ddclient
    (file-like ddclient)
@@ -959,8 +959,8 @@  (define-configuration ddclient-configuration
 file contains credentials for use by ddclient.  You are expected to create it
 manually.")
   (extra-options
-   (list '())
-   "Extra options will be appended to @file{ddclient.conf} file."))
+   (string "")
+   "Extra options will be appended verbatim to the @file{ddclient.conf} file."))
 
 (define (ddclient-account config)
   "Return the user accounts and user groups for CONFIG."