Hello!
Jan Nieuwenhuizen <jann...@gnu.org> skribis:
> Ludovic Courtès writes:
[...]
>>> + (jobs rottlog-jobs ; list of <mcron-job>
>>> + (default
>>> + (list #~(job
>>> + '(next-hour '(0))
>>> + (lambda ()
>>> + (system (string-append #$rottlog "/sbin/rottlog"))))
>>> + #~(job
>>> + '(next-hour '(12))
>>> + (lambda ()
>>> + (system (string-append #$rottlog
>>> "/sbin/rottlog"))))))))
>>
>> Please move (list …) to a global variable, to avoid code duplication
>> when the macro is expanded.
>
> ...moved to a function now...but I don't see what macro you mean (#~ ?)
> and when it gets expanded and how that leads to duplication.
The ‘rottlog-configuration’ is actually a macro. So at every call site
where a default value is used, the default value code is duplicated,
leading to code bloat if that default value is a big expression.
> I'm not sure if we should export the %default-rotations or if we should
> describe their contents in the manual.
Probably worth exporting and documenting, so people can ‘cons’ on it.
> From ee1be88f60d70de46009069da020c1bdc4993fd8 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <jann...@gnu.org>
> Date: Thu, 8 Sep 2016 01:20:43 +0200
> Subject: [PATCH] gnu: services: add rottlog.
>
> * gnu/services/admin.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> * doc/guix.texi (Log Rotation): Document it.
[...]
> +(define (rotation-config file kill)
> + (string-append file " {
> + sharedscripts
> + postrotate
> +" kill
> +" endscript
> + nocompress
> +}
> +"))
> +
> +(define (syslog-rotation-config file)
> + (let ((coreutils
> "/gnu/store/56x9fvx59i300wav3c193h84cp80bslr-coreutils-8.25")) ;; FIXME
Good point…
> + (rotation-config
> + file
> + (string-append
> + " "
> + coreutils "/bin/kill -HUP $(cat /var/run/syslog.pid) 2> /dev/null
> +"))))
> +
> +(define (simple-rotation-config file)
> + (rotation-config file ""))
> +
> +(define %default-rotations
> + `(("weekly" ,(plain-file "rottlog.weekly"
> + (string-append (string-join
> + (map syslog-rotation-config
> + %rotated-files)
> + "")
> + (simple-rotation-config
> + "/var/log/shepherd.log"))))))
I think we cannot use ‘plain-file’ here because of the computed ‘kill’
file name. So instead, this would be something along the lines of
(moving ‘string-append’ from the host side to the build side):
(define (syslog-rotation-config file)
#~(string-append #$file " {\n" …
#$coreutils "/bin/kill -HUP …"
"}\n"))
and:
(define %default-rotations
`(("weekly" ,(computed-file "rottlog.weekly"
#~(call-with-output-file #$output
(lambda (port)
(display #$(syslog-rotation-file …)
port)))))))
HTH!
Thanks again for taking the time and coping with half-baked advice!
;-)
Ludo’.
