git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] maintenance: disable cron on macOS

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Nov 10, 2021, 18:58 UTC
Message-ID
<nycvar.QRO.7.76.6.2111101953300.21127@tvgsbejvaqbjf.bet>
In-Reply-To
<pull.1075.git.1636569360002.gitgitgadget@gmail.com>
Hi Stolee,
On Wed, 10 Nov 2021, Derrick Stolee via GitGitGadget wrote:
Show 38 quoted lines
> From: Derrick Stolee <dstolee@microsoft.com>
>
> 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@microsoft.com>
> ---
>     [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

Show 64 quoted lines
>
>     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
>
>
Previous: Derrick Stolee via GitGitGadgetNext: Junio C Hamano
Message 2 of 6 in “maintenance: disable cron on macOS”
  1. maintenance: disable cron on macOSDerrick Stolee via GitGitGadget, Nov 10, 2021
  2. Johannes SchindelinNov 10, 2021
  3. Junio C HamanoNov 10, 2021
  4. Derrick StoleeNov 10, 2021
  5. Ævar Arnfjörð BjarmasonNov 10, 2021
  6. Derrick StoleeNov 10, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.