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

refactor(dockerfile): set shell options globally #32

Closed
wants to merge 1 commit into from

Conversation

guoard
Copy link

@guoard guoard commented Jan 5, 2024

This pull request aims to improve the consistency and reliability of Dockerfiles by introducing a global SHELL directive and setting options globally. Additionally, it introduces a new option, pipefail, to enhance error detection and prevent potential issues during the build process.

@joseluisq
Copy link
Owner

joseluisq commented Jan 5, 2024

Please help me to understand your changes. Here are my concerns.

  • Why ash and not bash?
  • I'm wondering if using a global SHELL will not interfere when users want to extend this image.
    I mean, if -euxo pipefail options will not be applied by default. If so, then some users could run into more script strictness that I do not know users will want. Subsequently, did you think that if this could break existing Dockerfiles by some users?

@guoard
Copy link
Author

guoard commented Jan 5, 2024

  1. By default Alpine Linux uses the busybox variant of the ash shell. you can check root user shell with this command:
docker run --rm alpine ash -c "cat /etc/passwd | grep root"
  1. Yes, The -euxo pipefail options are commonly used for strict error handling and debugging in shell scripts. While they can be helpful, they may indeed introduce unexpected behavior for users who extend your image.

What are your thoughts on adding the following line into the Dockerfile to leverage the pipefail option:

SHELL ["/bin/ash", "-o", "pipefail", "-c"]

@joseluisq
Copy link
Owner

joseluisq commented Jan 5, 2024

  1. By default Alpine Linux uses the busybox variant of the ash shell. you can check root user shell with this command:

Yes, you are right. I just forgot it for a while when I was writing the previous comment.
But just to be sure. Should be /bin/ash or /bin/sh? What I see everywhere is /bin/sh when using Alpine.

3. Yes, The -euxo pipefail options are commonly used for strict error handling and debugging in shell scripts. While they can be helpful, they may indeed introduce unexpected behavior for users who extend your image.

What are your thoughts on adding the following line into the Dockerfile to leverage the pipefail option:

SHELL ["/bin/ash", "-o", "pipefail", "-c"]

I do not you but I'm happy with having RUN set -euxo pipefail && ... per Docker RUN command and getting out of users' way.

@guoard
Copy link
Author

guoard commented Jan 5, 2024

But just to be sure. Should be /bin/ash or /bin/sh?

Yes, By default, both the BusyBox and Alpine Docker images include the Almquist Shell (/bin/ash). It's a lightweight fork of Bourne shell (/bin/sh) that aims to be minimal and fast.

I'm happy with having RUN set -euxo pipefail && ... per Docker RUN command and getting out of users' way.

Ok, I will try open another pull request.

@guoard guoard closed this Jan 5, 2024
@guoard guoard deleted the refactor/dockerfile branch January 5, 2024 13:32
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