Non-recursive make #4241

pull theuni wants to merge 8 commits into bitcoin:master from theuni:non-recursive-make changing 13 files +639 −607
  1. theuni commented at 6:35 PM on May 28, 2014: member

    I decided to take care of this before handling the libsecp256k1 changes, because this should end up making future work much less tangled.

    The point of this work is to avoid having separate logical builds for each directory. That causes complications because one target cannot know about changes in another directory. For example, if bitcoin/net.h changes, the qt target does not know that, and will continue to happily build/link against the stale intermediate libraries.

    With a non-recursive system, however, each target is aware of all of its own components. In the example above, changing src/net.h would force a rebuild of all files that include that header, regardless of where they're located. Non-recursive also means that lots of spaghetti can go away, because we can include any dependency anywhere we want.

    It also helps with parallelism because multiple targets can be compiling at once. A quick test shows that a fully ccached make -j8 of src/ (eliminating compiler time) is reduced from 20.4 sec to 13.7 sec. A make -j8 without ccache is reduced from 73.6 sec to 62.1.

    Good/Simple description of the difference here: https://www.flameeyes.eu/autotools-mythbuster/automake/nonrecursive.html

    Build logic moves from individual Makefile.am's to include files, which the main src/Makefile.am includes. This avoids having to manage a gigantic single Makefile.

    Stub Makefiles have been added to imitate previous behavior, for those who may have gotten used to doing a make or clean in specific subdirs. These can be expanded as desired, they're very simple for now.

    Most deviations from the original Makefile.am's involve eliminating $(BUILT_SOURCES), so that generated files are created as late as possible, and without requiring a vanilla 'make' (without a target) as they do currently.

  2. theuni commented at 6:38 PM on May 28, 2014: member

    @jgarzik You originally nack'd non-recursive, but I think your opinion has shifted since. You've got right of first refusal :)

  3. gavinandresen commented at 6:50 PM on May 28, 2014: contributor

    Untested ACK.

    Build-system changes like this shouldn't require extensive review; if the pull-tester is happy, I say pull it and do the final changes/tweaks as future pull requests.

  4. laanwj commented at 7:39 PM on May 28, 2014: member

    Build works fine for me.

    I tried to reproduce #3955:

    $ touch src/checkpoints.cpp
    $ make src/bitcoind
    make: `src/bitcoind' is up to date.
    

    Seems it doesn't detect the change in another directory even with the non-recursive make.

  5. theuni commented at 8:26 PM on May 28, 2014: member

    The top-level is disconnected from the others, that hasn't changed. You're looking for: make -C src bitcoind.

    I could move it all up a dir so that would work, but I'm not sure there's much to gain from that?

  6. sipa commented at 9:53 PM on May 28, 2014: member

    Don't care much, but changes look fine to me.

  7. theuni closed this on May 28, 2014

  8. theuni reopened this on May 28, 2014

  9. theuni commented at 11:22 PM on May 28, 2014: member

    Whoops, didn't mean to close.

  10. laanwj commented at 6:00 AM on May 29, 2014: member

    @theuni I don't mind, but I'd like to be able to close that issue and others that claim 'the build system is broken!&!!'. Thanks for explaining.

    ACK in any case as this speeds up the build.

  11. jgarzik commented at 7:06 PM on June 4, 2014: contributor

    No objection if others want it.

  12. build: Switch to non-recursive make
    Build logic moves from individual Makefile.am's to include files, which
    the main src/Makefile.am includes. This avoids having to manage a gigantic
    single Makefile.
    
    TODO: Move the rules from the old Makefile.include to where they actually
    belong and nuke the old file.
    65e8ba4dbe
  13. build: delete old Makefile.am's be4e9aeb14
  14. build: add stub makefiles for easier subdir builds 8b09ef7b63
  15. build: nuke Makefile.include from orbit
    Rules and targets no longer need to be shared between subdirectories, so
    this is no longer needed.
    6b9f0d5554
  16. build: Tidy up file generation output
    - Some file generation was still noisy, silence it.
    - AM_V_GEN is used rather than @ so that 'make V=1' works as intended
    - Cut down on file copies and moves when using sed, use pipes instead
    - Avoid the use of top_ and abs_ dirs where possible
    70c71c50ce
  17. build: avoid the use of top_ and abs_ dir paths
    Using them has the side effect of confusing the dependency-tracking logic.
    56c157d5e0
  18. build: quit abusing AM_CPPFLAGS
    Now that the build is non-recursive, adding to AM_CPPFLAGS means adding to
    _all_ cppflags.
    
    Logical groups of includes have been added instead, and are used individually
    by various targets.
    f4d81129f0
  19. build: fix version dependency efe6888407
  20. theuni renamed this:
    RFC: Non-recursive make
    Non-recursive make
    on Jun 5, 2014
  21. theuni commented at 8:18 PM on June 5, 2014: member

    Pushed a cleaned up and rebased version with some additional changes. On @gavinandresen's advise, I'm not going to bother breaking the move from .am's to .include's into smaller chunks for the sake of easier review. It's a pretty straightforward change.

    Now that we're non-recursive, things like AM_CPPFLAGS become global to all builds. I've removed our usage of those globals in favor of setting the flags per-target.

    Also (mainly to help in my testing of the changes) I've silenced the qt build spew. Now, the usual make V=1 shows the full output as would be expected.

    As discussed with @laanwj yesterday: This does not address the issue in #3955, but it doesn't regress it eaither. That can be addressed as a follow-up.

    I've removed the RFC description as (if pull-tester is happy) I believe this is now merge-ready.

  22. BitcoinPullTester commented at 8:24 PM on June 5, 2014: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/efe6888407186b8a468f357fa7084c0a8c3de503 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  23. laanwj commented at 5:57 AM on June 6, 2014: member

    Pulltester is happy. I've done some testing as well

    • Normal build: OK
    • Disablewallet build: OK
    • -DDEBUG_LOCKORDER build: OK

    Note that before pulling you need to remove the old generated Makefles, as otherwise you get a file collision error.

    rm src/test/Makefile src/qt/Makefile src/qt/test/Makefile
    

    (or alternatively, git clean -f -x -d to return to a clean working directory)

  24. laanwj merged this on Jun 6, 2014
  25. laanwj closed this on Jun 6, 2014

  26. laanwj referenced this in commit 71c0e80e7e on Jun 6, 2014
  27. sipa commented at 11:25 AM on June 6, 2014: member

    Doing make -j2 check in src/:

    It seems that when descending into make check-TESTS, all libbitcoin_server and libbitcoin_common objects are rebuilt (very fast, probably ccache'd), but still... I'd expect the makefiles to only build them once?

  28. pull[bot] referenced this in commit 05623c0216 on Jul 8, 2019
  29. jasonbcox referenced this in commit 3bee3612dd on Oct 9, 2020
  30. laanwj referenced this in commit 1591e35049 on May 10, 2021
  31. DrahtBot locked this on Sep 8, 2021

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: 2026-04-17 06:15 UTC

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