-
Notifications
You must be signed in to change notification settings - Fork 38.7k
gui: remove macOS start on login code #17567
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
Conversation
The macOS startup item code was disabled for builds targeting macOS > 10.11 in bitcoin#15208. Now that we require macOS 10.12 as a minimum, bitcoin#17550, we can remove the startup item code entirely, as the API we were using was removed in macOS 10.12.
|
ACK. If someone means to add this functionality again in the future, I think something like #9005 would be required. |
| GUI changes | ||
| ----------- | ||
|
|
||
| - The "Start Bitcoin Core on system login" option has been removed on macOS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please mention that it was already not working on recent MacOS versions (so effectively this is an user-invisible change).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will actually be user facing, as the current release binaries include this option in the preferences menu (0.19 was compiled with a macOS 10.10 minimum and targeting the 10.11 SDK).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the option was there, but it wasn't working on newer MacOS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The option was there, and it should have been working for all versions of macOS (I can only test up to 10.14). Note that there are plenty of ways for the user to create the "start at login" behaviour without us providing an option from within Bitcoin Core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks for explaining.
The way MacOS does things is really confusing here. So we need to remove the functionality because the calls are deprecated, but they still work fine, even on newer MacOS, if compiled with an older MacOS in mind.
Note that there are plenty of ways for the user to create the "start at login" behaviour without us providing an option from within Bitcoin Core.
I remember we used a different way based on scripts in the past. That way was deprecated, too 😠 Although annoying for users which rely on this functionality, I tend to agree it's not worth coming up with a new hack here every time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they still work fine, even on newer MacOS, if compiled with an older MacOS in mind
They work just fine even when compiling for the latest macOS
|
utACK 27d82b6 |
|
Tested ACK 27d82b6 - successfully compiled on 10.15.1 |
27d82b6 gui: remove macOS start on login code (fanquake) Pull request description: The macOS startup item code was disabled for builds targeting macOS > `10.11` in #15208. Now that we require macOS `10.12` as a minimum (#17550), we can remove the startup item code entirely. The API we were using, `LSSharedFileListItemCopyResolvedURL`, `LSSharedFileListCopySnapshot` etc, was removed in macOS `10.12` SDK. ACKs for top commit: jonasschnelli: utACK 27d82b6 jonasschnelli: Tested ACK 27d82b6 - successfully compiled on 10.15.1 Tree-SHA512: 7420757b91c7820e6a63280887155394547134a9cebcf3721af0284da23292627f94cd431241e033075b3fd86d79ace3ebf1b25d17763acbf71e07a742395409
27d82b6 gui: remove macOS start on login code (fanquake) Pull request description: The macOS startup item code was disabled for builds targeting macOS > `10.11` in bitcoin#15208. Now that we require macOS `10.12` as a minimum (bitcoin#17550), we can remove the startup item code entirely. The API we were using, `LSSharedFileListItemCopyResolvedURL`, `LSSharedFileListCopySnapshot` etc, was removed in macOS `10.12` SDK. ACKs for top commit: jonasschnelli: utACK 27d82b6 jonasschnelli: Tested ACK 27d82b6 - successfully compiled on 10.15.1 Tree-SHA512: 7420757b91c7820e6a63280887155394547134a9cebcf3721af0284da23292627f94cd431241e033075b3fd86d79ace3ebf1b25d17763acbf71e07a742395409
Summary: > The macOS startup item code was disabled for builds targeting macOS > > 10.11 in #15208. Now that we require macOS 10.12 as a minimum, #17550, > we can remove the startup item code entirely, as the API we were using > was removed in macOS 10.12. We have `OSX_MIN_VERSION=10.12` set in `depends/hosts/darwin.mk` This is a backport of [[bitcoin/bitcoin#15208 | PR15208]] and [[bitcoin/bitcoin#17567 | PR17567]] All code modified in `src/qt/guituil.cpp` in [[bitcoin/bitcoin#15208 | PR15208]] is removed in [[bitcoin/bitcoin#17567 | PR17567]], including everything related to *fix memory missmanagement*. Test Plan: I do not have a Mac to test this, so I rely on CI cross compilation tests. `ninja all check-all` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D8280
27d82b6 gui: remove macOS start on login code (fanquake) Pull request description: The macOS startup item code was disabled for builds targeting macOS > `10.11` in bitcoin#15208. Now that we require macOS `10.12` as a minimum (bitcoin#17550), we can remove the startup item code entirely. The API we were using, `LSSharedFileListItemCopyResolvedURL`, `LSSharedFileListCopySnapshot` etc, was removed in macOS `10.12` SDK. ACKs for top commit: jonasschnelli: utACK 27d82b6 jonasschnelli: Tested ACK 27d82b6 - successfully compiled on 10.15.1 Tree-SHA512: 7420757b91c7820e6a63280887155394547134a9cebcf3721af0284da23292627f94cd431241e033075b3fd86d79ace3ebf1b25d17763acbf71e07a742395409
The macOS startup item code was disabled for builds targeting macOS >
10.11in #15208. Now that we require macOS10.12as a minimum (#17550),we can remove the startup item code entirely. The API we were using,
LSSharedFileListItemCopyResolvedURL,LSSharedFileListCopySnapshotetc,was removed in macOS
10.12SDK.