threads / patch / 63318

patchmaintenance: fix launchctl calendar intervals

Subject: [PATCH 0/1] maintenance: fix launchctl calendar intervals

## tl;dr

5 messages between Apr 21, 2025 and Apr 23, 2025. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Josh Heinrichs· Apr 21, 2025, 05:46 UTC · lore
Hello!

While working to add git maintenance support to home-manager[1] on macOS, I noticed that the calendar intervals are set up incorrectly for the launchctl scheduler. With the current settings daily jobs run on the first six days of the month, and weekly jobs run daily. I've confirmed this behaviour by manually shifting my system time around and checking the system logs. This seems mostly harmless, and I think git maintenance is somewhat niche, so I don't think we need to worry about proactively correcting existing launchd configurations somehow.

[1] https://github.com/nix-community/home-manager 
Josh Heinrichs (1):
  maintenance: fix launchctl calendar intervals
 builtin/gc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
-- 
2.47.2
Josh Heinrichs· Apr 21, 2025, 05:46 UTC · re: Josh Heinrichs · lore

[PATCH 1/1] maintenance: fix launchctl calendar intervals

When using the launchctl scheduler, the weekly job runs daily, and the daily job runs on the first six days of each month. This appears to be due to specifying "Day" in the calendar intervals, which according to launchd.plist(5) is for specifying days of the month rather than days of the week. The behaviour of running a job on the 0th day is undocumented, but in my testing appears to be the same as not specifying "Day" in the calendar interval, in which case the job will run daily.

Use "Weekday" in the calendar intervals, which is the correct way to schedule jobs to run on specific days of the week.

Signed-off-by: Josh Heinrichs <joshiheinrichs@gmail.com>
---
 builtin/gc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to builtin/gc.c +2 −2
diff --git a/builtin/gc.c b/builtin/gc.c
index 99431fd467..cc13baa3bd 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -2075,7 +2075,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
 
 	case SCHEDULE_DAILY:
 		repeat = "<dict>\n"
-			 "<key>Day</key><integer>%d</integer>\n"
+			 "<key>Weekday</key><integer>%d</integer>\n"
 			 "<key>Hour</key><integer>0</integer>\n"
 			 "<key>Minute</key><integer>%d</integer>\n"
 			 "</dict>\n";
