Re: [PATCH] maintenance: use systemd timers on Linux

On Mon, May 10 2021, Lénaïc Huard wrote:

Le mercredi 5 mai 2021, 14:01:25 CEST Ævar Arnfjörð Bjarmason a écrit :

quoted

On Sun, May 02 2021, Eric Sunshine wrote:

quoted

On Sat, May 1, 2021 at 10:59 AM Lénaïc Huard [off-list ref] wrote:

quoted

+       strvec_push(&child.args, "--now");
+       strvec_pushf(&child.args, "git-maintenance@%s.timer", frequency);
+
+       if (start_command(&child))
+               die(_("failed to run systemctl"));
+       return finish_command(&child);
+}
+static int systemd_timer_write_unit_templates(const char *exec_path)
+{
+       unit = "[Unit]\n"
+              "Description=Optimize Git repositories data\n"
+              "\n"
+              "[Service]\n"
+              "Type=oneshot\n"
+              "ExecStart=\"%1$s/git\" --exec-path=\"%1$s\" for-each-repo
--config=maintenance.repo maintenance run --schedule=%%i\n"> 

I see that it's in POSIX, but do we use this `%n$s` directive
elsewhere in the Git source code? If not, I'd be cautious of
introducing it here. Maybe it's better to just use plain `%s` twice...

We use it in po/, so for sprintf() on systems that don't have
NO_GETTEXT=Y we already test it in the wild.

But no, I don't think anything in the main source uses it, FWIW I have a
WIP series in my own fork that I've cooked for a while that uses this, I
haven't run into any issues with it in either GitHub's CI
(e.g. Windows), or on the systems I myself test on.

I think it would be a useful canary to just take a change like this, we
can always change it to the form you suggest if it doesn't work out.

Based on this latest comment, I left the `%n$s` directive in the v2 of the 
patch.

Let me know if that’s still OK. Otherwise, I’d be happy to implement Eric’s 
suggestion.

Note however that this would be a “poor” canary to check if that directive is 
supported on all the platforms on which git has been ported.
Indeed, this code is executed only on systemd platforms, which means quite 
recent Linux systems.
Should this directive not be supported, I suppose it would be on more exotic 
systems.

Indeed, although we compile it on non-Linux platforms, so we'd expect to
get complaints from smarter non-Linux compilers if fprintf() is given an
unknown formatting directive.