Skip to content
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

feat: add libelf1 #363

Closed
wants to merge 1 commit into from
Closed

feat: add libelf1 #363

wants to merge 1 commit into from

Conversation

gajus
Copy link

@gajus gajus commented Mar 23, 2017

libelf1 is required by Flowtype (https://flowtype.org/) to store binaries and extract them at the runtime. Running flow-bin on a machine without libelf1 will product the following error:

/srv/node_modules/flow-bin/flow-linux64-v0.41.0/flow: error while loading shared libraries: libelf.so.1: cannot open shared object file: No such file or directory

https://www.npmjs.com/package/flow-bin is a popular program (over 880k downloads a month). Lack of an official Node.js image with flow-bin support results in a lot of avoidable image builds for the sole purpose of adding libelf1.

Adding libelf1 increases the final image size by 2MB (from 666MB to 668MB, measured using the 7.7 base image).

Fixes:

libelf1 is required by Flowtype (https://flowtype.org/) to store binaries and extract them at the runtime. Running `flow-bin` on a machine without libelf1 will product the following error:

```
/srv/node_modules/flow-bin/flow-linux64-v0.41.0/flow: error while loading shared libraries: libelf.so.1: cannot open shared object file: No such file or directory
```

https://www.npmjs.com/package/flow-bin is a popular program (over 880k downloads a month). Lack of an official Node.js image with flow-bin support results in a lot of avoidable image builds for the sole purpose of adding libelf1.

Adding libelf1 increases the final image size by 2MB (from 666MB to 668MB, measured using the 7.7 base image).
@gajus
Copy link
Author

gajus commented Mar 24, 2017

I might be wrong but it appears that the v4 is failing for an unrelated reason.

@Starefossen Can you take a look? It probably requires to simply restart the job.

@LaurentGoderre
Copy link
Member

The problem with adding library that are not essential to run node is that we would have to create a process to deal with every request of this type to determine what would go in and what wouldn't, which could get messy. I would rather have the minimum in the base image and implementers create their image on top of it.

@chorrell
Copy link
Contributor

I agree with @LaurentGoderre.

I'd also add that the slim variant was created because there were a lot of complaints about all the libraries and packages that are already preinstalled in the base Node.js image. The general consensus was that for production environments people would rather have a minimal Node.js image that they can build on and have more choice about what is installed.

@Starefossen
Copy link
Member

While I agree keeping the image as small as possible, the «main» Node.js image variant already contains quit a few packages which is strictly not necessary to run Node.js (yes, those are installed by the parent image, but still). Production container use cases should use the alpine or slim variants which is why I would would be +1 to include the libelf1 package for the main variant.

@chorrell
Copy link
Contributor

So with that logic I guess we just keep adding libraries and packages to the base node image whenever an issue like this comes up? Where do we draw the line?

@Starefossen
Copy link
Member

I see your point @chorrell; I can not recall too many cases like this in the past and as I mentioned to @gajus there are alternative Docker images with Node.js and pre-installed libelf available from Docker Hub.

@chorrell
Copy link
Contributor

chorrell commented Mar 30, 2017

Based on similar discussions in #367 I'm 👎 on this.

The base node image, for historical reasons, was more like a "developer" image where all kinds of things were pre-installed via buildpack-deps. It made for a pretty decent docker-on-your-laptop experience where (most) things just worked.

Image size has been a pretty considerable complaint because of this and lead to the creation of the slim variant. While using buildpack-depsmade sense at the time it's no longer relevant.

We usually recommend using the slim variant for production and installing the dependencies you need. We should be steering people towards using slim or alpine.

I guess the other, bigger issue here is that when you install a package with npm, you don't always know what system dependencies are required (e.g, the libelf1 lib). That's a pretty common issue we hit with the slim variant where a user of a given npm package is often unaware of what their app actually depends on. Maybe 80% of the time the main node image hides that from them, but I don't think that's such a good thing to do.

@gajus
Copy link
Author

gajus commented Mar 30, 2017

Understood. Thank you for all for the discussion.

It appears the majority is against the addition.

I will communicate an update when there is a resolution in the Flowtype community.

@gajus gajus closed this Mar 30, 2017
@montogeek
Copy link

I need this as well, @gajus Do you have a Docker image with Flow? Can you please share it?
I have https://hub.docker.com/r/montogeek/docker-yarn-flow/ but it is not working :/

@gajus
Copy link
Author

gajus commented Apr 4, 2017

I need this as well, @gajus Do you have a Docker image with Flow? Can you please share it?
I have https://hub.docker.com/r/montogeek/docker-yarn-flow/ but it is not working :/

A new version of Flowtype is coming out in a not too distant future that will drop libelf1 dependency. I will share an update here once it is out.

@mattbailey
Copy link

@gajus is there a issue to track removing libelf1 from flow-bin? Currently making it difficult for me to implement flow, since I use alpine base images.

@xdissent
Copy link

@mattbailey flow-bin won't work with alpine regardless because the flow builds are linked with glibc. IIRC they've expressed disinterest in including a musl build in flow-bin (but I wish they would!) For building flow from src in alpine you need the elfutils-dev package from edge. After compiling you only need elfutils-libelf.

@LaurentGoderre
Copy link
Member

@xdissent I think this is the case for many dependencies. Hopefully as alpine gains momentum, will become clear that maintaining musl binaries is a must for the ecosystem.

One thing that would be nice though would be a parameter in node specifying the compiler used so we can do detection on platform, architecture and compiler. Right now I haven't found a way to do that!

@gajus
Copy link
Author

gajus commented Apr 14, 2017

@gajus is there a issue to track removing libelf1 from flow-bin? Currently making it difficult for me to implement flow, since I use alpine base images.

flow/flow-bin#58

@gajus
Copy link
Author

gajus commented Jun 13, 2017

Addressed in Flow v0.48:

Removed the libelf dependency from Flow. This helps a few systems, like the standard node docker, that doesn't have libelf available.

https://github.com/facebook/flow/blob/master/Changelog.md#0480

@Starefossen
Copy link
Member

Great news @gajus 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants