build: mac deployment unification #20422

pull fanquake wants to merge 11 commits into bitcoin:master from fanquake:macdeploy_unify changing 7 files +181 −497
  1. fanquake commented at 4:13 am on November 19, 2020: member

    This consolidates our macOS build code so that .DS_Store generation is the same when running make deploy for macOS when building on Linux and macOS, rather than maintaining two version of code that essentially do the same thing (just slightly differently).

    It also removes unused code and any AppleScript usage, automates finding translation files and generally simplifies macdeployqtplus. It also gets rid of the annoying “popping up” behaviour during DMG generation, names the created image Bitcoin-Core.dmg rather than Bitcoin-Qt.dmg.

  2. fanquake added the label macOS on Nov 19, 2020
  3. fanquake added the label Build system on Nov 19, 2020
  4. laanwj commented at 9:27 am on November 19, 2020: member
    Concept ACK
  5. DrahtBot added the label Needs rebase on Nov 19, 2020
  6. DrahtBot commented at 12:19 pm on November 19, 2020: member

    🐙 This pull request conflicts with the target branch and needs rebase.

    Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

  7. fanquake force-pushed on Nov 19, 2020
  8. fanquake removed the label Needs rebase on Nov 19, 2020
  9. laanwj commented at 2:17 pm on November 23, 2020: member

    One outstanding issue is that there is either a bug, or a backwards incompatibility with the latest (2.1.0) version of mac_alias:

    It’s a bug. Looks like they tried to spread the expression over multiple lines but forgot to either add parenthesis around the tuple or \s.

    https://github.com/al45tair/mac_alias/blob/master/mac_alias/alias.py#L282

    Edit: created an upstream issue al45tair/mac_alias#9

  10. laanwj added the label Upstream on Nov 23, 2020
  11. RandyMcMillan commented at 10:55 pm on November 27, 2020: contributor
    Concept ACK!
  12. sipa deleted a comment on Nov 28, 2020
  13. build: automatically determine macOS translations 
    Rather than using OSX_QT_TRANSLATIONS which must be manually updated,
and we forget to update anyway, i.e: #19059, automatically find and copy
available translations from the translations directory.
    4d70d3d7fe
  14. macdeploy: remove codesigning argument 464b34d4c3
  15. macdeploy: remove add-resources argument 827d382aa7
  16. macdeploy: have a single level of logging output 
    4 different levels of verbosity is overkill for a fairly simple script, which
was always being run at 2 in any case.
    0ab4018c12
  17. macdeploy: assume plistlib is available 
    We already require Python 3.5 or later
    32347cd56a
  18. macdeploy: consolidate .DS_Store generation 
    Rather than two lots of logic doing roughly the same thing, dependent on if
you're compiling on Linux or macOS, combine the .DS store generation into
macdeployqtplus.

This also removes the -fancy and -volname options.
    6390a04862
  19. macdeploy: move qt_conf to where it's used ccb0325b1b
  20. macdeploy: remove existing Bitcoin-Core.dmg if present adaa26202b
  21. macdeploy: remove runHDIUtil in favor of directly calling subprocess.run a42aa94c54
  22. macdeploy: use Python 3.6 5d2cbdf772
  23. fanquake force-pushed on Nov 30, 2020
  24. build: mac_alias 2.1.1 b685f60a08
  25. fanquake commented at 7:40 am on November 30, 2020: member

    Edit: created an upstream issue al45tair/mac_alias#9

    Great, thanks. The upstream issue has been resolved with the release of mac_alias 2.1.1. I’ve added an additional commit to this PR to switch to that version in depends. I’ve also updated the PR description to remove no-longer-relevant info.

    This PR should now be ready for review.

  26. in contrib/macdeploy/macdeployqtplus:19 in b685f60a08 
    15@@ -16,9 +16,13 @@
