You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Anthony Baker <ab...@apache.org> on 2016/11/09 05:08:43 UTC
Review Request 53596: GEODE-1808 Remove broken check for jdk1.7.0_72
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53596/
-----------------------------------------------------------
Review request for geode, Jacob Barrett and Kirk Lund.
Repository: geode
Description
-------
The logic for evaluating jdk versions is incorrect for 3-digit builds.
Since we require a jdk1.8 version anyway, remove the check.
Diffs
-----
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java e1b2007c12804d3fb668ef42e976b8f5a1bcb3d5
geode-core/src/main/java/org/apache/geode/internal/cache/MinimumSystemRequirements.java a2c1874d9b37ee0268a6848a4c4ca98dc92cc16f
geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java fbe18d048bfce2bb1ce7fb30962f97bec52b5091
Diff: https://reviews.apache.org/r/53596/diff/
Testing
-------
Thanks,
Anthony Baker
Re: Review Request 53596: GEODE-1808 Remove broken check for
jdk1.7.0_72
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53596/#review155446
-----------------------------------------------------------
Ship it!
Ship It!
- Kirk Lund
On Nov. 9, 2016, 5:08 a.m., Anthony Baker wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53596/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2016, 5:08 a.m.)
>
>
> Review request for geode, Jacob Barrett and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> The logic for evaluating jdk versions is incorrect for 3-digit builds.
> Since we require a jdk1.8 version anyway, remove the check.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java e1b2007c12804d3fb668ef42e976b8f5a1bcb3d5
> geode-core/src/main/java/org/apache/geode/internal/cache/MinimumSystemRequirements.java a2c1874d9b37ee0268a6848a4c4ca98dc92cc16f
> geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java fbe18d048bfce2bb1ce7fb30962f97bec52b5091
>
> Diff: https://reviews.apache.org/r/53596/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anthony Baker
>
>
Re: Review Request 53596: GEODE-1808 Remove broken check for
jdk1.7.0_72
Posted by anilkumar gingade <ag...@pivotal.io>.
> On Nov. 9, 2016, 5:36 p.m., anilkumar gingade wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java, line 1189
> > <https://reviews.apache.org/r/53596/diff/1/?file=1558982#file1558982line1189>
> >
> > This is verifying java version when cache is getting created and warns the user if its not compatible version...I think we should keep this instead of removing it...
>
> Anthony Baker wrote:
> Anil, this is verifying that the java version is at least 1.7.0_72. It was originally added due to an xml parsing bug in certain jdk versions. We now require jdk1.8 as a minimum where this is not an issue. In fact, if you try to run geode with a 1.7 jdk you get a class version exception.
>
> If you look at the version parsing logic, it's broken anyway when trying to compare {version}_NN with {version}_NNN. This shows up when trying to run with a jdk9 jvm.
Got it, thanks...
- anilkumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53596/#review155455
-----------------------------------------------------------
On Nov. 9, 2016, 5:08 a.m., Anthony Baker wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53596/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2016, 5:08 a.m.)
>
>
> Review request for geode, Jacob Barrett and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> The logic for evaluating jdk versions is incorrect for 3-digit builds.
> Since we require a jdk1.8 version anyway, remove the check.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java e1b2007c12804d3fb668ef42e976b8f5a1bcb3d5
> geode-core/src/main/java/org/apache/geode/internal/cache/MinimumSystemRequirements.java a2c1874d9b37ee0268a6848a4c4ca98dc92cc16f
> geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java fbe18d048bfce2bb1ce7fb30962f97bec52b5091
>
> Diff: https://reviews.apache.org/r/53596/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anthony Baker
>
>
Re: Review Request 53596: GEODE-1808 Remove broken check for
jdk1.7.0_72
Posted by Anthony Baker <ab...@apache.org>.
> On Nov. 9, 2016, 5:36 p.m., anilkumar gingade wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java, line 1189
> > <https://reviews.apache.org/r/53596/diff/1/?file=1558982#file1558982line1189>
> >
> > This is verifying java version when cache is getting created and warns the user if its not compatible version...I think we should keep this instead of removing it...
Anil, this is verifying that the java version is at least 1.7.0_72. It was originally added due to an xml parsing bug in certain jdk versions. We now require jdk1.8 as a minimum where this is not an issue. In fact, if you try to run geode with a 1.7 jdk you get a class version exception.
If you look at the version parsing logic, it's broken anyway when trying to compare {version}_NN with {version}_NNN. This shows up when trying to run with a jdk9 jvm.
- Anthony
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53596/#review155455
-----------------------------------------------------------
On Nov. 9, 2016, 5:08 a.m., Anthony Baker wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53596/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2016, 5:08 a.m.)
>
>
> Review request for geode, Jacob Barrett and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> The logic for evaluating jdk versions is incorrect for 3-digit builds.
> Since we require a jdk1.8 version anyway, remove the check.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java e1b2007c12804d3fb668ef42e976b8f5a1bcb3d5
> geode-core/src/main/java/org/apache/geode/internal/cache/MinimumSystemRequirements.java a2c1874d9b37ee0268a6848a4c4ca98dc92cc16f
> geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java fbe18d048bfce2bb1ce7fb30962f97bec52b5091
>
> Diff: https://reviews.apache.org/r/53596/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anthony Baker
>
>
Re: Review Request 53596: GEODE-1808 Remove broken check for
jdk1.7.0_72
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53596/#review155455
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
<https://reviews.apache.org/r/53596/#comment225432>
This is verifying java version when cache is getting created and warns the user if its not compatible version...I think we should keep this instead of removing it...
- anilkumar gingade
On Nov. 9, 2016, 5:08 a.m., Anthony Baker wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53596/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2016, 5:08 a.m.)
>
>
> Review request for geode, Jacob Barrett and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> The logic for evaluating jdk versions is incorrect for 3-digit builds.
> Since we require a jdk1.8 version anyway, remove the check.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java e1b2007c12804d3fb668ef42e976b8f5a1bcb3d5
> geode-core/src/main/java/org/apache/geode/internal/cache/MinimumSystemRequirements.java a2c1874d9b37ee0268a6848a4c4ca98dc92cc16f
> geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java fbe18d048bfce2bb1ce7fb30962f97bec52b5091
>
> Diff: https://reviews.apache.org/r/53596/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anthony Baker
>
>