[TRAFODION-3241]support both jdk1.7 and jdk1.8 #1753
Conversation
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3046/ |
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/3046/ |
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3049/ |
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/3049/ |
@svarnau, @selvaganesang, @narendragoyal, do any of you have concerns on this change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of comments.
core/conn/jdbcT4/Makefile
Outdated
mv target/jdbcT4-${TRAFODION_VER}.zip ../clients | ||
#ln -sf ${TRAF_HOME}/export/lib/jdbcT4-${TRAFODION_VER}.jar ${TRAF_HOME}/export/lib/jdbcT4.jar | ||
`cd ${TRAF_HOME}/export/lib;ln -sf jdbcT4-${TRAFODION_VER}.jar jdbcT4.jar` | ||
@if [ ! -f target/*.jar ]; then \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of target/*.jar, should it check for the exact file that we are trying to build. What if TRAFODION_VER changes or there is some other JAR already in the 'target' directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If *.jar expands to more than one file, won't that generate a shell syntax error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@narendragoyal @svarnau sure, it should be an exactly name.
core/conn/jdbcT4/Makefile
Outdated
|
||
clean: | ||
-$(MAVEN) clean | grep ERROR | ||
$(RM) build_jdbct4.log ${TRAF_HOME}/export/lib/jdbcT4*.jar | ||
@if [ -f target/*.jar ]; then \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just my opinion :) : I think the 'clean' target should not be conditional on the existence of the target/JAR, it should execute the commands and perhaps the 'maven clean' will cleanup the intermediate class files also (even when a target/.jar does not exist).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The classes are in folder "target". if this folder does not exist, 'mvn clean' actually will do nothing. So here yes it should b -d target.
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3055/ |
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/3055/ |
@kevinxu021 Are we compiling the sources to be compatible with JDK 1.7 or JDK 1.8? I am asking because I couldn't decipher from the changes and I am not familiar with maven rules. Also, I see the hard coded versions 1.7 is being changed to use property such as source.version and target.version |
@selvaganesang it uses "[1.8,)" to activate the property "<javadoc.opts>-Xdoclint:none</javadoc.opts>", and with jdk1.8 you will got error while "mvn site". this is a preparation for our project running on jdk1.8. If this changes made, you can compile this project both on jdk1.7 and jdk1.8. So it's '-Xdoclint:none' stop JDK1.8 compatibility. |
@kevinxu021 Thanks for the explanation. But, I am not sure if it answered my question. Can you please be specific if it is using JDK 1.8 or JDK1.7 to compile and produce JDK 1.7 compatible classes, This will allow the jar files to be used with JDK1.7 and JDK1.8. If so, you can proceed with merging this PR. BTW sorry for the delay in responding. |
@selvaganesang Sorry for confused you. Here supporting JDK1.7 and JDK1.8 means compilation. Before the changes, those projects cannot be compiled on JDK1.8 actually. Then if we upgrade JDK from JDK1.7(current version) to JDK1.8, you will see some error on compilation time. This fixes are for that, to keep the code being compiled successfully on JDK1.7 and JDK1.8. |
Closed because of conflict. |
Previously, it doesnot support jdk1.8 because of site. Here use profile as a solution. By the way, to accelerate to the compilation speed, add if-else in Makefile that if you want to re-compile, please do make clean first. With my experiences, most of the time if the error is not in DCS/JDBC, you might not want to recompile this module.