Copilot commented on code in PR #3826:
URL: https://github.com/apache/avro/pull/3826#discussion_r3448143435
##########
share/docker/Dockerfile:
##########
@@ -43,38 +44,41 @@ RUN apt-get -qqy update \
curl \
doxygen \
flex \
- g++ \
- gcc \
git \
gnupg2 \
golang-go \
hugo \
- libboost-all-dev \
+ libboost-dev \
+ libboost-math-dev \
+ libboost-random-dev \
+ libboost-test-dev \
+ libbz2-dev \
libfontconfig1-dev \
libfreetype6-dev \
libglib2.0-dev \
libjansson-dev \
libreadline-dev \
libsnappy-dev \
- libsnappy1v5 \
libssl-dev \
- make \
+ libyaml-dev \
+ libzstd-dev \
openjdk-11-jdk \
openjdk-17-jdk \
openjdk-21-jdk \
perl \
source-highlight \
subversion \
+ tmux \
+ unzip \
Review Comment:
`tmux` is newly added to the base apt package list, but it isn’t mentioned
in the PR description’s change list (which emphasizes image-size reduction). If
it’s not required for builds/tests, consider dropping it to avoid growing the
image; otherwise, the PR description should call out the new dependency
explicitly.
##########
share/docker/Dockerfile:
##########
@@ -178,46 +188,53 @@ RUN case "${BUILDARCH:?}" in \
&& for url in \
https://downloads.python.org/pypy/pypy3.11-v7.3.20-"$pypyarch".tar.bz2 \
https://downloads.python.org/pypy/pypy3.10-v7.3.19-"$pypyarch".tar.bz2 \
- ; do curl -fsSL "$url" | tar -xvjpf -; \
+ ; do curl -fsSL "$url" | tar -xjpf -; \
done \
&& ln -s pypy3.11* pypy3.11 && ln -s pypy3.10* pypy3.10
# Note: python3-distutils was removed with Python 3.12; see here for migration
advise: https://peps.python.org/pep-0632/#migration-advice
-RUN apt-get -qqy install --no-install-recommends mypy \
+RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
+ --mount=type=cache,target=/var/lib/apt/lists,sharing=locked \
+ apt-get -qqy update \
+ && apt-get -qqy install --no-install-recommends mypy \
python3 \
python3.10 \
python3.11 \
python3.12 \
python3.13 \
python3.14 \
python3.14-dev \
- && apt-get -qqy clean \
- && curl -LsSf https://astral.sh/uv/0.10.4/install.sh | sh
+ && curl -fsSL -o /tmp/uv-install.sh https://astral.sh/uv/0.11.23/install.sh \
+ && sh /tmp/uv-install.sh \
+ && rm /tmp/uv-install.sh
# Install Ruby
-RUN apt-get -qqy install ruby-full \
- && apt-get -qqy clean
-RUN mkdir -p /tmp/lang/ruby/lib/avro && mkdir -p /tmp/share
-COPY lang/ruby/* /tmp/lang/ruby/
-COPY share/VERSION.txt /tmp/share/
-RUN gem install bundler --no-document && \
- apt-get install -qqy libyaml-dev && \
- cd /tmp/lang/ruby && bundle install
-
-# Install Rust
-RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y
--default-toolchain 1.73.0
-ENV PATH=$PATH:/root/.cargo/bin/:/root/.local/bin
+RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
+ --mount=type=cache,target=/var/lib/apt/lists,sharing=locked \
+ apt-get -qqy update \
+ && apt-get -qqy install --no-install-recommends ruby ruby-dev
# Install .NET SDK
-RUN cd /opt ; \
- wget https://dot.net/v1/dotnet-install.sh ; \
- bash ./dotnet-install.sh --channel "6.0" --install-dir "/opt/dotnet" ; \
- bash ./dotnet-install.sh --channel "7.0" --install-dir "/opt/dotnet" ; \
- bash ./dotnet-install.sh --channel "8.0" --install-dir "/opt/dotnet" ;
+RUN set -eux && \
+ wget -nv -O /opt/dotnet-install.sh https://dot.net/v1/dotnet-install.sh &&
\
+ bash /opt/dotnet-install.sh --channel "6.0" --install-dir "/opt/dotnet" &&
\
+ bash /opt/dotnet-install.sh --channel "7.0" --install-dir "/opt/dotnet" &&
\
+ bash /opt/dotnet-install.sh --channel "8.0" --install-dir "/opt/dotnet" &&
\
+ rm /opt/dotnet-install.sh
-ENV PATH=$PATH:/opt/dotnet
+ENV PATH=$PATH:/opt/dotnet:/root/.local/bin
# Since we want the JDK21 as a default, we have to re-prepend it to the PATH.
-RUN update-java-alternatives -s "java-1.21.*"
+RUN update-java-alternatives -s "java-1.21.*"
+
+# Install Ruby gems
+RUN --mount=type=bind,source=lang/ruby/Gemfile,target=/tmp/lang/ruby/Gemfile \
+
--mount=type=bind,source=lang/ruby/avro.gemspec,target=/tmp/lang/ruby/avro.gemspec
\
+ --mount=type=bind,source=lang/ruby/Manifest,target=/tmp/lang/ruby/Manifest
\
+ --mount=type=bind,source=share/VERSION.txt,target=/tmp/share/VERSION.txt \
+ --mount=type=cache,target=/usr/local/bundle/cache \
+ mkdir -p /tmp/lang/ruby/lib/avro /tmp/share && \
+ gem install bundler --no-document && \
+ cd /tmp/lang/ruby && bundle install
Review Comment:
The Ruby `bundle install` step only bind-mounts `Gemfile`, `avro.gemspec`,
and `Manifest`, but the gemspec’s `s.files` is derived from `Manifest` and
includes many additional files (e.g., `lib/avro.rb`, `LICENSE`, etc.). Bundler
installs the local gem when `gemspec` is used in the Gemfile, and that install
step can fail if the files listed in the gemspec aren’t present at install
time. Consider staging the full `lang/ruby` tree into a writable temp directory
(so the Gemfile can still write `lib/avro/VERSION.txt`) and cleaning it up
after `bundle install` so the image size benefit remains.
--
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]
