build: no-longer fail default configure if BDB isn’t available #23168

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:dont_fail_no_bdb_default_configure changing 1 files +14 −5
  1. fanquake commented at 5:13 am on October 4, 2021: member

    Inline with moving to descriptor (sqlite) wallets by default for 0.23, this adapts the build system so that a default ./configure invocation no-longer fails if BDB isn’t present. Currently, if configure is run with no options, and no BDB is present, we’ll fail with:

    0checking for Berkeley DB C++ headers... no
1configure: error: libdb_cxx headers missing, Bitcoin Core requires this library for BDB wallet support (--without-bdb to disable BDB wallet support)

    If descriptor wallets are to be the default, this behaviour no longer makes sense, as a builder should be able to configure and build, to use a wallet, without BDB installed, and without passing additional arguments, i.e --without-bdb or --with-incompatible-bdb, to configure.

    With this change, running configure will no-longer fail, but will instead print:

    0checking for Berkeley DB C++ headers... no
1configure: WARNING: libdb_cxx headers missing
2configure: WARNING: Bitcoin Core requires this library for BDB (legacy) wallet support
3configure: WARNING: Passing --without-bdb will suppress this warning
4checking for sqlite3 >= 3.7.17... yes
5checking whether to build wallet with support for sqlite... yes
  2. fanquake added the label Build system on Oct 4, 2021
  3. jarolrod commented at 5:40 am on October 4, 2021: member
    strong concept ack
  4. MarcoFalke added the label DrahtBot Guix build requested on Oct 4, 2021
  5. practicalswift commented at 8:14 am on October 4, 2021: contributor

    Strong Strong Concept ACK

    This will greatly improve the first time Bitcoin Core build experience by allowing the expected and familiar ./configure && make routine :)

  6. darosior commented at 9:09 am on October 4, 2021: member
    Concept ACK
  7. laanwj commented at 10:37 am on October 4, 2021: member

    What about --with-incompatible-bdb this is hardwired into my fingers now—

    without passing additional arguments, i.e –without-bdb or –with-incompatible-bdb, to configure.

    Thanks :smile:

  8. amadeuszpawlik commented at 4:28 pm on October 4, 2021: contributor
    Very nice!
  9. hebasto commented at 7:20 pm on October 4, 2021: member
    Concept ACK.
  10. hebasto commented at 7:34 pm on October 4, 2021: member

    Maybe also replace an error with a warning and disabling BDB support for the following case:

    0$ ./autogen.sh 
1$ ./configure --quiet
2configure: error: Found Berkeley DB other than 4.8, required for portable BDB wallets (--with-incompatible-bdb to ignore or --without-bdb to disable BDB wallet support)

    ?

  11. fanquake commented at 10:54 pm on October 4, 2021: member

    and disabling BDB support for the following case:

    What do you mean by disabling BDB support? I think this should remain as an error for now.

    On second thought, we probably could make this into a warning, and not build when the incompatible flag is missing. Still forcing builders to opt into using incompatible versions but no-longer failing if some bdb is installed.

  12. DrahtBot commented at 2:53 am on October 5, 2021: member

    Guix builds

    File commit 9e530c6352c3e3d4f2936bbbb1bcb34ff9ca6378(master) commit 07115545e73b77c615399023c000286b55478b21(master and this pull)
    *.tar.gz 1b1aae06add6cb19... 6dfa8b20e03eff49...
    guix_build.log 6b3a2a468bf3c035... a599afdfd0d29c8c...
    guix_build.log.diff 16ebe0b94d003750...
  13. DrahtBot removed the label DrahtBot Guix build requested on Oct 5, 2021
  14. build: no-longer fail default configure if BDB isn't available 
    Inline with moving to descriptor (sqlite) wallets by default for 0.23,
this adapts the build system so that a default `./configure` invocation
no-longer fails if BDB isn't present. Currently, if configure is run
with no options, and no BDB is present, we'll fail with:
```bash
checking for Berkeley DB C++ headers... no
configure: error: libdb_cxx headers missing, Bitcoin Core requires this library for BDB wallet support (--without-bdb to disable BDB wallet support)
```

If descriptor wallets are to be the default, this behaviour no longer
makes sense, as a builder should be able to configure and build, to use
a wallet, without BDB installed, and without passing additional
arguments, i.e `--without-bdb` or `--with-incompatible-bdb`, to
configure.

With this change, running configure will no-longer fail, and will
instead print:
```bash
checking for Berkeley DB C++ headers... no
configure: WARNING: libdb_cxx headers missing
configure: WARNING: Bitcoin Core requires this library for BDB (legacy) wallet support
configure: WARNING: Passing --without-bdb will suppress this warning
checking for sqlite3 >= 3.7.17... yes
checking whether to build wallet with support for sqlite... yes
```
    747cd17404
  15. fanquake force-pushed on Oct 5, 2021
  16. fanquake commented at 3:39 am on October 5, 2021: member

    Maybe also replace an error with a warning and disabling BDB support for the following case:

    I’ve done this now.

  17. in build-aux/m4/bitcoin_find_bdb48.m4:62 in 747cd17404 
    60+        AC_MSG_WARN([BDB (legacy) wallets opened by this build will not be portable!])
61+        use_bdb=yes
62       ],[
63-        AC_MSG_ERROR([Found Berkeley DB other than 4.8, required for portable BDB wallets (--with-incompatible-bdb to ignore or --without-bdb to disable BDB wallet support)])
64+        AC_MSG_WARN([Found Berkeley DB other than 4.8])
65+        AC_MSG_WARN([BDB (legacy) wallets opened by this build would not be portable!])
    hebasto commented at 7:10 pm on October 10, 2021:
    In this case the build won’t support BDB wallets at all (use_bdb=no).
    fanquake commented at 10:54 pm on October 10, 2021:

    Yes, that’s intended. We don’t want to fail in the case –with-incompatible-bdb isn’t passed.

    Isn’t that exactly what you asked for: #23168 (comment)?

    hebasto commented at 10:57 pm on October 10, 2021:

    Isn’t that exactly what you asked for: #23168 (comment)?

    Yes, it is.

    My concerns are about message text, not about logic.

    I mean “BDB (legacy) wallets” cannot be “opened by this build”, no?

    fanquake commented at 11:01 pm on October 10, 2021:
    I think the messaging is fine as is. We are just warning that there is an incompatible BDB present. Lack of BDB wallet support is also part of the configure summary.
  18. hebasto approved
  19. hebasto commented at 11:09 pm on October 10, 2021: member
    ACK 747cd17404832604c50d03d58e11ba816bb229f7, tested on Linux Mint 20.2 (x86_64) with the (un)installed system packages libdb-dev and libdb++-dev.
  20. fanquake merged this on Oct 11, 2021
  21. fanquake closed this on Oct 11, 2021
  22. fanquake deleted the branch on Oct 11, 2021
  23. katesalazar commented at 8:49 am on October 11, 2021: contributor
    Once decided BDB is going out, the earliest people gets the shock about removing it, the better. This is a good idea. 👍
  24. sidhujag referenced this in commit db3af6da6a on Oct 11, 2021
  25. DrahtBot locked this on Oct 30, 2022


fanquake jarolrod practicalswift darosior laanwj amadeuszpawlik hebasto DrahtBot katesalazar

Labels
Build system

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