conoro/yunmai-data-extract

Support non-Windows OS(es)

Closed this issue · 3 comments

I ran into a couple of issues using this on OSX and will shortly test this on Linux also.

I found that the bundled node_modules/leveldown was compiled for Windows, so in order to run this code I had to remove the node_modules/leveldown directory before running npm install, which installed a version of the library that was compatible with my OS.

Obviously you had some reason for bundling that particular build, but I figured I'd submit this and you can do as you wish with it 😁

Thanks for packaging this up! I revisited my own (somewhat cruder) attempt to do the same tonight, and I was glad to find your project.

Yep, looks like bundling node_modules/leveldown is preventing npm i from installing the correct build for the current platform.

I can confirm that on both OSX and Linux after:

  • making a fresh clone of the repo
  • following install instructions from README
  • running node index.js

I saw the following error (Linux version below), which indicates that build/Release/leveldown.node has been built for another platform.

chris@lp:~/Projects/src/yunmai-data-extract$ node index.js 

~/Projects/src/yunmai-data-extract/node_modules/levelup/lib/leveldown.js:31
    throw requireError(e)
    ^
LevelUPError: Failed to require LevelDOWN (~/Projects/src/yunmai-data-extract/node_modules/leveldown/build/Release/leveldown.node: invalid ELF header). Try `npm install leveldown` if it's missing
    at requireError (~/Projects/src/yunmai-data-extract/node_modules/levelup/lib/leveldown.js:37:10)
    at getLevelDOWN (~/Projects/src/yunmai-data-extract/node_modules/levelup/lib/leveldown.js:31:11)
    at LevelUP.open (~/Projects/src/yunmai-data-extract/node_modules/levelup/lib/levelup.js:112:34)
    at new LevelUP (~/Projects/src/yunmai-data-extract/node_modules/levelup/lib/levelup.js:84:8)
    at LevelUP (~/Projects/src/yunmai-data-extract/node_modules/levelup/lib/levelup.js:45:44)
    at Object.<anonymous> (~/Projects/src/yunmai-data-extract/index.js:22:10)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Function.Module.runMain (module.js:693:10)
    at startup (bootstrap_node.js:191:16)
    at bootstrap_node.js:612:3

This PR proposes removing that directory tree from the git repo, which will allow npm to download and build (if required) the correct platform-specific build when modules are installed. For many platforms / environments, leveldown has a prebuilt version available; for others npm will attempt to build a suitable working version.

If the reason for including the directory is to save build time during tests, it's possible to use a prebuild cache without putting those resources into the source tree.

(Would also be possible to remove the .exe currently in the source tree and offer it as a release binary as well - see https://help.github.com/articles/distributing-large-binaries/ for more on that.)

Apologies for not replying on these tickets, I missed the notifications. I'll look at them both this week.

Thanks. All of those changes made and updated version of Level to get rid of security warnings.