From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=)
Subject: Re: [PATCH] gnu: rottlog: rotate messages daily.
Date: Thu, 22 Sep 2016 00:32:35 +0900
Message-ID: <87shsti7zg.fsf@gnu.org>
References: <87d1lfziew.fsf@gnu.org> <87k2eoox3p.fsf@gnu.org>
	<877fanyjoy.fsf@gnu.org> <87zinhve2w.fsf@gnu.org>
	<87zinelg1t.fsf@gnu.org> <87eg4ou7h5.fsf@gnu.org>
	<87mvjbqxzf.fsf@gnu.org> <874m5itjv5.fsf@gnu.org>
	<87wpi7p2wh.fsf@gnu.org> <87eg4epach.fsf@gnu.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Return-path: <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42472)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1bmjW5-0000rn-HK
	for guix-devel@gnu.org; Wed, 21 Sep 2016 11:32:58 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1bmjW1-0000p4-Am
	for guix-devel@gnu.org; Wed, 21 Sep 2016 11:32:56 -0400
In-Reply-To: <87eg4epach.fsf@gnu.org> (Jan Nieuwenhuizen's message of "Tue, 20
	Sep 2016 22:47:26 +0200")
List-Id: "Development of GNU Guix and the GNU System distribution."
	<guix-devel.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/guix-devel/>
List-Post: <mailto:guix-devel@gnu.org>
List-Help: <mailto:guix-devel-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=subscribe>
Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org
Sender: "Guix-devel" <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
To: Jan Nieuwenhuizen <janneke@gnu.org>
Cc: guix-devel@gnu.org

Hello!

Jan Nieuwenhuizen <janneke@gnu.org> skribis:

> Ludovic Court=C3=A8s 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 =E2=80=A6) to a global variable, to avoid code duplica=
tion
>> 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 =E2=80=98rottlog-configuration=E2=80=99 is actually a macro.  So at eve=
ry 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 =E2=80=98cons=E2=80=
=99 on it.

> From ee1be88f60d70de46009069da020c1bdc4993fd8 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <janneke@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-coreutil=
s-8.25")) ;; FIXME

Good point=E2=80=A6

> +    (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 =E2=80=98plain-file=E2=80=99 here because of the comp=
uted =E2=80=98kill=E2=80=99
file name.  So instead, this would be something along the lines of
(moving =E2=80=98string-append=E2=80=99 from the host side to the build sid=
e):

  (define (syslog-rotation-config file)
    #~(string-append #$file " {\n" =E2=80=A6
                     #$coreutils "/bin/kill -HUP =E2=80=A6"
                     "}\n"))

and:

  (define %default-rotations
    `(("weekly" ,(computed-file "rottlog.weekly"
                                #~(call-with-output-file #$output
                                    (lambda (port)
                                      (display #$(syslog-rotation-file =E2=
=80=A6)
                                               port)))))))
HTH!

Thanks again for taking the time and coping with half-baked advice!
;-)

Ludo=E2=80=99.