Re: [PATCH v3] base: Annotate output with job information.

[Maxim Cournoyer]

Hello again!

Dale Mellor <mcron-lsfnyl@rdmp.org> writes:

> On Tue, 2021-08-24 at 00:20 -0400, Maxim Cournoyer wrote:
>> Before this change, it was difficult to discern which job
>> emitted which
>> output, as there was no information connecting the job to the
>> output it
>> produced.  This change rectifies that by annotating each line
>> output by
>> cron/mcron with a prefix that contains a timestamp and the job
>> name.  It also
>> reports about when the job runs and whether it completed
>> successfully or
>> failed.
>
>   This is great, but which version of Guile does it require?  I'm
> using the latest Debian 11 with its guile-3.0.5 and get the
> following
>
> In procedure dispatch-exception:
> Syntax error:
> mcron/base.scm:319:2: definition in expression context, where
> definitions are not allowed, in form (define (log-data data) (let
> ((name (job-data:name data)) (pid (job-data:pid data)) (port
> (job-data:port data)) (cont (job-data:continuation data)))
> (define (read-line*) (call-with-prompt (quote continue) (lambda
> () (parameterize ((current-read-waiter (lambda (_) (abort-to-
> prompt (quote continue))))) (if cont (begin (set-job-data-
> continuation! data #f) (cont)) (read-line port (quote split)))))
> (lambda (partial-continuation) (cons (quote suspended) partial-
> continuation)))) (define (format-line line) (let* ((cr-index
> (string-index-right line #\return)) (line* (if cr-index (string-
> drop line (#{1+}# cr-index)) line))) (format #t "~@?" (%log-
> format) timestamp pid name line*))) (let loop ((line+delim (read-
> line*))) (match line+delim (((quote suspended) . partial-
> continuation) (set-job-data-continuation! data partial-
> continuation)) ((line ? eof-object?) (close port) (unless (eof-
> object? line) (format-line line))) ((line . _) (format-line line)
> (loop (read-line*)))))))

That's strange; I would have thought 3.0.5 should have been new enough.

>   Mcron should really be compilable on this system.  (I would
> further point out that configure.ac currently allows Guile 3.0,
> 2.2 and 2.0!)

Oh, apologies, I hadn't consider that GNU Mcron should be compatible
with these (much) older Guiles.  I'm running Mcron with this patch on my
Guix System, which uses Guile 3.0.2.  I believe I was using Guile 3.0.7
to test during development.

I'll look if it's possible to make this change compatible with 2.0 and
2.2, but if it was to require a lot of work (haven't checked yet), I'd
question if it's really worth it.

> Dale
>
> ps. It would be useful also if you could provide patches against
> commits on the master branch so that we can 'git am' them
> directly.

The previous patch (the error handling on a non-existing home directory)
was based on master 97cdae129d48752dad8e7a424cbcacce32a0bfd9, while this
one was made on top.

> pps. I'm totally in support of this excellent work getting merged
> with mcron master.

I'm happy to read it!  It was a surprisingly difficult (tricky) thing to
implement, I found.  Perhaps due to my inexperience messing with 'dup'
and friends.

Thank you for taking a look!

Maxim