bitcoin-core bundles leveldb

Hi,

This old bug#34170 [1] provides only the ’Subject’ as elements.

The package bitcore-core still provides ’leveldb’:

and I am not sure to get what is the issue. Some explanations?

1: http://issues.guix.gnu.org/issue/34170

Cheers,

simon

Hi,

On Fri, 03 Dec 2021 at 11:29, zimoun <zimon.toutoune@gmail.com> wrote:

I am closing. Feel free to reopen if I miss something.

Cheers,

simon

zimoun schreef op vr 03-12-2021 om 11:29 [+0100]:

The issue is that bitcoin-core bundles leveldb (which you have shown

is still the case), even though leveldb is packaged in Guix. See, e.g.

‘(guix)Snippets versus Phases’ and the following from ‘(guix)Submitting

Patches’:

7. Make sure the package does not use bundled copies of software

already available as separate packages.

Sometimes, packages include copies of the source code of their

dependencies as a convenience for users. However, as a

distribution, we want to make sure that such packages end up using

the copy we already have in the distribution, if there is one.

This improves resource usage (the dependency is built and stored

only once), and allows the distribution to make transverse changes

such as applying security updates for a given software package in a

single place and have them affect the whole system—something that

bundled copies prevent.

Greetings,

Maxime.

Hi Maxime,

On Wed, 05 Jan 2022 at 10:39, Maxime Devos <maximedevos@telenet.be> wrote:

Thanks, I missed that ’leveldb’ is packaged in Guix. Indeed, it should

preferably be used.

So reopen. :-)

Cheers,

simon

Simon, Maxime, Danny,

Thanks for CCing me on this message! The rationale for bundling leveldb in Bitcoin Core goes a bit beyond convenience, it is several things:

1. The original reason for sub-treeing is that Bitcoin Core used to maintain its own version of leveldb with its own fixes here: https://github.com/bitcoin-core/leveldb-subtree https://github.com/bitcoin-core/leveldb-subtree , since then most of these fixes have been upstreamed as of: https://github.com/bitcoin/bitcoin/pull/17398 https://github.com/bitcoin/bitcoin/pull/17398

2. We also used to support using an external leveldb, however, it seems that it was fragile to rely on external projects to maintain ABI compatibility, see the quoted IRC bug report here: https://github.com/bitcoin/bitcoin/pull/23282 https://github.com/bitcoin/bitcoin/pull/23282 . Reasonable minds may disagree on this point, especially coming from Guix where patching is convenient.

In addition to the above, Bitcoin Core experienced a hard fork in 2013 due to database incompatibilities, which has predisposed maintainers towards a more stringent approach with pinning dependencies and their configure/build-time flags. See: https://blog.bitmex.com/bitcoins-consensus-forks/#was-the-2013-incident-a-hardfork https://blog.bitmex.com/bitcoins-consensus-forks/#was-the-2013-incident-a-hardfork

Let me know if I can provide more context!

Cheers,

Carl Dong

Hi,

Carl Dong schreef op wo 05-01-2022 om 12:40 [-0500]:

Seems reasonable to me, but the bitcoin project is upstreaming the

changes it made and most are already upstream, so I would prefer

to use upstream's leveldb.

The quoted ABI incompatibility

<Talkless> bitcoind fails to start with undefined symbol:

_ZTIN7leveldb6LoggerE in Debian Sid after leveldb upgraded from 1.22 to

1.23: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996486

doesn't apply in Guix, because guix uses RUNPATH and multiple library

versions can exist in the same store (in different directories in the

store).

I doubt that Guix has sufficient Bitcoin Core users to cause

a hard fork, but yes, this is an understandable reason to bundle

things. But any such problem seems easy to resolve (at the guix side):

we could simply temporarily switch to an older version of leveldb.

If desired, it would also be possible to do something in-between

unbundling and using bitcoin's leveldb: define a 'leveldb/bitcoin'

variant of the 'leveldb' package (using package/inherit or (package

(inherit ...) ...)), add it as input to the 'bitcoin' package and tell

and/or patch bitcoin's buid scripts to use that leveldb.

As source code, use an appropriate commit from

https://github.com/bitcoin-core/leveldb-subtree (and add a comment

to the definition of bitcoin-core to keep leveldb/bitcoin in-sync).

A benefit of this approach (if done properly, with (origin (inherit

...) ...) such that patches of 'leveldb' are inherited) above the

status quo, is that is that if for some reason 'leveldb' is patched in

Guix, then 'leveldb/bitcoin' receives the patch as well. Another

benefit is that the dependency 'googletest' and 'benchmark' of leveldb

would remain unbundled.

Greetings,

Maxime.

Yes I think that would be a splendid idea! With regards to patching bitcoin’s builds scripts, Bitcoin Knots follows Bitcoin Core closely, but has a bunch of patches which allow for using system libs, so that might be good to reference: https://github.com/bitcoin/bitcoin/compare/master...bitcoinknots:21.x-syslibs https://github.com/bitcoin/bitcoin/compare/master...bitcoinknots:21.x-syslibs

FYI, according to https://github.com/bitcoin/bitcoin/pull/17398 https://github.com/bitcoin/bitcoin/pull/17398 , we are currently using the upstream LevelDB commit 0c40829872a9f00f38e11dc370ff8adb3e19f25b

Cheers,

Carl Dong

Hi Carl,

Carl Dong <contact@carldong.me> writes:

Would you like to give the idea discussed above a try and send a patch?

That'd bring closure to this old issue :-).

Thanks,

Maxim