Re: [PATCH] gnu: rottlog: rotate messages daily.

[Ludovic Courtès]

Halo!

Jan Nieuwenhuizen <address@hidden> skribis:

> Ludovic Courtès writes:

[...]

>> The configuration probably contain something like an list of name/config
>> pairs as well as an ‘rc’ file, along these lines:
>>
>>   (rottlog-configuration
>>     (periods `(("weekly" . ,(file-append rottlog "/etc/weekly))
>>                ("daily" . ,(plain-file "daily" "…"))))
>>     (rc-file (file-append rottlog "/etc/rc")))
>
> I think I have this too now.
>
>> Of course we should provide default config values that take care of
>> common files such as /var/log/{messages,Xorg.0.log} in a reasonable way.
>
> Hmm, yes.  Input appreciated here!  Currently I just use the rottlog
> defaults except for using daily instead of weekly rotations, only
> rotating /var/log/messages.  On my box, that seems to be the only file
> that really needs rotation atm.  Is that sane?

I would add /var/log/{secure,shepherd.log}, but weekly is probably
enough.

> From 8b5e5245b066f13c4cef6e5371cdcb84f2d84085 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> 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.

I think we’re pretty much there!

> +(define-record-type* <rottlog-configuration>
> +  rottlog-configuration make-rottlog-configuration
> +  rottlog-configuration?
> +  (rottlog rottlog-rottlog (default rottlog))
> +  (rc-file rottlog-rc-file)
                             ^
With: (default (file-append rottlog "/etc/rc")).

> +  (periods rottlog-periods)

Maybe s/periods/periodic-rotations/ ?
Also with default rotation.

> +  (jobs rottlog-jobs))

We should have a sane default here, like running it twice a day (which
can’t hurt because “The logfiles cannot be modified multiple times per
period”, says the manual.)

It’s a good idea to mark the expected types of each field in the margin
(until we have a type system ;-)).

> +(define (files-alist->directory files)
> +  (define builder
> +    #~(begin
> +        (use-modules (ice-9 match))
> +        (mkdir #$output)
> +
> +        (for-each (lambda (name file)
> +                    (symlink file (string-append #$output "/" name)))
> +                  '#$(map car files)
> +                  '#$(map cdr files))))
> +  
> +    (computed-file "rottlog" builder))

You can remove it and use ‘file-union’ from (gnu services), which is
roughly the same (it expects a list of tuples instead of a list of
pairs.)

> +(define* (rottlog-service

This can be omitted.  It’s enough to expose ‘rottlog-service-type’ and
‘rottlog-configuration’.

The last thing that’s missing is a “Log Rotation” section in guix.texi,
with cross-references to the rottlog manual, like “Scheduled Job
Execution” does.  Could you look into it?

Thank you!

Ludo’.