[JDK1.6 compatibile task #695 and #676] Update Amino codebase to use JDK 1.6 APIs by maheshrajus · Pull Request #811 · amino-os/Amino.Run

maheshrajus · 2019-07-05T14:14:38Z

PR is raised for the issues [Experience #656] Update codebase to use JDK 1.6 APIs #695 and [Bug #661] Gradle build succeeds despite using higher version of Java APIs. #676
If any APIs used which are higher than JDK 1.6 version then compilation issues will be thrown
JAVA6_HOME need to be set if we want to compile the code with JDK1.6
ex: export JAVA6_HOME=/usr/lib/jvm/jdk1.6.X.X
this we can set in gradle.properties also.

Note:
Document changes we can handle in the issue: #694

Compilation Report:

Sample Error: [when we used lambda expression in Amino.Run code]

…codebase to use JDK 1.6 APIs

VenuReddy2103 · 2019-07-09T15:54:23Z

+        if ((JavaVersion.VERSION_1_6 == sourceCompatibility)
+                && (System.env.JAVA6_HOME) != null) {
+            options.fork = true
+            options.bootstrapClasspath = files("$System.env.JAVA6_HOME/jre/lib/rt.jar")


Forcing system env var to be fixed JAVA6_HOME for java6 ? It is very stringent. How about programmatically doing that. May be, define a variable in gradle properties for java6 path and override the JAVA_HOME variable Or may be set org.gradle.java.home only for the build context? Or some other way.

…s#676] Update Amino codebase to use JDK 1.6 APIs

quinton-hoole · 2019-07-12T16:30:27Z

+            options.bootstrapClasspath = files("$System.env.JAVA6_HOME/jre/lib/rt.jar")
+        }
+        else {
+            options.bootstrapClasspath = files("$System.env.JAVA_HOME/jre/lib/rt.jar")


This just silently uses whatever java version is installed on the system? Isn't that exactly the opposite of what you're trying to achieve here?

What you want is to ensure that java 1.6 is installed and used to compile, otherwise generate an error (unless the user explicitly asks to use a different version of java).

@quinton-hoole

This just silently uses whatever java version is installed on the system? Isn't that exactly the opposite of what you're trying to achieve here?

Here when we set java version to JavaVersion.VERSION_1_6 and set JAVA6_HOME to JDK1.6 path then it will take the first case.
that is --> options.bootstrapClasspath = files("$System.env.JAVA6_HOME/jre/lib/rt.jar")
and it will compile code in 1.6 compatible.
Here JAVA6_HOME should point to JDK1.6
ex: export JAVA6_HOME=/usr/lib/jvm/JDK1.6

What you want is to ensure that java 1.6 is installed and used to compile, otherwise generate an error (unless the user explicitly asks to use a different version of java).

When we have only JAVA 1.6 installed in our system and tries to build then gradle is throwing an error by saying that it requires a 1.7+ java version.

So we are using 1.8 JAVA version for Gradle build and we are enforcing the build to use 1.6 java version by setting the below fields explicitly.

def javaVersion = JavaVersion.VERSION_1_6 sourceCompatibility = javaVersion targetCompatibility = javaVersion if (JavaVersion.VERSION_1_6.equals(javaVersion) && (System.env.JAVA6_HOME != null) { options.fork = true //for 1.6java compatible we have to set JAVA6_HOME to JDK1.6 path options.bootstrapClasspath = files("$System.env.JAVA6_HOME/jre/lib/rt.jar") } else { options.bootstrapClasspath = files("$System.env.JAVA_HOME/jre/lib/rt.jar") }

Currently, with JAVA6_HOME setting to JDK1.6 path, we can catch the code which does not comply with java1.6 version.

Here I am working for how we can set options.bootstrapClasspath to JDK1.6/jre/lib/rt.jar path in the JDK1.6 case without system env variable(JAVA6_HOME)

Please ask @VenuReddy2103 to explain.
My point was that the desired java version to compile Amino with should be explicitly specified somewhere in the gradle config files that users check out of git. And if it's not specified, an error should be generated.

Right now, if someone deletes the statement:

def javaVersion = JavaVersion.VERSION_1_6

or if they do not have JAVA6_HOME set in their environment, it is not well defined which java version will be used to compile with. I different version could be used on each developers' machine, which is what we're trying to avoid.

quinton-hoole · 2019-07-12T16:53:45Z

        public boolean equals(Object o) {
-            if (this == o) return true;
-            if (o == null || getClass() != o.getClass()) return false;
+            if (this == o) {


Not your fault, but neither equals() nor hashCode are correct:

https://stackoverflow.com/questions/2066917/overriding-equals-hashcode-in-sub-classes-considering-super-fields

ok, I will check this and modify

ScaleUpFrontendPolicy.Config do not inherit LoadBalancedFrontendPolicy.Config. They are independent classes to hold their respective config. So, this generation of equals() and hashCode() using IDE looks correct to me.

quinton-hoole

See inline comment. I don't think this does what it's intended to do?

…#676 ] Update codebase to use JDK 1.6 APIs

quinton-hoole · 2019-07-19T22:20:46Z

What is the status here @maheshrajus ? Did you discuss with @VenuReddy2103 ? Is this ready for review yet?

VenuReddy2103 · 2019-07-22T14:52:12Z

-        sourceCompatibility = javaVersion
-        targetCompatibility = javaVersion
-        options.bootstrapClasspath = files("$System.env.JAVA_HOME/jre/lib/rt.jar")
+            def jdkpath = project.property('jdkPath')


nit: Remove these additional tabs

VenuReddy2103 · 2019-07-22T14:54:55Z

-        sourceCompatibility = javaVersion
-        targetCompatibility = javaVersion
-        options.bootstrapClasspath = files("$System.env.JAVA_HOME/jre/lib/rt.jar")
+            def jdkpath = project.property('jdkPath')


nit: Rename jdkPath property to jdk6Path

VenuReddy2103 · 2019-07-22T14:58:03Z

+            def jdkpath = project.property('jdkPath')
+            def folder = new File(jdkpath)
+            if(!folder.exists()) {
+                throw new GradleException("JDK 1.6 path does not exist ! Please set the correct 'jdkPath' in gradle.properties file")


nit: Suggest string to be something like this - "JDK 1.6 path is not set. Please set 'jdk6Path' project property in gradle.properties file."

VenuReddy2103 · 2019-07-22T15:04:35Z

+            if(!folder.exists()) {
+                throw new GradleException("JDK 1.6 path does not exist ! Please set the correct 'jdkPath' in gradle.properties file")
+            }
+


I think, we should execute java --version using this JDK(the one with jdk6Path) internally, extract version and check if it is 1.6 or not. If not, throw exception with string - "jdk6Path project property is set with JDK 1.x path. Please set it with JDK 1.6 path."

VenuReddy2103 · 2019-07-22T15:50:36Z

What is the status here @maheshrajus ? Did you discuss with @VenuReddy2103 ? Is this ready for review yet?

Discussion Summary:

Our current gradle version(4.10) requires java 1.7 or later to be installed. It detects java version internally and throws below error if we don't have 1.7 or later version available. So we must have 1.7 or later installed.
Have jdk6Path property defined in properties file. This must be set with JDK 6 Path. Check and throw exceptions in following cases:
1. if jdk6Path property is not set
2. Or if that is set with some JDK other than 1.6. [We can extract java version with the provided jdk6Path property's bin folder, upon executing java --version internally]

NOTE: We just need JRE 1.6(actually require only rt.jar of 1.6). We don't need complete JDK 1.6 to be installed to compile source with 1.6 compatibility. Will check further on this.

maheshrajus · 2019-07-23T06:22:26Z

When JDK1.6 path not set:

When JDK1.8 path set:

Compilation & examples test report:

…s#676 ] Update codebase to use JDK 1.6 APIs

[JDK1.6 compatibile task amino-os#695 and amino-os#676] Update Amino …

0d5f53b

…codebase to use JDK 1.6 APIs

prostil requested review from AmitRoushan, VenuReddy2103 and quinton-hoole July 9, 2019 04:51

prostil assigned maheshrajus Jul 9, 2019

VenuReddy2103 reviewed Jul 9, 2019

View reviewed changes

Comment thread core/src/main/java/amino/run/app/DMSpec.java Outdated

VenuReddy2103 reviewed Jul 9, 2019

View reviewed changes

Comment thread build.gradle Outdated

VenuReddy2103 reviewed Jul 9, 2019

View reviewed changes

AmitRoushan reviewed Jul 10, 2019

View reviewed changes

Comment thread build.gradle Outdated

Comment thread core/src/integrationTest/java/amino/run/kernel/IntegrationTestBase.java

Comment thread core/src/integrationTest/java/amino/run/kernel/IntegrationTestBase.java

AmitRoushan reviewed Jul 10, 2019

View reviewed changes

Comment thread core/src/integrationTest/java/amino/run/kernel/IntegrationTestBase.java Outdated

Review Comments Fix:[JDK1.6 compatibile task amino-os#695 and amino-o…

47f5774

…s#676] Update Amino codebase to use JDK 1.6 APIs

quinton-hoole reviewed Jul 12, 2019

View reviewed changes

quinton-hoole requested changes Jul 12, 2019

View reviewed changes

Review comments Fix[JDK1.6 compatibile task amino-os#695 and amino-os…

b359d52

…#676 ] Update codebase to use JDK 1.6 APIs

maheshrajus changed the title ~~[JDK1.6 compatibile task #695 and #676] Update Amino codebase to use JDK 1.6 APIs~~ WIP-[JDK1.6 compatibile task #695 and #676] Update Amino codebase to use JDK 1.6 APIs Jul 18, 2019

VenuReddy2103 reviewed Jul 22, 2019

View reviewed changes

Review comments Fix-[JDK1.6 compatibile task amino-os#695 and amino-o…

74ab213

…s#676 ] Update codebase to use JDK 1.6 APIs

maheshrajus changed the title ~~WIP-[JDK1.6 compatibile task #695 and #676] Update Amino codebase to use JDK 1.6 APIs~~ [JDK1.6 compatibile task #695 and #676] Update Amino codebase to use JDK 1.6 APIs Jul 23, 2019

resolving conflicts

921f6ce

Uh oh!

Conversation

maheshrajus commented Jul 5, 2019

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

quinton-hoole Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VenuReddy2103 Jul 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

quinton-hoole left a comment

Choose a reason for hiding this comment

Uh oh!

quinton-hoole commented Jul 19, 2019

Uh oh!

VenuReddy2103 Jul 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VenuReddy2103 commented Jul 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maheshrajus commented Jul 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

quinton-hoole Jul 15, 2019 •

edited

Loading

VenuReddy2103 Jul 22, 2019 •

edited

Loading

VenuReddy2103 Jul 22, 2019 •

edited

Loading

VenuReddy2103 commented Jul 22, 2019 •

edited

Loading