diff mbox series

[RFC,1/5] maintenance: package systemd units

Message ID 20240318153257.27451-2-mg@max.gautier.name (mailing list archive)
State Superseded
Headers show
Series maintenance: use packaged systemd units | expand

Commit Message

Max Gautier March 18, 2024, 3:31 p.m. UTC
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

Comments

Patrick Steinhardt March 21, 2024, 12:37 p.m. UTC | #1
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
> 
>
Max Gautier March 21, 2024, 1:07 p.m. UTC | #2
On Thu, Mar 21, 2024 at 01:37:30PM +0100, Patrick Steinhardt wrote:
> 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.
> 

Hum, I wrote that in the cover, but you're right, it should be in the
commit itself.
Ack.

> > ...
> > 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

I copied the unit file which is defined in strings in builtin/gc.c,
which I delete in patch 3.
Should the moving be inside one commit, in order to explicit the fact
that it's only moving things around ?
Patrick Steinhardt March 21, 2024, 1:22 p.m. UTC | #3
On Thu, Mar 21, 2024 at 02:07:02PM +0100, Max Gautier wrote:
> On Thu, Mar 21, 2024 at 01:37:30PM +0100, Patrick Steinhardt wrote:
> > 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.
> > 
> 
> Hum, I wrote that in the cover, but you're right, it should be in the
> commit itself.
> Ack.
> 
> > > ...
> > > 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
> 
> I copied the unit file which is defined in strings in builtin/gc.c,
> which I delete in patch 3.
> Should the moving be inside one commit, in order to explicit the fact
> that it's only moving things around ?

I don't really think it's necessary. But pointing that out in the commit
message would go a long way to explain where exactly these definitions
come from.

Patrick
Max Gautier March 21, 2024, 1:38 p.m. UTC | #4
On Thu, Mar 21, 2024 at 01:37:30PM +0100, Patrick Steinhardt wrote:
> 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".
> 

We probably want that (conditional install) but I'm not sure where that
should go ; I'm not super familiar with autoconf. 

I just noticed than man 7 daemon (shipped by systemd) propose the
following snippet for installing systemd system services (should be easy
enough to adapt for user ervices, I think):

PKG_PROG_PKG_CONFIG()
AC_ARG_WITH([systemdsystemunitdir],
    [AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files])],,
    [with_systemdsystemunitdir=auto])
AS_IF([test "x$with_systemdsystemunitdir" = "xyes" -o "x$with_systemdsystemunitdir" = "xauto"], [
    def_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)

    AS_IF([test "x$def_systemdsystemunitdir" = "x"],
  [AS_IF([test "x$with_systemdsystemunitdir" = "xyes"],
   [AC_MSG_ERROR([systemd support requested but pkg-config unable to query systemd package])])
   with_systemdsystemunitdir=no],
  [with_systemdsystemunitdir="$def_systemdsystemunitdir"])])
AS_IF([test "x$with_systemdsystemunitdir" != "xno"],
     [AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])])
AM_CONDITIONAL([HAVE_SYSTEMD], [test "x$with_systemdsystemunitdir" != "xno"])

Would something like that work ?
Patrick Steinhardt March 21, 2024, 2:44 p.m. UTC | #5
On Thu, Mar 21, 2024 at 02:38:36PM +0100, Max Gautier wrote:
> On Thu, Mar 21, 2024 at 01:37:30PM +0100, Patrick Steinhardt wrote:
> > 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".
> > 
> 
> We probably want that (conditional install) but I'm not sure where that
> should go ; I'm not super familiar with autoconf. 
> 
> I just noticed than man 7 daemon (shipped by systemd) propose the
> following snippet for installing systemd system services (should be easy
> enough to adapt for user ervices, I think):
> 
> PKG_PROG_PKG_CONFIG()
> AC_ARG_WITH([systemdsystemunitdir],
>     [AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files])],,
>     [with_systemdsystemunitdir=auto])
> AS_IF([test "x$with_systemdsystemunitdir" = "xyes" -o "x$with_systemdsystemunitdir" = "xauto"], [
>     def_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)
> 
>     AS_IF([test "x$def_systemdsystemunitdir" = "x"],
>   [AS_IF([test "x$with_systemdsystemunitdir" = "xyes"],
>    [AC_MSG_ERROR([systemd support requested but pkg-config unable to query systemd package])])
>    with_systemdsystemunitdir=no],
>   [with_systemdsystemunitdir="$def_systemdsystemunitdir"])])
> AS_IF([test "x$with_systemdsystemunitdir" != "xno"],
>      [AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])])
> AM_CONDITIONAL([HAVE_SYSTEMD], [test "x$with_systemdsystemunitdir" != "xno"])
> 
> Would something like that work ?

