basho / eleveldb

Erlang LevelDB API
263 stars 176 forks source link

Add -stdlib=libc++ flag when building snappy on develop-3.0 #262

Open girishramnani opened 3 years ago

girishramnani commented 3 years ago

used the changes from https://github.com/basho/eleveldb/pull/258 and opened a PR again to develop-3.0

girishramnani commented 3 years ago

Shout out to @jvf for the original PR

martinsumner commented 3 years ago

Does this address an issue building on a particular OS? I know we set to use libc++ on OSX specifically - https://github.com/basho/eleveldb/blob/develop-3.0/c_src/build_deps.sh#L71-L75.

I think we only changed it on OSX as it was understood it wasn't a problem on another OS, but we haven't exhaustively tested.

girishramnani commented 3 years ago

I tried develop-3.0 on MacOS Big Sur and it failed and after adding this flag the build successfully happened. Hence thought I should open a PR

martinsumner commented 3 years ago

What do you get when you run uname -s on Big Sur?

girishramnani commented 3 years ago

Darwin

martinsumner commented 3 years ago

So that leaves me confused why the current OSX fix doesn't work, given that will add libc++ to the $CFLAGS $CXXFLAGS (when uname returns "Darwin")

girishramnani commented 3 years ago

Yeah, I was also confused but I can confirm it is working after the fix and before it wasn’t

martinsumner commented 3 years ago

The issue potentially is that your fix will change it for every OS, which we avoided previously; only setting this for OSX which is the only OS where we had the problem.

girishramnani commented 3 years ago

Ok let me see if There is a better way

martinsumner commented 3 years ago

It would be helpful if you can echo out in the script and confirm if these flags are being set as expected on Big Sur

martinsumner commented 3 years ago

@ThomasArts - did you say you had a problem with OSX builds? Was it this?

ThomasArts commented 3 years ago

Compiling eleveldb on MacOS X 10.15.7 without problem, but indeed, somehow the compiler is already instructed to use libc++ as stdlib: rm -f libleveldb.a ar -rs libleveldb.a db/builder.o db/c.o db/db_impl.o db/db_iter.o db/dbformat.o db/filename.o db/log_reader.o db/log_writer.o db/memtable.o db/repair.o db/table_cache.o db/version_edit.o db/version_set.o db/write_batch.o leveldb_ee/cache_warm.o leveldb_ee/compile_opt.o leveldb_ee/expiry_ee.o leveldb_ee/hot_backup.o leveldb_ee/riak_object.o table/block.o table/block_builder.o table/filter_block.o table/format.o table/iterator.o table/merger.o table/table.o table/table_builder.o table/two_level_iterator.o util/arena.o util/bloom.o util/bloom2.o util/cache.o util/cache2.o util/coding.o util/comparator.o util/crc32c.o util/db_list.o util/env.o util/env_posix.o util/expiry_os.o util/filter_policy.o util/flexcache.o util/hash.o util/histogram.o util/hot_threads.o util/logging.o util/murmurhash.o util/options.o util/perf_count.o util/status.o util/thread_tasks.o util/throttle.o port/port_posix.o util/lz4.o ar: creating archive libleveldb.a g++ -I /Users/thomas/Quviq/Customers/NHS/riak/_build/default/lib/eleveldb/c_src/system/include -stdlib=libc++ -I. -I./include -mmacosx-version-min=10.8 -DOS_MACOSX -stdlib=libc++ -DLEVELDB_PLATFORM_POSIX -DSNAPPY -DLEVELDB_VSN="2.0.36" -O2 -g -DNDEBUG -fPIC tools/leveldb_repair.cc -o leveldb_repair -L . -lleveldb -L/Users/thomas/Quviq/Customers/NHS/riak/_build/default/lib/eleveldb/c_src/system/lib -mmacosx-version-min=10.8 -lsnappy

ThomasArts commented 3 years ago

In other words, we should not need this PR to fix it.