-
Notifications
You must be signed in to change notification settings - Fork 335
Add more defensive mkdir
for GEM_HOME
#463
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question about $GEM_HOME
parent directories.
# adjust permissions of a few directories for running "gem install" as an arbitrary user | ||
RUN mkdir -p "$GEM_HOME" && chmod 1777 "$GEM_HOME" | ||
RUN set -eux; \ | ||
mkdir "$GEM_HOME"; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we expect all parent directories of $GEM_HOME
to exist? Removing the -p
causes mkdir
to fail if the directory already exists, which is desireable, but it also means parent directories are not automatically created, which would be bad if all of $GEM_HOME
's parent directory do not exist in the image layers prior to this RUN
command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like all the Dockerfiles have a mkdir -p /usr/local/etc
in a previous RUN
statement, so this should be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, and Ruby itself ends up in /usr/local
via make install
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should work because of the earlier mkdir -p /usr/local/etc
Changes: - docker-library/ruby@c069afc: Merge pull request docker-library/ruby#463 from infosiftr/mkdir-gem-home - docker-library/ruby@f69cac9: Add more defensive `mkdir` for `GEM_HOME`
Changes: - docker-library/ruby@2e432fc: Update 3.3 to 3.3.4 - docker-library/ruby@c069afc: Merge pull request docker-library/ruby#463 from infosiftr/mkdir-gem-home - docker-library/ruby@f69cac9: Add more defensive `mkdir` for `GEM_HOME`
This drops the
-p
argument inmkdir -p "$GEM_HOME"
to ensure that this validates our assumption that ourGEM_HOME
did not exist before this line and is indeed created (as a new, empty directory) by this line.cc @whalelines (whose idea validating better this was)