Bug 1939930 - git maintenance fails to install crontab
Summary: git maintenance fails to install crontab
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: cronie
Version: 33
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Matej Mužila
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-03-17 10:23 UTC by Matthias Andree
Modified: 2021-03-27 00:15 UTC (History)
12 users (show)

Fixed In Version: cronie-1.5.6-1.fc33 cronie-1.5.6-1.fc34
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-03-19 18:50:13 UTC
Type: Bug


Attachments (Terms of Use)
fix git-maintenance crontab command for cronie (1.19 KB, patch)
2021-03-17 14:16 UTC, Todd Zullinger
no flags Details | Diff
fix git-maintenance crontab command for cronie (1.65 KB, patch)
2021-03-17 16:16 UTC, Todd Zullinger
no flags Details | Diff

Description Matthias Andree 2021-03-17 10:23:21 UTC
Description of problem:
git maintenance start is incompatible with crontab

Version-Release number of selected component (if applicable):
git-2.30.2-1.fc33.x86_64

How reproducible:
100%

Steps to Reproduce:
1. cd to a git repository
2. run: git maintenance start

Actual results:
crontab: usage error: file name or - (for stdin) must be specified for replace
Usage:
 crontab [options] file
 crontab [options]
 crontab -n [hostname]

Options:
 -u <user>  define user
 -e         edit user's crontab
 -l         list user's crontab
 -r         delete user's crontab
 -i         prompt before deleting
 -n <host>  set host in cluster to run users' crontabs
 -c         get host in cluster to run users' crontabs
 -s         selinux context
 -V         print version and exit
 -x <mask>  enable debugging

Default operation is replace, per 1003.2
error: 'crontab' abgebrochen


Expected results:
installation of git background task

Additional info:
none

Comment 1 Todd Zullinger 2021-03-17 14:16:00 UTC
Created attachment 1764056 [details]
fix git-maintenance crontab command for cronie

The maintenance command is relatively new.

At a glance, it looks like the code is calling `crontab` with no arguments to get the edit mode, which cronie stopped supporting.  References:

https://github.com/cronie-crond/cronie/issues/28
https://github.com/cronie-crond/cronie/commit/39a67fdb6
39a67fd (crontab: Make crontab without arguments fail., 2019-02-15)

A patch like this fixes the issue for Fedora (also attached as a git formatted patch):

```
diff --git i/builtin/gc.c w/builtin/gc.c
index ef7226d7bc..9ed8058356 100644
--- i/builtin/gc.c
+++ w/builtin/gc.c
@@ -1904,6 +1904,7 @@ static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd)
        rewind(cron_list);
 
        strvec_split(&crontab_edit.args, cmd);
+       strvec_push(&crontab_list.args, "-");
        crontab_edit.in = -1;
        crontab_edit.git_cmd = 0;

```

However, I would be very reluctant to carry such a patch in the Fedora package.  It's unlikely to be acceptable upstream because it breaks on POSIX-compliant crontab implementations.

It's really a cronie "bug" here, since it deviates from POSIX.  I completely understand why cronie does this.  But I'm not sure that trying to fix such usage issues in commands like crontab is worth the cost.  Anyone running crontab should read the man page.

In the bigger picture it seems like the git maintenance command should setup a systemd timer on systems which use systemd, since crond may not even be installed or enabled.  But that's something for upstream to consider and someone interested in git maintenance to implement.  That may be worth bringing up on the git list (git@vger.kernel.org).

Comment 2 Todd Zullinger 2021-03-17 14:26:46 UTC
Re-assigning to cronie to register the "complaint" which Tomáš asked for in https://github.com/cronie-crond/cronie/issues/28 (2 years ago). :)

If there's been 1 complaint in 2 years, I can understand if cronie doesn't wish to revert to the POSIX-compliant behavior.  (Though I hope it does, on the grounds that users of cron should be willing to accept some of the warts of an old and widely-implemented command like crontab specified by POSIX.)

I'm not yet sure if I'd want to carry a local patch in git to support git maintenance with cronie or not.  The git maintenance code is relatively new and may change which will bring work to rebase it over time.  So even though it's a tiny patch it could cause more work than it's worth.

Comment 3 Tomáš Mráz 2021-03-17 14:45:28 UTC
I have no plan to change the crontab behavior in upstream cronie to revert back. The posix mandated behavior is really stupid as accidentally typing crontab without any options will overwrite your crontab unless you do CTRL-C.

But thinking about it more, perhaps there is one thing that could be done - test whether stdin is TTY and if not revert back to the POSIX behavior. This might work for you and it would fix the accidental overwrite as well.

Comment 4 Matthias Andree 2021-03-17 14:54:36 UTC
if (isatty(0)) { /* print helpful comment to user */ }

=> https://github.com/cronie-crond/cronie/issues/28#issuecomment-801143756

Comment 5 Todd Zullinger 2021-03-17 14:59:34 UTC
Understood.  No argument that the POSIX behavior is an ugly foot gun.  I didn't know that it would overwrite the crontab as easily either (I guess ^C is deeply ingrained in me).

Thanks for considering the fallback to POSIX behavior.  That does seem like a good middle ground -- I've implemented that sort of test in python command-line tools which can be used in pipelines as well as directly.  If the implementation isn't too bad it would be a nice way to handle things.

I do hope that eventually there is support for systemd timers in git maintenance which will make this particular method of hitting the issue moot.  But that's not an itch of mine. :)

Comment 6 Todd Zullinger 2021-03-17 16:16:08 UTC
Created attachment 1764110 [details]
fix git-maintenance crontab command for cronie

Comment 7 Fedora Update System 2021-03-17 16:30:50 UTC
FEDORA-2021-93a62df830 has been submitted as an update to Fedora 34. https://bodhi.fedoraproject.org/updates/FEDORA-2021-93a62df830

Comment 8 Fedora Update System 2021-03-17 16:30:55 UTC
FEDORA-2021-b55af90afe has been submitted as an update to Fedora 33. https://bodhi.fedoraproject.org/updates/FEDORA-2021-b55af90afe

Comment 9 Todd Zullinger 2021-03-17 19:00:35 UTC
Thanks for the impressively quick fix Tomáš!  Nicely done.

I tested cronie-1.5.6 with git maintenance (on the 32 laptop I have handy).  It works perfectly when called by git and it continues to provide the expected usage output when run directly.

I shall try to spin up an f33 and/or f34 system to test the official updates and leave positive karma in bodhi.

Comment 10 Fedora Update System 2021-03-18 03:28:55 UTC
FEDORA-2021-b55af90afe has been pushed to the Fedora 33 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2021-b55af90afe`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-b55af90afe

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 11 Fedora Update System 2021-03-18 21:46:32 UTC
FEDORA-2021-93a62df830 has been pushed to the Fedora 34 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2021-93a62df830`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-93a62df830

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 12 Fedora Update System 2021-03-19 18:50:13 UTC
FEDORA-2021-b55af90afe has been pushed to the Fedora 33 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 13 Fedora Update System 2021-03-27 00:15:38 UTC
FEDORA-2021-93a62df830 has been pushed to the Fedora 34 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.