[2/3] maintenance: use launchctl on macOS

Commit Message

Derrick Stolee Nov. 3, 2020, 2:03 p.m. UTC

From: Derrick Stolee <[e-mail]>

The existing mechanism for scheduling background maintenance is done
through cron. The 'crontab -e' command allows updating the schedule
while cron itself runs those commands. While this is technically
supported by macOS, it has some significant deficiencies:

1. Every run of 'crontab -e' must request elevated privileges through
   the user interface. When running 'git maintenance start' from the
   Terminal app, it presents a dialog box saying "Terminal.app would
   like to administer your computer. Administration can include
   modifying passwords, networking, and system settings." This is more
   alarming than what we are hoping to achieve. If this alert had some
   information about how "git" is trying to run "crontab" then we would
   have some reason to believe that this dialog might be fine. However,
   it also doesn't help that some scenarios just leave Git waiting for
   a response without presenting anything to the user. I experienced
   this when executing the command from a Bash terminal view inside
   Visual Studio Code.

2. While cron initializes a user environment enough for "git config
   --global --show-origin" to show the correct config file information,
   it does not set up the environment enough for Git Credential Manager
   Core to load credentials during a 'prefetch' task. My prefetches
   against private repositories required re-authenticating through UI
   pop-ups in a way that should not be required.

The solution is to switch from cron to the Apple-recommended [1]
'launchd' tool.

[1] https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPSystemStartup/Chapters/ScheduledJobs.html

The basics of this tool is that we need to create XML-formatted
"plist" files inside "~/Library/LaunchAgents/" and then use the
'launchctl' tool to make launchd aware of them. The plist files
include all of the scheduling information, along with the command-line
arguments split across an array of <string> tags.

For example, here is my plist file for the weekly scheduled tasks:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0"><dict>
<key>Label</key><string>org.git-scm.git.weekly</string>
<key>ProgramArguments</key>
<array>
<string>/usr/local/libexec/git-core/git</string>
<string>--exec-path=/usr/local/libexec/git-core</string>
<string>for-each-repo</string>
<string>--config=maintenance.repo</string>
<string>maintenance</string>
<string>run</string>
<string>--schedule=weekly</string>
</array>
<key>StartCalendarInterval</key>
<array>
<dict>
<key>Day</key><integer>0</integer>
<key>Hour</key><integer>0</integer>
<key>Minute</key><integer>0</integer>
</dict>
</array>
</dict>
</plist>

The schedules for the daily and hourly tasks are more complicated
since we need to use an array for the StartCalendarInterval with
an entry for each of the six days other than the 0th day (to avoid
colliding with the weekly task), and each of the 23 hours other
than the 0th hour (to avoid colliding with the daily task).

The "Label" value is currently filled with "org.git-scm.git.X"
where X is the frequency. We need a different plist file for each
frequency.

The launchctl command needs to be aligned with a user id in order
to initialize the command environment. This must be done using
the 'launchctl bootstrap' subcommand. This subcommand is new as
of macOS 10.11, which was released in September 2015. Before that
release the 'launchctl load' subcommand was recommended. The best
source of information on this transition I have seen is available
at [2].

[2] https://babodee.wordpress.com/2016/04/09/launchctl-2-0-syntax/

To remove a schedule, we must run 'launchctl bootout' with a valid
plist file. We also need to 'bootout' a task before the 'bootstrap'
subcommand will succeed, if such a task already exists.

We can verify the commands that were run by 'git maintenance start'
and 'git maintenance stop' by injecting a script that writes the
command-line arguments into GIT_TEST_CRONTAB.

Signed-off-by: Derrick Stolee <[e-mail]>
---
 builtin/gc.c           | 209 +++++++++++++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh |  52 +++++++++-
 t/test-lib.sh          |   4 +
 3 files changed, 262 insertions(+), 3 deletions(-)

Comments

Eric Sunshine Nov. 3, 2020, 6:45 p.m. UTC | #1

On Tue, Nov 3, 2020 at 9:05 AM Derrick Stolee via GitGitGadget
<[e-mail]> wrote:
> maintenance: use launchctl on macOS

