Updated make file for docker building to perform extra sanity tests#526
Updated make file for docker building to perform extra sanity tests#526trzecieu wants to merge 3 commits into
Conversation
- `--embeddded` mode is the now the default - embedded cache and ports is also the default - emsdk now fully polulates the cache by default so no need to do that. - always "activate" the SDK avoiding that need things to work without that. - avoid making extra symlinks. - remove python2
|
Nice! Does that mean that the changes in my branch pass the tests and we are good the land it? |
| @@ -11,7 +11,21 @@ ifndef version | |||
| endif | |||
|
|
|||
| build: .TEST | |||
There was a problem hiding this comment.
Hmm .. I just realized that that ".TEST" target here is no used is it? $(error) can just be in column zero and fire when the Makefile is parsed. No need to special ".TEST" rule I think.
Not related to this PR though .
There was a problem hiding this comment.
Not sure if I understand, the .TEST is meant to verify 'arguments' and make sure make is feed with version variable:
make: Entering directory '/home/trzeci/Projects/emsdk/docker'
Makefile:10: *** argument 'version' is not set. Please call `make version=SOME_VERSION ...`. Stop.
make: Leaving directory '/home/trzeci/Projects/emsdk/docker'
4706f76 to
e3df1d9
Compare
| 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 |
There was a problem hiding this comment.
Could we split this into two jobs build-docker-image and publish-docker-image and have build-docker-image run always with no filter.. so we can build and test the image on all PRs?
There was a problem hiding this comment.
(No need to make this part of this PR .
There was a problem hiding this comment.
Sounds good, that will require some experiments to make sure it works like intended, let's make another PR for that.
There was a problem hiding this comment.
I can take care of that if you like.. since it would be nice to verify that docker build and test as part of my PR and this one.
e3df1d9 to
fcea0b6
Compare
They do! : ) |
|
@sbc100 This PR bases on your branch |
14d77da to
447c32f
Compare
|
Do you fancy re-basing on top of master now that other changes have landed? (If not I can cherry-pick into a new PR I guess.. but if you have time that would be great). |
|
Sorry I didn't mean to close this PR. I can't seem to reopen now for some reason. |
|
No worries, I'll create new one anchored to master |
Extension to: #523
I've added tests on top of your branch, so that it will be easier to maintain this image.
CC: @sbc100: