Page MenuHomeFreeBSD

cron: add log suppression and mail suppression for successful runs
AcceptedPublic

Authored by freebsd_t.lastninja.net on Apr 25 2019, 5:16 AM.

Details

Reviewers
kevans
bcr
Group Reviewers
manpages
Summary

I would like to see FreeBSD provide the crontab extensions found in OpenBSD
that implement -n to suppress mail on successful run and -q to suppress
logging of command execution.

The -q option appears decades old, but -n is relatively new. The
original proposal by Job Snijder can be found here [1], and gives very
convincing reasons for inclusion in base.

This patch is a nearly identical port of OpenBSD cron for -q and -n
features. It is written to follow existing conventions and style of the
existing codebase. I'm also not good at writing manpages but gave it my best
shot.

[1]: https://marc.info/?l=openbsd-tech&m=152874866117948&w=2

PR: 237538

Test Plan

I tested each of the following crontab entries in isolation:

# should both log and send email
* * * * * date

# should log but not send email (check to preserve original behaviour)
* * * * * date >/dev/null

# should only send email, but won't show up in log
* * * * * -q date

# should not send email
* * * * * -n date

# should not send email or log
* * * * * -n -q date

# should send email because of ping failure
* * * * * -n -q ping -c 1 5.5.5.5

I also tested to ensure the command options work using the
/etc/crontab format which requires user to be specified.

Also, I tested the empty command entry, e.g.:

* * * * *

This will log but never send an email.

However, because of the way the parsing works, an empty command
is not supported in conjunction with command options. So:

* * * * * -q

will not parse as valid crontab syntax.

I suspect this is the same behaviour as OpenBSD crontab, and
frankly makes sense - since an empty command entry only purpose
is to provide a signal in the cron logs.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 23922
Build 22842: arc lint + arc unit

Event Timeline

freebsd_t.lastninja.net edited the test plan for this revision. (Show Details)Apr 25 2019, 5:17 AM
freebsd_t.lastninja.net edited the test plan for this revision. (Show Details)Apr 25 2019, 5:20 AM
freebsd_t.lastninja.net edited the summary of this revision. (Show Details)Apr 25 2019, 5:24 AM
freebsd_t.lastninja.net edited the summary of this revision. (Show Details)
  • crontab: add -n to not mail if run is successful
  • crontab: add -n to not mail if run is successful
freebsd_t.lastninja.net edited the summary of this revision. (Show Details)Apr 25 2019, 2:10 PM
freebsd_t.lastninja.net edited the summary of this revision. (Show Details)
freebsd_t.lastninja.net set the repository for this revision to rS FreeBSD src repository.
pi added a subscriber: pi.Apr 25 2019, 6:54 PM
bcr added a subscriber: bcr.Apr 26 2019, 9:37 AM

Can you run "mandoc -Tlint" and textproc/igor over the man page? Thanks!

  • crontab: add -q option to suppress logging
  • crontab: add -n to not mail if run is successful
In D20046#431547, @bcr wrote:

Can you run "mandoc -Tlint" and textproc/igor over the man page? Thanks!

Hi I have run both commands and rectified all warnings introduced by my change.

Thanks.

bcr accepted this revision.Apr 26 2019, 1:01 PM

OK, thanks. Approved for the manpages part.

This revision is now accepted and ready to land.Apr 26 2019, 1:01 PM
freebsd_t.lastninja.net edited the test plan for this revision. (Show Details)Apr 27 2019, 11:55 AM

Sorry for the long delay... I think just these really minor style nits.

usr.sbin/cron/cron/do_command.c
99

Kill this blank line

539

Please move this comment to its own spacing after beginning and before end of comment markers

543

I'd lean towards moving the comment to the line before it, but the style here is all fairly broken (already, before this change) with respect to style(9)

usr.sbin/cron/lib/entry.c
440

Space after switch keyword

danfe added a subscriber: danfe.Jul 11 2019, 3:07 AM
danfe added inline comments.
usr.sbin/cron/cron/do_command.c
101

Since you're changing these lines anyway, would it make sense to drop the register specifier?

546

This comment violates our style(9).

553

This one as well.

559

I'm not a native speaker, but shouldn't executed be followed by an adverb, not adjective?

kevans added inline comments.Jul 11 2019, 3:43 AM
usr.sbin/cron/cron/do_command.c
546

It matches the style of the rest of the file, which is more important and allowed by style(9) last time I read it.

553

Ditto above comment

559

Indeed, successfully