Skip to content

Docker improvements #571

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Sep 2, 2020
Merged

Docker improvements #571

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da0432a
Docker improvements
kleisauke Aug 1, 2020
1274e7a
Remove check for asm2wasm
kleisauke Aug 22, 2020
bbcc24c
Merge remote-tracking branch 'upstream/master' into docker-improvements
kleisauke Aug 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,13 @@ jobs:
name: install docker
command: apt-get update -q && apt-get install -q -y docker.io
- setup_remote_docker
# Build and test the tip-of-tree build of EMSDK
- run:
name: build
command: docker build --network host ./docker
command: make -C ./docker version=tot build
- run:
name: test
command: make -C ./docker version=tot test

publish-docker-image:
executor: bionic
Expand All @@ -109,16 +113,15 @@ jobs:
- setup_remote_docker
- run:
name: build
command: docker build --network host --build-arg EMSCRIPTEN_VERSION=${CIRCLE_TAG} --tag emscripten/emsdk:${CIRCLE_TAG} ./docker
command: make -C ./docker version=${CIRCLE_TAG} build
- run:
name: tag image
command: docker tag emscripten/emsdk:${CIRCLE_TAG} emscripten/emsdk:latest
name: test
command: make -C ./docker version=${CIRCLE_TAG} test
- run:
name: push image
command: |
docker login -u "$DOCKER_USER" -p "$DOCKER_PASS"
docker push emscripten/emsdk:${CIRCLE_TAG}
docker push emscripten/emsdk:latest
make -C ./docker version=${CIRCLE_TAG} alias=latest push

