kevva / bin-wrapper

Binary wrapper that makes your programs seamlessly available as local dependencies
MIT License
152 stars 66 forks source link

prefers to use system binary if available #51

Closed torvitas closed 7 years ago

torvitas commented 7 years ago

see #50

torvitas commented 7 years ago

Hi,

the failing checks do not look like I caused them to fail at all. Is there anything else I can do to get this one merged?

Greetings, Sascha

torvitas commented 7 years ago

Sadly the detection whether bin-wrapper is calling it self does not work. I have no clue how to solve that.

AndreKR commented 7 years ago

What's the problem currently?

matteocng commented 7 years ago

@torvitas you didn't cause the failing checks, #53 (fixes the test) and #54 (fixes the linting errors) fix them.

Hoping this one can get merged, using the system binary is very helpful in small Docker containers without build tools installed.

torvitas commented 7 years ago

It's actually not that easy to ensure bin-wrapper doesn't call it self. I just switched away from node-alpine to avoid having to use system binaries over what bin-wrapper provides.

AndreKR commented 7 years ago

Is this fix already released? I'm still getting the same error with a package (pngquant-bin) that I think is using bin-wrapper.

Here's how to reproduce on a machine that has Docker.

docker run -it --rm alpine:latest /bin/sh
apk update
apk add nodejs nodejs-npm

Install pngquant and verify that it's working:

apk add pngquant --update-cache --repository http://dl-3.alpinelinux.org/alpine/edge/community --allow-untrusted
pngquant
# Output should be:
# pngquant, 2.10.1 (July 2017), by Kornel Lesinski, Greg Roelofs.
# ...

Then try to install pngquant-bin:

cd /root
mkdir test
cd test
echo '{}' > package.json
npm install --save pngquant-bin

Expected result: Successful installation using the pngquant that was previously installed on the system.

Actual result:

> pngquant-bin@3.1.1 postinstall /root/test/node_modules/pngquant-bin
> node lib/install.js

  ⚠ spawn /root/test/node_modules/pngquant-bin/vendor/pngquant ENOENT
  ⚠ pngquant pre-build test failed
  ℹ compiling from source
  ✖ Error:
[some build error]
torvitas commented 7 years ago

It is not merged, and it does not work correctly the way it is. It is necessary to prevent bin-wrapper to call it self. Feel free to improve.

AndreKR commented 7 years ago

How to reproduce this problem with bin-wrapper calling itself?

torvitas commented 7 years ago

The problem is that node extends $PATH. If I remember correctly this should happen if there is no system binary.