Skip to content
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

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

Open
wants to merge 1 commit into
base: develop-3.0
Choose a base branch
from

Conversation

girishramnani
Copy link

used the changes from #258 and opened a PR again to develop-3.0

@girishramnani
Copy link
Author

Shout out to @jvf for the original PR

@martinsumner
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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

@girishramnani
Copy link
Author

Darwin

@martinsumner
Copy link
Contributor

martinsumner commented Apr 1, 2021

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
Copy link
Author

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

@martinsumner
Copy link
Contributor

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
Copy link
Author

Ok let me see if There is a better way

@martinsumner
Copy link
Contributor

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
Copy link
Contributor

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

@ThomasArts
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants