Closed tamird closed 8 years ago
LGTM
I tried checking out this PR in c-rocksdb
and running a test in cockroach
(I was investigating #5855 and wanted to see if it works with the newer rocksdb). I get this:
/tmp/go-build175573054/github.com/cockroachdb/cockroach/sql/_test/sql.test: symbol lookup error: /tmp/go-build175573054/github.com/cockroachdb/cockroach/sql/_test/sql.test: undefined symbol: _ZTVN7rocksdb14PosixDirectoryE
I did do a make clean
before. Not sure if I should have done something else.
Yeah, I see that locally as well. Will investigate tomorrow.
As discovered with @tamird, we need to add -DROCKSDB_LIB_IO_POSIX
to cgo_flags.go
.
Updated all the flags using build_tools/build_detect_platform
, and added instructions to import.sh
Did you intend to get rid of -DNDEBUG
and -DLZ4
? The latter seems ok as we're not using lz4 compression with rocksdb, but the former is more problematic as it turns on all sorts of debugging code.
Oops, I sure didn't. Reverted both.
On Mon, Mar 28, 2016 at 10:25 AM, Peter Mattis notifications@github.com wrote:
Did you intend to get rid of -DNDEBUG and -DLZ4? The latter seems ok as we're not using lz4 compression with rocksdb, but the former is more problematic as it turns on all sorts of debugging code.
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/cockroachdb/c-rocksdb/pull/16#issuecomment-202415039
With @petermattis' help, this is good to go. Merging.
Reapplied a69d4e23b715271498914f6b9e302a89177fb519 on top because it hasn't made it into a release yet.
@marc @petermattis @bdarnell