Skip to content

Commit

Permalink
maintenance: use launchctl on macOS
Browse files Browse the repository at this point in the history
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 <dstolee@microsoft.com>
  • Loading branch information
derrickstolee committed Nov 3, 2020
1 parent d35f1aa commit 4271f40
Show file tree
Hide file tree
Showing 3 changed files with 262 additions and 3 deletions.
209 changes: 209 additions & 0 deletions builtin/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
{
Expand Down
52 changes: 49 additions & 3 deletions t/t7900-maintenance.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 &&
Expand Down
4 changes: 4 additions & 0 deletions t/test-lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4271f40

Please sign in to comment.