kevva / bin-wrapper

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

use system binary if possible #56

Open digitalkaoz opened 6 years ago

digitalkaoz commented 6 years ago

this PR allows using system binaries.

digitalkaoz commented 6 years ago

the failing test cases are addressed by #53 and #54

digitalkaoz commented 6 years ago

this fixes (among others) #50

sindresorhus commented 6 years ago

I don't think that's a good idea. The system binary might be outdated and missing CLI flags that the user has supplied or differing behavior.

digitalkaoz commented 6 years ago

Or the other way around ;) Then we should think about another aproach. Currently the whole imagemin ecosystem is unusable on node:alpine...i searched for the most generalistic solution to fix it for all packages...the version compare still happens, so i think its a workable solution

Sindre Sorhus notifications@github.com schrieb am Mi., 27. Dez. 2017, 20:14:

I don't think that's a good idea. The system binary might be outdated and missing CLI flags that the user has supplied or differing behavior.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kevva/bin-wrapper/pull/56#issuecomment-354163643, or mute the thread https://github.com/notifications/unsubscribe-auth/AAR61yrqtMICYJd1RkobHjpYvXQ-42zWks5tEpcQgaJpZM4RDmNi .

davazp commented 6 years ago

I think it is important to provide a way to delegate the download of the package to the user.

Right now we have a problem with an outage of Sourceforge in some regions. And some CI environments there is an intentional constraint to not use the network at all.

The system-wide install executable can be a bit unpredictable. Other alternatives? We could have a config file mapping URLs to locally downloaded files or similar.

I am not sure if it is doable but the ideal thing would be not to require the network at all.

digitalkaoz commented 6 years ago

As i said, this PR also checks the defined version of the system binary (if any found). So i still dont see a Problem in merging this...

David Vázquez Púa notifications@github.com schrieb am Fr., 2. März 2018, 11:37:

I think it is important to provide a way to delegate the download of the package to the user.

Right now we have a problem with an outage of Sourceforge in some regions. And some CI environments there is an intentional constraint to not use the network at all.

The system-wide install executable can be a bit unpredictable. Other alternatives? We could have a config file mapping URLs to locally downloaded files or similar.

I am not sure if it is doable but the ideal thing would be not to require the network at all.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kevva/bin-wrapper/pull/56#issuecomment-369886488, or mute the thread https://github.com/notifications/unsubscribe-auth/AAR611vZ0BkuT6ObhhT_aV6BHw2lG5Z1ks5taSD_gaJpZM4RDmNi .

davazp commented 6 years ago

@digitalkaoz I think it is better than not having it at all. But the result of the package will change if somebody has that dependency installed. Which could be installed for other reasons and not be an explicit step by the user. That is what I meant with unpredictable.

But again, we need something in place.

Another question, is the version checked just to verify the globally installed program works? Or if the version doesn't match it will try to download it?

polarathene commented 5 years ago

Just spent past day with my alpine docker image, turned out build issues were with imagemin dependencies(cwebp, pngquant(this ones binary worked fine), mozjpeg). If I use local packages my image size is about 110MB vs around 300MB as the binary dependencies are only sourced/built during npm/yarn install so the packages required to build them(due to only pngquant having a compatible binary) need to remain..

There is no version enforcement in these packages(presumably because they're linking to their own as source URLs to pull the correct binary). Even so, I could build those via source in another image using multi-stage build, copy the binaries over and have the much smaller image size.

+1 for merging the PR so system binaries can be used :)

For the issues with predictability, you could warn in the output that system binaries were found and are being used, or if system binary usage when available should be more of a niche thing, allow usage via environment variable? I could then just add one in my use-case and be much happier with less dependencies and faster builds.

ri0t commented 4 years ago

Just spent past day with my alpine docker image, turned out build issues were with imagemin dependencies(cwebp, pngquant(this ones binary worked fine), mozjpeg). If I use local packages my image size is about 110MB vs around 300MB as the binary dependencies are only sourced/built during npm/yarn install so the packages required to build them(due to only pngquant having a compatible binary) need to remain..

Yeah, build time for arm64 increases a lot with all that binary building. For the same reason i had to kick out node-sass, which doesn't provide 64 bit arm stuff anymore.

andir commented 4 years ago

Is there any way to move this forward? I am also running into a situation as described above. In my situation I am trying to build software using Nix where there is no network access during the build phase and I instead provide "system" binaries into the environment.

foosinn commented 4 years ago

I would also love to see this merged.

We are running a ton of daily builds, each compiling the tools on its own, which takes up to 3 minutes, a lot of cpu time and wastes energy.

About the complaints: What do you think about an OS-Env variable to Opt-In into this behavior? A simple BIN_WRAPPER_USE_OS_PACKAGES=true would allow for opting into that behavior.

Personally i would rather like to see an opt-out, but i'm more than happy with either of the options.

Thanks!

infabo commented 4 years ago

Absolutely! BIN_WRAPPER_USE_OS_PACKAGES=true would be an option to make everyone happy.

infabo commented 4 years ago

a lot of cpu time and wastes energy.

lots of wasted CO2. Carbon footprint explodes, cause maintainer refuses to provide a solution here - because of personal sensitivities. There were several alternative approaches discussed already.