Re: [PATCH v2 3/6] maintenance: simplify systemctl calls

From: Max Gautier <mg@max.gautier.name>
Date: Mar 23, 2024, 10:25 UTC
Message-ID: <Zf6uISqdCn9Ld4ir@framework>
In-Reply-To: <CAPig+cTeXMw8pTS9on_CSJraPDYKSLaFd82LEtWjFgOBzH4=yg@mail.gmail.com>

On Fri, Mar 22, 2024 at 07:09:18PM -0400, Eric Sunshine wrote:

Show 32 quoted lines

> On Fri, Mar 22, 2024 at 6:13 PM Max Gautier <mg@max.gautier.name> wrote:
> > The systemctl invocation to enable or disable the git maintenance timers
> > is needlessly complicated:
> > - systemctl does not mind at all enabling already enabled units, nor
> >   disabling already disabled units.
> 
> Has systemctl behavior changed...
> 
> > Use only one systemctl invocation per `git maintenance start/stop`.
> > Transparently pass its status and output.
> > Add the --force option to override conflicting symlinks to previous
> > instances of our units files which lived in $XDG_CONFIG_HOME.
> >
> > Signed-off-by: Max Gautier <mg@max.gautier.name>
> > ---
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > @@ -2303,70 +2303,35 @@ static int is_systemd_timer_available(void)
> > -        * Disabling the systemd unit while it is already disabled makes
> > -        * systemctl print an error.
> > -        * Let's ignore it since it means we already are in the expected state:
> > -        * the unit is disabled.
> 
> ... since this and...
> 
> > -                * Disabling an already disabled systemd unit makes
> > -                * systemctl fail.
> > -                * Let's ignore this failure.
> 
> ... this were written?
> 
> If so, do we still need to worry about older systems in which
> systemctl prints errors and/or fails outright?

I tried the following on systemd source $ git log -L :do_unit_file_disable:src/shared/install.c $ git log -L :unit_file_disable:src/shared/install.c` $ git log --grep 'already disabled' $ git log --grep 're disable' $ git log --grep 'disabled unit' $ git log --grep 'systemctl disable'

That yields nothing indicating a change (in the commit messages, I didn't look at the code in detail) when disabling already disabled units, and testing on systemd 255 disable already disabled units (normal and templated, same thing) without complaining or an error status. Nothing relevant I could find on github in the issues or PRs either.

systemctl does error out or print warnings in other conditions, like missing [Install] section, but that's not something we should ignore.

Philip Wood asked that question here in the original thread:

> What is the exit code of systemctl if a unit is already enabled and we 
> try to enbale it again (and the same for disabling a disabled unit)?

https://lore.kernel.org/git/3fd17223-8667-24be-2e65-f1970d411bdf@gmail.com/

But I can't find a follow-up email from Lénaïc, and searching for "disabled" in the whole thread only yield the comment I'm removing. Not sure if Lénaïc still follows the list, but maybe he can comment on that ? To me it looks like it was not necessary to begin with, but I might have just missed the discussion about it.

Adding Phillip to the discussion as well.

-- 
Max Gautier