release: require macOS 10.10+#13617
Conversation
|
0f57c60 could be a scripted-diff |
|
Concept ACK |
src/util.cpp
Outdated
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.
depends/README.md
Outdated
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. |
jonasschnelli
left a comment
jonasschnelli
left a comment
There was a problem hiding this comment.
Concept ACK.
Changes look good.
Does someone has a 10.10 build environment? I guess would be great to test on 10.9 (failcase-test) and on 10.10.
src/qt/guiutil.h
Outdated
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
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):
|
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 |
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`
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`
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
10 participants
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.