git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Todd Zullinger <tmz@pobox.com>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Cc: Renato Botelho <garga@freebsd.org>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: git maintenance broken on FreeBSD
Date: Fri, 12 Aug 2022 23:42:42 -0400	[thread overview]
Message-ID: <YvcdskzUkocUv/d7@pobox.com> (raw)
In-Reply-To: <YvZnQFVMZZmz9TIX@danh.dev>

Đoàn Trần Công Danh wrote:
> 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.

> 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

  reply	other threads:[~2022-08-13  3:46 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-12 13:51 git maintenance broken on FreeBSD Renato Botelho
2022-08-12 14:44 ` Đoàn Trần Công Danh
2022-08-13  3:42   ` Todd Zullinger [this message]
2022-08-13  5:02     ` Junio C Hamano
2022-08-13 15:37       ` Đoàn Trần Công Danh
2022-08-13 17:26         ` Junio C Hamano
2022-08-13 17:35           ` brian m. carlson
2022-08-15 13:22             ` Derrick Stolee
2022-08-15 16:09               ` Junio C Hamano
2022-08-23  1:01               ` [PATCH] gc: use temporary file for editing crontab brian m. carlson
2022-08-23  9:12                 ` Johannes Schindelin
2022-08-23 17:06                   ` Derrick Stolee
2022-08-23 21:15                     ` brian m. carlson
2022-08-24 16:06                       ` Junio C Hamano
2022-08-28 21:41                 ` [PATCH v2] " brian m. carlson
2022-08-29  6:46                   ` Junio C Hamano
2022-08-29 10:52                   ` Renato Botelho
2022-08-30 13:27                   ` Derrick Stolee
2022-08-30 20:40                   ` [PATCH] test-crontab: minor memory and error handling fixes Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YvcdskzUkocUv/d7@pobox.com \
    --to=tmz@pobox.com \
    --cc=congdanhqx@gmail.com \
    --cc=garga@freebsd.org \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).