A few comments below (not necessarily worth a re-roll)...

> The launchctl command needs to be aligned with a user id in order
> to initialize the command environment. This must be done using
> the 'launchctl bootstrap' subcommand. This subcommand is new as
> of macOS 10.11, which was released in September 2015. Before that
> release the 'launchctl load' subcommand was recommended. The best
> source of information on this transition I have seen is available
> at [2].

It's not clear whether or not this is saying that git-maintenance will
dynamically adapt to work with modern and older 'launchctl'. A glance
at the actual code reveals that it knows only about modern
'bootstrap'. Perhaps this could be a bit clearer by saying that it
only supports modern versions, and that support for older versions can
be added later if needed. (For those of us who are stuck with 10-20
year old hardware and OS versions, 2015 isn't that long ago.)

> Signed-off-by: Derrick Stolee <[e-mail]>
> ---
> diff --git a/builtin/gc.c b/builtin/gc.c
> @@ -1491,6 +1491,214 @@ static int maintenance_unregister(void)
> +static int bootout(const char *filename)
> +{
> +       int result;
> +       struct strvec args = STRVEC_INIT;
> +       char *uid = get_uid();
> +       const char *launchctl = getenv("GIT_TEST_CRONTAB");
> +       if (!launchctl)
> +               launchctl = "/bin/launchctl";
> +
> +       strvec_split(&args, launchctl);
> +       strvec_push(&args, "bootout");
> +       strvec_pushf(&args, "gui/%s", uid);
> +       strvec_push(&args, filename);
> +
> +       result = run_command_v_opt(args.v, 0);
> +
> +       strvec_clear(&args);
> +       free(uid);
> +       return result;
> +}
> +
> +static int bootstrap(const char *filename)
> +{
> +       int result;
> +       struct strvec args = STRVEC_INIT;
> +       char *uid = get_uid();
> +       const char *launchctl = getenv("GIT_TEST_CRONTAB");
> +       if (!launchctl)
> +               launchctl = "/bin/launchctl";
> +
> +       strvec_split(&args, launchctl);
> +       strvec_push(&args, "bootstrap");
> +       strvec_pushf(&args, "gui/%s", uid);
> +       strvec_push(&args, filename);
> +
> +       result = run_command_v_opt(args.v, 0);
> +
> +       strvec_clear(&args);
> +       free(uid);
> +       return result;
> +}

The bootout() and bootstrap() functions seem to be identical except
for one string literal. Code could be reduced by refactoring and
passing that string literal in as an argument.

> +static int remove_plist(enum schedule_priority schedule)
> +{
> +       const char *frequency = get_frequency(schedule);
> +       char *name = get_service_name(frequency);
> +       char *filename = get_service_filename(name);
> +       int result = bootout(filename);
> +       free(filename);
> +       free(name);
> +       return result;
> +}

The result of get_service_name() is only ever passed to
get_service_filename(). If get_service_filename() made the call to
get_service_name() itself, it would free up callers from having to
remember to free(name), thus reducing the likelihood of a possible
leak.

> +static int schedule_plist(const char *exec_path, enum schedule_priority schedule)
> +{
> +       plist = fopen(filename, "w");
> +
> +       if (!plist)
> +               die(_("failed to open '%s'"), filename);

You can replace the fopen() and die() with a single call to xfopen().

> +       /* bootout might fail if not already running, so ignore */
> +       bootout(filename);
> +       if (bootstrap(filename))
> +               die(_("failed to bootstrap service %s"), filename);

I'm guessing that 'launchctl bootout' won't print a confusing and
unexpected error message if the plist is not presently registered?

> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> @@ -389,12 +389,58 @@ test_expect_success 'stop from existing schedule' '
> +test_expect_success MACOS_MAINTENANCE 'start and stop macOS maintenance' '
> +       echo "#!/bin/sh\necho \$@ >>args" >print-args &&
> +       chmod a+x print-args &&

Note that $@ loses its special magic if not surrounded by quotes, thus
acts just like $*. So, either use "$@" or $* depending upon your
requirements, but in the case of 'echo', it's just not going to matter
at all, so $* is fine.

To construct the script, you can do this instead, which is easier to
read and handles the 'chmod' for you:

    write_script print-args <<-\EOF
    echo $* >>args
    EOF

> +       for frequency in hourly daily weekly
> +       do
> +               PLIST="$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
> +               grep schedule=$frequency "$PLIST" &&
> +               echo "bootout gui/$UID $PLIST" >>expect &&
> +               echo "bootstrap gui/$UID $PLIST" >>expect || return 1
> +       done &&

My gut feeling is that this would be more robust if you manually
determine UID in the test script the same way as the git-maintenance
command itself does using '/usr/bin/id -u' rather than relying upon
inheriting UID from the user's environment.

> +       # stop does not remove plist files, but boots them out

Is this desirable? Should git-maintenance do a better job of cleaning
up after itself?

Derrick Stolee Nov. 3, 2020, 9:21 p.m. UTC | #2

On 11/3/2020 1:45 PM, Eric Sunshine wrote:
> On Tue, Nov 3, 2020 at 9:05 AM Derrick Stolee via GitGitGadget
> <[e-mail]> wrote:
>> maintenance: use launchctl on macOS
> 
> A few comments below (not necessarily worth a re-roll)...
> 
>> The launchctl command needs to be aligned with a user id in order
>> to initialize the command environment. This must be done using
>> the 'launchctl bootstrap' subcommand. This subcommand is new as
>> of macOS 10.11, which was released in September 2015. Before that
>> release the 'launchctl load' subcommand was recommended. The best
>> source of information on this transition I have seen is available
>> at [2].
> 
> It's not clear whether or not this is saying that git-maintenance will
> dynamically adapt to work with modern and older 'launchctl'. A glance
> at the actual code reveals that it knows only about modern
> 'bootstrap'. Perhaps this could be a bit clearer by saying that it
> only supports modern versions, and that support for older versions can
> be added later if needed. (For those of us who are stuck with 10-20
> year old hardware and OS versions, 2015 isn't that long ago.)

Yes, this is a strange place to be. How far do we go back to support
as many users as possible? How many users will be simultaneously
stuck on an old version of macOS _and_ interested in updating to this
latest version of Git? Is that worth the extra functionality to detect
the the OS version and change commands?

The good news is that this patch doesn't lock us in to the
boot(strap|out) subcommands too much. We could add in load/unload
subcommands for systems that are too old. However, I did think it
was prudent to take the currently-recommended option for fear that
Apple will completely _delete_ the load/unload options in an
upcoming release.

This makes me realize that I should update the documentation to give
pointers for how to view the schedules for each platform:

- Windows: Open "Task Scheduler"
- macOS: 'launchctl list | grep org.git-scm.git'
- Others: 'crontab -l'

>> +static int bootstrap(const char *filename)
>> +{
>> +       int result;
>> +       struct strvec args = STRVEC_INIT;
>> +       char *uid = get_uid();
>> +       const char *launchctl = getenv("GIT_TEST_CRONTAB");
>> +       if (!launchctl)
>> +               launchctl = "/bin/launchctl";
>> +
>> +       strvec_split(&args, launchctl);
>> +       strvec_push(&args, "bootstrap");
>> +       strvec_pushf(&args, "gui/%s", uid);
>> +       strvec_push(&args, filename);
>> +
>> +       result = run_command_v_opt(args.v, 0);
>> +
>> +       strvec_clear(&args);
>> +       free(uid);
>> +       return result;
>> +}
> 
> The bootout() and bootstrap() functions seem to be identical except
> for one string literal. Code could be reduced by refactoring and
> passing that string literal in as an argument.

Good point. Or a simple boolean value for "add" or "remove".
 
>> +static int remove_plist(enum schedule_priority schedule)
>> +{
>> +       const char *frequency = get_frequency(schedule);
>> +       char *name = get_service_name(frequency);
>> +       char *filename = get_service_filename(name);
>> +       int result = bootout(filename);
>> +       free(filename);
>> +       free(name);
>> +       return result;
>> +}
> 
> The result of get_service_name() is only ever passed to
> get_service_filename(). If get_service_filename() made the call to
> get_service_name() itself, it would free up callers from having to
> remember to free(name), thus reducing the likelihood of a possible
> leak.

You're right. In an earlier version I thought I needed to add the
name in the XML, but it turns out I did not.

>> +static int schedule_plist(const char *exec_path, enum schedule_priority schedule)
>> +{
>> +       plist = fopen(filename, "w");
>> +
>> +       if (!plist)
>> +               die(_("failed to open '%s'"), filename);
> 
> You can replace the fopen() and die() with a single call to xfopen().

Thanks! I'll use that in several places and try to remember next time.

>> +       /* bootout might fail if not already running, so ignore */
>> +       bootout(filename);
>> +       if (bootstrap(filename))
>> +               die(_("failed to bootstrap service %s"), filename);
> 
> I'm guessing that 'launchctl bootout' won't print a confusing and
> unexpected error message if the plist is not presently registered?

You're right, it does. It also returns with a non-zero exit code.
Along with your later suggestion to clear the .plist files, we will
want to have several conditions to not error out during a case where
the task is not scheduled.

>> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
>> @@ -389,12 +389,58 @@ test_expect_success 'stop from existing schedule' '
>> +test_expect_success MACOS_MAINTENANCE 'start and stop macOS maintenance' '
>> +       echo "#!/bin/sh\necho \$@ >>args" >print-args &&
>> +       chmod a+x print-args &&
> 
> Note that $@ loses its special magic if not surrounded by quotes, thus
> acts just like $*. So, either use "$@" or $* depending upon your
> requirements, but in the case of 'echo', it's just not going to matter
> at all, so $* is fine.
> 
> To construct the script, you can do this instead, which is easier to
> read and handles the 'chmod' for you:
> 
>     write_script print-args <<-\EOF
>     echo $* >>args
>     EOF

TIL. thanks.

>> +       for frequency in hourly daily weekly
>> +       do
>> +               PLIST="$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
>> +               grep schedule=$frequency "$PLIST" &&
>> +               echo "bootout gui/$UID $PLIST" >>expect &&
>> +               echo "bootstrap gui/$UID $PLIST" >>expect || return 1
>> +       done &&
> 
> My gut feeling is that this would be more robust if you manually
> determine UID in the test script the same way as the git-maintenance
> command itself does using '/usr/bin/id -u' rather than relying upon
> inheriting UID from the user's environment.

Yeah, you're right. Thanks!

>> +       # stop does not remove plist files, but boots them out
> 
> Is this desirable? Should git-maintenance do a better job of cleaning
> up after itself?

Yes, let's clear up these .plist files.

Thanks!
-Stolee

Eric Sunshine Nov. 3, 2020, 10:27 p.m. UTC | #3

On Tue, Nov 3, 2020 at 4:22 PM Derrick Stolee <[e-mail]> wrote:
> On 11/3/2020 1:45 PM, Eric Sunshine wrote:
> > It's not clear whether or not this is saying that git-maintenance will
> > dynamically adapt to work with modern and older 'launchctl'. A glance
> > at the actual code reveals that it knows only about modern
> > 'bootstrap'. Perhaps this could be a bit clearer by saying that it
> > only supports modern versions, and that support for older versions can
> > be added later if needed. (For those of us who are stuck with 10-20
> > year old hardware and OS versions, 2015 isn't that long ago.)
>
> Yes, this is a strange place to be. How far do we go back to support
> as many users as possible? How many users will be simultaneously
> stuck on an old version of macOS _and_ interested in updating to this
> latest version of Git? Is that worth the extra functionality to detect
> the the OS version and change commands?

I don't think this patch series needs to answer these questions
provided that it doesn't close the door to someone adding
older-version support down the road. My review comment was more about
the commit message being clearer about the choice -- supporting only
recent 'launchctl' -- being made by this series. (And perhaps the
documentation could mention that it requires a reasonably modern
'launchctl'.)

> This makes me realize that I should update the documentation to give
> pointers for how to view the schedules for each platform:
>
> - Windows: Open "Task Scheduler"
> - macOS: 'launchctl list | grep org.git-scm.git'
> - Others: 'crontab -l'

Good idea.

I haven't looked at the documentation, but if it doesn't already, I
wonder if it should give examples of how to set these up by hand or
how to customize the ones created by git-maintenance itself. I was
also wondering if git-maintenance could have a mode in which it
generates the template file(s) for you but doesn't actually
activate/install it, instead providing instructions for
activation/installation. That way, people could modify the scheduling
file before actually activating it. However, this may all be outside
the scope of the patch series, and could be done later if desired.

Derrick Stolee Nov. 4, 2020, 1:33 p.m. UTC | #4

On 11/3/2020 5:27 PM, Eric Sunshine wrote:
> On Tue, Nov 3, 2020 at 4:22 PM Derrick Stolee <[e-mail]> wrote:
>> On 11/3/2020 1:45 PM, Eric Sunshine wrote:
>>> It's not clear whether or not this is saying that git-maintenance will
>>> dynamically adapt to work with modern and older 'launchctl'. A glance
>>> at the actual code reveals that it knows only about modern
>>> 'bootstrap'. Perhaps this could be a bit clearer by saying that it
>>> only supports modern versions, and that support for older versions can
>>> be added later if needed. (For those of us who are stuck with 10-20
>>> year old hardware and OS versions, 2015 isn't that long ago.)
>>
>> Yes, this is a strange place to be. How far do we go back to support
>> as many users as possible? How many users will be simultaneously
>> stuck on an old version of macOS _and_ interested in updating to this
>> latest version of Git? Is that worth the extra functionality to detect
>> the the OS version and change commands?
> 
> I don't think this patch series needs to answer these questions
> provided that it doesn't close the door to someone adding
> older-version support down the road. My review comment was more about
> the commit message being clearer about the choice -- supporting only
> recent 'launchctl' -- being made by this series. (And perhaps the
> documentation could mention that it requires a reasonably modern
> 'launchctl'.)

Thanks. I'll be sure to make the commit message more clear.
 
>> This makes me realize that I should update the documentation to give
>> pointers for how to view the schedules for each platform:
>>
>> - Windows: Open "Task Scheduler"
>> - macOS: 'launchctl list | grep org.git-scm.git'
>> - Others: 'crontab -l'
> 
> Good idea.
> 
> I haven't looked at the documentation, but if it doesn't already, I
> wonder if it should give examples of how to set these up by hand or
> how to customize the ones created by git-maintenance itself. I was
> also wondering if git-maintenance could have a mode in which it
> generates the template file(s) for you but doesn't actually
> activate/install it, instead providing instructions for
> activation/installation. That way, people could modify the scheduling
> file before actually activating it. However, this may all be outside
> the scope of the patch series, and could be done later if desired.

Outside of the technical details, the biggest questions I've
tried to handle with the background maintenance feature has
been to balance customization with ease-of-use. My philosophy
is that users fall into a few expertise buckets, and have
different expectations:

* Beginners will never know about background maintenance and
  so will never run "git maintenance start" or set the config
  values.

* Intermediate users might discover "git maintenance start"
  and will appreciate that they don't need to learn cron to
  set up a good default schedule.

* Advanced users will read the documentation and use Git
  config settings to customize their maintenance tasks and
  schedule.

* Expert users might decide the task schedule available by
  "git maintenance start" is too restrictive, so they create
  their own background schedule with custom tasks. They might
  not even run the maintenance builtin and opt instead for
  'git gc' or 'git repack' directly!

My main target has been these "intermediate" users who might
run the command and forget about it. However, I've also tried
to keep the advanced users in mind with the advanced config
options available.

Your comment about documentation demonstrates a way to serve
the advanced and expert users by providing a clear framework
for discovering what Git is doing under the hood and how to
modify or adapt that to their needs. It is also important to
demonstrate how to set up schedules in a similar way without
having them be overwritten by a later "git maintenance start"
command.

I will give this a shot in v2. Thanks.

-Stolee

Derrick Stolee Nov. 4, 2020, 2:17 p.m. UTC | #5

On 11/3/2020 4:21 PM, Derrick Stolee wrote:
> On 11/3/2020 1:45 PM, Eric Sunshine wrote:
>> On Tue, Nov 3, 2020 at 9:05 AM Derrick Stolee via GitGitGadget
>>> +static int remove_plist(enum schedule_priority schedule)
>>> +{
>>> +       const char *frequency = get_frequency(schedule);
>>> +       char *name = get_service_name(frequency);
>>> +       char *filename = get_service_filename(name);
>>> +       int result = bootout(filename);
>>> +       free(filename);
>>> +       free(name);
>>> +       return result;
>>> +}
>>
>> The result of get_service_name() is only ever passed to
>> get_service_filename(). If get_service_filename() made the call to
>> get_service_name() itself, it would free up callers from having to
>> remember to free(name), thus reducing the likelihood of a possible
>> leak.
> 
> You're right. In an earlier version I thought I needed to add the
> name in the XML, but it turns out I did not.

As I go through the effort to remove get_service_name() I find that
actually the name is used in one place in the XML file:

+		   "<key>Label</key><string>%s</string>\n"

This "Label" should match the filename, I believe. I can still
be more careful about how often this name is actually required.

Thanks,
-Stolee

Patch

diff --git a/builtin/gc.c b/builtin/gc.c
index c1f7d9bdc2..fa0ae63a80 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1491,6 +1491,214 @@  static int maintenance_unregister(void)
 	return run_command(&config_unset);
 }
 
+#if defined(__APPLE__)
+
+static char *get_service_name(const char *frequency)
+{
+	struct strbuf label = STRBUF_INIT;
+	strbuf_addf(&label, "org.git-scm.git.%s", frequency);
+	return strbuf_detach(&label, NULL);
+}
+
+static char *get_service_filename(const char *name)
+{
+	char *expanded;
+	struct strbuf filename = STRBUF_INIT;
+	strbuf_addf(&filename, "~/Library/LaunchAgents/%s.plist", name);
+
+	expanded = expand_user_path(filename.buf, 1);
+	if (!expanded)
+		die(_("failed to expand path '%s'"), filename.buf);
+
+	strbuf_release(&filename);
+	return expanded;
+}
+
+static const char *get_frequency(enum schedule_priority schedule)
+{
+	switch (schedule) {
+	case SCHEDULE_HOURLY:
+		return "hourly";
+	case SCHEDULE_DAILY:
+		return "daily";
+	case SCHEDULE_WEEKLY:
+		return "weekly";
+	default:
+		BUG("invalid schedule %d", schedule);
+	}
+}
+
+static char *get_uid(void)
+{
+	struct strbuf output = STRBUF_INIT;
+	struct child_process id = CHILD_PROCESS_INIT;
+
+	strvec_pushl(&id.args, "/usr/bin/id", "-u", NULL);
+	if (capture_command(&id, &output, 0))
+		die(_("failed to discover user id"));
+
+	strbuf_trim_trailing_newline(&output);
+	return strbuf_detach(&output, NULL);
+}
+
+static int bootout(const char *filename)
+{
+	int result;
+	struct strvec args = STRVEC_INIT;
+	char *uid = get_uid();
+	const char *launchctl = getenv("GIT_TEST_CRONTAB");
+	if (!launchctl)
+		launchctl = "/bin/launchctl";
+
+	strvec_split(&args, launchctl);
+	strvec_push(&args, "bootout");
+	strvec_pushf(&args, "gui/%s", uid);
+	strvec_push(&args, filename);
+
+	result = run_command_v_opt(args.v, 0);
+
+	strvec_clear(&args);
+	free(uid);
+	return result;
+}
+
+static int bootstrap(const char *filename)
+{
+	int result;
+	struct strvec args = STRVEC_INIT;
+	char *uid = get_uid();
+	const char *launchctl = getenv("GIT_TEST_CRONTAB");
+	if (!launchctl)
+		launchctl = "/bin/launchctl";
+
+	strvec_split(&args, launchctl);
+	strvec_push(&args, "bootstrap");
+	strvec_pushf(&args, "gui/%s", uid);
+	strvec_push(&args, filename);
+
+	result = run_command_v_opt(args.v, 0);
+
+	strvec_clear(&args);
+	free(uid);
+	return result;
+}
+
+static int remove_plist(enum schedule_priority schedule)
+{
+	const char *frequency = get_frequency(schedule);
+	char *name = get_service_name(frequency);
+	char *filename = get_service_filename(name);
+	int result = bootout(filename);
+	free(filename);
+	free(name);
+	return result;
+}
+
+static int remove_plists(void)
+{
+	return remove_plist(SCHEDULE_HOURLY) ||
+		remove_plist(SCHEDULE_DAILY) ||
+		remove_plist(SCHEDULE_WEEKLY);
+}
+
+static int schedule_plist(const char *exec_path, enum schedule_priority schedule)
+{
+	FILE *plist;
+	int i;
+	const char *preamble, *repeat;
+	const char *frequency = get_frequency(schedule);
+	char *name = get_service_name(frequency);
+	char *filename = get_service_filename(name);
+
+	if (safe_create_leading_directories(filename))
+		die(_("failed to create directories for '%s'"), filename);
+	plist = fopen(filename, "w");
+
+	if (!plist)
+		die(_("failed to open '%s'"), filename);
+
+	preamble = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+		   "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
+		   "<plist version=\"1.0\">"
+		   "<dict>\n"
+		   "<key>Label</key><string>%s</string>\n"
+		   "<key>ProgramArguments</key>\n"
+		   "<array>\n"
+		   "<string>%s/git</string>\n"
+		   "<string>--exec-path=%s</string>\n"
+		   "<string>for-each-repo</string>\n"
+		   "<string>--config=maintenance.repo</string>\n"
+		   "<string>maintenance</string>\n"
+		   "<string>run</string>\n"
+		   "<string>--schedule=%s</string>\n"
+		   "</array>\n"
+		   "<key>StartCalendarInterval</key>\n"
+		   "<array>\n";
+	fprintf(plist, preamble, name, exec_path, exec_path, frequency);
+
+	switch (schedule) {
+	case SCHEDULE_HOURLY:
+		repeat = "<dict>\n"
+			 "<key>Hour</key><integer>%d</integer>\n"
+			 "<key>Minute</key><integer>0</integer>\n"
+			 "<dict>\n";
+		for (i = 1; i <= 23; i++)
+			fprintf(plist, repeat, i);
+		break;
+
+	case SCHEDULE_DAILY:
+		repeat = "<dict>\n"
+			 "<key>Day</key><integer>%d</integer>\n"
+			 "<key>Hour</key><integer>0</integer>\n"
+			 "<key>Minute</key><integer>0</integer>\n"
+			 "</dict>\n";
+		for (i = 1; i <= 6; i++)
+			fprintf(plist, repeat, i);
+		break;
+
+	case SCHEDULE_WEEKLY:
+		fprintf(plist,
+			"<dict>\n"
+			"<key>Day</key><integer>0</integer>\n"
+			"<key>Hour</key><integer>0</integer>\n"
+			"<key>Minute</key><integer>0</integer>\n"
+			"</dict>\n");
+		break;
+
+	default:
+		/* unreachable */
+		break;
+	}
+	fprintf(plist, "</array>\n</dict>\n</plist>\n");
+
+	/* bootout might fail if not already running, so ignore */
+	bootout(filename);
+	if (bootstrap(filename))
+		die(_("failed to bootstrap service %s"), filename);
+
+	fclose(plist);
+	free(filename);
+	free(name);
+	return 0;
+}
+
+static int add_plists(void)
+{
+	const char *exec_path = git_exec_path();
+
+	return schedule_plist(exec_path, SCHEDULE_HOURLY) ||
+		schedule_plist(exec_path, SCHEDULE_DAILY) ||
+		schedule_plist(exec_path, SCHEDULE_WEEKLY);
+}
+
+static int platform_update_schedule(int run_maintenance, int fd)
+{
+	if (run_maintenance)
+		return add_plists();
+	else
+		return remove_plists();
+}
+#else
 #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE"
 #define END_LINE "# END GIT MAINTENANCE SCHEDULE"
 
@@ -1585,6 +1793,7 @@  static int platform_update_schedule(int run_maintenance, int fd)
 		fclose(cron_list);
 	return result;
 }
+#endif
 
 static int update_background_schedule(int run_maintenance)
 {
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 20184e96e1..f0210aa206 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -367,7 +367,7 @@  test_expect_success 'register and unregister' '
 	test_cmp before actual
 '
 
-test_expect_success 'start from empty cron table' '
+test_expect_success !MACOS_MAINTENANCE 'start from empty cron table' '
 	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
 
 	# start registers the repo
@@ -378,7 +378,7 @@  test_expect_success 'start from empty cron table' '
 	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
 '
 
-test_expect_success 'stop from existing schedule' '
+test_expect_success !MACOS_MAINTENANCE 'stop from existing schedule' '
 	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
 
 	# stop does not unregister the repo
@@ -389,12 +389,58 @@  test_expect_success 'stop from existing schedule' '
 	test_must_be_empty cron.txt
 '
 
-test_expect_success 'start preserves existing schedule' '
+test_expect_success !MACOS_MAINTENANCE 'start preserves existing schedule' '
 	echo "Important information!" >cron.txt &&
 	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
 	grep "Important information!" cron.txt
 '
 
+test_expect_success MACOS_MAINTENANCE 'start and stop macOS maintenance' '
+	echo "#!/bin/sh\necho \$@ >>args" >print-args &&
+	chmod a+x print-args &&
+
+	rm -f args &&
+	GIT_TEST_CRONTAB="./print-args" git maintenance start &&
+
+	# start registers the repo
+	git config --get --global maintenance.repo "$(pwd)" &&
+
+	# ~/Library/LaunchAgents
+	ls "$HOME/Library/LaunchAgents" >actual &&
+	cat >expect <<-\EOF &&
+	org.git-scm.git.daily.plist
+	org.git-scm.git.hourly.plist
+	org.git-scm.git.weekly.plist
+	EOF
+	test_cmp expect actual &&
+
+	rm expect &&
+	for frequency in hourly daily weekly
+	do
+		PLIST="$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
+		grep schedule=$frequency "$PLIST" &&
+		echo "bootout gui/$UID $PLIST" >>expect &&
+		echo "bootstrap gui/$UID $PLIST" >>expect || return 1
+	done &&
+	test_cmp expect args &&
+
+	rm -f args &&
+	GIT_TEST_CRONTAB="./print-args"  git maintenance stop &&
+
+	# stop does not unregister the repo
+	git config --get --global maintenance.repo "$(pwd)" &&
+
+	# stop does not remove plist files, but boots them out
+	rm expect &&
+	for frequency in hourly daily weekly
+	do
+		PLIST="$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
+		grep schedule=$frequency "$PLIST" &&
+		echo "bootout gui/$UID $PLIST" >>expect || return 1
+	done &&
+	test_cmp expect args
+'
+
 test_expect_success 'register preserves existing strategy' '
 	git config maintenance.strategy none &&
 	git maintenance register &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4a60d1ed76..620ffbf3af 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1703,6 +1703,10 @@  test_lazy_prereq REBASE_P '
 	test -z "$GIT_TEST_SKIP_REBASE_P"
 '
 
+test_lazy_prereq MACOS_MAINTENANCE '
+	launchctl list
+'
+
 # Ensure that no test accidentally triggers a Git command
 # that runs 'crontab', affecting a user's cron schedule.
 # Tests that verify the cron integration must set this locally