workflows:
flake8:
Expand Down
25 changes: 25 additions & 0 deletions .dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Ignore all subdirectories
*/*

# Allow to run the test script inside the Docker container
!/docker/test_dockerimage.sh

# Allow the Dockerfile for future re-creation/reference
!/docker/Dockerfile

# Ignore unnecessary files inside top-level directory
*.bat
*.csh
*.fish
*.ps1
*.pyc
.emscripten
.emscripten.old
.emscripten_cache
.emscripten_cache__last_clear
.emscripten_sanity
.emscripten_sanity_wasm
.flake8
emscripten-releases-tot.txt
legacy-*-tags.txt
README.md
20 changes: 4 additions & 16 deletions docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
FROM debian:buster AS stage_build

ARG EMSCRIPTEN_VERSION=1.39.20
ARG EMSCRIPTEN_VERSION=tot
ENV EMSDK /emsdk

# ------------------------------------------------------------------------------
Expand All @@ -18,12 +18,12 @@ RUN echo "## Start building" \
python3-pip \
&& echo "## Done"

RUN echo "## Get EMSDK" \
&& git clone https://github.com/emscripten-core/emsdk.git ${EMSDK} \
&& echo "## Done"
# Copy the contents of this repository to the container
COPY . ${EMSDK}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I my version of this change I used ADD . /emsdk: https://github.com/emscripten-core/emsdk/compare/docker_use_local_emsdk?expand=1#diff-ebacf6f6ae4ee68078bb16454b23247dR26

I guess they are the same .. i can't remember why I chose one over the other.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't make much difference: https://stackoverflow.com/a/24958548/10952119. I think COPY is fine here.


RUN echo "## Install Emscripten" \
&& cd ${EMSDK} \
&& if [ "$EMSCRIPTEN_VERSION" = "tot" ]; then ./emsdk update-tags; fi \
&& ./emsdk install ${EMSCRIPTEN_VERSION} \
&& echo "## Done"

Expand Down Expand Up @@ -115,18 +115,6 @@ RUN echo "## Update and install packages" \
&& rm -rf /usr/share/man/??_* \
&& echo "## Done"

# ------------------------------------------------------------------------------
# Internal test suite of tools that this image provides
COPY test_dockerimage.sh /emsdk/

RUN echo "## Internal Testing of image" \
&& /emsdk/test_dockerimage.sh \
&& echo "## Done"

# ------------------------------------------------------------------------------
# Copy this Dockerimage into image, so that it will be possible to recreate it later
COPY Dockerfile /emsdk/dockerfiles/emscripten-core/emsdk/

# ------------------------------------------------------------------------------
# Use commonly used /src as working directory
WORKDIR /src
Expand Down
21 changes: 14 additions & 7 deletions docker/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
#!/usr/bin/env make
# A Makefile to build, test, tag and publish the Emscripten SDK Docker container.

# Emscripten version to build: Should match the version that has been already released.
# i.e.: 1.38.45, 1.38.45-upstream
# i.e.: 1.39.18
version =
alias =

image_name ?= emscripten/emsdk

.TEST:
ifndef version
$(error argument 'version' is not set. Please call `make version=SOME_VERSION ...`)
endif

build: .TEST
docker build --network host --build-arg=EMSCRIPTEN_VERSION=${version} --tag emscripten/emsdk:${version} .
build: Dockerfile .TEST
cd .. && docker build --network host --build-arg=EMSCRIPTEN_VERSION=${version} -t ${image_name}:${version} -f docker/$< .

test: test_dockerimage.sh .TEST
# test as non-root
docker run --rm -u `id -u`:`id -g` -w /emsdk/docker --net=host ${image_name}:${version} \
bash $<

push: .TEST
docker push emscripten/emsdk:${version}
docker push ${image_name}:${version}
ifdef alias
docker tag emscripten/emsdk:${version} emscripten/emsdk:${alias}
docker push emscripten/emsdk:${alias}
docker tag ${image_name}:${version} ${image_name}:${alias}
docker push ${image_name}:${alias}
endif
47 changes: 18 additions & 29 deletions docker/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Dockerfile for EMSDK

This Dockerfile builds a self-contained version of emsdk that enables emscripten to be used without any
This Dockerfile builds a self-contained version of Emscripten SDK that enables Emscripten to be used without any
other installation on the host system.

It is published at https://hub.docker.com/u/emscripten/emsdk
Expand All @@ -23,7 +23,7 @@ EOF
# compile with docker image
docker run \
--rm \
-v $(pwd):$(pwd) \
-v $(pwd):/src \
-u $(id -u):$(id -g) \
emscripten/emsdk \
emcc helloworld.cpp -o helloworld.js
Expand Down Expand Up @@ -51,7 +51,7 @@ This image requires to specify following build arguments:

| arg | description |
| --- | --- |
| `EMSCRIPTEN_VERSION` | One of released version of Emscripten. For example `1.38.45`<br/> Can be used with `-upstream` variant like: `1.38.45-upstream`<br /> Minimal supported version is **1.38.40**|
| `EMSCRIPTEN_VERSION` | One of released version of Emscripten. For example `1.39.17`<br/> Minimal supported version is **1.39.0**|

**Building**

Expand All @@ -60,13 +60,14 @@ This step will build Dockerfile as given tag on local machine
# using docker
docker build \
--network host \
--build-arg=EMSCRIPTEN_VERSION=1.38.43-upstream \
--tag emscripten/emsdk:1.38.43-upstream \
--build-arg=EMSCRIPTEN_VERSION=1.39.17 \
-t emscripten/emsdk:1.39.17 \
-f docker/Dockerfile \
.
```
```bash
# using predefined make target
make version=1.38.43-upstream build
make version=1.39.17 build test
```

**Tagging**
Expand All @@ -79,35 +80,24 @@ This step will take local image and push to default docker registry. You need to

```bash
# using docker
docker push emscripten/emsdk:1.38.43-upstream
docker push emscripten/emsdk:1.39.17
```
```bash
# using predefined make target
make version=1.38.43-upstream push
make version=1.39.17 push
```

In case of pushing the most recent version, this version should be also tagged as `latest` or `latest-upstream` and pushed.
In case of pushing the most recent version, this version should be also tagged as `latest` and pushed.
```bash
# using docker cli

# in case of fastcomp variant (default backend)
docker tag emscripten/emsdk:1.38.43 emscripten/emsdk:latest
docker tag emscripten/emsdk:1.39.17 emscripten/emsdk:latest
docker push emscripten/emsdk:latest

# in case of upstream variant
docker tag emscripten/emsdk:1.38.43-upstream emscripten/emsdk:latest-upstream
docker push emscripten/emsdk:latest-upstream

```

```bash
# using predefined make target

make version=1.38.43-upstream alias=latest-upstream push

# using make
make version=1.39.17 alias=latest push
```


### Extending

If your project uses packages that this image doesn't provide you might want to:
Expand All @@ -117,20 +107,19 @@ If your project uses packages that this image doesn't provide you might want to:
1. create own Dockerfile that holds:
```dockerfile
# Point at any base image that you find suitable to extend.
FROM emscripten/emsdk:1.38.25
FROM emscripten/emsdk:1.39.17

# Install required tools that are useful for your project i.e. ninja-build
RUN apt update && apt install -y ninja-build

```

2. build it
```shell
```bash
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this make a difference anywhere?

Copy link
Collaborator Author

@kleisauke kleisauke Jul 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't make any difference, bash is an alias for shell within GitHub flavored markdown. I think @trzecieu changed this to align it with the other code blocks.

docker build -t extended_emscripten .
```

3. test
```shell
```bash
docker run --rm extended_emscripten ninja --version
# Python 2.7.16
# 1.10.0
```

21 changes: 16 additions & 5 deletions docker/test_dockerimage.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
#!/bin/bash
#!/usr/bin/env bash
set -ex

sudo -u nobody `which emcc` --version
if [ $EUID -eq 0 ]; then
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this check is needed for Podman (which is the default container engine on Fedora 32). Podman provides a Docker-compatible command line front end that can simply alias the Docker cli (alias docker=podman).

Without this check, it will fail to perform the tests (probably because Podman is running in rootless mode).

Details 
$ make -C ./docker version=tot test
make: Entering directory '/home/kleisauke/emsdk/docker'
# test as non-root
docker run --rm -u `id -u`:`id -g` -w /emsdk/docker --net=host emscripten/emsdk:tot \
	bash test_dockerimage.sh
++ which emcc
+ sudo -u nobody /emsdk/upstream/emscripten/emcc --version
sudo: unable to resolve host pc-kaw: Name or service not known

We trust you have received the usual lecture from the local System
Administrator. It usually boils down to these three things:

    #1) Respect the privacy of others.
    #2) Think before you type.
    #3) With great power comes great responsibility.

sudo: no tty present and no askpass program specified
make: *** [Makefile:20: test] Error 1
make: Leaving directory '/home/kleisauke/emsdk/docker'

sudo -u nobody `which emcc` --version
fi

which asm2wasm
which llvm-ar
which emsdk
node --version
Expand All @@ -15,5 +16,15 @@ emcc --version
java -version
cmake --version

# cleanup after test
find ${EMSDK} -name "*.pyc" -exec rm {} \;
exit_code=0

# test emcc compilation
echo 'int main() { return 0; }' | emcc -o /tmp/main.js -xc -
node /tmp/main.js || exit_code=$?
if [ $exit_code -ne 0 ]; then
echo "Node exited with non-zero exit code: $exit_code"
exit $exit_code
fi

# test embuilder
embuilder build zlib --force
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I under standstand now, the cleanup was being done not inside the docker image but here in the docker subdirectory.

In that I case I think the old way was better. No need to use /tmp just write the files here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I cannot mount volumes using the CircleCI Docker executor, writing the compiled output to the docker subdirectory is not possible. Would you rather use the CircleCI Docker executor (current state) or write to the compiled output to the docker subdirectory (and therefore using the machine executor)?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I think its fine to use the /tmp directory inside the container assuming that is what is happening? Since that directory will be destroyed once the container finished running right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. The /tmp is only used inside the container and then cleaned up afterwards once the container is finished (thanks to the --rm parameter).