shoothzj commented on PR #4333: URL: https://github.com/apache/bookkeeper/pull/4333#issuecomment-2131235213
>> +1 for not hard-coding the path. >> Can we merge these two changed command into the previous one? The new one will generate more layers in docker Thank you for raising the concern regarding the setting of the `JAVA_HOME` environment variable in our Dockerfile. Iād like to address the points raised and explain the reasoning behind the current approach. #### 1. Setting `JAVA_HOME` with `ENV` The suggestion to dynamically set the `JAVA_HOME` using commands like `dirname` and `readlink` within the `ENV` directive is not feasible. Docker does not support executing commands within the `ENV` directive. This limitation means we cannot use: Like I commented in issue before, I have tested on my laptop with my personal image. It won't work. ``` āā[$] <> cat Dockerfile FROM shoothzj/base:jdk17 ENV JAVA1_HOME $(dirname $(dirname $(readlink -f $(which java)))) ``` ``` docker run -it --rm test bash root@f28cdaac85a6:/# echo $JAVA1_HOME $(dirname $(dirname $(readlink -f $(which java)))) root@f28cdaac85a6:/# exit ``` ### 2. Impact of an Extra Layer Regarding the concern about adding an extra layer in the Docker image, while it is ideal to minimize layers for efficiency, in this case, the impact is negligible. Our Dockerfile already includes several `ENV` directives, and one additional layer for correctly setting `JAVA_HOME` ensures the functionality is as expected without introducing complexity or potential errors. -- 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]
