added JAVA_HOME environment variable #28

soupdiver · 2015-03-05T04:31:07Z

Fixes #27

md5 · 2015-03-05T05:16:40Z

I think this should be /usr/lib/jvm/java-VERSION-openjdk-amd64/jre for all cases.

I just ran this test program against all the combinations:

public class Test {
    public static void main(String... argz) {
        System.out.println(System.getProperty("java.home"));
    }
}

With that file as Test.java, I did the following:

docker build --rm -v "$(pwd):/work" -w /work java:6-jdk javac Test.java
for version in 6 7 8; do for flavor in jdk jre; do \
    docker run --rm -v $(pwd):/work -w /work java:$version-$flavor java -cp . Test; done; done

This was the output:

/usr/lib/jvm/java-6-openjdk-amd64/jre
/usr/lib/jvm/java-6-openjdk-amd64/jre
/usr/lib/jvm/java-7-openjdk-amd64/jre
/usr/lib/jvm/java-7-openjdk-amd64/jre
/usr/lib/jvm/java-8-openjdk-amd64/jre
/usr/lib/jvm/java-8-openjdk-amd64/jre

soupdiver · 2015-03-05T05:21:54Z

Based on this the path is different for JDK and JRE

md5 · 2015-03-05T05:34:40Z

Perhaps you're right that JAVA_HOME and java.home shouldn't necessarily match, though I'm not sure that SO link counts for much.

Looking at Tomcat's catalina.sh as an example, it actually looks for both JAVA_HOME and JRE_HOME, preferring JRE_HOME (cf. https://github.com/apache/tomcat/blob/7cf92c21bde40bc9d3b9145e7a6781ab6b591d53/bin/catalina.sh#L47-L52 and https://github.com/apache/tomcat/blob/7cf92c21bde40bc9d3b9145e7a6781ab6b591d53/bin/setclasspath.sh)

soupdiver · 2015-03-05T05:37:51Z

ok so there is a difference between JAVA_HOME and JRE_HOME interesting.
I'm also not very familiar with JAVA so I'm not sure about the differences.

IMHO relying on JRE_HOME seems better. What do you think?

Or we could simply define both to cover more use cases

md5 · 2015-03-05T05:39:01Z

I don't think that JRE_HOME is something that belongs in this image. It's not anywhere near as common as JAVA_HOME.

soupdiver · 2015-03-05T05:41:07Z

As I said I'm not very familiar with JAVA so I'm not sure what is more common or where the differences are.
So should I change the PR to use /jre in all cases?

md5 · 2015-03-05T05:45:48Z

I'm actually not sure.

What was the use case you were trying to address by adding JAVA_HOME? I know it's pretty common, but did you actually have an app that wouldn't work without it?

soupdiver · 2015-03-05T07:24:16Z

I described my my situation in #27
I had to edit my default Java keystore to add a certificate.
And everything I found was just referencing to $JAVA_HOME for the location and this was not defined.
So not that it was not working without the variable but I had to go manually into the container to figure out the location.

md5 · 2015-03-05T14:00:39Z

I just noticed that openjdk-N-jre-headless includes this file a file called JAVA_HOME that looks like this:

JAVA_HOME

Legacy use of JAVA_HOME

As of the latest versions of java-common there really should
be no need for JAVA_HOME as Java(TM) should be completely
integrated with your Debian GNU/Linux system.

Selecting OpenJDK 7 as the default Java(TM) implementation

Please see README.alternatives for instructions on how
to make OpenJDK 7 executables and man pages the default on your system.

cf. docker run --rm java:8 find -name JAVA_HOME -exec cat {} \;

The version in the java:6 package also has this wording in that file:

For reference, the setting of JAVA_HOME for Sun Java 6 is:
JAVA_HOME=/usr/lib/jvm/java-6-sun

So if it makes sense to add JAVA_HOME at all, I think you want to use /usr/lib/jvm/java-VERSION-openjdk-amd64 ~~as you've already done~~.

LGTM (sorta)

soupdiver · 2015-03-05T20:50:18Z

So so should I update my PR or should we consider JAVA_HOME as legacy?

md5 · 2015-03-05T20:55:21Z

I think you should update your PR, but I'm personally +0 on this change.

Perhaps just documenting JAVA_HOME would be enough.

yosifkit · 2015-03-05T20:57:07Z

I think having JAVA_HOME will help; it can't really hurt anything. I agree that the /jre is not needed; when I tested single mode hadoop, it only needed the /usr/lib/jvm/java-7-openjdk-amd64 even when using the jre image.

soupdiver · 2015-03-05T20:59:56Z

ok I've updated the PR

yosifkit · 2015-03-05T21:09:30Z

LGTM, @tianon?

tianon · 2015-03-05T21:26:38Z

I'm also totally +0 on whether this should be a code change or a docs change, especially since it's a legacy thing.

What I'm definitely -1 on is having it be hard-coded in each Dockerfile like this with no verification that it's correct.

I think what I'd be more comfortable with is having some way to verify that the value is correct after our install, and having the whole build bail if it's not:

ENV JAVA_VERSION 6b34
ENV JAVA_DEBIAN_VERSION 6b34-1.13.6-1~deb7u1

RUN apt-get update && apt-get install -y openjdk-6-jdk="$JAVA_DEBIAN_VERSION" && rm -rf /var/lib/apt/lists/*

ENV JAVA_HOME /usr/lib/jvm/java-6-openjdk-amd64
RUN set -x && [ "$JAVA_HOME" = "$(readlink -f "$(dirname "$(readlink -f "$(which java)")")/../..")" ]

Example:

$ for v in 6 7 8; do for a in $v-jre $v-jdk; do { echo "FROM java:$a"; echo "ENV JAVA_HOME /usr/lib/jvm/java-$v-openjdk-amd64"; echo 'RUN set -x && [ "$JAVA_HOME" = "$(readlink -f "$(dirname "$(readlink -f "$(which java)")")/../..")" ]'; } | docker build -; done; done
Sending build context to Docker daemon 2.048 kB
Sending build context to Docker daemon 
Step 0 : FROM java:6-jre
 ---> bf4469de640a
Step 1 : ENV JAVA_HOME /usr/lib/jvm/java-6-openjdk-amd64
 ---> Running in 1a03bea755dc
 ---> 124dbafcd548
Removing intermediate container 1a03bea755dc
Step 2 : RUN set -x && [ "$JAVA_HOME" = "$(readlink -f "$(dirname "$(readlink -f "$(which java)")")/../..")" ]
 ---> Running in f92e985520a5
+ which java
+ readlink -f /usr/bin/java
+ dirname /usr/lib/jvm/java-6-openjdk-amd64/jre/bin/java
+ readlink -f /usr/lib/jvm/java-6-openjdk-amd64/jre/bin/../..
+ [ /usr/lib/jvm/java-6-openjdk-amd64 = /usr/lib/jvm/java-6-openjdk-amd64 ]
 ---> a3c08eeff205
Removing intermediate container f92e985520a5
Successfully built a3c08eeff205
Sending build context to Docker daemon 2.048 kB
Sending build context to Docker daemon 
Step 0 : FROM java:6-jdk
 ---> ac9411624983
Step 1 : ENV JAVA_HOME /usr/lib/jvm/java-6-openjdk-amd64
 ---> Running in 875657e67ecd
 ---> 1af4b405173f
Removing intermediate container 875657e67ecd
Step 2 : RUN set -x && [ "$JAVA_HOME" = "$(readlink -f "$(dirname "$(readlink -f "$(which java)")")/../..")" ]
 ---> Running in f98017de94cc
+ which java
+ readlink -f /usr/bin/java
+ dirname /usr/lib/jvm/java-6-openjdk-amd64/jre/bin/java
+ readlink -f /usr/lib/jvm/java-6-openjdk-amd64/jre/bin/../..
+ [ /usr/lib/jvm/java-6-openjdk-amd64 = /usr/lib/jvm/java-6-openjdk-amd64 ]
 ---> 5b46b59fa73d
Removing intermediate container f98017de94cc
Successfully built 5b46b59fa73d
Sending build context to Docker daemon 2.048 kB
Sending build context to Docker daemon 
Step 0 : FROM java:7-jre
 ---> d0b904b99306
Step 1 : ENV JAVA_HOME /usr/lib/jvm/java-7-openjdk-amd64
 ---> Running in 185d27736972
 ---> d0d84a069215
Removing intermediate container 185d27736972
Step 2 : RUN set -x && [ "$JAVA_HOME" = "$(readlink -f "$(dirname "$(readlink -f "$(which java)")")/../..")" ]
 ---> Running in 08526a5df2bb
+ which java
+ readlink -f /usr/bin/java
+ dirname /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java
+ readlink -f /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/../..
+ [ /usr/lib/jvm/java-7-openjdk-amd64 = /usr/lib/jvm/java-7-openjdk-amd64 ]
 ---> d28718162603
Removing intermediate container 08526a5df2bb
Successfully built d28718162603
Sending build context to Docker daemon 2.048 kB
Sending build context to Docker daemon 
Step 0 : FROM java:7-jdk
 ---> 513c90950366
Step 1 : ENV JAVA_HOME /usr/lib/jvm/java-7-openjdk-amd64
 ---> Running in ad67dbc6013b
 ---> e893fcd7d191
Removing intermediate container ad67dbc6013b
Step 2 : RUN set -x && [ "$JAVA_HOME" = "$(readlink -f "$(dirname "$(readlink -f "$(which java)")")/../..")" ]
 ---> Running in 4cdb109cf11f
+ which java
+ readlink -f /usr/bin/java
+ dirname /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java
+ readlink -f /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/../..
+ [ /usr/lib/jvm/java-7-openjdk-amd64 = /usr/lib/jvm/java-7-openjdk-amd64 ]
 ---> b906c66c2670
Removing intermediate container 4cdb109cf11f
Successfully built b906c66c2670
Sending build context to Docker daemon 2.048 kB
Sending build context to Docker daemon 
Step 0 : FROM java:8-jre
 ---> 7aaf9421464d
Step 1 : ENV JAVA_HOME /usr/lib/jvm/java-8-openjdk-amd64
 ---> Running in 9547bbd478ef
 ---> 43cca47c38c6
Removing intermediate container 9547bbd478ef
Step 2 : RUN set -x && [ "$JAVA_HOME" = "$(readlink -f "$(dirname "$(readlink -f "$(which java)")")/../..")" ]
 ---> Running in 326a591e149a
+ which java
+ readlink -f /usr/bin/java
+ dirname /usr/lib/jvm/java-8-openjdk-amd64/jre/bin/java
+ readlink -f /usr/lib/jvm/java-8-openjdk-amd64/jre/bin/../..
+ [ /usr/lib/jvm/java-8-openjdk-amd64 = /usr/lib/jvm/java-8-openjdk-amd64 ]
 ---> 8e6183c64018
Removing intermediate container 326a591e149a
Successfully built 8e6183c64018
Sending build context to Docker daemon 2.048 kB
Sending build context to Docker daemon 
Step 0 : FROM java:8-jdk
 ---> ed632caf877b
Step 1 : ENV JAVA_HOME /usr/lib/jvm/java-8-openjdk-amd64
 ---> Running in 4f291a67a9b0
 ---> a6193c9085a5
Removing intermediate container 4f291a67a9b0
Step 2 : RUN set -x && [ "$JAVA_HOME" = "$(readlink -f "$(dirname "$(readlink -f "$(which java)")")/../..")" ]
 ---> Running in b552769a249a
+ which java
+ readlink -f /usr/bin/java
+ dirname /usr/lib/jvm/java-8-openjdk-amd64/jre/bin/java
+ readlink -f /usr/lib/jvm/java-8-openjdk-amd64/jre/bin/../..
+ [ /usr/lib/jvm/java-8-openjdk-amd64 = /usr/lib/jvm/java-8-openjdk-amd64 ]
 ---> 24cfc8d56faa
Removing intermediate container b552769a249a
Successfully built 24cfc8d56faa

soupdiver · 2015-03-05T21:38:24Z

@tianon I also thought about setting the env variable via the command you used but as the Java version itself is hardcoded it is, in my opinion, it is also ok to hardcode the path to JAVA_HOME and it would be less cryptic.
But I can't give any real strong arguments beside my own opinion, so I would be ok with both solutions.

yosifkit · 2015-03-05T21:42:44Z

@soupdiver, it seems @tianon was just adding a RUN line that would fail if the JAVA_HOME did not match the installed java, so that there would be a failsafe protecting us from ourselves.

md5 · 2015-03-05T21:44:45Z

Would it be sufficient for that check to be in update.sh?

md5 · 2015-03-05T21:45:18Z

Or in a test in official-images?

soupdiver · 2015-03-05T21:47:28Z

@yosifkit Oh, sorry totally overlooked that

yosifkit · 2015-03-05T21:50:52Z

I am not sure where we would want it.

update.sh: it could autofill the line in the Dockerfile, but would spin up more containers.
tests: nice to have more tests, but it is late in the process to find this possible "bug"

yosifkit · 2015-03-05T21:51:29Z

in the Dockerfile, easy, but adds unnecessary layer for users

javadev · 2015-11-19T14:29:51Z

openjdk-8-jre/Dockerfile

@@ -9,6 +9,7 @@ RUN apt-get update && apt-get install -y unzip && rm -rf /var/lib/apt/lists/*

 ENV JAVA_VERSION 8u40~b22
 ENV JAVA_DEBIAN_VERSION 8u40~b22-2
+ENV JAVA_HOME /usr/lib/jvm/java-8-openjdk-amd64


/usr/lib/jvm/java-8-openjdk-amd64 for openjdk-8-jre ?

added JAVA_HOME environment variable

ed54c77

javadev reviewed Nov 19, 2015
View reviewed changes

tianon mentioned this pull request Jan 5, 2016

Add explicit JAVA_HOME environment variable #61

Merged

yosifkit closed this in #61 Jan 5, 2016

added JAVA_HOME environment variable #28

added JAVA_HOME environment variable #28

Uh oh!

Conversation

soupdiver commented Mar 5, 2015

Uh oh!

md5 commented Mar 5, 2015

Uh oh!

soupdiver commented Mar 5, 2015

Uh oh!

md5 commented Mar 5, 2015

Uh oh!

soupdiver commented Mar 5, 2015

Uh oh!

md5 commented Mar 5, 2015

Uh oh!

soupdiver commented Mar 5, 2015

Uh oh!

md5 commented Mar 5, 2015

Uh oh!

soupdiver commented Mar 5, 2015

Uh oh!

md5 commented Mar 5, 2015

Uh oh!

soupdiver commented Mar 5, 2015

Uh oh!

md5 commented Mar 5, 2015

Uh oh!

yosifkit commented Mar 5, 2015

Uh oh!

soupdiver commented Mar 5, 2015

Uh oh!

yosifkit commented Mar 5, 2015

Uh oh!

tianon commented Mar 5, 2015

Uh oh!

soupdiver commented Mar 5, 2015

Uh oh!

yosifkit commented Mar 5, 2015

Uh oh!

md5 commented Mar 5, 2015

Uh oh!

md5 commented Mar 5, 2015

Uh oh!

soupdiver commented Mar 5, 2015

Uh oh!

yosifkit commented Mar 5, 2015

Uh oh!

yosifkit commented Mar 5, 2015

Uh oh!

javadev Nov 19, 2015

Choose a reason for hiding this comment

Uh oh!

Uh oh!