Merge branch 'js/maintenance-launchctl-fix'

"git maintenance" scheduler fix for macOS.

* js/maintenance-launchctl-fix:
  maintenance: skip bootout/bootstrap when plist is registered
  maintenance: create `launchctl` configuration using a lock file

2 files changed, 87 insertions, 21 deletions

diff --git a/builtin/gc.c b/builtin/gc.c
index 6ce5ca4..9a48eb5 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1600,18 +1600,40 @@ static int launchctl_remove_plists(const char *cmd)
 		launchctl_remove_plist(SCHEDULE_WEEKLY, cmd);
 }
 
+static int launchctl_list_contains_plist(const char *name, const char *cmd)
+{
+	int result;
+	struct child_process child = CHILD_PROCESS_INIT;
+	char *uid = launchctl_get_uid();
+
+	strvec_split(&child.args, cmd);
+	strvec_pushl(&child.args, "list", name, NULL);
+
+	child.no_stderr = 1;
+	child.no_stdout = 1;
+
+	if (start_command(&child))
+		die(_("failed to start launchctl"));
+
+	result = finish_command(&child);
+
+	free(uid);
+
+	/* Returns failure if 'name' doesn't exist. */
+	return !result;
+}
+
 static int launchctl_schedule_plist(const char *exec_path, enum schedule_priority schedule, const char *cmd)
 {
-	FILE *plist;
-	int i;
+	int i, fd;
 	const char *preamble, *repeat;
 	const char *frequency = get_frequency(schedule);
 	char *name = launchctl_service_name(frequency);
 	char *filename = launchctl_service_filename(name);
-
-	if (safe_create_leading_directories(filename))
-		die(_("failed to create directories for '%s'"), filename);
-	plist = xfopen(filename, "w");
+	struct lock_file lk = LOCK_INIT;
+	static unsigned long lock_file_timeout_ms = ULONG_MAX;
+	struct strbuf plist = STRBUF_INIT, plist2 = STRBUF_INIT;
+	struct stat st;
 
 	preamble = "<?xml version=\"1.0\"?>\n"
 		   "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
@@ -1630,7 +1652,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
 		   "</array>\n"
 		   "<key>StartCalendarInterval</key>\n"
 		   "<array>\n";
-	fprintf(plist, preamble, name, exec_path, exec_path, frequency);
+	strbuf_addf(&plist, preamble, name, exec_path, exec_path, frequency);
 
 	switch (schedule) {
 	case SCHEDULE_HOURLY:
@@ -1639,7 +1661,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
 			 "<key>Minute</key><integer>0</integer>\n"
 			 "</dict>\n";
 		for (i = 1; i <= 23; i++)
-			fprintf(plist, repeat, i);
+			strbuf_addf(&plist, repeat, i);
 		break;
 
 	case SCHEDULE_DAILY:
@@ -1649,32 +1671,59 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
 			 "<key>Minute</key><integer>0</integer>\n"
 			 "</dict>\n";
 		for (i = 1; i <= 6; i++)
-			fprintf(plist, repeat, i);
+			strbuf_addf(&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");
+		strbuf_addstr(&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");
-	fclose(plist);
+	strbuf_addstr(&plist, "</array>\n</dict>\n</plist>\n");
+
+	if (safe_create_leading_directories(filename))
+		die(_("failed to create directories for '%s'"), filename);
+
+	if ((long)lock_file_timeout_ms < 0 &&
+	    git_config_get_ulong("gc.launchctlplistlocktimeoutms",
+				 &lock_file_timeout_ms))
+		lock_file_timeout_ms = 150;
+
+	fd = hold_lock_file_for_update_timeout(&lk, filename, LOCK_DIE_ON_ERROR,
+					       lock_file_timeout_ms);
 
-	/* bootout might fail if not already running, so ignore */
-	launchctl_boot_plist(0, filename, cmd);
-	if (launchctl_boot_plist(1, filename, cmd))
-		die(_("failed to bootstrap service %s"), filename);
+	/*
+	 * Does this file already exist? With the intended contents? Is it
+	 * registered already? Then it does not need to be re-registered.
+	 */
+	if (!stat(filename, &st) && st.st_size == plist.len &&
+	    strbuf_read_file(&plist2, filename, plist.len) == plist.len &&
+	    !strbuf_cmp(&plist, &plist2) &&
+	    launchctl_list_contains_plist(name, cmd))
+		rollback_lock_file(&lk);
+	else {
+		if (write_in_full(fd, plist.buf, plist.len) < 0 ||
+		    commit_lock_file(&lk))
+			die_errno(_("could not write '%s'"), filename);
+
+		/* bootout might fail if not already running, so ignore */
+		launchctl_boot_plist(0, filename, cmd);
+		if (launchctl_boot_plist(1, filename, cmd))
+			die(_("failed to bootstrap service %s"), filename);
+	}
 
 	free(filename);
 	free(name);
+	strbuf_release(&plist);
+	strbuf_release(&plist2);
 	return 0;
 }
 
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 58f46c7..fc16ac2 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -582,6 +582,23 @@ test_expect_success 'start and stop macOS maintenance' '
 	test_line_count = 0 actual
 '
 
+test_expect_success 'use launchctl list to prevent extra work' '
+	# ensure we are registered
+	GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance start &&
+
+	# do it again on a fresh args file
+	rm -f args &&
+	GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance start &&
+
+	ls "$HOME/Library/LaunchAgents" >actual &&
+	cat >expect <<-\EOF &&
+	list org.git-scm.git.hourly
+	list org.git-scm.git.daily
+	list org.git-scm.git.weekly
+	EOF
+	test_cmp expect args
+'
+
 test_expect_success 'start and stop Windows maintenance' '
 	write_script print-args <<-\EOF &&
 	echo $* >>args