Build docker image on Circle CI by Squareys · Pull Request #513 · emscripten-core/emsdk

Squareys · 2020-06-03T16:46:56Z

As per emscripten-core/emscripten#11078 and #372.

@trzecieu @sbc100
Very very rough draft for building the docker image on Circle CI.

TODO

Build docker image
Properly specify the tag(s) for docker hub
Trigger build only on ~~emscripten-releases-tags.txt changes.~~ tags
- Double check that this is really just triggering on tags.
What's the process of getting the docker hub deploy key? They should be specified through secret variables in Circle CI and probably could be done as a final step by one of the project maintainers.
~~Build various variations of the image? (i.e. I remember emscripten-slim and a ubuntu based version in trzeci's original images.)~~

sbc100

Very excited to see progress here.

sbc100 · 2020-06-03T18:41:46Z

.circleci/config.yml

+      - checkout
+      - run: docker build -t emscripten-core/emsdk:$CIRCLE_BRANCH docker/
+      - run: |
+          echo "$DOCKER_PASS" | docker login --username $DOCKER_USER --password-stdin


Is this the recommended way to authenticate the push ? I wonder if we can pass login credentials directly to the push command somehow?

This is from the CircleCI docs, yes. It is definitely possible to pass the password directly:

docker login -u "$DOCKER_USER" -p "$DOCKER_PASS" ...

Pattern from the CircleCI docs, seems fine

Looks odd, but also I've found that the recommended - more secure way for login.

Error: Cannot perform an interactive login from a non TTY device

Looks like --password-stdin is not accepted here anyways? 🤔

trzecieu · 2020-06-04T00:52:19Z

.circleci/config.yml

+    machine: true
+    steps:
+      - checkout
+      - run: docker build -t emscripten-core/emsdk:$CIRCLE_BRANCH docker/


We do have docker.io organization called emscripten/emsdk.
I don't know who manages this organisation, looks like @haraldreingruber has access to another project emscripten/emscripten-ci - maybe he knows more :)

I seem to remember deciding on emscripten/emsdk for this image, right? Or am I remembering wrong?

Most likely you're right. And looking at the name of this repo it makes sense.
In this case there should be a following action on https://hub.docker.com/orgs/emscripten

The org (https://hub.docker.com/orgs/emscripten) was created by @kripken, right?
It's odd, but I don't see any other members...

Squareys · 2020-06-04T06:40:59Z

Trigger build only on emscripten-releases-tags.txt changes.

@sbc100 It looks like whenever this file changes, emsdk is tagged. Could we instead make this job trigger on tags instead? That would be very straight forward to achieve.

*edit: I implemented this suggestion now. Unsure whether this works, though, since CircleCI seems a bit confusing in this regard and the job has been triggered by my last push, despite not being a tag.

Squareys · 2020-06-04T14:08:00Z

From my side this is pretty much ready, what's left is:

Ensure this is running only on tags (I set up a filter which should make the job ignored for all branches, but included for all tags). The $CIRCLE_TAG variable is only set for tags, hence it fails for this PR.
Set DOCKER_PASS and DOCKER_USER secret variables in the Circle CI settings after merge.

So, this is ready to be merged from my side.

.circleci/config.yml

Squareys · 2020-06-05T14:05:11Z

@sbc100 I fixed or commented on the issues you found. Tell me if there is anything else I can do.

sbc100 · 2020-06-05T19:17:43Z

lgtm.

What are the next steps? It looks like the CI is currently failing... and I guess we need to figure out how to inject the credentials?

Squareys · 2020-06-05T20:10:25Z

Injecting the credentials should be done after merge by someone who has access to the Circle CI backend and the docker hub namespace (@kripken? See #513 (comment)).

The CI is failing because it is not running for a tag, hence CIRCLE_TAG variable is missing aswell. Once the PR is merged, it will only run on tags.

Also, I guess this needs to be run once for all the existing previous tags. Probably sufficient to do that manually.

sbc100 · 2020-06-05T21:24:42Z

The CI is failing because it is not running for a tag, hence CIRCLE_TAG variable is missing aswell. Once the PR is merged, it will only run on tags.

It seems a little odd to me that this would run on this PR at all.. if its only supposed to run on tags.. why is it running here?

Signed-off-by: Squareys <[email protected]>

Squareys · 2020-06-05T22:20:11Z

It seems a little odd to me that this would run on this PR at all.. if its only supposed to run on tags.. why is it running here?

Uh, I figured it out, it needed 2 more spaces of indentation...

sbc100 reviewed Jun 3, 2020

View reviewed changes

trzecieu reviewed Jun 4, 2020

View reviewed changes

Squareys force-pushed the circle-ci-build-docker-image branch 9 times, most recently from 1fbc527 to 648f665 Compare June 4, 2020 14:00

Squareys marked this pull request as ready for review June 4, 2020 14:01

Squareys force-pushed the circle-ci-build-docker-image branch from 648f665 to fa7c2d9 Compare June 4, 2020 14:03

Squareys requested a review from sbc100 June 4, 2020 14:08

Squareys changed the title ~~WIP: Build docker image on Circle CI~~ Build docker image on Circle CI Jun 4, 2020

sbc100 reviewed Jun 4, 2020

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

.circleci/config.yml Show resolved Hide resolved

.circleci/config.yml Show resolved Hide resolved

Squareys force-pushed the circle-ci-build-docker-image branch from fa7c2d9 to 2774148 Compare June 4, 2020 19:56

Squareys requested a review from sbc100 June 5, 2020 08:58

Squareys force-pushed the circle-ci-build-docker-image branch 2 times, most recently from 1705baa to bff9fe3 Compare June 5, 2020 22:15

Set up build-docker-image job build docker image for tags on Circle CI

43b221d

Signed-off-by: Squareys <[email protected]>

Squareys force-pushed the circle-ci-build-docker-image branch from bff9fe3 to 43b221d Compare June 5, 2020 22:17

sbc100 merged commit 24d8848 into emscripten-core:master Jun 5, 2020

Squareys mentioned this pull request Jun 6, 2020

Followup to #368 - Emscripten Docker Image #372

Closed

trzecieu mentioned this pull request Jun 7, 2020

Advertise the official emscripten image trzecieu/emscripten-docker#69

Open

yoshihitoh mentioned this pull request Jul 7, 2020

[contrib] Fix single-file compilation error on Emscripten build. facebook/zstd#2227

Merged

Conversation

Squareys commented Jun 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbc100 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Squareys commented Jun 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Squareys commented Jun 4, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Squareys commented Jun 5, 2020

Uh oh!

sbc100 commented Jun 5, 2020

Uh oh!

Squareys commented Jun 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbc100 commented Jun 5, 2020

Uh oh!

Squareys commented Jun 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Squareys commented Jun 3, 2020 •

edited

Loading

Squareys commented Jun 4, 2020 •

edited

Loading

Squareys commented Jun 5, 2020 •

edited

Loading