Re: [PATCH v2 2/6] maintenance: use packaged systemd units

[Eric Sunshine <sunshine@sunshineco.com>]

On Sat, Mar 23, 2024 at 4:21 AM Max Gautier <mg@max.gautier.name> wrote:
> [...]
> Package the systemd user units (timer and service) with git in
> $(prefix)/lib/systemd/user (or $XDG_DATA_HOME for $HOME installation),
> and remove code for writing and deleting the units from builtin/gc.c.
> Determine the correct git path at install time by for the service unit.
>
> Detect systemd timers support (at install time) by relying on systemctl
> presence, since we use it as the control interface for the systemd
> scheduler.
>
> Signed-off-by: Max Gautier <mg@max.gautier.name>
> ---
> Notes:
>     I'm not completely sure if it's ok to do install time templating like
>     this, but I couldn't find a similar enough example in the Makefile. Any
>     suggestion for a better way ?
>
> diff --git a/Makefile b/Makefile
> @@ -3469,6 +3469,11 @@ install: all
> +ifdef SYSTEMD_USER_UNIT_DIR
> +       $(INSTALL) -Dm 644 -t '$(DESTDIR_SQ)$(SYSTEMD_USER_UNIT_DIR)' systemd/user/git-maintenance@.timer
> +       sed 's+@BINDIR@+$(bindir_SQ)+' systemd/user/git-maintenance@.service.in | \
> +               $(INSTALL) -Dm 644 /dev/stdin '$(DESTDIR_SQ)$(SYSTEMD_USER_UNIT_DIR)/git-maintenance@.service'
> +endif

This is the first use of /dev/stdin in the project and I might worry a
bit about portability. Granted, a system in which systemd is installed
is likely to have /dev/stdin available, but it's often a good idea to
be cautious when introducing something new into the project.

I would think it would be possible to instead generate the
`git-maintenance@.service` file locally from the template
`git-maintenance@.service.in` as part of the normal build process, and
then install the built `git-maintenance@.service` at "install" time.
That seems more in line with how other resources are handled, avoids
the novel use of /dev/stdin, and answers the question you ask above.

> diff --git a/config.mak.uname b/config.mak.uname
> @@ -68,6 +68,16 @@ ifeq ($(uname_S),Linux)
> +       ifeq ($(shell command -v systemctl >/dev/null ?&& echo y),y)

What is "?&&"?

> +               XDG_DATA_HOME ?= $(HOME)/.local/share
> +               # systemd user units of programm installed in the home directory
> +               # (meaning prefix == $HOME) shall go in XDG_DATA_HOME
> +               # (from man 5 systemd.unit)

s/programm/program/