Added support for "node-gyp" build & update deps #39

justmoon · 2012-07-24T19:14:31Z

This pull request:

Switches from node-waf to node-gyp, this fixes build issues with Node.js 0.8.x on MacOS, see make node-leveldb ready for node 0.8.x #38
Updates Snappy to r65 - fixes several bugs, see changelog
Updates LevelDB to 1.5.0 - fixes several bugs & improves performance, see changelog

Tested with Node.js 0.8.3 on Ubuntu.

…ion for coffee-script now

gflarity · 2012-07-24T19:37:17Z

Looks like you might have missed a file? (../deps/snappy/snappy.h:45:33: error: snappy-stubs-public.h: No such file or directory). Here's the full output:

What a great team effort.

gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
  CXX(target) Release/obj.target/snappy/deps/snappy/snappy.o
In file included from ../deps/snappy/snappy.cc:29:
../deps/snappy/snappy.h:45:33: error: snappy-stubs-public.h: No such file or directory
In file included from ../deps/snappy/snappy.cc:29:
../deps/snappy/snappy.h:59: error: 'uint32' has not been declared
../deps/snappy/snappy.h:69: error: 'string' has not been declared
../deps/snappy/snappy.h:78: error: 'string' has not been declared
In file included from ../deps/snappy/snappy-internal.h:34,
                 from ../deps/snappy/snappy.cc:30:
