Remove launchjob files during uninstall, even if they are not loaded #14730
Conversation
jawshooah
added
the
awaiting maintainer feedback
| paths = paths.map { |elt| Pathname(elt) }.select(&:exist?) | ||
| paths.each do |path| | ||
| @command.run!('/bin/rm', :args => ['-f', '--', path], :sudo => with_sudo) | ||
| end | ||
| plist_status = @command.run('/bin/launchctl', :args => ['list', service], :sudo => with_sudo, :print_stderr => false).stdout | ||
| if %r{^\{}.match(plist_status) | ||
| result = @command.run!('/bin/launchctl', :args => ['remove', service], :sudo => with_sudo) |
There was a problem hiding this comment.
@jawshooah thanks for a fix!
is there any specific reason to rm .plist file and only then launchctl remove?
This is a reversal of previous flow and feels unnatural.
I'd assume it works just fine, but we shouldn't expect this to continue working if Apple rewrites launchctl daemon once again.
It feels safer and further-proof to launchctl remove first and only then rm .plist.
There was a problem hiding this comment.
Good point. I tested locally and there were no problems, but there's no guarantee that won't change. Thanks for the review!
Pathname does not expand '~', so we need to give it the absolute path to the HOME directory.
|
Looks fine to me. We should cut a release after this is in (there were other small fixes in the meantime as well) |
Remove launchjob files during uninstall, even if they are not loaded
|
Are you cutting the new release, or do you want me to do it? |
|
Took care of it myself. |
adidalal
removed
the
awaiting maintainer feedback
This fixes the original implementation in #8523 to correctly remove user launchjob files, and to do so even when the jobs have not been loaded.
Fixes #14728