Re: [PATCH v2 1/1] maintenance: use systemd timers on Linux

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 10, 2021, 18:25 UTC
Message-ID: <CAPig+cRvs+d-Tv4WyuD967v5Pbfhmwj-ruwyBXhwcR=5chQAmQ@mail.gmail.com>
In-Reply-To: <3fd17223-8667-24be-2e65-f1970d411bdf@gmail.com>

On Mon, May 10, 2021 at 2:04 PM Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 7 quoted lines

> On 09/05/2021 22:32, Lénaïc Huard wrote:
> > +static int systemd_timer_enable_unit(int enable,
> > +                                  enum schedule_priority schedule,
> > +                                  const char *cmd)
>
> The cmd argument is pointless, it will always be "systemctl" and you
> have even hard coded that value into the error message below.

The reason that `cmd` is passed around everywhere is that the actual command can be overridden by GIT_TEST_MAINT_SCHEDULER which allows the test script to mock up a scheduler command rather than running the real scheduler command. I haven't read the new version of the patch closely yet, but after a quick scan, I'm pretty confident that this is still the case (despite the aggressive changes the patch makes to the areas around GIT_TEST_MAINT_SCHEDULER).

As for hardcoding the command name in the error message, that seems perfectly fine since, under normal circumstances, it _will_ be that command (it's only different when testing).