[bug#56046] [PATCH opensmtpd-records v3] services (opensmtpd): add opensmtpd records to enhance opensmtpd-configuration.

[Liliana Marie Prikler]

Am Dienstag, dem 27.12.2022 um 19:16 -0500 schrieb Joshua Branson:
> Are you giving me a triple A+ ? :) Org generated the the like that. I
> think you mentioned that I should use fractions last time. Sorry I
> did not do that.
Do you have to convert your documentation from org?  Writing Texinfo
code manually is an option, as is generating it from define-
configuration IIRC.  There is also nothing wrong with manually touching
up generated docs, but I imagine doing so consistently might be a bit
more adventurous.

> If I wait 'til I implement every one of your suggestions, I will
> probably never submit it. I am really probably "perfecting" this
> service.
You can submit whatever, but don't expect me or any other committer to
upstream the patches while there are open points to address.
> 

> > Instead of a string, take 'no-verify as symbol perhaps?
> 
> Sounds good to me. May I ask why you prefer a symbol instead of a
> string?
Symbols can be compared with eq?, case et al.

> > > -;;;
> > >  ;;; OpenSMTPD.
> > >  ;;;
> > > +;;; This next bit of code helps me create my own sanitizer
> > > functions.
> > > +
> > > +;; some fieldnames have a default value of #f, which is ok. 
> > > They
> > > cannot have
> > > +;; a value of #t.
> > > +;; for example opensmtpd-table-data can be #f, BUT NOT true.
> > > +;; my/sanitize procedure tests values to see if they are of the
> > > right kind.
> > > +;; procedure false? is needed to allow fields like 'values' to
> > > be
> > > blank,
> > > +;; (empty), or #f BUT also have a value like a list of strings.
> > Use less egocentric comments ;)
> 
> I'm not sure what you mean here? I know I had a comment in my task
> list that said something like my sanitizer function are probably
> better than those found in guix. Apologies for that.
For what it's worth, it definitely wasn't I. [1]

> > 
> > > +(define (false? var)
> > > +  (eq? #f var))
> > > +
> > > +;; TODO I have to have this procedure, or I need to change
> > > my/sanitize
> > > +;; procedure.
> > > +(define (my-file-exists? file)
> > > +  (and (string? file)
> > > +       (access? file F_OK)))
> > Does file-exists? not work for you?
> 
> The file-exists? function causes my-sanitize function to break.
Why?

> I think. 
Prove it.

> If you get rid of it, then what happens when a user types in (file
> 4), you get an raise-exception. 
(file-exists? "(file 4)") ; => #f

> I can probably just rework my-sanitizer function to
> deal with that possibility, but I have not yet. I would love some
> guidance on how to do that. Because I feel like having to handle that
> exception is hard.

>From the Guile manual:

 -- Scheme Procedure: stat object [exception-on-error?]
 -- C Function: scm_stat (object, exception_on_error)
[...]
     If the optional EXCEPTION_ON_ERROR argument is true, which is the
     default, an exception will be raised if the underlying system call
     returns an error, for example if the file is not found or is not
     readable.  Otherwise, an error will cause ‘stat’ to return ‘#f’.

Now, in (ice-9 boot-9), file-exists? is defined (assuming posix) as

      (lambda (str)
        (->bool (stat str #f)))

Thus, I am pretty sure that no exception should be raised from the
check ;)

> > 
> > > +;; This procedure takes in a var and a list of procedures.  It
> > > loops
> > > through
> > > +;; list of procedures passing in var to each.
> > > +;; if one procedure returns #t, the function returns true. 
> > > Otherwise #f.
> > > +;; TODO for fun rewrite this using map
> > > +;; If I rewrote it in map, then it may help with sanitizing.
> > > +;; eg: I could then potentially easily sanitize vars with lambda
> > > procedures.
> > > +(define (is-value-right-type? var list-of-procedures record
> > > fieldname)
> > > +  (if (null? list-of-procedures)
> > > +      #f
> > > +      (if ((car list-of-procedures) var)
> > > +          #t
> > > +          (is-value-right-type? var (cdr list-of-procedures)
> > > record
> > > +                                fieldname))))
> > Alternatively, (any (cut <> var) list-of-procedures).
> 
> You mentioned that in the last review, I just can't figure out how to
> use your
> suggestion. This is the code that I have in the task list WIP:
> 
> *** TODO simplify my sanitizing funcions  (any (cut <> var))
> 
> #+BEGIN_SRC scheme
> (use-modules (ice-9 curried-definitions)
>              (srfi srfi-26))
> 
> (define (((expect-any predicates) record field) var)
>   (if (any (cut <> var) predicates)
>       var
>       (begin
>         ;; code code code
>         ;; how do I tell the user which function failed?
>         (display "error")
>         (throw 'bad! var))))
All of them failed, that's the point.  As for constructing a string
from a list of procedures, see list-of-procedures->string.

> ;; here is how you use it.
>   (name opensmtpd-table-name ;; string
>         (default #f)
>         (sanitize (lambda (var)
>                     (((expect-any (list string? number?)) "hello"
> "that") var))))
> 
> #+END_SRC
> 
> Does that look close to what you want? I feel like it is way off, but
> I don't know. Honestly when I say this suggestion I was completely
> blown away, I have been using (any ) and (every) in a few places to
> get rid of some uses of primitive eval.
I don't see that, but I do see functions that have been dropped still
mentioned in the ChangeLog.  Another hint at this patch being too
convoluted for its own sake ;)

> > This procedure needs a proper name, like sanitize/check-type, but
> > more importantly, why not simply use define-configuration?
> 
> Yes! I have slowly been realizing that I have been clumsily re-
> inventing define-configuration. I hope to switch to define-
> configuration, because a lot of this code would go away. But I need
> to explore how define-configuration works.
> That would be quite a major change. :)
Manchmal erspart einem monatelange Implementier-Arbeit einen Nachmittag
in der Bücherei.

Cheers

[1] https://issues.guix.gnu.org/issue/56046#4-lineno323