From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, lenaic@lhuard.fr,
Derrick Stolee <derrickstolee@github.com>,
Derrick Stolee <dstolee@microsoft.com>
Subject: [PATCH] maintenance: disable cron on macOS
Date: Wed, 10 Nov 2021 18:35:59 +0000 [thread overview]
Message-ID: <pull.1075.git.1636569360002.gitgitgadget@gmail.com> (raw)
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.
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
next reply other threads:[~2021-11-10 18:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-10 18:35 Derrick Stolee via GitGitGadget [this message]
2021-11-10 18:58 ` [PATCH] maintenance: disable cron on macOS Johannes Schindelin
2021-11-10 19:47 ` Junio C Hamano
2021-11-10 20:01 ` Derrick Stolee
2021-11-10 20:56 ` Ævar Arnfjörð Bjarmason
2021-11-10 21:06 ` Derrick Stolee
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=pull.1075.git.1636569360002.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=derrickstolee@github.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=lenaic@lhuard.fr \
/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).