ctubbsii commented on code in PR #384: URL: https://github.com/apache/accumulo-website/pull/384#discussion_r1175935000
########## Dockerfile: ########## @@ -0,0 +1,30 @@ +FROM ruby:2.7.8-slim-bullseye as base + +RUN apt-get update && apt-get install -y --no-install-recommends \ + build-essential \ + git \ + curl \ + && rm -rf /var/lib/apt/lists/* + +WORKDIR /site + +COPY Gemfile /site/Gemfile +COPY Gemfile.lock /site/Gemfile.lock + +RUN gem update --system && bundle install && gem cleanup Review Comment: > Docker is operating on it's own separate filesystem layer so nothing installed in the container will affect anything on the given system. This accomplishes the same separation as setting `GEM_HOME` to a user directory. My main concern isn't separation between the container filesystem and the local filesystem, but the user being able to comprehend the analogous behavior inside the container to the local system. Specifically, in the local system, there is a separation between the OS's package manager and the gem package management by setting `GEM_HOME` so that the gem package manager stores stuff in a different location than the locations where RPM or DEB packages get installed. This separation is comprehensible in the README because `GEM_HOME` is explicitly set. It's not clear how this separation occurs inside the container. > > If you're interested, the [Ruby image spec](https://hub.docker.com/_/ruby) calls out various environment variables that are set in the image, including `GEM_HOME` which is set to `/usr/local/bundle`. Perfect. In that case, I think all that's needed to help connect the behavior of this container to the instructions in the README is to add a simple comment above the gem commands that mention something like: `GEM_HOME is set by the container`. You probably don't even need to specify that it's `/usr/local/bundle`, because that could change. Just knowing that it's already set should be enough to connect one's understanding of the README to what's going on in the container. > > The explicit `gem install bundler` command isn't needed as the Gemfile.lock requires Ruby 2.7 to be the minimum installed and bundler was added as a default gem in 2.6. Not really relevant for this PR, but just to highlight this challenge for some: Annoyingly, in Fedora 37, even though ruby.rpm is 3.1 and rubygems.rpm is 3.3, rubygems-bundler.rpm still ships in a separate optional RPM, even though it's supposed to be a default gem since 2.6 and should be included with rubygems.rpm. I think there was a discussion about fixing this, but I lost track of the discussion. > > So, I don't see this diverging from the detailed readme instructions as `GEM_HOME` is already set in the container and the next command to run is `bundle install`. Now that I understand what it's doing (thanks for digging in and explaining it), I agree it's not different. I think just a simple comment can help connect the dots for future devs trying to figure this out. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
