gemini-code-assist[bot] commented on code in PR #38440:
URL: https://github.com/apache/beam/pull/38440#discussion_r3219855206
##########
sdks/java/container/Dockerfile:
##########
@@ -24,22 +24,21 @@ ARG TARGETARCH
ARG pull_licenses
-ADD target/slf4j-api.jar /opt/apache/beam/jars/
-ADD target/slf4j-jdk14.jar /opt/apache/beam/jars/
-ADD target/jcl-over-slf4j.jar /opt/apache/beam/jars/
-ADD target/log4j-over-slf4j.jar /opt/apache/beam/jars/
-ADD target/log4j-to-slf4j.jar /opt/apache/beam/jars/
-ADD target/beam-sdks-java-harness.jar /opt/apache/beam/jars/
-
-# Required to use jamm as a javaagent to get accurate object size measuring
-# COPY fails if file is not found, so use a wildcard for open-module-agent.jar
-# since it is only included in Java 9+ containers
-COPY target/jamm.jar target/open-module-agent.jar /opt/apache/beam/jars/
-
-COPY target/${TARGETOS}_${TARGETARCH}/boot /opt/apache/beam/
-
-COPY target/LICENSE /opt/apache/beam/
-COPY target/NOTICE /opt/apache/beam/
+# Dependency jars
+COPY target/slf4j-api.jar \
+ target/slf4j-jdk14.jar \
+ target/jcl-over-slf4j.jar \
+ target/log4j-over-slf4j.jar \
+ target/log4j-to-slf4j.jar \
+ # Required to use jamm as a javaagent to get accurate object size
measuring
+ target/jamm.jar \
+ /opt/apache/beam/jars/
Review Comment:
The comment on line 33 is placed inside a multi-line `COPY` command.
Dockerfile syntax does not support comments between line continuation
characters (`\`). This will cause the build to fail because the parser will
treat the comment and subsequent lines as arguments to the `COPY` command. The
comment should be moved above the command.
```
# Dependency jars
# Required to use jamm as a javaagent to get accurate object size measuring
COPY target/slf4j-api.jar \
target/slf4j-jdk14.jar \
target/jcl-over-slf4j.jar \
target/log4j-over-slf4j.jar \
target/log4j-to-slf4j.jar \
target/jamm.jar \
/opt/apache/beam/jars/
```
##########
sdks/java/container/Dockerfile:
##########
@@ -24,22 +24,21 @@ ARG TARGETARCH
ARG pull_licenses
-ADD target/slf4j-api.jar /opt/apache/beam/jars/
-ADD target/slf4j-jdk14.jar /opt/apache/beam/jars/
-ADD target/jcl-over-slf4j.jar /opt/apache/beam/jars/
-ADD target/log4j-over-slf4j.jar /opt/apache/beam/jars/
-ADD target/log4j-to-slf4j.jar /opt/apache/beam/jars/
-ADD target/beam-sdks-java-harness.jar /opt/apache/beam/jars/
-
-# Required to use jamm as a javaagent to get accurate object size measuring
-# COPY fails if file is not found, so use a wildcard for open-module-agent.jar
-# since it is only included in Java 9+ containers
-COPY target/jamm.jar target/open-module-agent.jar /opt/apache/beam/jars/
-
-COPY target/${TARGETOS}_${TARGETARCH}/boot /opt/apache/beam/
-
-COPY target/LICENSE /opt/apache/beam/
-COPY target/NOTICE /opt/apache/beam/
+# Dependency jars
+COPY target/slf4j-api.jar \
+ target/slf4j-jdk14.jar \
+ target/jcl-over-slf4j.jar \
+ target/log4j-over-slf4j.jar \
+ target/log4j-to-slf4j.jar \
+ # Required to use jamm as a javaagent to get accurate object size
measuring
+ target/jamm.jar \
+ /opt/apache/beam/jars/
+
+# Built jars
+COPY target/open-module-agent.jar target/beam-sdks-java-harness.jar
/opt/apache/beam/jars/
Review Comment:
The `open-module-agent.jar` is only present in Java 9+ builds. Using a
specific filename in `COPY` will cause the build to fail if the file is missing
(e.g., in Java 8 environments). Using a wildcard (e.g.,
`target/open-module-agent.jar*`) makes this file optional and ensures the build
remains robust across different Java versions.
```
COPY target/open-module-agent.jar* target/beam-sdks-java-harness.jar
/opt/apache/beam/jars/
```
##########
sdks/go/container/Dockerfile:
##########
@@ -16,16 +16,20 @@
# limitations under the License.
###############################################################################
-FROM gcr.io/distroless/base-nossl-debian12:latest
+FROM gcr.io/distroless/base-nossl-debian12:latest AS base_image
+
+# Distroless base image has many layers. Squash into single layer
+FROM scratch
+COPY --from=base_image / /
+
LABEL Author "Apache Beam <[email protected]>"
ARG TARGETOS
ARG TARGETARCH
ADD target/${TARGETOS}_${TARGETARCH}/boot /opt/apache/beam/
Review Comment:
Use `COPY` instead of `ADD` for local files when no extraction is required.
This is a Docker best practice and maintains consistency with the improvements
made in the Java Dockerfile.
```
COPY target/${TARGETOS}_${TARGETARCH}/boot /opt/apache/beam/
```
##########
sdks/java/container/Dockerfile:
##########
@@ -24,22 +24,21 @@ ARG TARGETARCH
ARG pull_licenses
-ADD target/slf4j-api.jar /opt/apache/beam/jars/
-ADD target/slf4j-jdk14.jar /opt/apache/beam/jars/
-ADD target/jcl-over-slf4j.jar /opt/apache/beam/jars/
-ADD target/log4j-over-slf4j.jar /opt/apache/beam/jars/
-ADD target/log4j-to-slf4j.jar /opt/apache/beam/jars/
-ADD target/beam-sdks-java-harness.jar /opt/apache/beam/jars/
-
-# Required to use jamm as a javaagent to get accurate object size measuring
-# COPY fails if file is not found, so use a wildcard for open-module-agent.jar
-# since it is only included in Java 9+ containers
-COPY target/jamm.jar target/open-module-agent.jar /opt/apache/beam/jars/
-
-COPY target/${TARGETOS}_${TARGETARCH}/boot /opt/apache/beam/
-
-COPY target/LICENSE /opt/apache/beam/
-COPY target/NOTICE /opt/apache/beam/
+# Dependency jars
+COPY target/slf4j-api.jar \
+ target/slf4j-jdk14.jar \
+ target/jcl-over-slf4j.jar \
+ target/log4j-over-slf4j.jar \
+ target/log4j-to-slf4j.jar \
+ # Required to use jamm as a javaagent to get accurate object size
measuring
+ target/jamm.jar \
+ /opt/apache/beam/jars/
+
+# Built jars
+COPY target/open-module-agent.jar target/beam-sdks-java-harness.jar
/opt/apache/beam/jars/
+
+# Built binary with licenses
+COPY target/${TARGETOS}_${TARGETARCH}/boot target/LICENSE target/NOTICE
/opt/apache/beam/
# copy third party licenses
ADD target/third_party_licenses /opt/apache/beam/third_party_licenses/
Review Comment:
For local files and directories, `COPY` is preferred over `ADD` unless you
specifically need the auto-extraction feature of `ADD` (e.g., for tarballs).
Using `COPY` is more explicit and follows Docker best practices, consistent
with other changes in this pull request.
```
COPY target/third_party_licenses /opt/apache/beam/third_party_licenses/
```
--
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]
