Skip to content
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.

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

Allow run_shell_setup during Docker build #312

Merged
merged 8 commits into from
Nov 20, 2024

Conversation

jabez007
Copy link
Contributor

@jabez007 jabez007 commented Nov 15, 2024

In reference to this issue
#311

The idea is to use an ARG in the Dockerfile as a flag to indicate that we are in a build so that the install.sh will still execute run_shell_setup, and since we are in a Docker build pass the option to run_shell_setup so that the prompts are just answered with their defaults.

an example Dockerfile then would look something like

FROM debian:stable-slim

# Define a build argument to indicate the Docker build context
ARG DOCKER_BUILD=true

RUN apt-get update && \
  apt-get install -y \
  bash \
  curl \
  unzip

ENV SHELL="/bin/bash"

#RUN curl -fsSL https://deno.land/install.sh | sh
RUN curl -fsSL https://raw.githubusercontent.com/jabez007/deno_install/refs/heads/fix-docker/install.sh | sh

CMD ["/bin/bash"]

@nathanwhit
Copy link
Member

Hello, thanks for the PR!

I think instead of using an ARG and special casing docker builds, we should check if the user passed in -y or --yes, and if so run the shell setup regardless of whether it's an interactive terminal.

The steps for that would be roughly:

  1. Update the arg parsing here to check for -y/--yes, and if it's present set some var (should_run_shell_setup or something)
  2. Update the condition to be if { [ -z "$CI" ] && [ -t 1 ]; } || $should_run_shell_setup;

That way you could install deno in a dockerfile by just passing -y, so it would be consistent across environments instead of being dockerfile specific

@jabez007
Copy link
Contributor Author

So then the example Dockerfile would be something like

FROM debian:stable-slim

RUN apt-get update && \
  apt-get install -y \
  bash \
  curl \
  unzip

ENV SHELL="/bin/bash"

#RUN curl -fsSL https://deno.land/install.sh | sh -s -- -y
RUN curl -fsSL https://raw.githubusercontent.com/jabez007/deno_install/refs/heads/fix-docker/install.sh | sh -s -- -y

CMD ["/bin/bash"]

Copy link
Member

@nathanwhit nathanwhit left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@nathanwhit nathanwhit merged commit 7cc82d6 into denoland:master Nov 20, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants