build: prune Boost headers in depends #24742

pull fanquake wants to merge 3 commits into bitcoin:master from fanquake:prune_boost changing 2 files +102 −4
  1. fanquake commented at 7:24 pm on April 2, 2022: member

    The Boost 1.81.0 tarball is ~118mb, and expands to much larger than that, however we end up with ~150mb of headers copied into the /include/boost dir in depends. This is a lot by itself, and even more when it’s 170mb * 9 (HOSTS for a guix build).

    With the changes in this PR, we end up with ~50mb of Boost headers in depends, which with some creative patching/pruning, could be trimmed even further. i.e sometimes you end up pulling in an entire boost module, because of a single include in another header we use, but in code that we don’t actually need. In other cases there are deprecated headers which are still being used, which could be removed if the modules we care about stopped using them. I will open some PRs upstream to try and improve that situation, ie: https://github.com/boostorg/multi_index/pull/57.

  2. fanquake added the label Brainstorming on Apr 2, 2022
  3. fanquake added the label Build system on Apr 2, 2022
  4. hebasto commented at 5:28 am on April 3, 2022: member
    Concept ACK.
  5. DrahtBot commented at 6:44 am on April 3, 2022: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK hebasto, laanwj

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    No conflicts as of last run.

  6. DrahtBot added the label Needs rebase on Apr 4, 2022
  7. laanwj commented at 1:53 pm on April 4, 2022: member

    Concept ACK. I’d slightly prefer a full list of accepted headers in a separate file, instead of a list of commands in the makefile having -r directives which could come to include more files if boost changes.

    Or is this very long or unwieldy? 33mb of headers still sounds insane to me.

  8. fanquake force-pushed on Apr 5, 2022
  9. fanquake commented at 2:07 pm on April 5, 2022: member

    I’d slightly prefer a full list of accepted headers in a separate file, instead of a list of commands in the makefile having -r directives which could come to include more files if boost changes.

    Or is this very long or unwieldy? 33mb of headers still sounds insane to me.

    There doesn’t seem to be a super straight forward approach to doing this, hence why I opened it up for some brainstorming.

    One alternative is instead of trying to copy what we need, we just prune what we know we definitely don’t need. I played around with this, and got the headers down to around 70mb, which is still much better than 170. This approach is also simpler code-wise, and probably easier to maintain across different HOSTS. There is still the potential of Boost introducing more cross-module dependency going forward, but I think the risk of that being a problem is low.

  10. laanwj commented at 2:10 pm on April 5, 2022: member

    One alternative is instead of trying to copy what we need, we just prune what we know we definitely don’t need.

    I definitely like the white-list approach is better. It gives an exact overview of what we need. And might, at some point, when whittled down enough, include in the repository.

    Agree the other way around is bound to be easier.

    There is still the potential of Boost introducing more cross-module dependency going forward

    Yes, whatever you do, this is something new that needs to be maintained when the boost version in depends changes.

  11. DrahtBot removed the label Needs rebase on Apr 5, 2022
  12. DrahtBot added the label Needs rebase on Apr 6, 2022
  13. fanquake force-pushed on Apr 8, 2022
  14. fanquake force-pushed on Apr 8, 2022
  15. DrahtBot removed the label Needs rebase on Apr 8, 2022
  16. fanquake force-pushed on Apr 8, 2022
  17. fanquake force-pushed on Apr 8, 2022
  18. fanquake force-pushed on Apr 8, 2022
  19. fanquake force-pushed on Apr 8, 2022
  20. fanquake force-pushed on Apr 8, 2022
  21. fanquake force-pushed on Apr 8, 2022
  22. fanquake force-pushed on Apr 8, 2022
  23. fanquake force-pushed on Apr 8, 2022
  24. fanquake force-pushed on Apr 9, 2022
  25. fanquake commented at 7:17 pm on April 9, 2022: member

    Sorry for all the force push noise, this now (CI passing) seems to be in a working state for all HOSTS. Boost Process is a mess, and is the primary reason for dragging in a huge amount of extra crap that we certainly don’t otherwise use; i.e filesystem, system, asio, fusion, winapi + more.

    I’ve opened a few more minor PR’s upstream to remove some deprecated header usage from Boost modules. Will run a Guix build shortly.

  26. jarolrod commented at 0:02 am on April 11, 2022: member

    GUIX hashes, mine match @fanquake

     0$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
     1
     246cf751df6fa97b0ebf30d4004af8860d23b33ce9545efdf2b8887bf7211e03d  guix-build-b58baa4fc174/output/aarch64-linux-gnu/SHA256SUMS.part
     3a5dd51b0444502cb49f2fd01fcbd1d446df5551cb6123c8a2472a50f11a4ff37  guix-build-b58baa4fc174/output/aarch64-linux-gnu/bitcoin-b58baa4fc174-aarch64-linux-gnu-debug.tar.gz
     41629cf1426f6d2f63dd1ce82106acc5f3db57c9e447afd8ec92bbe2e074208da  guix-build-b58baa4fc174/output/aarch64-linux-gnu/bitcoin-b58baa4fc174-aarch64-linux-gnu.tar.gz
     540e18f7bcdca4c70317fb33e60f9c8bbf0ade0cfbb4b24fbd029673942efa073  guix-build-b58baa4fc174/output/arm-linux-gnueabihf/SHA256SUMS.part
     6541248ef1c544dc3191ae2c2040161379391c63ea16796a7f29726538fdf6192  guix-build-b58baa4fc174/output/arm-linux-gnueabihf/bitcoin-b58baa4fc174-arm-linux-gnueabihf-debug.tar.gz
     777d87510676bf73a613ca796f905fd531b878ff2ead04450cb4881991021bd12  guix-build-b58baa4fc174/output/arm-linux-gnueabihf/bitcoin-b58baa4fc174-arm-linux-gnueabihf.tar.gz
     85d9d47ead9e2668ce2ac6f22f7ebe6115d0def77a77bd9fa9a5575b25516e0a0  guix-build-b58baa4fc174/output/arm64-apple-darwin/SHA256SUMS.part
     9afaf25547be9afa45dfd7a683ed8da4fa5c82eb2a8d0debda295bb7b7b732786  guix-build-b58baa4fc174/output/arm64-apple-darwin/bitcoin-b58baa4fc174-arm64-apple-darwin-unsigned.dmg
    10a1f907703199e56160f5893a7bf4821433e232ecbddc1a2a984ff4760c3c69b6  guix-build-b58baa4fc174/output/arm64-apple-darwin/bitcoin-b58baa4fc174-arm64-apple-darwin-unsigned.tar.gz
    1188f193122b439b7100144de8147799a04ccdbe43763386f3aeb28797a087834d  guix-build-b58baa4fc174/output/arm64-apple-darwin/bitcoin-b58baa4fc174-arm64-apple-darwin.tar.gz
    125e1e6830764108952b20ea4610b1024ed50e95ebb566c48e7f2425536ad09dd0  guix-build-b58baa4fc174/output/dist-archive/bitcoin-b58baa4fc174.tar.gz
    131e3754946815a85a5c4056184dce1ba9bd3f963fbc0cda0361100a46bd0f938d  guix-build-b58baa4fc174/output/powerpc64-linux-gnu/SHA256SUMS.part
    1475a860707324864d026b8fe9acc63cfb6234cb8c6dfc91b6b534098d58fd210f  guix-build-b58baa4fc174/output/powerpc64-linux-gnu/bitcoin-b58baa4fc174-powerpc64-linux-gnu-debug.tar.gz
    1545e5c819b7fb01b27fa8a4140a5ec8a1459fe236da25dd0e29468b7be1b584da  guix-build-b58baa4fc174/output/powerpc64-linux-gnu/bitcoin-b58baa4fc174-powerpc64-linux-gnu.tar.gz
    164914872d4abe7f1d47e0671a7c9da0a4489287afcdf470d1fbe2a82b075a4c1e  guix-build-b58baa4fc174/output/powerpc64le-linux-gnu/SHA256SUMS.part
    174df7b855cb0bcc811aec8d268f92e4c6f514fa19a007c09885284cbf41514dfa  guix-build-b58baa4fc174/output/powerpc64le-linux-gnu/bitcoin-b58baa4fc174-powerpc64le-linux-gnu-debug.tar.gz
    18d840a42585565fe918d515f1f241bfc67995f7233c399947996582de3fe09c77  guix-build-b58baa4fc174/output/powerpc64le-linux-gnu/bitcoin-b58baa4fc174-powerpc64le-linux-gnu.tar.gz
    19c4c7078cc03b7028f8f1c3dafa0b74bf46a6ab31965b42194e12136ddc32dbf7  guix-build-b58baa4fc174/output/riscv64-linux-gnu/SHA256SUMS.part
    207d27f30f6182602b369fed813f9792b04ca3e27a9f4358b31f99e801830c798a  guix-build-b58baa4fc174/output/riscv64-linux-gnu/bitcoin-b58baa4fc174-riscv64-linux-gnu-debug.tar.gz
    21619d4a17b7c07021cb657129ee1ddd2596dd734c92f6b268c54452ad7a505610  guix-build-b58baa4fc174/output/riscv64-linux-gnu/bitcoin-b58baa4fc174-riscv64-linux-gnu.tar.gz
    2200339ad6afe4654d62f0d12e2f5107c818aa945871ede0646814c0907427987c  guix-build-b58baa4fc174/output/x86_64-apple-darwin/SHA256SUMS.part
    2368ac374e44dd5472d51f6dd3781b975ac12aa010d110a39565cd7ce2b913ed07  guix-build-b58baa4fc174/output/x86_64-apple-darwin/bitcoin-b58baa4fc174-x86_64-apple-darwin-unsigned.dmg
    2464bd9290fcf52b548c5395c762af27a3b17522eb87fcadcb1b65bcdecd13f635  guix-build-b58baa4fc174/output/x86_64-apple-darwin/bitcoin-b58baa4fc174-x86_64-apple-darwin-unsigned.tar.gz
    2535c4a937c06abbb394bd4ab513379749565abae340062e5fb078caa07d3c4f1e  guix-build-b58baa4fc174/output/x86_64-apple-darwin/bitcoin-b58baa4fc174-x86_64-apple-darwin.tar.gz
    26e4f3a6d318e64fe604945a9d481a072d3c605f3b0e0f01b68f279957d7d6948e  guix-build-b58baa4fc174/output/x86_64-linux-gnu/SHA256SUMS.part
    27b9195e1827127f3f2e0aa5ddca47e663bbebc81bd382418c1ea735bb5be21e44  guix-build-b58baa4fc174/output/x86_64-linux-gnu/bitcoin-b58baa4fc174-x86_64-linux-gnu-debug.tar.gz
    286f25fc9f175e2c8099b800844b5939add2af909199632eeb71cf3a53619f89ea  guix-build-b58baa4fc174/output/x86_64-linux-gnu/bitcoin-b58baa4fc174-x86_64-linux-gnu.tar.gz
    298356d62337c59f5c789091e3dbe2a04d40305f765a7cad0a132d3ceed94afe3f  guix-build-b58baa4fc174/output/x86_64-w64-mingw32/SHA256SUMS.part
    3022d9f0189ab3fcfe32079800e6370983f2928870d710a29a8e40b2b6f3ff545d  guix-build-b58baa4fc174/output/x86_64-w64-mingw32/bitcoin-b58baa4fc174-win64-debug.zip
    316ca3efae06a92e0ebab4331e1b63c9d2b66f2f948b3e6a4355a02a6873d37f4d  guix-build-b58baa4fc174/output/x86_64-w64-mingw32/bitcoin-b58baa4fc174-win64-setup-unsigned.exe
    323565563ef4fe75afb7d6a5db38e39717ec9e1ad9f3c5973d2eaf2380a48436f8  guix-build-b58baa4fc174/output/x86_64-w64-mingw32/bitcoin-b58baa4fc174-win64-unsigned.tar.gz
    33f14f2ef9760f87a9ce33eef7a5eac523c0452293899ea84230fda8a9ad15b40c  guix-build-b58baa4fc174/output/x86_64-w64-mingw32/bitcoin-b58baa4fc174-win64.zip
    
  27. fanquake force-pushed on Apr 26, 2022
  28. fanquake renamed this:
    [POC] build: prune Boost headers in depends
    build: prune Boost headers in depends
    on Apr 26, 2022
  29. fanquake commented at 9:06 am on April 26, 2022: member
    Rebased past #22953. Updated the PR description. Might split out one other related change.
  30. fanquake force-pushed on May 19, 2022
  31. fanquake force-pushed on Jun 1, 2022
  32. fanquake force-pushed on Jun 10, 2022
  33. fanquake force-pushed on Jul 5, 2022
  34. fanquake force-pushed on Jul 17, 2022
  35. fanquake force-pushed on Aug 1, 2022
  36. fanquake force-pushed on Aug 13, 2022
  37. fanquake commented at 1:34 pm on August 13, 2022: member
    Rebased on master, #25803, and a commit from 25696 that updates Boost to 1.80.0. Also updated to remove boost/detail/winapi, which is a duplicate of boost/winapi, and no-longer needed.
  38. fanquake force-pushed on Aug 16, 2022
  39. fanquake force-pushed on Aug 23, 2022
  40. fanquake referenced this in commit 3c537f1cc8 on Sep 21, 2022
  41. fanquake force-pushed on Sep 21, 2022
  42. fanquake commented at 12:33 pm on September 21, 2022: member
    Split (and extended) part of this out into #26148 after discussion with @theuni. Also now rebased for Boost 1.80.0, and based on #25465. Please review either of those two PRs first.
  43. fanquake force-pushed on Sep 21, 2022
  44. fanquake commented at 3:38 pm on September 21, 2022: member
    Updated to include the BOOST_ASIO_STANDALONE commit, and drop some additional dirs from depends, including filesystem.
  45. sidhujag referenced this in commit a7834914f5 on Sep 23, 2022
  46. fanquake force-pushed on Nov 30, 2022
  47. fanquake force-pushed on Dec 8, 2022
  48. DrahtBot added the label Needs rebase on Jan 13, 2023
  49. fanquake force-pushed on Jan 13, 2023
  50. DrahtBot removed the label Needs rebase on Jan 13, 2023
  51. fanquake force-pushed on Feb 2, 2023
  52. fanquake commented at 12:01 pm on February 2, 2023: member
    Rebased for #25465.
  53. fanquake marked this as ready for review on Feb 2, 2023
  54. in depends/packages/boost.mk:104 in c08c0a7524 outdated
    102+  cp boost/utility.hpp $($(package)_staging_prefix_dir)/include/boost && \
    103+  cp -r boost/variant $($(package)_staging_prefix_dir)/include/boost && \
    104+  cp boost/version.hpp $($(package)_staging_prefix_dir)/include/boost && \
    105+  cp boost/visit_each.hpp $($(package)_staging_prefix_dir)/include/boost && \
    106+  cp boost/weak_ptr.hpp $($(package)_staging_prefix_dir)/include/boost && \
    107+  cp -r boost/winapi $($(package)_staging_prefix_dir)/include/boost
    


    maflcko commented at 2:07 pm on February 2, 2023:
    How painful is it to update this when boost changes?

    fanquake commented at 2:45 pm on February 2, 2023:
    It’s not too awful, and should mostly be removing headers (there are already a few we can drop in the next Boost version). Note that the removal of the winapi dir here is because boost ships it twice (one in boost/winapi and another copy in boost/detail/winapi). If we can (hopefully) get rid of external signer there are a large amount of headers we could drop from here.
  55. hebasto commented at 2:41 pm on February 2, 2023: member

    c08c0a7524f4f821a0caa977e3e7324727e475cc

    Changes look correct, but I’m also curios about maintainability

  56. theuni commented at 8:46 pm on February 2, 2023: member

    It’s really nice to have a list of exactly what headers we need, nice work.

    How painful is it to update this when boost changes?

    I’m nervous about this too. Is it possible to group the copies somewhat? Like “these are required for external signer”? Or are they so intertwined that it doesn’t make sense to try?

    I’m also a little nervous about the procedure. When updating to a new boost version, it’s possible that we accidentally forget to ship a new header and it gets picked up from a different version on the local filesystem instead, no? I guess we’d notice that quickly enough, but I don’t love the trial/error requirement for updates.

  57. fanquake commented at 4:35 pm on February 5, 2023: member

    Like “these are required for external signer”? Or are they so intertwined that it doesn’t make sense to try?

    I think it’s a bit too much of a mess. I had considered splitting up our depends boost package into multiple, and having options to turn various bits on and off, but, the overlap in headers, and complication in handling that probably isn’t worth it.

    As mentioned above, I’d much rather us remove these nice-to-have, but no-one really uses (not 0, but in terms of instances of bitcoind, the % of users is close enough to 0) features, which are a blocker for removing/integrating the bare-minium of Boost we need (with a signals2 replacement this isn’t too far-fetched).

    Dependencies like Boost Process remain fairly poorly maintained, in a adhoc fasion, by a single person upstream (very similar to upnpc and libnatpmp), and I think it continues to makes less and less sense for these kinds of dependencies to be a part of this project.

  58. jarolrod commented at 4:42 pm on February 5, 2023: member

    I use external signer

    the % of users is close enough to 0

    There is no evidence for such a claim, aside from an issue wasn’t detected by windows users

  59. fanquake force-pushed on Feb 14, 2023
  60. fanquake commented at 3:24 pm on February 14, 2023: member
    Dropped one additional unused header.
  61. fanquake force-pushed on Mar 3, 2023
  62. DrahtBot added the label Needs rebase on Mar 9, 2023
  63. fanquake force-pushed on Mar 9, 2023
  64. DrahtBot removed the label Needs rebase on Mar 9, 2023
  65. depends: Boost 1.82.0
    https://www.boost.org/users/history/version_1_82_0.html
    2f4cbb98ab
  66. build: disable Boost user config 34274b52b1
  67. build: only copy used Boost headers in depends
    Something similar was initially suggested in #24301.
    e3b3138410
  68. fanquake force-pushed on Apr 17, 2023
  69. fanquake commented at 10:27 am on April 17, 2023: member
    Added a commit to make this based on Boost 1.82.0.
  70. DrahtBot added the label CI failed on May 31, 2023
  71. DrahtBot removed the label CI failed on May 31, 2023
  72. fanquake commented at 2:08 pm on August 14, 2023: member
    Closing for now.
  73. fanquake closed this on Aug 14, 2023

  74. fanquake deleted the branch on Nov 6, 2023
  75. PastaPastaPasta referenced this in commit c88f3870b1 on Apr 3, 2024
  76. bitcoin locked this on Nov 5, 2024

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 09:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me