New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for building against system LevelDB #2241

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@luke-jr
Member

luke-jr commented Jan 30, 2013

Status: Tests succeed, looks complete.

This is based on top of #2243 (LevelDB build bugfixes).

This is necessary for proper downstream distro packaging, and useful for people who have LevelDB installed for other reasons anyway. Undocumented to infer an unsupported status, but I could add a brief blurb if that's desirable. Only implemented for Linux (ie, not makefile.) for now - I assume OSX and Windows won't be packaging LevelDB/Bitcoin themselves anytime soon. Note that this does NOT remove the copied leveldb from the code, and will still build with that by default.

To test with system LevelDB:

  1. Optional: Delete or move src/leveldb so any attempt to use it errors explicitly
  2. Install LevelDB on your system as a shared library
  3. qmake bitcoin-qt.pro USE_SYSTEM_LEVELDB=1 && make
  4. cd src
  5. make -f makefile.unix USE_SYSTEM_LEVELDB=1 bitcoind
  6. make -f makefile.unix USE_SYSTEM_LEVELDB=1 test_bitcoin

Step 6 (test_bitcoin) will fail if your system LevelDB does not support memenv. This is expected behaviour.

To test included LevelDB still works:

  1. Restore src/leveldb
  2. Remove system LevelDB library/package
  3. qmake bitcoin-qt.pro && make
  4. cd src
  5. make -f makefile.unix bitcoind
  6. make -f makefile.unix test_bitcoin

SIDE EFFECT:
Bitcoin-Qt and bitcoind no longer include the LevelDB memenv module (only test_bitcoin uses it) - saves 423 KB

@maqifrnswa

This comment has been minimized.

Show comment
Hide comment
@maqifrnswa

maqifrnswa Feb 26, 2013

Contributor

Debian is at least looking at including memenv, won't happen until after Wheezy is released though:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=701075

Contributor

maqifrnswa commented Feb 26, 2013

Debian is at least looking at including memenv, won't happen until after Wheezy is released though:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=701075

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Feb 26, 2013

Member

@maqifrnswa memenv is only needed for test_bitcoin.

Member

luke-jr commented Feb 26, 2013

@maqifrnswa memenv is only needed for test_bitcoin.

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Jun 19, 2013

Contributor

ACK

Though this will certainly need updating, pending autotools work

Contributor

jgarzik commented Jun 19, 2013

ACK

Though this will certainly need updating, pending autotools work

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jun 19, 2013

Member

I'd delay this until after autotoolification, as it would need to be redone from scratch otherwise anyway.

However, all but test_bitcoin not linking against libmemenv.a seems meaningful by itself.

Member

sipa commented Jun 19, 2013

I'd delay this until after autotoolification, as it would need to be redone from scratch otherwise anyway.

However, all but test_bitcoin not linking against libmemenv.a seems meaningful by itself.

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Jul 16, 2013

Member

In the meantime, I fixed a bug building test_bitcoin against system leveldb (memenv.h is not in a subdirectory)

Member

luke-jr commented Jul 16, 2013

In the meantime, I fixed a bug building test_bitcoin against system leveldb (memenv.h is not in a subdirectory)

@BitcoinPullTester

This comment has been minimized.

Show comment
Hide comment
@BitcoinPullTester

BitcoinPullTester Jul 21, 2013

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/9f62e7c190d27b14b834d6d085ad136c496e83d0 for binaries and test log.

This could happen for one of several reasons:

  1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in contrib/test-scripts)
  2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
  3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
  4. The test suite fails on either Linux i386 or Win32
  5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

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.

BitcoinPullTester commented Jul 21, 2013

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/9f62e7c190d27b14b834d6d085ad136c496e83d0 for binaries and test log.

This could happen for one of several reasons:

  1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in contrib/test-scripts)
  2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
  3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
  4. The test suite fails on either Linux i386 or Win32
  5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

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.

@Diapolo

This comment has been minimized.

Show comment
Hide comment
@Diapolo

Diapolo Oct 10, 2013

This needs rebase because of Autotools, dunno if it's even required...

Diapolo commented Oct 10, 2013

This needs rebase because of Autotools, dunno if it's even required...

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Oct 16, 2013

Member

Closing this as autotools will need a completely different solution.

Member

laanwj commented Oct 16, 2013

Closing this as autotools will need a completely different solution.

@laanwj laanwj closed this Oct 16, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment