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]