search

creationix / node-leveldb

NodeJS bindings to levelDB - a fast and lightweight key/value database library
http://code.google.com/p/leveldb/
113 stars 32 forks source link

Unfortunate fixes necessary to compile under Mac OS X 10.5.8, using built-in i686-apple-darwin9-gcc-4.0.1 #5

Closed carter-thaxton closed 13 years ago

carter-thaxton commented 13 years ago

Hey Tim,

I'm curious if these changes affect your build on Linux. I had to make a number of changes in order to get a build of leveldb working on my Mac.

I've run into these kinds of public/private issues with nested classes and friends before. They're due to a very subtle ambiguity in the C++ standard. I think the version of g++ that comes with XCode for Leopard (4.0.1) interprets the standard a little differently from gcc on linux. I'd be happy to explain in more detail if you like.

The LP64 thing in the leveldb source seems to be broken for Mac OS X 10.5.8. They've clearly gone out of their way to make some special considerations for 64-bit Mac, but I think it's a brittle dependency. The change I've made is more portable, I believe.

Also, I think the changes I made to wscript are just good practice. They removed a lot of warnings for me. It should be unnecessary to supply "-l" to the compiler phase. It's only necessary during linking, which is done via ldflags instead of cxxflags.

I know most of these changes are not related to your node add-on, but are more appropriately part of the leveldb source. How would you prefer to deal with these kinds of things?

By the way, I love the library. I may dive in and work on some async I/O.

Thanks,

carter-thaxton commented 13 years ago

Btw, I just submitted a pull request to dizzyd, so these changes can end up in the "portable" branch that you're using.

It looks like dizzyd's branch is already a few commits ahead of what's being used in node-leveldb. In any case, if my patch is accepted there, we won't need to do anything special in node-leveldb, except stay in sync.

tilgovi commented 13 years ago

Was this on the latest HEAD which uses the "portable" branch of leveldb?

tilgovi commented 13 years ago

Oh, I see. Nevermind. No action needed. Close when we merge happens on portable branch.

tilgovi commented 13 years ago

Landed in 04a80d05e5aadacabab6c5f9699e639d3b05d8bf it seems.