fuwaneko / node-protobuf

Google Protocol Buffers wrapper for Node.js [UNMAINTAINED]
181 stars 42 forks source link

Updated README.md with additional linux dep #69

Closed webmakersteve closed 8 years ago

webmakersteve commented 8 years ago

The linking script requires pkg-config which is not preinstalled on all linux distributions.

fuwaneko commented 8 years ago

pkg-config is not required, you can provide env variable LIBPROTOBUF.

webmakersteve commented 8 years ago

That means you need to run npm install with that environment variable otherwise it won't resolve. Perhaps we can make a preinstall hook for NPM making sure 1. the protobuf-devel headers and library are available on the machine, and 2. it can find them whether through the environment variable or pkg-config?

Also, would it make sense then to at least mention pkg-config in the README? Perhaps not as a requirement, but it is still useful to know that THAT is the alternative to setting an explicit environment variable when running a routine npm install.

Thanks. Happy to do some of the leg work for these changes if you're interested in them.

fuwaneko commented 8 years ago

That means you need to run npm install with that environment variable otherwise it won't resolve.

Yes, and what is the problem here? It's a common practice. You either rely on auto-detection, in this case pkg-config, or provide path to library yourself. Many software packages do that with 3rd-party libs.

Perhaps we can make a preinstall hook

It's already how it works.

I agree that README needs some updates.

webmakersteve commented 8 years ago

Documenting it would be a fine solution. Currently in the README, only the Windows installation makes any reference to the environment variable.

When I suggested a preinstall hook I didn't mean there wasn't one at all, it was just that it didn't log the real source of the error and the npm install would fail. It was easy enough to determine it didn't know where the location of the library was, but it may make it easier if it checks if pkg config exists or the env var is set before it runs it, so it could log, e.g., "Could not find libproto-devel. Is it installed? Specify LIBPROTOBUF to tell me where." Or whatever it is.

Would be easy enough to add that to lp.sh. Or even just updating the linux README so it documented that environment variable's use.

It's just a small thing anyway, so I'm not attached to any "solution" for solving this. Was just trying to help.