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

Added an alpine variant #156

Merged
merged 3 commits into from
Nov 10, 2016
Merged

Added an alpine variant #156

merged 3 commits into from
Nov 10, 2016

Conversation

LaurentGoderre
Copy link
Member

No description provided.

@ChasonTang
Copy link

I think you shouldn't build node.js again,instead,you can downloads binary from offcial website.secondly, you should mark the package when you install them,for example:

apk add --no-cache --virtual .build-deps \
        curl

so that you can remove it when it complete.

@LaurentGoderre
Copy link
Member Author

@ChasonTang I tried that approach but the built binaries don't work on alpine

@LaurentGoderre LaurentGoderre force-pushed the alpine branch 3 times, most recently from 0674d0e to e673d1d Compare April 25, 2016 14:02
@chorrell
Copy link
Contributor

For --virtual .build-deps can you list the packages out in alpha-numeric order? It will make it easier to see differences if we ever need to change that list in the future. e.g.:

RUN apk add --no-cache --virtual .build-deps \
binutils-gold \
curl \
g++ \
gcc \
gnupg \
libgcc \
libstdc++ \
linux-headers \
make \
paxctl \
python \
tar 

@chorrell
Copy link
Contributor

@LaurentGoderre @ChasonTang

There were plans to setup testing of Node.js on Alpine Linux and to eventually provide official binaries for Alpine Linux:

#107 (comment)

I'm not sure where things are with that work though.

@LaurentGoderre LaurentGoderre force-pushed the alpine branch 3 times, most recently from 49653ce to c659d89 Compare April 25, 2016 14:22
@LaurentGoderre
Copy link
Member Author

@chorrell if we merge #162 I can make this one sequence

@LaurentGoderre LaurentGoderre force-pushed the alpine branch 2 times, most recently from b3ddb62 to 4561c97 Compare April 25, 2016 14:30
@chorrell
Copy link
Contributor

I merged #162

@LaurentGoderre
Copy link
Member Author

This should also be dependent on #164

@chorrell
Copy link
Contributor

Does #164 solve the broken build issue?

Also, it might be good to put this on hold for a bit. I'd like to know where things we at with the build group's plan to test on Alpine before we take this any further.

@LaurentGoderre
Copy link
Member Author

@chorrell Not it doesn't but it simplifies this PR since it wont need to change the update script anymore

@Vanuan
Copy link

Vanuan commented Apr 25, 2016

@ChasonTang

I think you shouldn't build node.js again,instead,you can downloads binary from official website

An official website doesn't provide pre-built binaries for musl libc.
Maybe you meant alpine's repository? But those also don't have pre-built binaries for all versions:
https://hub.docker.com/r/iron/node/tags/

@ChasonTang
Copy link

@Vanuan oh,it's my fault,alpine node.js must build in the container.

@LaurentGoderre
Copy link
Member Author

For some odd reason, the build only fails on node 0.10 for the alpine variants....

@chorrell
Copy link
Contributor

I don't think it's the image, it has something to do with accessing the gpg key server. I have another PR that's failing in the same place:

https://travis-ci.org/nodejs/docker-node/builds/125849371

The PR just removes the -q from docker build in the test-build.sh script.

@chorrell
Copy link
Contributor

gpg: requesting key DBE9B9C5 from hkp server ha.pool.sks-keyservers.net
gpg: keyserver timed out
gpg: keyserver receive failed: keyserver error

@LaurentGoderre
Copy link
Member Author

For mine it's not the key, it fails the build around libuv...

@LaurentGoderre
Copy link
Member Author

@LaurentGoderre
Copy link
Member Author

This is very unfortunate....the Travis Build doesn't have enough time to build all the alpine variants. Maybe it should be matrixed?

@Vanuan
Copy link

Vanuan commented Apr 30, 2016

So, can we build it for alpine one-time, put it somewhere, and then download it in the Dockerfile?

@Vanuan
Copy link

Vanuan commented Apr 30, 2016

As I can see, there's always the need to have 2 Dockerfiles: one for building, another for using it.
The missing link is a mechanism of providing assets from the "build" image to the "production" image.
Currently, distributions's repositories (or custom urls) are used for that.
Alpine doesn't seem to support multiple version of the same package: http://dl-3.alpinelinux.org/alpine/edge/main/x86_64/
So custom url should be used.

I.e., prebuilt images should be placed to https://nodejs.org/dist/
Maybe under "node-$VERSION-linux-musl-$ARCH.tar.gz

@LaurentGoderre
Copy link
Member Author

I like that idea @Vanuan !

@LaurentGoderre LaurentGoderre force-pushed the alpine branch 2 times, most recently from 1e6fcb9 to 611588d Compare November 9, 2016 19:35
@LaurentGoderre
Copy link
Member Author

I did the v4.6.2 update

@chorrell
Copy link
Contributor

chorrell commented Nov 9, 2016

Yes, 7.1 would replace 7.0

@LaurentGoderre LaurentGoderre force-pushed the alpine branch 4 times, most recently from 5722c3a to 1d6ffe1 Compare November 10, 2016 01:56
@LaurentGoderre
Copy link
Member Author

Ok, after a lot of rebasing, I updated to what it should be

@LaurentGoderre
Copy link
Member Author

The build passed even if it's not showing up here again

@chorrell
Copy link
Contributor

Yeah, the builds look good even though the status in the PR still shows it's pending:

https://travis-ci.org/nodejs/docker-node/builds/174661216

I think we're good to merge this now :)

@chorrell chorrell merged commit 4ac0faf into nodejs:master Nov 10, 2016
@chorrell
Copy link
Contributor

I'll work on the PR for the docker hub

@LaurentGoderre
Copy link
Member Author

Wooohooo! Thanks so much for working with me to get this in!

@shouze
Copy link

shouze commented Dec 7, 2016

@LaurentGoderre a decent icu is missing in this image. Ok you've compiled node exactly like the official way node binaries are distributed so that's great... but when you have to deal with intl/icu in some node apps (this is my case in many apps) this is pain in the ass to deal with full-icu package since a long time... and things don't get better with things like yarn for example as you can see yarnpkg/yarn#2124.

So... I suggest to just add https://github.com/rezzza/docker-node/blob/master/6/Dockerfile#L24 and https://github.com/rezzza/docker-node/blob/master/6/Dockerfile#L48 in official alpine (and non alpine maybe, I don't use them) containers. Ok the generated container will be a little bigger but I guess it's a good tradeoff until some day full-icu will get fixed.

On my side I can keep going building my own images - I've done that since at least 1 year - but I doubt to be the only guy in that situation and bringing a decent solution to everyone would be nice no?

@LaurentGoderre
Copy link
Member Author

@shouze the lines referred don't seem to work for me? One point to a GPG key, the other to nowhere. In any case, could we discuss this in a new issue so that others may chime in? I don't like having discussions in closed PRs

@shouze
Copy link

shouze commented Dec 7, 2016

@shouze
Copy link

shouze commented Dec 7, 2016

The resulting container is around 27 MB (in contrast with the 18 MB alpine one ATM).

@LaurentGoderre
Copy link
Member Author

I'm not opposed to it but could you create an issue for this to get comments from others?

@shouze
Copy link

shouze commented Dec 7, 2016

@LaurentGoderre yes of course #286 feel free to tag it question

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.