-
Notifications
You must be signed in to change notification settings - Fork 691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: macos status #287
base: master
Are you sure you want to change the base?
Fix: macos status #287
Conversation
I'm going to test an approach where we ask |
Status is correctly detecting "Not Installed" and "Stopped" now, but I think I managed to break |
I tested in master and was experiencing the same issue around starting the service. It doesn't error, but the service process doesn't actually start. It may be such that we just need to check if the service does not have a start condition, and manually start it ourselves (immediately) after loading it. In addition I'm uncomfortable with this Edit: Thankfully |
59e574c works fine, but userID management is not our responsibility. This passes tests for me locally, but I need to rebase a collection of patches and run them on the CI. Will flip this over to "ready" afterwards. |
Works on my machine seal of approval: https://github.com/djdv/go-filesystem-utils/actions/runs/1056920024 |
This needs some refinement, but I'm putting it up early.
Follow up to #273
I encountered problems in my own tests related to detecting that the service is not installed.
Pre patch: https://github.com/djdv/go-filesystem-utils/runs/3106784696?check_suite_focus=true#step:7:36
Post patch: https://github.com/djdv/go-filesystem-utils/runs/3108004664?check_suite_focus=true#step:7:58
Still getting a status related failure further down the test so I'll have to see what's going on there.
As-is, this removes the need to check the error string value to determine if it's a stderr-err or something else.Replacing that logic with a wrapped error value so we can use
errors.Is
instead.And changes how we get data from stdio during run, and how we interpret the response from
launchctl
.Edit: This was a simplification but still too complicated, the need for this went away in future commits.
In addition, I recognize how hacky this is, and the variable stuttering. Considering there doesn't seem to be a launchd API this is probably as good as it can get.
For the variable naming, I'm going to look at it again later, but might not be able to change it. We're kind of stuck with
stderr
already havingerr
as its suffix. Suggestions for any of the names is welcomed.(Also, this incorporates some code from another commit, I might have messed up one of the branches while trying to separate them. Going to review that and submit some others later)