../deps/snappy/snappy-stubs-internal.h:94: error: 'uint32' does not name a type
../deps/snappy/snappy-stubs-internal.h:95: error: 'int64' does not name a type
../deps/snappy/snappy-stubs-internal.h: In function 'void snappy::UnalignedCopy64(const void*, void*)':
../deps/snappy/snappy-stubs-internal.h:195: error: expected type-specifier before 'uint64'
../deps/snappy/snappy-stubs-internal.h:195: error: expected `>' before 'uint64'
../deps/snappy/snappy-stubs-internal.h:195: error: expected `(' before 'uint64'
../deps/snappy/snappy-stubs-internal.h:195: error: 'uint64' was not declared in this scope
../deps/snappy/snappy-stubs-internal.h:195: error: expected primary-expression before '>' token
../deps/snappy/snappy-stubs-internal.h:195: error: ISO C++ forbids declaration of 'type name' with no type
../deps/snappy/snappy-stubs-internal.h:195: error: expected `>' before 'uint64'
../deps/snappy/snappy-stubs-internal.h:195: error: expected `(' before 'uint64'
../deps/snappy/snappy-stubs-internal.h:195: error: expected primary-expression before '>' token
../deps/snappy/snappy-stubs-internal.h:195: error: expected `)' before ';' token
../deps/snappy/snappy-stubs-internal.h:195: error: expected `)' before ';' token
../deps/snappy/snappy-stubs-internal.h:200: error: expected type-specifier before 'uint32'
../deps/snappy/snappy-stubs-internal.h:200: error: expected `>' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:200: error: expected `(' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:200: error: 'uint32' was not declared in this scope
../deps/snappy/snappy-stubs-internal.h:200: error: expected primary-expression before '>' token
../deps/snappy/snappy-stubs-internal.h:200: error: ISO C++ forbids declaration of 'type name' with no type
../deps/snappy/snappy-stubs-internal.h:200: error: expected `>' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:200: error: expected `(' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:200: error: expected primary-expression before '>' token
../deps/snappy/snappy-stubs-internal.h:200: error: expected `)' before ';' token
../deps/snappy/snappy-stubs-internal.h:200: error: expected `)' before ';' token
../deps/snappy/snappy-stubs-internal.h:201: error: expected type-specifier before 'uint32'
../deps/snappy/snappy-stubs-internal.h:201: error: expected `>' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:201: error: expected `(' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:201: error: expected primary-expression before '>' token
../deps/snappy/snappy-stubs-internal.h:201: error: ISO C++ forbids declaration of 'type name' with no type
../deps/snappy/snappy-stubs-internal.h:201: error: expected `>' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:201: error: expected `(' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:201: error: expected primary-expression before '>' token
../deps/snappy/snappy-stubs-internal.h:201: error: expected `)' before ';' token
../deps/snappy/snappy-stubs-internal.h:201: error: expected `)' before ';' token
../deps/snappy/snappy-stubs-internal.h: At global scope:
../deps/snappy/snappy-stubs-internal.h:289: error: 'uint16' does not name a type
../deps/snappy/snappy-stubs-internal.h:290: error: 'uint16' does not name a type
../deps/snappy/snappy-stubs-internal.h:292: error: 'uint32' does not name a type
../deps/snappy/snappy-stubs-internal.h:293: error: 'uint32' does not name a type
../deps/snappy/snappy-stubs-internal.h:300: error: 'uint16' does not name a type
../deps/snappy/snappy-stubs-internal.h:304: error: 'uint16' has not been declared
../deps/snappy/snappy-stubs-internal.h:308: error: 'uint32' does not name a type
../deps/snappy/snappy-stubs-internal.h:312: error: 'uint32' has not been declared
../deps/snappy/snappy-stubs-internal.h: In static member function 'static void snappy::LittleEndian::Store16(void*, int)':
../deps/snappy/snappy-stubs-internal.h:305: error: expected type-specifier before 'uint16'
../deps/snappy/snappy-stubs-internal.h:305: error: expected `>' before 'uint16'
../deps/snappy/snappy-stubs-internal.h:305: error: expected `(' before 'uint16'
../deps/snappy/snappy-stubs-internal.h:305: error: 'uint16' was not declared in this scope
../deps/snappy/snappy-stubs-internal.h:305: error: expected primary-expression before '>' token
../deps/snappy/snappy-stubs-internal.h:305: error: 'FromHost16' was not declared in this scope
../deps/snappy/snappy-stubs-internal.h:305: error: expected `)' before ';' token
../deps/snappy/snappy-stubs-internal.h: In static member function 'static void snappy::LittleEndian::Store32(void*, int)':
../deps/snappy/snappy-stubs-internal.h:313: error: expected type-specifier before 'uint32'
../deps/snappy/snappy-stubs-internal.h:313: error: expected `>' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:313: error: expected `(' before 'uint32'
../deps/snappy/snappy-stubs-internal.h:313: error: 'uint32' was not declared in this scope
../deps/snappy/snappy-stubs-internal.h:313: error: expected primary-expression before '>' token
../deps/snappy/snappy-stubs-internal.h:313: error: 'FromHost32' was not declared in this scope
../deps/snappy/snappy-stubs-internal.h:313: error: expected `)' before ';' token
../deps/snappy/snappy-stubs-internal.h: At global scope:
../deps/snappy/snappy-stubs-internal.h:321: error: 'uint32' has not been declared
../deps/snappy/snappy-stubs-internal.h:326: error: 'uint32' has not been declared
../deps/snappy/snappy-stubs-internal.h:327: error: 'uint64' has not been declared
../deps/snappy/snappy-stubs-internal.h:330: error: ISO C++ forbids declaration of 'DISALLOW_COPY_AND_ASSIGN' with no type
../deps/snappy/snappy-stubs-internal.h:349: error: 'snappy::Bits::Log2Floor' declared as an 'inline' variable
../deps/snappy/snappy-stubs-internal.h:349: error: 'int snappy::Bits::Log2Floor' is not a static member of 'class snappy::Bits'
../deps/snappy/snappy-stubs-internal.h:349: error: 'uint32' was not declared in this scope
../deps/snappy/snappy-stubs-internal.h:349: error: expected ',' or ';' before '{' token
../deps/snappy/snappy.cc:1118: error: expected `}' at end of input
make[1]: *** [Release/obj.target/snappy/deps/snappy/snappy.o] Error 1

pconstr · 2012-07-24T19:40:41Z

I'm getting the exact same problem as gflarity

justmoon · 2012-07-24T19:46:31Z

My bad, it looks like snappy-stubs-public.h is generated from snappy-stubs-public.h.in by autoconf. I'll look at porting it to gyp.

justmoon · 2012-07-24T19:55:22Z

The Chromium source holds the solution: http://src.chromium.org/svn/trunk/src/third_party/snappy/snappy.gyp

They have three different directories for each of the operating systems with pregenerated config.h / snappy-stubs-public.h headers and then just add the respective build directory with OS conditionals.

I'll "borrow" their solution. ;)

justmoon · 2012-07-24T20:20:09Z

Ok, try again now! I also found out why it was working for me - I had snappy-stubs-public.h in my /usr/local/include/. Sorry about that. :)

pconstr · 2012-07-24T21:28:00Z

both npm install and npm test ran without errors on Mac OS 10.6.8 (node 0.8.1)

my8bird · 2012-07-25T01:11:15Z

Two things I found

This did not build for me with v0.6.8. While not surprising just wanted to make sure we were all on the same page since the travis build file indicates 0.6 should still work.
When I ran the tests with v0.8.3 I got the following error. I can look into it but wanted to first know if others saw it already. I am running Ubuntu 12.04 if that is a key to anyone else.

  ✖ 1 of 41 tests failed:

  1) Iterator db.forRange() should iterate until limit key:

      actual expected

      107

  AssertionError: "107" == ""
      at i (/home/nathan/src/node-leveldb/test/iterator-test.coffee:202:18)
      at exports.Iterator.Iterator.forRange.next (/home/nathan/src/node-leveldb/lib/leveldb/iterator.js:142:9)
      at exports.Iterator.Iterator._wrapSeek (/home/nathan/src/node-leveldb/lib/leveldb/iterator.js:62:28)

justmoon · 2012-07-25T04:49:12Z

This did not build for me with v0.6.8. While not surprising just wanted to make sure we were all on the same page since the travis build file indicates 0.6 should still work.

Good point. From what I understand 0.6.13+ should include node-gyp and should work. Travis uses Node.js 0.6.19, so the .travis.yml can stay as it is. This branch works fine on Travis for 0.6 and 0.8.

The question is do we want to use >=0.6.13 as the minimum version in package.json?

When I ran the tests with v0.8.3 I got the following error. I can look into it but wanted to first know if others saw it already. I am running Ubuntu 12.04 if that is a key to anyone else.

I'm running the same and I haven't seen this, awaiting your feedback!

mikepb · 2012-07-25T04:54:03Z

The tests failures happen randomly. I haven't the time to track down the issue. There's a lot of multithreading involved, that may be the issue. I haven't looked at the library in months, so it's all going over my head.

Great work on the updates 👍

justmoon · 2012-07-25T05:06:29Z

The tests failures happen randomly.

I opened a ticket for it here: #40. I think the key to debugging those issues is to record a bunch of logs from the random failures and collect them in one place.

(That is assuming that @my8bird comes back confirming that this was indeed one of those random failures, of course.)

my8bird · 2012-07-25T14:32:32Z

The test error appears to be random so we can drop that from this thread since the #40 exists for that.

Secondly, I pulled down v0.6.19 to test that also and it built and passed all tests. Given that I would say we update the package.json to put 0.6.13 or 19 as the minimum the library will work with so that users know what to expect.

It is really cool to see all of the interest in this pull, thanks a lot all.

Added support for "node-gyp" build & update deps

pconstr and others added 11 commits July 22, 2012 15:45

recover leveldb.gyp before it got moved out to chromium

88abb03

move from node-waf to node-gyp, adapting leveldb.gyp

7a51edd

will hopefully work on node 0.8.x

011573a

package is a reversed word now in coffee-script

9e43f04

bumped node version and mocha version, mocha requires --compilers opt…

96f6fe1

…ion for coffee-script now

modfied leveldb.gyp to build a static_library

9fa779a

don't delete the build dir, it's needed

92e5afa

Update Snappy to r65.

f598e95

Add snappy.gyp and enable by default.

f6fbfd5

Updated LevelDB to 1.5.0.

97ca7a8

Fix "path.existsSync" deprecation warning.

41c53f6

justmoon mentioned this pull request Jul 24, 2012

make node-leveldb ready for node 0.8.x #38

Closed

Fix Snappy portability headers.

193b096

my8bird added a commit that referenced this pull request Jul 25, 2012

Merge pull request #39 from justmoon/gypbuild

c7de25a

Added support for "node-gyp" build & update deps

my8bird merged commit c7de25a into my8bird:master Jul 25, 2012

my8bird mentioned this pull request Jul 25, 2012

doesn't work with node v0.8.1 #36

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for "node-gyp" build & update deps #39

Added support for "node-gyp" build & update deps #39

justmoon commented Jul 24, 2012

gflarity commented Jul 24, 2012

pconstr commented Jul 24, 2012

justmoon commented Jul 24, 2012

justmoon commented Jul 24, 2012

justmoon commented Jul 24, 2012

pconstr commented Jul 24, 2012

my8bird commented Jul 25, 2012

justmoon commented Jul 25, 2012

mikepb commented Jul 25, 2012

justmoon commented Jul 25, 2012

my8bird commented Jul 25, 2012

Added support for "node-gyp" build & update deps #39

Added support for "node-gyp" build & update deps #39

Conversation

justmoon commented Jul 24, 2012

gflarity commented Jul 24, 2012

pconstr commented Jul 24, 2012

justmoon commented Jul 24, 2012

justmoon commented Jul 24, 2012

justmoon commented Jul 24, 2012

pconstr commented Jul 24, 2012

my8bird commented Jul 25, 2012

justmoon commented Jul 25, 2012

mikepb commented Jul 25, 2012

justmoon commented Jul 25, 2012

my8bird commented Jul 25, 2012