@@ -2086,7 +2086,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
 	case SCHEDULE_WEEKLY:
 		strbuf_addf(&plist,
 			    "<dict>\n"
-			    "<key>Day</key><integer>0</integer>\n"
+			    "<key>Weekday</key><integer>0</integer>\n"
 			    "<key>Hour</key><integer>0</integer>\n"
 			    "<key>Minute</key><integer>%d</integer>\n"
 			    "</dict>\n",
-- 
2.47.2
Junio C Hamano· Apr 21, 2025, 17:42 UTC · re: Josh Heinrichs · lore

Re: [PATCH 1/1] maintenance: fix launchctl calendar intervals

Josh Heinrichs <joshiheinrichs@gmail.com> writes:
Show 13 quoted lines
> When using the launchctl scheduler, the weekly job runs daily, and the
> daily job runs on the first six days of each month. This appears to be
> due to specifying "Day" in the calendar intervals, which according to
> launchd.plist(5) is for specifying days of the month rather than days of
> the week. The behaviour of running a job on the 0th day is undocumented,
> but in my testing appears to be the same as not specifying "Day" in the
> calendar interval, in which case the job will run daily.
>
> Use "Weekday" in the calendar intervals, which is the correct way to
> schedule jobs to run on specific days of the week.
>
> Signed-off-by: Josh Heinrichs <joshiheinrichs@gmail.com>
> ---

With a bit wider context, the patch looks like so. As I do not use macOS or launchctl myself, my guess may be way off but please bear with me.

Show 22 quoted lines
>  builtin/gc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git i/builtin/gc.c w/builtin/gc.c
> index 99431fd467..cc13baa3bd 100644
> --- i/builtin/gc.c
> +++ w/builtin/gc.c
> @@ -2072,24 +2072,24 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
>  		for (i = 1; i <= 23; i++)
>  			strbuf_addf(&plist, repeat, i, minute);
>  		break;
>  
>  	case SCHEDULE_DAILY:
>  		repeat = "<dict>\n"
> -			 "<key>Day</key><integer>%d</integer>\n"
> +			 "<key>Weekday</key><integer>%d</integer>\n"
>  			 "<key>Hour</key><integer>0</integer>\n"
>  			 "<key>Minute</key><integer>%d</integer>\n"
>  			 "</dict>\n";
>  		for (i = 1; i <= 6; i++)
>  			strbuf_addf(&plist, repeat, i, minute);
>  		break;

So we used to say that "we want the task to run on days 1 thru 6 (6 days in total) of a month at certain minute of the day past midnight", which clearly not how people think of "daily" task.

Updated one specifies the task to run on day 1 (Monday) thru 6 (Saturday) of a week. The manual page seems to say that day 0 and day 7 of a week are both Sundays.

  https://www.manpagez.com/man/5/launchd.plist/ look for
  StartCalendarInterval to read about Day and Weekday.

Curious that day 7/0 are ignored. Other schedulers are somewhat inconsistent but generally agree, so I won't worry about changing it in this patch. schtasks_schedule_task() excludes Sundays, and crontab_update_schedule() says that it wants to run daily scheduled tasks on day 1-6. systemd_timer_write_timer_file() however seems to say that it wants to run things on Tue..Sun (excluding Mondays), which may want to be corrected, I dunno.

Show 10 quoted lines
>  	case SCHEDULE_WEEKLY:
>  		strbuf_addf(&plist,
>  			    "<dict>\n"
> -			    "<key>Day</key><integer>0</integer>\n"
> +			    "<key>Weekday</key><integer>0</integer>\n"
>  			    "<key>Hour</key><integer>0</integer>\n"
>  			    "<key>Minute</key><integer>%d</integer>\n"
>  			    "</dict>\n",
>  			    minute);
>  		break;

This one used to say "run on 0th day of the month", but now it says "run on 0th day of the week", which is on Sundays. Makes sense.

Will queue.  Thanks.
Derrick Stolee· Apr 23, 2025, 19:25 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] maintenance: fix launchctl calendar intervals

On 4/21/2025 1:42 PM, Junio C Hamano wrote:
Show 12 quoted lines
> Josh Heinrichs <joshiheinrichs@gmail.com> writes:
> 
>> When using the launchctl scheduler, the weekly job runs daily, and the
>> daily job runs on the first six days of each month. This appears to be
>> due to specifying "Day" in the calendar intervals, which according to
>> launchd.plist(5) is for specifying days of the month rather than days of
>> the week. The behaviour of running a job on the 0th day is undocumented,
>> but in my testing appears to be the same as not specifying "Day" in the
>> calendar interval, in which case the job will run daily.
>>
>> Use "Weekday" in the calendar intervals, which is the correct way to
>> schedule jobs to run on specific days of the week.
Wow, good find! Thank you for submitting a fix for this issue.

Thanks, -Stolee

Junio C Hamano· Apr 23, 2025, 20:31 UTC · re: Derrick Stolee · lore

Re: [PATCH 1/1] maintenance: fix launchctl calendar intervals

Derrick Stolee <stolee@gmail.com> writes:
Show 15 quoted lines
> On 4/21/2025 1:42 PM, Junio C Hamano wrote:
>> Josh Heinrichs <joshiheinrichs@gmail.com> writes:
>> 
>>> When using the launchctl scheduler, the weekly job runs daily, and the
>>> daily job runs on the first six days of each month. This appears to be
>>> due to specifying "Day" in the calendar intervals, which according to
>>> launchd.plist(5) is for specifying days of the month rather than days of
>>> the week. The behaviour of running a job on the 0th day is undocumented,
>>> but in my testing appears to be the same as not specifying "Day" in the
>>> calendar interval, in which case the job will run daily.
>>>
>>> Use "Weekday" in the calendar intervals, which is the correct way to
>>> schedule jobs to run on specific days of the week.
>
> Wow, good find! Thank you for submitting a fix for this issue.

Yes, good find indeed. Tweaked in your Acked-by and will merge to 'next'.

Thanks, all.

← back to recent threads