Copilot commented on code in PR #3826:
URL: https://github.com/apache/avro/pull/3826#discussion_r3448069940


##########
share/docker/Dockerfile:
##########
@@ -99,28 +103,29 @@ RUN set -eux; \
 ENV PATH="/opt/maven/bin:${PATH}"
 
 # Install nodejs
-RUN curl -sSL https://deb.nodesource.com/setup_24.x \
-  | bash \
- && apt-get -qqy install nodejs \
- && apt-get -qqy clean \
+RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
+    --mount=type=cache,target=/var/lib/apt/lists,sharing=locked \
+    --mount=type=cache,target=/root/.npm \
+    curl -sSL https://deb.nodesource.com/setup_24.x | bash \
+ && apt-get -qqy install --no-install-recommends nodejs \
  && npm install -g grunt-cli \
- && npm install -g browserify \
- && npm cache clean --force
+ && npm install -g browserify
 
 # Install PHP
-RUN apt-get -qqy install --no-install-recommends libzstd-dev \
-                                                 libbz2-dev \
-                                                 php \
+RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
+    --mount=type=cache,target=/var/lib/apt/lists,sharing=locked \
+    apt-get -qqy install --no-install-recommends php \
                                                  php-bz2 \

Review Comment:
   This `apt-get install` relies on `/var/lib/apt/lists` already being 
populated (from a previous step). With BuildKit cache mounts, that state can be 
missing when earlier layers are restored from a registry cache or when the 
builder cache is pruned, leading to `Unable to locate package ...`. Add an 
`apt-get update` in this RUN (it will still be fast with the cache mount).



##########
share/docker/Dockerfile:
##########
@@ -178,46 +183,49 @@ 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 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 -LsSf https://astral.sh/uv/0.11.23/install.sh | 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 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 && \
+    gem install bundler --no-document && \
+    cd /tmp/lang/ruby && bundle install

Review Comment:
   The BuildKit bind mount targets `/tmp/share/VERSION.txt`, but the command 
only creates `/tmp/lang/ruby/...` and never ensures `/tmp/share` exists. With 
`--mount=type=bind`, the parent directory typically must exist or the build 
fails at mount time. Create `/tmp/share` before running `bundle install`.



##########
lang/py/pyproject.toml:
##########
@@ -24,10 +24,10 @@ version = "1.13.0"
 requires-python = ">=3.10"
 authors = [{name = "Apache Avro", email = "[email protected]"}]
 keywords = ["avro", "serialization", "rpc"]
-license-files = ["LICENSE.txt"]
+license = "Apache-2.0"
+license-files = ["avro/LICENSE"]

Review Comment:
   `[project].license` is specified as a plain string, but with 
`setuptools.build_meta` this field is expected to follow PEP 621’s schema (a 
table with `text` or `file`). As written, some build backends will reject the 
project metadata and fail packaging. Use a PEP 621-compliant license table 
(e.g., reference the existing `avro/LICENSE` file).



##########
share/docker/Dockerfile:
##########
@@ -178,46 +183,49 @@ 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 install --no-install-recommends mypy \
                                                  python3 \

Review Comment:
   This `apt-get install` can fail if `/var/lib/apt/lists` isn't already 
populated (e.g., when earlier layers come from a registry cache but the 
BuildKit cache mount is empty). Adding `apt-get update` inside this RUN keeps 
it robust while still benefiting from the cache mount.



##########
share/docker/Dockerfile:
##########
@@ -178,46 +183,49 @@ 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 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 -LsSf https://astral.sh/uv/0.11.23/install.sh | 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 install --no-install-recommends ruby ruby-dev

Review Comment:
   This `apt-get install` depends on apt lists having been populated by an 
earlier step. With BuildKit cache mounts, that state may be missing when 
reusing only layer cache (e.g., `--cache-from`) or after cache pruning, leading 
to intermittent build failures. Add an `apt-get update` here as well (the cache 
mount will still keep it fast).



##########
share/docker/Dockerfile:
##########
@@ -138,12 +143,14 @@ RUN mkdir tmp && cd tmp \
  && echo "extension=snappy.so" > 
"/etc/php/${PHP8_VERSION}/cli/conf.d/10-snappy.ini" \
  && cd .. && rm -rf php-ext-snappy \
  && php -m \
- && apt-get -qqy clean
+ && rm -rf /tmp/lang/php
 
 RUN curl -sS https://getcomposer.org/installer | php -- --version=2.8.12 
--install-dir=/usr/local/bin --filename=composer
 
 # Install Perl modules
-RUN apt-get -qqy install --no-install-recommends libcompress-raw-zlib-perl \
+RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
+    --mount=type=cache,target=/var/lib/apt/lists,sharing=locked \
+    apt-get -qqy install --no-install-recommends libcompress-raw-zlib-perl \
                                                  libcpan-uploader-perl \

Review Comment:
   This `apt-get install` assumes `/var/lib/apt/lists` is already populated 
from a previous layer. With cache mounts, that can be empty when building from 
a remote layer cache or after cache pruning, causing package resolution 
failures. Add `apt-get update` in this RUN (the cache mount will still prevent 
repeated downloads).



-- 
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]

Reply via email to