colinwerner commented on code in PR #2052:
URL: https://github.com/apache/activemq/pull/2052#discussion_r3692449451
##########
assembly/src/docker/Dockerfile:
##########
@@ -16,7 +16,7 @@
# limitations under the License.
################################################################################
-ARG java_version=17
+ARG java_version=21
Review Comment:
@mattrpav Right, it is not filtered. It comes through as a build arg
instead; the docker profiles pass java_version via fabric8's args block in
assembly/pom.xml, which overrides the ARG default. On main as-is:
```
mvn clean install -DskipTests -Pdocker -Ddocker-java-version=17
docker run --rm apache/activemq:6.4.0-SNAPSHOT java -version
```
gives Temurin-17.0.19+10. Without the `-D` you get 25. Same Dockerfile, only
the property changed.
So `mvn -Pdocker` quietly builds 17 today; this just sets the property to
25 so they match.
That said, dropping the arg and the property might be the better fix. The
version is declared in two places right now, and they can drift, which is what
happened here; #2257 updated the Dockerfile and the property stayed at 17. If
it only lives in the Dockerfile, then `docker build` and `mvn -Pdocker` cannot
disagree, and there is no `-D` to remember. The one thing it costs is
overriding the JDK per build from the command line. Happy to redo it that way
if you prefer. Either way, please let me know...
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
