basho / eleveldb

Erlang LevelDB API
263 stars 176 forks source link

Depend on system snappy (With OSX fix) #272

Closed codeadict closed 1 year ago

codeadict commented 1 year ago

This builds on top of @hmmr 's https://github.com/basho/eleveldb/pull/271.

Fixes https://github.com/basho/eleveldb/issues/270

codeadict commented 1 year ago

@hmmr @ioolkos @martinsumner This one is passing now on both Linux and OSX, don't have a FreeBSD box myself but should be fine there too 🤞🏽 . See passing tests at https://github.com/codeadict/eleveldb/actions/runs/4239145446/jobs/7366922850

ioolkos commented 1 year ago

Confirming this works on Ubuntu. ➜ eleveldb git:(272) ldd -r _build/default/lib/eleveldb/priv/eleveldb.so shows no more missing Snappy symbols.

codeadict commented 1 year ago

Looks like Github actions are stuck on the OTP 22 job:

Job is about to start running on the hosted runner: GitHub Actions 3 (hosted)

Might be a good idea to cancel and re-run them.

ioolkos commented 1 year ago

Just made a visit here, scanning through #269 to #271 on which this PR is based. Seems to keep the FreeBSD fixes done previously. Looks good to me, but more importantly let me know when I can help test any specifics.

codeadict commented 1 year ago

Indeed @ioolkos , confirmed it works on FreeBSD:

Building LevelDB...
gmake LDFLAGS="" -C leveldb all
gmake[2]: Entering directory '/root/eleveldb/c_src/leveldb'
gmake[2]: Nothing to be done for 'all'.
gmake[2]: Leaving directory '/root/eleveldb/c_src/leveldb'
gmake LDFLAGS="-L/usr/local/lib -lsnappy -lpthread" -C leveldb tools
gmake[2]: Entering directory '/root/eleveldb/c_src/leveldb'
gmake[2]: Nothing to be done for 'tools'.
gmake[2]: Leaving directory '/root/eleveldb/c_src/leveldb'
for tool in leveldb_repair perf_dump sst_rewrite sst_scan; do cp leveldb/$tool /root/eleveldb/priv/; done
gmake[1]: Leaving directory '/root/eleveldb/c_src'
===> Analyzing applications...
===> Compiling eleveldb
===> Compiling c_src/eleveldb.cc
===> Compiling c_src/refobjects.cc
===> Compiling c_src/workitems.cc
===> Linking /root/eleveldb/priv/eleveldb.so
root@freebsd:~/eleveldb # uname -a
FreeBSD freebsd 13.1-RELEASE FreeBSD 13.1-RELEASE releng/13.1-n250148-fc952ac2212 GENERIC arm64
ioolkos commented 1 year ago

@hmmr, @martinsumner do you see a possibility to merge this?

hmmr commented 1 year ago

There's a fair bit of details specific to OSX way beyond my expertise, so I have to trust those details are done right. As long as it builds on linux (which it does), I'm giving my +1, but others should have a look.

ioolkos commented 1 year ago

@hmmr thanks for your answer and your +1. Fair enough on the OSX remark. @codeadict can consult on this, maybe ensuring enough "separation" between OSX and Linux builds is enough. Or maybe that's already cleanly given here.

martinsumner commented 1 year ago

Will do some testing of my own over the next couple of days just to double-check everything is all OK. I don't have the skills or knowledge to do a full review of the changes myself, but I think it might be fair to say there has been sufficient eyes on this, and sufficient testing to move forward.

This PR does resolve the problem we had running Riak on OSX Catalina, which is helpful to me personally.

@nsaadouni - do you have any objections to this being merged?

codeadict commented 1 year ago

I'm free for any help needed on this, have some macs to test with different versions but also the CI in this PR runs on OSX ensuring this works well in OSX with all the OTP versions supported.