DomT4 / homebrew-autoupdate

:tropical_drink: An easy, convenient way to automatically update Homebrew.
BSD 2-Clause "Simplified" License
995 stars 55 forks source link

Use StartCalendarInterval in addition to StartInterval #28

Closed danielcompton closed 3 years ago

danielcompton commented 4 years ago

StartCalendarInterval

The man page for launchd.plist says:

     StartInterval 
     This optional key causes the job to be started every N seconds. If the system is
     asleep during the time of the next scheduled interval firing, that interval will be
     missed due to shortcomings in kqueue(3).  If the job is running during an interval
     firing, that interval firing will likewise be missed.

     StartCalendarInterval 
     This optional key causes the job to be started every calendar interval as specified.
     Missing arguments are considered to be wildcard. The semantics are similar to
     crontab(5) in how firing dates are specified. Multiple dictionaries may be specified
     in an array to schedule multiple calendar intervals.

     Unlike cron which skips job invocations when the computer is asleep, launchd will
     start the job the next time the computer wakes up.  If multiple intervals transpire
     before the computer is woken, those events will be coalesced into one event upon
     wake from sleep.

     Note that StartInterval and StartCalendarInterval are not aware of each other. They
     are evaluated completely independently by the system.

This currently uses StartInterval:

https://github.com/DomT4/homebrew-autoupdate/blob/9aa7e9cbe6baea53dc2ff4e2679cabf1de3424d5/lib/autoupdate/start.rb#L91-L92.

This means that Homebrew autoupdate may not be run if it happens to be asleep when the timer would normally fire. I've noticed this myself a few times and was surprised by it.

I think it could be good to use StartCalendarInterval, either as an option, or a replacement for StartInterval. StartCalendarInterval is basically a cron expression.

An easier alternative would be to document this limitation, and encourage people to check often enough that they won't miss too many updates from their computer being asleep.

RunAtLoad

The man page also says:

     RunAtLoad 
     This optional key is used to control whether your job is launched once at the time
     the job is loaded. The default is false. This key should be avoided, as speculative
     job launches have an adverse effect on system-boot and user-login scenarios.

We should probably remove this key to avoid doing a bunch of work in the background on boot.

ProcessType

We should also look at setting the ProcessType to Background

     ProcessType 
     This optional key describes, at a high level, the intended purpose of the job.  The
     system will apply resource limits based on what kind of job it is. If left unspeci-
     fied, the system will apply light resource limits to the job, throttling its CPU
     usage and I/O bandwidth. This classification is preferable to using the
     HardResourceLimits, SoftResourceLimits and Nice keys. The following are valid val-
     ues:

           Background
           Background jobs are generally processes that do work that was not directly
           requested by the user. The resource limits applied to Background jobs are
           intended to prevent them from disrupting the user experience.
DomT4 commented 3 years ago

@danielcompton Did you want to follow up on https://github.com/DomT4/homebrew-autoupdate/pull/27 or are you happy for someone else to explore the changes discussed here?

danielcompton commented 3 years ago

Happy for anyone to take this, I don't have a lot of time at the moment sorry!

One question to answer about this is whether we would want to continue to support --start [interval], or to only support (for example) --cron 0 0 * * *. It is probably fine to keep supporting both, but I'd suggest changing the default to --cron (or whatever it's called) as that has semantics closer to what people expect and want.

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.