Re: git maintenance broken on FreeBSD

From: Todd Zullinger <tmz@pobox.com>
Date: Aug 13, 2022, 03:42 UTC
Message-ID: <YvcdskzUkocUv/d7@pobox.com>
In-Reply-To: <YvZnQFVMZZmz9TIX@danh.dev>

Đoàn Trần Công Danh wrote:

Show 19 quoted lines

> On 2022-08-12 10:51:03-0300, Renato Botelho <garga@FreeBSD.org> wrote:
>> As reported at [1], git maintenance is not working on FreeBSD.  I didn't
>> find the time to dig into it but it seems like it's calling crontab using
>> parameters not supported on FreeBSD.
>> 
>> [1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=260746
> 
> It seems like FreeBSD's cron is vixie-cron which requires <file>
> passed to crontab(1).
> 
>      The crontab command conforms to IEEE Std 1003.2 (“POSIX.2”) with the
>      exception that the dangerous variant of calling crontab without a file
>      name in the first form of the command is not allowed by this
>      implementation.  The pseudo-filename ‘-’ must be specified to read from
>      standard input.  The new command syntax differs from previous versions of
>      Vixie Cron, as well as from the classic SVR3 syntax.
> 
> I think other crontab implementation also accept "-" as filename for stdin.
> At least cronie, fcron, dcron, and busybox's crontab both supports "-" as stdin.

A similar issue was noted in Fedora with cronie shortly after the git maintenance command was released:

    https://bugzilla.redhat.com/show_bug.cgi?id=1939930#c1

I noted that a patch just like the one below would suffice, but I was concerned that it wouldn't be welcome here because the behavior of crontab was specified by POSIX (even though it's very unfriendly and, apparently, supported by fewer and fewer implementations).

If a change like this is made, aren't we trading one group of broken users for another? It would fix users of newer systems at the expense of those on older systems, I would suspect.

Show 16 quoted lines

> I think this patch can fix FreeBSD's problem:
> 
> ---- 8< -----
> diff --git a/builtin/gc.c b/builtin/gc.c
> index eeff2b760e..45d908def3 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -2087,6 +2087,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)
>  	rewind(cron_list);
>  
>  	strvec_split(&crontab_edit.args, cmd);
> +	strvec_push(&crontab_edit.args, "-");
>  	crontab_edit.in = -1;
>  	crontab_edit.git_cmd = 0;
>  
> ---- 8< ---------

In the end, cronie adjusted it's behavior, which was similar to that of the newer vixie-cron, in 8b0241f (Partially revert the behavior of crontab command without arguments, 2021-03-17)¹. It now behaves as required by POSIX if stdin is not a TTY. That seems like a reasonable compromise and perhaps vixie-cron would be willing to do the same?

¹ https://github.com/cronie-crond/cronie/commit/8b0241f

-- 
Todd