Probably. But while we do have autoconf wired up, the primary way of
building Git does not use it, but rather uses `config.mak.uname`. I'd
recommend to have a look at it to figure out how we handle other
build-time options there.

Patrick
Max Gautier March 21, 2024, 2:48 p.m. UTC | #6
On Thu, Mar 21, 2024 at 02:38:36PM +0100, Max Gautier wrote:
> On Thu, Mar 21, 2024 at 01:37:30PM +0100, Patrick Steinhardt wrote:
> > 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".
> > 
> 
> We probably want that (conditional install) but I'm not sure where that
> should go ; I'm not super familiar with autoconf. 
> 
> I just noticed than man 7 daemon (shipped by systemd) propose the
> following snippet for installing systemd system services (should be easy
> enough to adapt for user ervices, I think):
> 
> PKG_PROG_PKG_CONFIG()
> AC_ARG_WITH([systemdsystemunitdir],
>     [AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files])],,
>     [with_systemdsystemunitdir=auto])
> AS_IF([test "x$with_systemdsystemunitdir" = "xyes" -o "x$with_systemdsystemunitdir" = "xauto"], [
>     def_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)
> 
>     AS_IF([test "x$def_systemdsystemunitdir" = "x"],
>   [AS_IF([test "x$with_systemdsystemunitdir" = "xyes"],
>    [AC_MSG_ERROR([systemd support requested but pkg-config unable to query systemd package])])
>    with_systemdsystemunitdir=no],
>   [with_systemdsystemunitdir="$def_systemdsystemunitdir"])])
> AS_IF([test "x$with_systemdsystemunitdir" != "xno"],
>      [AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])])
> AM_CONDITIONAL([HAVE_SYSTEMD], [test "x$with_systemdsystemunitdir" != "xno"])
> 
> Would something like that work ?

Ugh, that snippet apparently requires that I delete aclocal.m4, execute
`autoreconf --install`, then concat together the previous aclocal.m4 and
the newly generated one.

I'm adding the last authors on aclocal.m4 to the discussion, maybe they
have some lights ? (Though they don't have commits in the last ten
years, so it might be a very long shot ...)
Max Gautier March 21, 2024, 2:49 p.m. UTC | #7
On Thu, Mar 21, 2024 at 03:44:05PM +0100, Patrick Steinhardt wrote:
> On Thu, Mar 21, 2024 at 02:38:36PM +0100, Max Gautier wrote:
> > On Thu, Mar 21, 2024 at 01:37:30PM +0100, Patrick Steinhardt wrote:
> > > 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".
> > > 
> > 
> > We probably want that (conditional install) but I'm not sure where that
> > should go ; I'm not super familiar with autoconf. 
> > 
> > I just noticed than man 7 daemon (shipped by systemd) propose the
> > following snippet for installing systemd system services (should be easy
> > enough to adapt for user ervices, I think):
> > 
> > PKG_PROG_PKG_CONFIG()
> > AC_ARG_WITH([systemdsystemunitdir],
> >     [AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files])],,
> >     [with_systemdsystemunitdir=auto])
> > AS_IF([test "x$with_systemdsystemunitdir" = "xyes" -o "x$with_systemdsystemunitdir" = "xauto"], [
> >     def_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)
> > 
> >     AS_IF([test "x$def_systemdsystemunitdir" = "x"],
> >   [AS_IF([test "x$with_systemdsystemunitdir" = "xyes"],
> >    [AC_MSG_ERROR([systemd support requested but pkg-config unable to query systemd package])])
> >    with_systemdsystemunitdir=no],
> >   [with_systemdsystemunitdir="$def_systemdsystemunitdir"])])
> > AS_IF([test "x$with_systemdsystemunitdir" != "xno"],
> >      [AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])])
> > AM_CONDITIONAL([HAVE_SYSTEMD], [test "x$with_systemdsystemunitdir" != "xno"])
> > 
> > Would something like that work ?
> 
> Probably. But while we do have autoconf wired up, the primary way of
> building Git does not use it, but rather uses `config.mak.uname`. I'd
> recommend to have a look at it to figure out how we handle other
> build-time options there.
> 
> Patrick

I'll take a look, thanks.
diff mbox series

Patch

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)
 
 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
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