Subject: Re: [PATCH] maintenance: use systemd timers on Linux

From: Đoàn Trần Công Danh <[email-masked]>
Date: Wed, 5 May 2021 19:19:33 +0700

On 2021-05-02 15:10:05+0100, Phillip Wood <[email-masked]> wrote:
> On 02/05/2021 07:45, Eric Sunshine wrote:
> > On Sat, May 1, 2021 at 10:59 AM Lénaïc Huard <[email-masked]> wrote:
> > > The existing mechanism for scheduling background maintenance is done
> > > through cron. On Linux systems managed by systemd, systemd provides an
> > > alternative to schedule recurring tasks: systemd timers.
> > 
> > Thanks for working on this. While `cron` has been the go-to standard
> > for decades, `systemd` is certainly widespread enough that it makes
> > sense to support it, as well.
> 
> Yes, thank you for working on this, it will be very useful to users like me
> who use a linux distribution that does not install a cron daemon by default
> but relies on systemd instead.
> 
> > > The main motivations to implement systemd timers in addition to cron
> > > are:
> > > * cron is optional and Linux systems running systemd might not have it
> > >    installed.
> > > * The execution of `crontab -l` can tell us if cron is installed but not
> > >    if the daemon is actually running.
> 
> Can we use systemctl to see if it is running (and enabled so we know it will
> be restarted after a reboot)?

Not sure if I understand this suggestion.
However, non-systemd systems doesn't have systemctl command to begin
with.

> > > * With systemd, each service is run in its own cgroup and its logs are
> > >    tagged by the service inside journald. With cron, all scheduled tasks
> > >    are running in the cron daemon cgroup and all the logs of the
> > >    user-scheduled tasks are pretended to belong to the system cron
> > >    service.
> > >    Concretely, a user that doesn’t have access to the system logs won’t
> > >    have access to the log of its own tasks scheduled by cron whereas he
> > >    will have access to the log of its own tasks scheduled by systemd
> > >    timer.
> > 
> > The last point is somewhat compelling. A potential counterargument is
> > that `cron` does send email to the user by default if any output is
> > generated by the cron job. However, it seems quite likely these days
> > that many systems either won't have local mail service enabled or the
> > user won't bother checking the local mailbox. It's a minor point, but
> > if you re-roll it might make sense for the commit message to expand
> > the last point by saying that although `cron` attempts to send email,
> > that email may go unseen by the user.
> > 
> > > In order to schedule git maintenance, we need two unit template files:
> > > * ~/.config/systemd/user/git-maintenance@.service
> > >    to define the command to be started by systemd and
> > > * ~/.config/systemd/user/git-maintenance@.timer
> > >    to define the schedule at which the command should be run.

I think it would be better to change ~/.config here to
$XDG_CONFIG_HOME, as others also points out in another comments.

[..snip..]

> > > +`git maintenance start` will overwrite these files and start the timer
> > > +again with `systemctl --user`, so any customization should be done by
> > > +creating a drop-in file
> > > +`~/.config/systemd/user/git-maintenance@.service.d/*.conf`.

Ditto.

> > Will `systemd` users generally understand what filename to create in
> > the "...@.service.d/" directory, and will they know what to populate
> > the file with? (Genuine question; I've never dealt with that.)
> 
> I think it would be helpful to explicitly mention the file names (I don't
> think I could tell you what they are without reading the relevant systemd
> man page)

[..snip..]

> > > +static char *systemd_timer_timer_filename()
> > > +{
> > > +       const char *filename = "~/.config/systemd/user/git-maintenance@.timer";
> > > +       char *expanded = expand_user_path(filename, 0);
> > > +       if (!expanded)
> > > +               die(_("failed to expand path '%s'"), filename);
> > > +
> > > +       return expanded;
> > > +}
> > 
> > I was curious whether this would fail if `.config/systemd/user/`
> > didn't already exist, but looking at the implementation of
> > expand_user_path() , I see that it doesn't require the path to already
> > exist if you pass 0 for the second argument as you do here. Okay.
> 
> Do we need to worry about $XDG_CONFIG_HOME rather than hard coding
> "~/.config/". There is a function xdg_config_home() that takes care of this.

-- 
Danh