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 sys/sysmacros.h for build on modern glibc #86

Merged
merged 1 commit into from
Dec 9, 2018

Conversation

lachesis
Copy link
Contributor

@lachesis lachesis commented Dec 7, 2018

Port of https://github.com/facebook/rocksdb/pull/2208/files

Without this, npm i rocksdb fails on my arch linux machine.

glibc version:

Version         : 2.28-5
Build Date      : Thu 11 Oct 2018 01:18:28 AM PDT
Install Date    : Wed 24 Oct 2018 10:43:56 PM PDT

Error:

  CXX(target) Release/obj.target/leveldb/deps/leveldb/leveldb-rocksdb/util/iostats_context.o
  CXX(target) Release/obj.target/leveldb/deps/leveldb/leveldb-rocksdb/util/io_posix.o
../deps/leveldb/leveldb-rocksdb/util/io_posix.cc: In function ‘size_t rocksdb::{anonymous}::GetLogicalBufferSize(int)’:
../deps/leveldb/leveldb-rocksdb/util/io_posix.cc:57:7: error: ‘major’ was not declared in this scope
   if (major(buf.st_dev) == 0) {
       ^~~~~
../deps/leveldb/leveldb-rocksdb/util/io_posix.cc:67:55: error: ‘major’ was not declared in this scope
   snprintf(path, kBufferSize, "/sys/dev/block/%u:%u", major(buf.st_dev),
                                                       ^~~~~
../deps/leveldb/leveldb-rocksdb/util/io_posix.cc:68:12: error: ‘minor’ was not declared in this scope
            minor(buf.st_dev));
            ^~~~~
../deps/leveldb/leveldb-rocksdb/util/io_posix.cc:68:12: note: suggested alternative: ‘mknod’
            minor(buf.st_dev));
            ^~~~~
            mknod
make: *** [deps/leveldb/leveldb.target.mk:299: Release/obj.target/leveldb/deps/leveldb/leveldb-rocksdb/util/io_posix.o] Error 1
make: Leaving directory '/home/eswanson/projects/rockdove/node_modules/rocksdb/build'
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/usr/lib/node_modules/node-gyp/lib/build.js:262:23)
gyp ERR! stack     at ChildProcess.emit (events.js:182:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:254:12)
gyp ERR! System Linux 4.19.4-arch1-1-ARCH
gyp ERR! command "/usr/bin/node" "/usr/lib/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /home/eswanson/projects/rockdove/node_modules/rocksdb
gyp ERR! node -v v11.2.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok 
npm WARN ts-mock-imports@1.2.2 requires a peer of typescript@>=2.6.1 < 3.2 but none is installed. You must install peer dependencies yourself.
npm WARN ts-mock-imports@1.2.2 requires a peer of sinon@>= 4.1.2 < 7 but none is installed. You must install peer dependencies yourself.
npm WARN rockdove@0.0.1 No repository field.
npm WARN rockdove@0.0.1 No license field.

@vweevers
Copy link
Member

vweevers commented Dec 7, 2018

Thanks for your contribution and welcome!

This will become obsolete, because we're moving RocksDB to a git submodule (#82).

@lachesis
Copy link
Contributor Author

lachesis commented Dec 7, 2018

Excellent, that is a better fix for sure. I needed this change to begin using rocksdb right away, so I figured I would offer a pull as well.

@filoozom
Copy link
Contributor

filoozom commented Dec 8, 2018

Might as well merge this for now. We'd also need to find a more recent version of RocksDB with sys/sysmacros.h and that doesn't break this whole library.

@vweevers
Copy link
Member

vweevers commented Dec 8, 2018

Roger. We can considering merging this, because #82 isn't ready yet and afterwards we'd also need to update RocksDB to a more recent version.

@ralphtheninja WDYT?

@vweevers
Copy link
Member

vweevers commented Dec 8, 2018

@filoozom beat me to it :)

@filoozom
Copy link
Contributor

filoozom commented Dec 8, 2018

Looks like you're lucky, this commit is really old: facebook/rocksdb@3b4d1b7. Shouldn't be a problem to update to there once the time has come to merge the Git Submodule.

Copy link
Contributor

@filoozom filoozom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upstream's PR: facebook/rocksdb#2208

@vweevers vweevers merged commit d50ce56 into Level:master Dec 9, 2018
@vweevers
Copy link
Member

Released in 3.0.3

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