Hi Stolee, On Wed, 10 Nov 2021, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > > In eba1ba9 (maintenance: `git maintenance run` learned > `--scheduler=<scheduler>`, 2021-09-04), we introduced the ability to > specify a scheduler explicitly. This led to some extra checks around > whether an alternative scheduler was available. This added the > functionality of removing background maintenance from schedulers other > than the one selected. > > On macOS, cron is technically available, but running 'crontab' triggers > a UI prompt asking for special permissions. This is the major reason why > launchctl is used as the default scheduler. The is_crontab_available() > method triggers this UI prompt, causing user disruption. > > Remove this disruption by using an #ifdef to prevent running crontab > this way on macOS. This has the unfortunate downside that if a user > manually selects cron via the '--scheduler' option, then adjusting the > scheduler later will not remove the schedule from cron. The > '--scheduler' option ignores the is_available checks, which is how we > can get into this situation. > > Extract the new check_crontab_process() method to avoid making the > 'child' variable unused on macOS. The method is marked MAYBE_UNUSED > because it has no callers on macOS. > > Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > --- > [For 2.34.0 Release] maintenance: disable cron on macOS > > This one is really tricky because we can't notice anything is wrong > without running git maintenance start or git maintenance stop > interactively on macOS. The tests pass just fine because the UI alert > gets automatically ignored during the test suite. > > This is a bit of a half-fix: it avoids the UI alert, but has a corner > case of not un-doing the cron schedule if a user manages to select it > (under suitable permissions such that it succeeds). For the purpose of > the timing of the release, I think this is an appropriate hedge. I agree. We can revisit this once v2.34.0 is released, and then determine a better layer to prevent this, or alternatively warn very loudly about it. Thanks, Dscho > > Thanks! -Stolee > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1075%2Fderrickstolee%2Fmaintenance-cron-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1075/derrickstolee/maintenance-cron-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1075 > > builtin/gc.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/builtin/gc.c b/builtin/gc.c > index 26709311601..bcef6a4c8da 100644 > --- a/builtin/gc.c > +++ b/builtin/gc.c > @@ -1999,15 +1999,11 @@ static int schtasks_update_schedule(int run_maintenance, int fd) > return schtasks_remove_tasks(); > } > > -static int is_crontab_available(void) > +MAYBE_UNUSED > +static int check_crontab_process(const char *cmd) > { > - const char *cmd = "crontab"; > - int is_available; > struct child_process child = CHILD_PROCESS_INIT; > > - if (get_schedule_cmd(&cmd, &is_available)) > - return is_available; > - > strvec_split(&child.args, cmd); > strvec_push(&child.args, "-l"); > child.no_stdin = 1; > @@ -2022,6 +2018,25 @@ static int is_crontab_available(void) > return 1; > } > > +static int is_crontab_available(void) > +{ > + const char *cmd = "crontab"; > + int is_available; > + > + if (get_schedule_cmd(&cmd, &is_available)) > + return is_available; > + > +#ifdef __APPLE__ > + /* > + * macOS has cron, but it requires special permissions and will > + * create a UI alert when attempting to run this command. > + */ > + return 0; > +#else > + return check_crontab_process(cmd); > +#endif > +} > + > #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE" > #define END_LINE "# END GIT MAINTENANCE SCHEDULE" > > > base-commit: 6c220937e2b26d85920bf2d38ff2464a0d57fd6b > -- > gitgitgadget > >
