head/usr.sbin/cron/cron/cron.8
20	typo
head/usr.sbin/cron/cron/database.c
80	This means that modifying an existing file in /etc/cron.d is not detected. That is an uncommon thing to do.
127	We have filesystems that return DT_UNKNOWN (such as NFS with default options), in which case an fstatat is required. This check also prevents following symlinks in /etc/cron.d which Debian's cron allows.

bapt marked an inline comment as done.Oct 31 2016, 11:40 PM

bapt added inline comments.

head/usr.sbin/cron/cron/database.c
80	That is the same behaviour as how users crontabs are detected to have been modified. I find it pretty convenient :) might be inefficient if lots of cron files have been added any better idea?
127	Fixed thanks

This is a long-awaited feature that we have been missing, and it's great we finally have it implemented.

As much as I welcome this, I found a couple of differences in features this implementation has from Debian's, so let me point them out in inline comments.

head/usr.sbin/cron/cron/database.c
80	The cron(8) man page reads: "Note that the crontab(1) command updates the modification time of the spool directory whenever it changes a crontab." So for user crontabs it is transparent to users because they always edit their crontab via crontab(1). On the other hand there's no tool support for /etc/cron.d/*, so you have to manually edit a file in it just to find out crond will not reload it unless you touch /etc/cron.d by hand. I think we should follow Debian's cron which properly checks modification times of individual files in cron.d: https://anonscm.debian.org/cgit/pkg-cron/pkg-cron.git/tree/database.c?id=1215fdaddba158f32b10f0cc041e634505d50cb8#n118
131	Loading all crontabs into a single namespace that is the same as /etc/crontab ("root") means environment variable settings defined in a file will be overwritten by those in another. So, one crontab file installed by a package may affect system jobs, which does not seem great to me. Debian seems to allocate separate namespaces for each cron.d file by using the filenames: https://anonscm.debian.org/cgit/pkg-cron/pkg-cron.git/tree/database.c?id=1215fdaddba158f32b10f0cc041e634505d50cb8#n247

In D8400#189384, @knu wrote:

This is a long-awaited feature that we have been missing, and it's great we finally have it implemented.

As much as I welcome this, I found a couple of differences in features this implementation has from Debian's, so let me point them out in inline comments.

I agree with both comments, I do not have much time right now to work on this, right now, do not hesitate to propose a patch if you have time otherwise I will do it as soon as I can find free time