Use setup-emsdk instead of emscripten dockerfile in samples.yml. #18422

ScottTodd · 2024-09-03T21:52:37Z

Also delete the now unused emscripten.Dockerfile (it's technically still referenced in a commented out cross_compile_and_test build, but that can be added back as needed using this same technique).

Progress on #15332 - one less Dockerfile to maintain.

Tested here: https://github.com/ScottTodd/iree/actions/runs/10691087132/job/29636852886 (this samples.yml workflow runs on a nightly schedule).

skip-ci: no impact on other workflows

saienduri

lgtm

saienduri · 2024-09-03T22:01:39Z

.github/workflows/samples.yml

@@ -61,7 +61,6 @@ jobs:

  web:
    runs-on: ubuntu-20.04
-    container: gcr.io/iree-oss/emscripten@sha256:2dd4c52f1bb499ab365aad0111fe5538b685d88af38636b409b0cf6a576ab214
    env:
      VENV_DIR: ${{ github.workspace }}/.venv


might be cleaner to add env variables here in future btw

Trueee... I could simplify the various places this is done, like

iree/.github/workflows/pkgci_test_android.yml

Lines 64 to 69 in 3bdd909

- name: Install build dependencies

run: |

sudo apt update

sudo apt install -y ninja-build

echo "CC=clang" >> $GITHUB_ENV

echo "CXX=clang++" >> $GITHUB_ENV

Do we prefer one over the other? Most workflows I've looked in seem to follow https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables#defining-environment-variables-for-a-single-workflow and not https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#setting-an-environment-variable but I don't know what we should prefer.

ScottTodd · 2024-09-03T22:05:08Z

build_tools/docker/dockerfiles/emscripten.Dockerfile

-# Emscripten writes into its cache location (outside of the CMake build
-# directory).
-# We can either
-#   (A) Grant broad write permissions to the cache directory to be able to run
-#       our scripts under different users.
-#   (B) Mount a user home directory when using the image.
-# Since (A) requires less configuration, we'll do that. If multiple tools would
-# want a user directory (like Bazel), we should switch to (B).
-# See https://github.com/emscripten-core/emsdk/issues/535
-RUN mkdir -p "${EM_CACHE}" && chmod -R 777 "${EM_CACHE}"
-
-# Normally we'd run `source emsdk_env.sh`, but that doesn't integrate with
-# Docker's environment properties model. Instead, we directly extend the path
-# to include the directories suggested by `emsdk activate`.
-ENV PATH="${EMSDK}:${EMSDK}/upstream/emscripten:$PATH"


Good riddance to these hacks 🥳

Follow-up to #18422 (comment) and #18421 (comment). This style is more compact, but it does remove some context that could be useful - that the CC/CXX environment variables are for the build toolchain and are related to CMake and Ninja. The 'runtime' workflow is cross platform and also sets this conditionally: https://github.com/iree-org/iree/blob/1124be8025995b64804e9f02b62a0354b25dfb2b/.github/workflows/ci.yml#L112-L124 See https://github.com/iree-org/iree-template-runtime-cmake/blob/main/.github/workflows/build-and-test.yml for a more comprehensive example where we also branch on gcc and clang in the same matrix.

…e-org#18422) Also delete the now unused `emscripten.Dockerfile` (it's technically still referenced in a commented out `cross_compile_and_test` build, but that can be added back as needed using this same technique). Progress on iree-org#15332 - one less Dockerfile to maintain. Tested here: https://github.com/ScottTodd/iree/actions/runs/10691087132/job/29636852886 (this samples.yml workflow runs on a nightly schedule). skip-ci: no impact on other workflows

…org#18425) Follow-up to iree-org#18422 (comment) and iree-org#18421 (comment). This style is more compact, but it does remove some context that could be useful - that the CC/CXX environment variables are for the build toolchain and are related to CMake and Ninja. The 'runtime' workflow is cross platform and also sets this conditionally: https://github.com/iree-org/iree/blob/1124be8025995b64804e9f02b62a0354b25dfb2b/.github/workflows/ci.yml#L112-L124 See https://github.com/iree-org/iree-template-runtime-cmake/blob/main/.github/workflows/build-and-test.yml for a more comprehensive example where we also branch on gcc and clang in the same matrix.

…e-org#18422) Also delete the now unused `emscripten.Dockerfile` (it's technically still referenced in a commented out `cross_compile_and_test` build, but that can be added back as needed using this same technique). Progress on iree-org#15332 - one less Dockerfile to maintain. Tested here: https://github.com/ScottTodd/iree/actions/runs/10691087132/job/29636852886 (this samples.yml workflow runs on a nightly schedule). skip-ci: no impact on other workflows

…org#18425) Follow-up to iree-org#18422 (comment) and iree-org#18421 (comment). This style is more compact, but it does remove some context that could be useful - that the CC/CXX environment variables are for the build toolchain and are related to CMake and Ninja. The 'runtime' workflow is cross platform and also sets this conditionally: https://github.com/iree-org/iree/blob/1124be8025995b64804e9f02b62a0354b25dfb2b/.github/workflows/ci.yml#L112-L124 See https://github.com/iree-org/iree-template-runtime-cmake/blob/main/.github/workflows/build-and-test.yml for a more comprehensive example where we also branch on gcc and clang in the same matrix.

ScottTodd added 2 commits September 3, 2024 14:41

Use setup-emsdk instead of emscripten dockerfile in samples.yml.

6a73ecb

Delete emscripten.Dockerfile.

104ad6b

ScottTodd added infrastructure Relating to build systems, CI, or testing cleanup 🧹 platform/web 🌐 Web-specific build, execution, benchmarking, and deployment labels Sep 3, 2024

ScottTodd requested review from marbre and saienduri September 3, 2024 21:55

saienduri approved these changes Sep 3, 2024

View reviewed changes

saienduri reviewed Sep 3, 2024

View reviewed changes

ScottTodd commented Sep 3, 2024

View reviewed changes

ScottTodd merged commit 1124be8 into iree-org:main Sep 3, 2024
23 checks passed

ScottTodd deleted the infra-web-docker branch September 3, 2024 22:05

This was referenced Sep 3, 2024

Drop Docker and remote ccache usage from publish_website.yml. #18421

Merged

Set CC and CXX environment variables using 'env' in workflows. #18425

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use setup-emsdk instead of emscripten dockerfile in samples.yml. #18422

Use setup-emsdk instead of emscripten dockerfile in samples.yml. #18422

ScottTodd commented Sep 3, 2024

saienduri left a comment

saienduri Sep 3, 2024

ScottTodd Sep 3, 2024

marbre Sep 3, 2024 •

edited

Loading

ScottTodd Sep 3, 2024

	- name: Install build dependencies
	run: \|
	sudo apt update
	sudo apt install -y ninja-build
	echo "CC=clang" >> $GITHUB_ENV
	echo "CXX=clang++" >> $GITHUB_ENV

Use setup-emsdk instead of emscripten dockerfile in samples.yml. #18422

Use setup-emsdk instead of emscripten dockerfile in samples.yml. #18422

Conversation

ScottTodd commented Sep 3, 2024

saienduri left a comment

Choose a reason for hiding this comment

saienduri Sep 3, 2024

Choose a reason for hiding this comment

ScottTodd Sep 3, 2024

Choose a reason for hiding this comment

marbre Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

ScottTodd Sep 3, 2024

Choose a reason for hiding this comment

marbre Sep 3, 2024 •

edited

Loading