Skip to content

Conversation

@fanquake
Copy link
Member

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.

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.
@laanwj
Copy link
Member

laanwj commented Nov 23, 2019

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.
Copy link
Member

@laanwj laanwj Nov 23, 2019

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).

Copy link
Member Author

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).

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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.

Copy link

@DimitarNestorov DimitarNestorov Apr 12, 2020

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

@jonasschnelli
Copy link
Contributor

utACK 27d82b6

@jonasschnelli
Copy link
Contributor

Tested ACK 27d82b6 - successfully compiled on 10.15.1

fanquake added a commit that referenced this pull request Nov 26, 2019
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
@fanquake fanquake merged commit 27d82b6 into bitcoin:master Nov 26, 2019
@fanquake fanquake deleted the remove_macos_startup_item branch November 26, 2019 16:02
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 27, 2019
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
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 5, 2020
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
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
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
kwvg added a commit to kwvg/dash that referenced this pull request Aug 26, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 30, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 31, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants