-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use system binary if possible #56
base: master
Are you sure you want to change the base?
Conversation
this fixes (among others) #50 |
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. |
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
<#56 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAR61yrqtMICYJd1RkobHjpYvXQ-42zWks5tEpcQgaJpZM4RDmNi>
.
|
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. |
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
<#56 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAR611vZ0BkuT6ObhhT_aV6BHw2lG5Z1ks5taSD_gaJpZM4RDmNi>
.
|
@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? |
copied from kevva#56
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. |
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. |
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. |
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 Personally i would rather like to see an opt-out, but i'm more than happy with either of the options. Thanks! |
Absolutely! |
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. |
this PR allows using system binaries.