TooTallNate / node-lame

Node.js native bindings to libmp3lame & libmpg123
MIT License
567 stars 113 forks source link

Add support for x87 targets #74

Closed ndfred closed 7 years ago

ndfred commented 7 years ago

The package would throw this error when trying to install on Linux 32 bit targets:

gyp: name 'mpg123_cpu' is not defined while evaluating condition 'mpg123_cpu=="arm_nofpu"' in 
deps/mpg123/mpg123.gyp while loading dependencies of binding.gyp while trying to load binding.gyp

Copy the ia32 config over and tweak the gyp files to get it to build correctly (I couldn't figure out how to link instead of copy, happy to fix if there is a way to do that and still be compatible with Windows).

This allows me to run airsonos on my Synology NAS with Debian chroot, and I would assume fixes compatibility for all 32 bit Linux setups.

TooTallNate commented 7 years ago

Wow I've never heard of x87 CPU architecture!

ndfred commented 7 years ago

Me neither, apparently that's what the Intel Atom CE5335 is :)

Tried redefining target_arch but couldn't find a way. Maybe I should generate the config headers instead of copying, remember how you did that?

ndfred commented 7 years ago

Also the build failure seems unrelated to my changes, any idea how to fix that?

ndfred commented 7 years ago

Just confirmed airsonos is working, so this fix is definitely functional on x87 CPUs

TooTallNate commented 7 years ago

Maybe I should generate the config headers instead of copying, remember how you did that?

  ./configure --enable-static --disable-shared --with-pic --disable-rpath --disable-frontend --disable-gtktest

From: https://github.com/TooTallNate/node-lame/tree/master/deps/lame/config

TooTallNate commented 7 years ago

Also, are you compiling Node.js from source? Or how are you installing it?

ndfred commented 7 years ago

I actually have a Debian chroot and installed node from apt.

Trying to run both configure scripts I get config.h files that only differ by the features to enable and libraries to link to which seems more system than architecture specific, we should probably ignore these and keep the copies from the ia32 folders: https://gist.github.com/ndfred/1df7f5315ad2a5aa01d1319c03024387

Leaves us with the build failure, which looks like an issue in the nan module: https://travis-ci.org/TooTallNate/node-lame/builds/226387511

ndfred commented 7 years ago

@TooTallNate: is this OK to merge?