Skip to content

Docker: avoid sourcing emsdk_env.sh in interactive shells #1220

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

Closed
wants to merge 3 commits into from
Closed

Docker: avoid sourcing emsdk_env.sh in interactive shells #1220

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f1e626
Docker: avoid sourcing `emsdk_env.sh` in interactive shells
kleisauke May 28, 2023
4ef1012
Docker: clarify env variables origin
kleisauke May 31, 2023
b3dc670
Docker: don't set umask to 0000 in interactive shells
kleisauke May 31, 2023
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
7 changes: 2 additions & 5 deletions docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,8 @@ FROM ubuntu:jammy AS stage_deploy

COPY --from=stage_build /emsdk /emsdk

# Fallback in case Emscripten isn't activated.
# This will let use tools offered by this image inside other Docker images
# (sub-stages) or with custom / no entrypoint
# Set up emsdk environment
# This corresponds to the environment variables set during `source ./emsdk_env.sh`
ENV EMSDK=/emsdk \
EMSDK_NODE=/emsdk/node/15.14.0_64bit/bin/node \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seeing this now I think we should remove EMSDK_NODE here.. its not used for anything that I know of.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, EMSDK it only used locally within this file.. is there some way to declare it as local?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed the EMSDK_NODE env with commit f7c22d0.

The EMSDK env is probably still needed since it is also set by the emsdk_env.sh script, I clarified the origin of these env variables with commit 5d2b22c.

PATH="/emsdk:/emsdk/upstream/emscripten:/emsdk/upstream/bin:/emsdk/node/15.14.0_64bit/bin:${PATH}"
Expand All @@ -78,8 +77,6 @@ ENV EMSDK=/emsdk \
RUN echo "## Create emscripten user (1000:1000)" \
&& groupadd --gid 1000 emscripten \
&& useradd --uid 1000 --gid emscripten --shell /bin/bash --create-home emscripten \
&& echo "umask 0000" >> /etc/bash.bashrc \
&& echo ". /emsdk/emsdk_env.sh" >> /etc/bash.bashrc \
&& echo "## Done"

# ------------------------------------------------------------------------------
Expand Down