16 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
17 #
18 
19-import subprocess, sys, re, os, shutil, stat, os.path, time
20-from string import Template
21+import plistlib
    dergoegge commented at 2:27 pm on December 2, 2020:
    This import was only used by the -fancy option and could also be removed since it is now unused.
    fanquake commented at 8:51 am on December 8, 2020:
    Good catch. However I’m going to merge this now, and we’ll take care of the leftover import shortly.
  27. dergoegge approved
  28. dergoegge commented at 2:55 pm on December 2, 2020: member
    ACK b685f60a08007e0ae8a5564ee68cd94f9015d899 - Less and cleaner code looks good. I tested this with make deploy and everything still works + the popup during DMG generation is gone.
  29. willcl-ark commented at 3:18 pm on December 2, 2020: member
    utACK. I don’t have an Apple Developer account so can’t get the SDK to test with I’m afraid.
  30. fanquake merged this on Dec 8, 2020
  31. fanquake closed this on Dec 8, 2020
  32. sidhujag referenced this in commit c3efae2c44 on Dec 8, 2020
  33. Sjors commented at 9:43 am on December 10, 2020: member

    Tested (via master) on macOS and it seems to work.

    We should add an instruction before make deploy to install the Python dependency:

    0pip3 install ds_store
  34. fanquake deleted the branch on Feb 9, 2021
  35. fanquake referenced this in commit dca6c90329 on Jun 9, 2021
  36. random-zebra referenced this in commit fbebade1e4 on Jun 11, 2021
  37. fanquake referenced this in commit 4fdd0ff9ee on Jul 20, 2021
  38. josibake referenced this in commit 7cf7fd0749 on Jul 21, 2021
  39. hebasto referenced this in commit 2d5ce10be8 on Jul 22, 2021
  40. sidhujag referenced this in commit 3dd6b1462e on Jul 23, 2021
  41. hebasto commented at 10:13 am on September 14, 2021: member

    It appears this PR breaks make deploy on macOS 10.14.6 (18G9323):

     0$ make deploy
 1...
 2created: /Users/hebasto/bitcoin/Bitcoin-Core.temp.dmg
 3+ Applying fancy settings +
 4+ Finalizing .dmg disk image +
 5"disk23" ejected.
 6hdiutil: convert failed - Resource temporarily unavailable
 7Traceback (most recent call last):
 8  File "/Users/hebasto/bitcoin/./contrib/macdeploy/macdeployqtplus", line 687, in <module>
 9    run(["hdiutil", "convert", tempname, "-format", "UDZO", "-o", appname, "-imagekey", "zlib-level=9"], check=True, universal_newlines=True)
10  File "/usr/local/Cellar/python@3.9/3.9.7/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 528, in run
11    raise CalledProcessError(retcode, process.args,
12subprocess.CalledProcessError: Command '['hdiutil', 'convert', 'Bitcoin-Core.temp.dmg', '-format', 'UDZO', '-o', 'Bitcoin-Core', '-imagekey', 'zlib-level=9']' returned non-zero exit status 1.
13make: *** [Makefile:1282: Bitcoin-Core.dmg] Error 1

    Newer macOS versions—10.15 and 11.x—are not affected.

  42. fanquake commented at 9:40 am on September 16, 2021: member

    hdiutil: convert failed - Resource temporarily unavailable

    This looks like an intermittent issue, and I would be somewhat surprised if the changes here worked on macOS 10.15 but not 10.14. I don’t think there’s anything we’re doing that would be specific to the OS, or a certain version of a tool, like hdiutil etc.

    Does make deploy work on 10.14 with this change reverted? Without any more info there’s not a lot to go on here.

  43. hebasto commented at 9:45 am on September 16, 2021: member

    Does make deploy work on 10.14 with this change reverted?

    Yes, it does.

    I think @jarolrod could also confirm such behavior?

    I would be somewhat surprised if the changes here worked on macOS 10.15 but not 10.14.

    Me too.

  44. janus referenced this in commit d74124b967 on Oct 29, 2021
  45. DrahtBot locked this on Oct 30, 2022


fanquake laanwj DrahtBot RandyMcMillan dergoegge willcl-ark Sjors hebasto

Labels
macOS Build system Upstream

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-11-21 12:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me