You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2021/03/15 16:27:33 UTC
[GitHub] [orc] yuokada opened a new pull request #655: ORC-694: Update docker files for adding Java11 support
yuokada opened a new pull request #655:
URL: https://github.com/apache/orc/pull/655
<!--
Thanks for sending a pull request! Here are some tips for you:
1. File a JIRA issue first and use it as a prefix of your PR title, e.g., `ORC-001: Fix ABC`.
2. Use your PR title to summarize what this PR proposes instead of describing the problem.
3. Make PR title and description complete because these will be the permanent commit log.
4. If possible, provide a concise and reproducible example to reproduce the issue for a faster review.
5. If the PR is unfinished, use GitHub PR Draft feature.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-802335172
Thank you, @yuokada and @omalley .
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yuokada commented on a change in pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
yuokada commented on a change in pull request #655:
URL: https://github.com/apache/orc/pull/655#discussion_r595650384
##########
File path: docker/ubuntu18/Dockerfile
##########
@@ -32,9 +33,9 @@ RUN apt-get install -y \
make \
curl \
maven \
- openjdk-8-jdk \
+ openjdk-${jdk}-jdk \
tzdata
-RUN update-java-alternatives --set java-1.8.0-openjdk-amd64
+RUN update-java-alternatives --set java-1.${jdk}.0-openjdk-amd64
Review comment:
I believe so. It properly is set.
```
~/w/I/o/docker ❯❯❯ docker run -it orc-ubuntu18-jdk11 update-java-alternatives -l
java-1.11.0-openjdk-amd64 1111 /usr/lib/jvm/java-1.11.0-openjdk-amd64
~/w/I/o/docker ❯❯❯ docker run -it orc-ubuntu18-jdk11 java -version
openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment (build 11.0.10+9-Ubuntu-0ubuntu1.18.04)
OpenJDK 64-Bit Server VM (build 11.0.10+9-Ubuntu-0ubuntu1.18.04, mixed mode, sharing)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yuokada edited a comment on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
yuokada edited a comment on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-801516449
@dongjoon-hyun
Just questions.
For debian 10, the OS image already use ~~jdk10~~ jdk11. Should we support jdk8 on it?
For debian 9 and ubuntu16, the openjdk-11 deb package isn't provided for both OSes.
Do we need to support JDK 11 in both?
Or should we skip building an image on those OSes?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yuokada edited a comment on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
yuokada edited a comment on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-801516449
@dongjoon-hyun
Just questions.
For debian 10, the OS image already use jdk10. Should we support jdk8 on it?
For debian 9 and ubuntu16, the openjdk-11 deb package isn't provided for both OSes.
Do we need to support JDK 11 in both?
Or should we skip building an image on those images?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yuokada commented on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
yuokada commented on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-799854873
Created: https://github.com/apache/orc/pull/656
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yuokada commented on a change in pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
yuokada commented on a change in pull request #655:
URL: https://github.com/apache/orc/pull/655#discussion_r594829201
##########
File path: docker/README.md
##########
@@ -21,7 +21,7 @@ The scripts are:
A base image for each OS is built using:
cd docker/$os
- docker build -t orc-$os .
+ docker build -t "orc-$os-jdk8" --build-arg jdk=8 . OR docker build -t "orc-$os-jdk11" --build-arg jdk=11 .
Review comment:
Is this change easy to understand?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #655:
URL: https://github.com/apache/orc/pull/655#discussion_r595399929
##########
File path: docker/ubuntu20-clang/Dockerfile
##########
@@ -31,9 +32,9 @@ RUN apt-get install -y \
make \
curl \
maven \
- openjdk-8-jdk \
+ openjdk-${jdk}-jdk \
tzdata
-RUN update-java-alternatives --set java-1.8.0-openjdk-amd64
+RUN update-java-alternatives --set java-1.${jdk}.0-openjdk-amd64
Review comment:
ditto.
##########
File path: docker/ubuntu20/Dockerfile
##########
@@ -32,9 +33,9 @@ RUN apt-get install -y \
make \
curl \
maven \
- openjdk-8-jdk \
+ openjdk-${jdk}-jdk \
tzdata
-RUN update-java-alternatives --set java-1.8.0-openjdk-amd64
+RUN update-java-alternatives --set java-1.${jdk}.0-openjdk-amd64
Review comment:
ditto.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #655:
URL: https://github.com/apache/orc/pull/655#discussion_r596300252
##########
File path: docker/run-all.sh
##########
@@ -32,9 +32,11 @@ function failure {
rm -f logs/pids.txt logs/*.log
start=`date`
-for os in `cat os-list.txt`; do
- echo "Building $os"
- ( cd $os && docker build -t "orc-$os" . ) > logs/$os-build.log 2>&1 || exit 1
+for jdk in 8 11; do
+ for os in `cat os-list.txt`; do
+ echo "Building $os"
Review comment:
Could you show `$jdk` together?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #655:
URL: https://github.com/apache/orc/pull/655#discussion_r595399669
##########
File path: docker/ubuntu18/Dockerfile
##########
@@ -32,9 +33,9 @@ RUN apt-get install -y \
make \
curl \
maven \
- openjdk-8-jdk \
+ openjdk-${jdk}-jdk \
tzdata
-RUN update-java-alternatives --set java-1.8.0-openjdk-amd64
+RUN update-java-alternatives --set java-1.${jdk}.0-openjdk-amd64
Review comment:
Ur, is this correct, `java-1.11.0-openjdk-amd64`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yuokada edited a comment on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
yuokada edited a comment on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-801516449
@dongjoon-hyun
Just questions.
For debian 10, the OS image already use jdk10. Should we support jdk8 on it?
For debian 9 and ubuntu16, the openjdk-11 deb package isn't provided for both OSes.
Do we need to support JDK 11 in both?
Or should we skip building an image on those OSes?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-801548429
If there is no JDK11 package, we had better document it and skip JDK11 for old OSes inside the above script like run-all.sh.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] omalley closed pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
omalley closed pull request #655:
URL: https://github.com/apache/orc/pull/655
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] pgaref commented on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
pgaref commented on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-801358248
Thanks @yuokada and @dongjoon-hyun for the work on this! Left some minor comments.
Seems to work well locally!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yuokada commented on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
yuokada commented on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-801516449
@dongjoon-hyun
Just questions.
For debian 10, the OS image already use jdk10. Should we support jdk8 on it?
For debian 9 and ubuntu16, the openjdk-11 deb package isn't provided for both OSes.
Do we need to support JDK 11 in both?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yuokada commented on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
yuokada commented on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-799837292
@dongjoon-hyun
`MAINTAINER` command is deprecated since docker 1.13 or later. So I replaced it with the `LABEL` command.
https://docs.docker.com/engine/reference/builder/#maintainer-deprecated
But, we don't need to change it in this PR.
Should we create another ticket to replace `MAINTAINER` command?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yuokada commented on a change in pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
yuokada commented on a change in pull request #655:
URL: https://github.com/apache/orc/pull/655#discussion_r595650885
##########
File path: docker/ubuntu20/Dockerfile
##########
@@ -32,9 +33,9 @@ RUN apt-get install -y \
make \
curl \
maven \
- openjdk-8-jdk \
+ openjdk-${jdk}-jdk \
tzdata
-RUN update-java-alternatives --set java-1.8.0-openjdk-amd64
+RUN update-java-alternatives --set java-1.${jdk}.0-openjdk-amd64
Review comment:
```
~/w/I/o/docker ❯❯❯ docker run -it orc-ubuntu20-jdk11 update-java-alternatives -l
java-1.11.0-openjdk-amd64 1111 /usr/lib/jvm/java-1.11.0-openjdk-amd64
~/w/I/o/docker ❯❯❯ docker run -it orc-ubuntu20-jdk11 java -version
openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment (build 11.0.10+9-Ubuntu-0ubuntu1.20.04)
OpenJDK 64-Bit Server VM (build 11.0.10+9-Ubuntu-0ubuntu1.20.04, mixed mode, sharing)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] pgaref commented on a change in pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #655:
URL: https://github.com/apache/orc/pull/655#discussion_r596317114
##########
File path: docker/README.md
##########
@@ -21,7 +21,7 @@ The scripts are:
A base image for each OS is built using:
Review comment:
Lets add above that:
`run-all.sh`, `run-one.sh` and `reinit.sh` tests both on jdk8 and 11 across OSes
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #655:
URL: https://github.com/apache/orc/pull/655#issuecomment-799851117
Shall we do that in a separate PR because it's irrelevant to Java?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] pgaref commented on a change in pull request #655: ORC-694: Update docker files for adding Java11 support
Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #655:
URL: https://github.com/apache/orc/pull/655#discussion_r596314939
##########
File path: docker/README.md
##########
@@ -21,7 +21,7 @@ The scripts are:
A base image for each OS is built using:
cd docker/$os
- docker build -t orc-$os .
+ docker build -t "orc-$os-jdk8" --build-arg jdk=8 . OR docker build -t "orc-$os-jdk11" --build-arg jdk=11 .
Review comment:
Lets make it more explicit.
For jdk8: docker build -t "orc-$os-jdk8" --build-arg jdk=8 .
For jdk11: docker build -t "orc-$os-jdk11" --build-arg jdk=11 .
##########
File path: docker/README.md
##########
@@ -21,7 +21,7 @@ The scripts are:
A base image for each OS is built using:
Review comment:
Lets add above that:
`run-all.sh` and `run-one.sh` tests both on jdk8 and 11 across OSes
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org