release: require macOS 10.10+ #13617
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
|
0f57c60 could be a scripted-diff |
|
Concept ACK |
theuni
reviewed
Jul 9, 2018
src/util.cpp
Outdated
| @@ -719,8 +719,8 @@ fs::path GetDefaultDataDir() | |||
| pathRet = fs::path("/"); | |||
| else | |||
| pathRet = fs::path(pszHome); | |||
| #ifdef MAC_OSX | |||
| // Mac | |||
| #ifdef __APPLE__ | |||
There was a problem hiding this comment.
Do we for sure mean "Apple" in all of these changes and not just "macOS"? This would affect iOS as well, which seems likely incorrect here. I suspect that we want to keep using MAC_OSX (which we define ourselves) so that we can differentiate later if needed.
There was a problem hiding this comment.
I've dropped this change and replaced it with a scripted-diff that converts APPLE usage to MAC_OSX.
theuni
reviewed
Jul 9, 2018
| @@ -22,7 +22,7 @@ Common `host-platform-triplets` for cross compilation are: | |||
|
|
|||
| - `i686-w64-mingw32` for Win32 | |||
| - `x86_64-w64-mingw32` for Win64 | |||
| - `x86_64-apple-darwin11` for macOS | |||
| - `x86_64-apple-darwin14` for macOS | |||
There was a problem hiding this comment.
@theuni In the readme it's cosmetic, darwin11 refers to OS X 10.7, darwin14 should be 10.10.
However good catch, as we should also be changing this in travis.yml and gitian-osx.yml ?
There was a problem hiding this comment.
Huh, how does this even compile on travis/gitian, given that support for 10.7 has been removed?
There was a problem hiding this comment.
I've never seen a case where this changes anything in practice. As a test, I just had a look at gcc's configure.ac though, and it at least checks for >=10 in a few places. So I guess we should keep these in sync to be on the safe side.
There was a problem hiding this comment.
Updated in travis.yml and gitian-osx.yml
Note to reviewers: This pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
| @@ -233,19 +230,7 @@ namespace GUIUtil | |||
| void mouseReleaseEvent(QMouseEvent *event); | |||
| }; | |||
|
|
|||
| #if defined(Q_OS_MAC) | |||
There was a problem hiding this comment.
Has someone verified that this BUG no longer appears on OSX 10.10+?
There was a problem hiding this comment.
@jonasschnelli I haven't been able to recreate it yet, will test further though.
There was a problem hiding this comment.
The new URL for the bug appears to be https://bugreports.qt.io/browse/QTBUG-15631 but that bug seems unrelated to the comment.
There was a problem hiding this comment.
@kallewoof I mentioned in the PR description, from what I can tell the correct link is https://bugreports.qt.io/browse/QTBUG-20880
src/util.cpp
Outdated
| @@ -719,8 +719,8 @@ fs::path GetDefaultDataDir() | |||
| pathRet = fs::path("/"); | |||
| else | |||
| pathRet = fs::path(pszHome); | |||
| #ifdef MAC_OSX | |||
| // Mac | |||
| #ifdef __APPLE__ | |||
fanquake
force-pushed
the
macos-10-10
branch
2 times, most recently
from
eb3a568 to
8fc3d60
Compare
|
Appears to time out, but builds on my travis when merged with #13634 |
fanquake
changed the title
|
Rebased on master. Removed the [wip] tag. There were a couple todos left, but they aren't critical for 0.17.0, and can be done for 0.18.0 alongside PRs like #13561. |
|
Should the darwin11 also be updated when |
|
tACK 5eb615c, modulo #13752 and the I also built depends, ran functional test with that, then ran regular tests without depends (see #13750). I switched GUI to Mandarin, looks nice. I didn't check if the progress bar uses CPU when disconnected. |
|
utACK after the Makefile change. |
-BEGIN VERIFY SCRIPT- sed -i 's/__APPLE__/MAC_OSX/g' src/compat/byteswap.h src/util.cpp -END VERIFY SCRIPT-
|
Added the makefile change to 26b15df |
|
Gitian builds for commit 1211b15 (master):
Gitian builds for commit 217e2a1720aed4b07d19c88027a15a104c3b0de9 (master and this pull):
|
maflcko
pushed a commit
that referenced
this pull request
Jul 25, 2018
3828a79 scripted-diff: prefer MAC_OSX over __APPLE__ (fanquake) fa6e841 gui: remove macOS ProgressBar workaround (fanquake) 68c2725 gui: remove SubstituteFonts (fanquake) 6c6dbd8 doc: mention that macOS 10.10 is now required (fanquake) 84b0cfa release: bump minimum required macOS to 10.10 (fanquake) 26b15df depends: set OSX_MIN_VERSION to 10.10 (fanquake) Pull request description: Closes #13362 d99abfddb0c8f2111340a6127e77cc686e0043d8 This workaround should no longer be required, as it should have only been in use when compiled with the 10.7 SDK, which we haven't been building with for a while now. 5bc5ae30982a0f0f6a9804b05d99434af770c724 The bugreport linked with this code is for an unrelated? issue, however from what I can tell the correct QTBUG is this one https://bugreports.qt.io/browse/QTBUG-20880. Reading though the discussion there, it seems that the way progress bars are animated changed in macOS 10.10. Qt was patched [here (5.5+)](https://codereview.qt-project.org/#/c/112379/): > Disable progress bar animations on 10.10 Yosemite and higher - the native style does not animate them any more. Keep the indeterminate progress bar animation. Given all of that, I don't think this is worth keeping around, as it would seem to only be useful in the case that a macOS user is compiling with a Qt < 5.5. That should be pretty unlikely, as we don't support downloaded Qt binaries, and brew currently provides [5.11.1](https://github.com/Homebrew/homebrew-core/blob/571b46213c70ca1573da6d0425b0bd6df34961ee/Formula/qt.rb). Tree-SHA512: 4278cb30cc9bcb313e166129ecf032c808995f8b51a3123637c47860a0010ac88f86f82ec44792153b6b1e5cca595f25013b2eaeae80194647b9ce4f7eaf32c1
|
@fanquake what was the rationale for using This (fairly old) blog post recommends a different strategy: http://nadeausoftware.com/articles/2012/01/c_c_tip_how_use_compiler_predefined_macros_detect_operating_system#OSXiOSandDarwin |
zkbot
added a commit
to zcash/zcash
that referenced
this pull request
Jul 30, 2020
Modernise macOS cross-compilation toolchain Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#13617 - Excluding the QT GUI changes. - bitcoin/bitcoin#17550 - bitcoin/bitcoin#16392 - Excluding the QT GUI changes. - bitcoin/bitcoin#18589 - bitcoin/bitcoin#19240 - bitcoin/bitcoin#19407 - bitcoin/bitcoin#17919 - Only the ancillary changes, not the `FORCE_USE_SYSTEM_CLANG` change. - bitcoin/bitcoin#19530 After these changes, macOS versions earlier than 10.12 are no longer supported. To cross-compile for macOS: - Follow the instructions in `contrib/macdeploy/README.md` to generate `Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers.tar.gz` (requires an Apple Developer Account) - Extract it into `depends/SDKs` (creating that folder first if it does not exist) - `HOST=x86_64-apple-darwin16 ./zcutil/build.sh`
zkbot
added a commit
to zcash/zcash
that referenced
this pull request
Aug 7, 2020
Modernise macOS cross-compilation toolchain Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#13617 - Excluding the QT GUI changes. - bitcoin/bitcoin#17550 - bitcoin/bitcoin#16392 - Excluding the QT GUI changes. - bitcoin/bitcoin#18589 - bitcoin/bitcoin#19240 - bitcoin/bitcoin#19407 - bitcoin/bitcoin#17919 - Only the ancillary changes, not the `FORCE_USE_SYSTEM_CLANG` change. - bitcoin/bitcoin#19530 After these changes, macOS versions earlier than 10.12 are no longer supported. To cross-compile for macOS: - Follow the instructions in `contrib/macdeploy/README.md` to generate `Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers.tar.gz` (requires an Apple Developer Account) - Extract it into `depends/SDKs` (creating that folder first if it does not exist) - `HOST=x86_64-apple-darwin16 ./zcutil/build.sh`
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Dec 17, 2020
3828a79 scripted-diff: prefer MAC_OSX over __APPLE__ (fanquake) fa6e841 gui: remove macOS ProgressBar workaround (fanquake) 68c2725 gui: remove SubstituteFonts (fanquake) 6c6dbd8 doc: mention that macOS 10.10 is now required (fanquake) 84b0cfa release: bump minimum required macOS to 10.10 (fanquake) 26b15df depends: set OSX_MIN_VERSION to 10.10 (fanquake) Pull request description: Closes bitcoin#13362 d99abfddb0c8f2111340a6127e77cc686e0043d8 This workaround should no longer be required, as it should have only been in use when compiled with the 10.7 SDK, which we haven't been building with for a while now. 5bc5ae30982a0f0f6a9804b05d99434af770c724 The bugreport linked with this code is for an unrelated? issue, however from what I can tell the correct QTBUG is this one https://bugreports.qt.io/browse/QTBUG-20880. Reading though the discussion there, it seems that the way progress bars are animated changed in macOS 10.10. Qt was patched [here (5.5+)](https://codereview.qt-project.org/#/c/112379/): > Disable progress bar animations on 10.10 Yosemite and higher - the native style does not animate them any more. Keep the indeterminate progress bar animation. Given all of that, I don't think this is worth keeping around, as it would seem to only be useful in the case that a macOS user is compiling with a Qt < 5.5. That should be pretty unlikely, as we don't support downloaded Qt binaries, and brew currently provides [5.11.1](https://github.com/Homebrew/homebrew-core/blob/571b46213c70ca1573da6d0425b0bd6df34961ee/Formula/qt.rb). Tree-SHA512: 4278cb30cc9bcb313e166129ecf032c808995f8b51a3123637c47860a0010ac88f86f82ec44792153b6b1e5cca595f25013b2eaeae80194647b9ce4f7eaf32c1
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Jul 2, 2021
3828a79 scripted-diff: prefer MAC_OSX over __APPLE__ (fanquake) fa6e841 gui: remove macOS ProgressBar workaround (fanquake) 68c2725 gui: remove SubstituteFonts (fanquake) 6c6dbd8 doc: mention that macOS 10.10 is now required (fanquake) 84b0cfa release: bump minimum required macOS to 10.10 (fanquake) 26b15df depends: set OSX_MIN_VERSION to 10.10 (fanquake) Pull request description: Closes bitcoin#13362 d99abfddb0c8f2111340a6127e77cc686e0043d8 This workaround should no longer be required, as it should have only been in use when compiled with the 10.7 SDK, which we haven't been building with for a while now. 5bc5ae30982a0f0f6a9804b05d99434af770c724 The bugreport linked with this code is for an unrelated? issue, however from what I can tell the correct QTBUG is this one https://bugreports.qt.io/browse/QTBUG-20880. Reading though the discussion there, it seems that the way progress bars are animated changed in macOS 10.10. Qt was patched [here (5.5+)](https://codereview.qt-project.org/#/c/112379/): > Disable progress bar animations on 10.10 Yosemite and higher - the native style does not animate them any more. Keep the indeterminate progress bar animation. Given all of that, I don't think this is worth keeping around, as it would seem to only be useful in the case that a macOS user is compiling with a Qt < 5.5. That should be pretty unlikely, as we don't support downloaded Qt binaries, and brew currently provides [5.11.1](https://github.com/Homebrew/homebrew-core/blob/571b46213c70ca1573da6d0425b0bd6df34961ee/Formula/qt.rb). Tree-SHA512: 4278cb30cc9bcb313e166129ecf032c808995f8b51a3123637c47860a0010ac88f86f82ec44792153b6b1e5cca595f25013b2eaeae80194647b9ce4f7eaf32c1
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #13362
d99abfddb0c8f2111340a6127e77cc686e0043d8
This workaround should no longer be required, as it should have only been in use when compiled with the 10.7 SDK, which we haven't been building with for a while now.
5bc5ae30982a0f0f6a9804b05d99434af770c724
The bugreport linked with this code is for an unrelated? issue, however from what I can tell the correct QTBUG is this one https://bugreports.qt.io/browse/QTBUG-20880. Reading though the discussion there, it seems that the way progress bars are animated changed in macOS 10.10.
Qt was patched here (5.5+):
Given all of that, I don't think this is worth keeping around, as it would seem to only be useful in the case that a macOS user is compiling with a Qt < 5.5. That should be pretty unlikely, as we don't support downloaded Qt binaries, and brew currently provides 5.11.1.