Re: [RFC PATCH 1/5] maintenance: package systemd units

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 3459 bytes --]

On Mon, Mar 18, 2024 at 04:31:15PM +0100, Max Gautier wrote:

It would be great to document _why_ we want to package the systemd units
alongside with Git.

> Signed-off-by: Max Gautier <mg@max.gautier.name>
> ---
>  Makefile                              |  4 ++++
>  systemd/user/git-maintenance@.service | 16 ++++++++++++++++
>  systemd/user/git-maintenance@.timer   |  9 +++++++++
>  3 files changed, 29 insertions(+)
>  create mode 100644 systemd/user/git-maintenance@.service
>  create mode 100644 systemd/user/git-maintenance@.timer
> 
> diff --git a/Makefile b/Makefile
> index 4e255c81f2..276b4373c6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -619,6 +619,7 @@ htmldir = $(prefix)/share/doc/git-doc
>  ETC_GITCONFIG = $(sysconfdir)/gitconfig
>  ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
>  lib = lib
> +libdir = $(prefix)/lib
>  # DESTDIR =
>  pathsep = :
>  
> @@ -1328,6 +1329,8 @@ BUILTIN_OBJS += builtin/verify-tag.o
>  BUILTIN_OBJS += builtin/worktree.o
>  BUILTIN_OBJS += builtin/write-tree.o
>  
> +SYSTEMD_USER_UNITS := $(wildcard systemd/user/*)
> +
>  # THIRD_PARTY_SOURCES is a list of patterns compatible with the
>  # $(filter) and $(filter-out) family of functions. They specify source
>  # files which are taken from some third-party source where we want to be
> @@ -3469,6 +3472,7 @@ install: all
>  	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
>  	$(INSTALL) $(INSTALL_STRIP) $(install_bindir_xprograms) '$(DESTDIR_SQ)$(bindir_SQ)'
>  	$(INSTALL) $(BINDIR_PROGRAMS_NO_X) '$(DESTDIR_SQ)$(bindir_SQ)'
> +	$(INSTALL) -Dm 644 -t '$(DESTDIR_SQ)$(libdir)/systemd/user' $(SYSTEMD_USER_UNITS)

I wonder whether we want to unconditionally install those units. Many of
the platforms that we support don't even have systemd available, so
certainly it wouldn't make any sense to install it on those platforms.

Assuming that this is something we want in the first place I thus think
that we should at least make this conditional and add some platform
specific quirk to "config.mak.uname".

>  ifdef MSVC
>  	# We DO NOT install the individual foo.o.pdb files because they
> diff --git a/systemd/user/git-maintenance@.service b/systemd/user/git-maintenance@.service
> new file mode 100644
> index 0000000000..87ac0c86e6
> --- /dev/null
> +++ b/systemd/user/git-maintenance@.service
> @@ -0,0 +1,16 @@
> +[Unit]
> +Description=Optimize Git repositories data
> +
> +[Service]
> +Type=oneshot
> +ExecStart=git for-each-repo --config=maintenance.repo \
> +          maintenance run --schedule=%i
> +LockPersonality=yes
> +MemoryDenyWriteExecute=yes
> +NoNewPrivileges=yes
> +RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_VSOCK
> +RestrictNamespaces=yes
> +RestrictRealtime=yes
> +RestrictSUIDSGID=yes
> +SystemCallArchitectures=native
> +SystemCallFilter=@system-service

Curious, but how did you arrive at these particular restrictions for the
unit? Might be something to explain in the commit message, as well.

Patrick

> diff --git a/systemd/user/git-maintenance@.timer b/systemd/user/git-maintenance@.timer
> new file mode 100644
> index 0000000000..40fbc77a62
> --- /dev/null
> +++ b/systemd/user/git-maintenance@.timer
> @@ -0,0 +1,9 @@
> +[Unit]
> +Description=Optimize Git repositories data
> +
> +[Timer]
> +OnCalendar=%i
> +Persistent=true
> +
> +[Install]
> +WantedBy=timers.target
> -- 
> 2.44.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]