release: require macOS 10.10+ #13617

pull fanquake wants to merge 6 commits into bitcoin:master from fanquake:macos-10-10 changing 13 files +14 −73
  1. fanquake commented at 2:54 am on July 9, 2018: member

    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+):

    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.

  2. fanquake added the label macOS on Jul 9, 2018
  3. fanquake added the label Build system on Jul 9, 2018
  4. fanquake added this to the milestone 0.17.0 on Jul 9, 2018
  5. fanquake requested review from jonasschnelli on Jul 9, 2018
  6. fanquake requested review from theuni on Jul 9, 2018
  7. Empact commented at 4:30 am on July 9, 2018: member
    0f57c60 could be a scripted-diff
  8. practicalswift commented at 1:08 pm on July 9, 2018: contributor
    Concept ACK
  9. in src/util.cpp:722 in da6db21492 outdated 
    718@@ -719,8 +719,8 @@ fs::path GetDefaultDataDir()
719         pathRet = fs::path("/");
720     else
721         pathRet = fs::path(pszHome);
722-#ifdef MAC_OSX
723-    // Mac
724+#ifdef __APPLE__
    theuni commented at 7:42 pm on July 9, 2018:
    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.
    jonasschnelli commented at 8:30 pm on July 10, 2018:
    Agree with @theuni
    fanquake commented at 8:15 am on July 11, 2018:
    I’ve dropped this change and replaced it with a scripted-diff that converts APPLE usage to MAC_OSX.
  10. in depends/README.md:25 in 8f14bc47d4 outdated 
    21@@ -22,7 +22,7 @@ Common `host-platform-triplets` for cross compilation are:
22 
23 - `i686-w64-mingw32` for Win32
24 - `x86_64-w64-mingw32` for Win64
25-- `x86_64-apple-darwin11` for macOS
26+- `x86_64-apple-darwin14` for macOS
    theuni commented at 7:45 pm on July 9, 2018:
    Does this change anything?
    fanquake commented at 2:24 am on July 10, 2018:
    @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 ?
    MarcoFalke commented at 5:02 pm on July 10, 2018:
    Huh, how does this even compile on travis/gitian, given that support for 10.7 has been removed?
    theuni commented at 6:04 pm on July 10, 2018:
    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.
    fanquake commented at 8:09 am on July 11, 2018:
    Updated in travis.yml and gitian-osx.yml
  11. DrahtBot commented at 11:09 pm on July 9, 2018: member
    • #13755 ([WIP] Full unicode support on Windows by ken2812221)
    • #13426 ([bugfix] Add u8path and u8string to fix encoding issue for Windows by ken2812221)

    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.

  12. in src/qt/guiutil.h:236 in da6db21492 outdated 
    229@@ -233,19 +230,7 @@ namespace GUIUtil
230         void mouseReleaseEvent(QMouseEvent *event);
231     };
232 
233-#if defined(Q_OS_MAC)
    jonasschnelli commented at 8:30 pm on July 10, 2018:
    Has someone verified that this BUG no longer appears on OSX 10.10+?
    fanquake commented at 8:09 am on July 11, 2018:
    @jonasschnelli I haven’t been able to recreate it yet, will test further though.
    kallewoof commented at 10:00 am on July 25, 2018:
    The new URL for the bug appears to be https://bugreports.qt.io/browse/QTBUG-15631 but that bug seems unrelated to the comment.
    fanquake commented at 10:01 am on July 25, 2018:
    @kallewoof I mentioned in the PR description, from what I can tell the correct link is https://bugreports.qt.io/browse/QTBUG-20880
  13. jonasschnelli commented at 8:31 pm on July 10, 2018: contributor
    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.
  14. fanquake force-pushed on Jul 11, 2018
  15. fanquake force-pushed on Jul 11, 2018
  16. MarcoFalke commented at 3:55 pm on July 11, 2018: member
    Appears to time out, but builds on my travis when merged with #13634
  17. laanwj removed this from the milestone 0.17.0 on Jul 19, 2018
  18. laanwj added this to the milestone 0.17.0 on Jul 19, 2018
  19. fanquake force-pushed on Jul 22, 2018
  20. fanquake renamed this:
    [WIP] release: require macOS 10.10+
    release: require macOS 10.10+
    on Jul 22, 2018
  21. fanquake commented at 4:27 am on July 22, 2018: member
    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.
  22. MarcoFalke commented at 12:43 pm on July 22, 2018: member
    Should the darwin11 also be updated when make download-osx in depends?
  23. Sjors commented at 2:46 pm on July 24, 2018: member

    tACK 5eb615c, modulo #13752 and the depends/Makefile darwin11 reference @MarcoFalke pointed out.

    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.

  24. theuni commented at 8:13 pm on July 24, 2018: member
    utACK after the Makefile change.
  25. depends: set OSX_MIN_VERSION to 10.10 26b15df99d
  26. release: bump minimum required macOS to 10.10 84b0cfa8b6
  27. doc: mention that macOS 10.10 is now required 6c6dbd8af5
  28. gui: remove SubstituteFonts 68c272527f
  29. gui: remove macOS ProgressBar workaround fa6e841e89
  30. scripted-diff: prefer MAC_OSX over __APPLE__ 
    -BEGIN VERIFY SCRIPT-
sed -i 's/__APPLE__/MAC_OSX/g' src/compat/byteswap.h src/util.cpp
-END VERIFY SCRIPT-
    3828a79711
  31. fanquake force-pushed on Jul 24, 2018
  32. fanquake commented at 11:36 pm on July 24, 2018: member
    Added the makefile change to 26b15df
  33. MarcoFalke added the label Needs gitian build on Jul 24, 2018
  34. DrahtBot commented at 4:21 am on July 25, 2018: member

    Gitian builds for commit 1211b15bf6c0b2904d90b96a9b3834c5cb9e7b4e (master):

    Gitian builds for commit 217e2a1720aed4b07d19c88027a15a104c3b0de9 (master and this pull):

  35. DrahtBot removed the label Needs gitian build on Jul 25, 2018
  36. kallewoof commented at 10:02 am on July 25, 2018: member

    utACK 3828a79

    I assume the __APPLE__ define is still available, as it is still used in

    https://github.com/bitcoin/bitcoin/blob/1211b15bf6c0b2904d90b96a9b3834c5cb9e7b4e/src/tinyformat.h#L157

  37. MarcoFalke merged this on Jul 25, 2018
  38. MarcoFalke closed this on Jul 25, 2018
  39. MarcoFalke referenced this in commit cf7f9ae34e on Jul 25, 2018
  40. fanquake deleted the branch on Aug 7, 2018
  41. Sjors commented at 5:13 pm on August 28, 2018: member

    @fanquake what was the rationale for using MAC_OSX? I can’t find any documentation about it.

    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

  42. zkbot referenced this in commit 2a39656e6d on Jul 30, 2020
  43. zkbot referenced this in commit e0692ed4df on Aug 7, 2020
  44. UdjinM6 referenced this in commit 39ff085409 on Dec 17, 2020
  45. CryptoCentric referenced this in commit 99e83cf056 on Jul 2, 2021
  46. DrahtBot locked this on Sep 8, 2021


fanquake Empact practicalswift theuni jonasschnelli MarcoFalke DrahtBot kallewoof Sjors

Labels
macOS Build system

Milestone
0.17.0

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-29 07:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me