You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@curator.apache.org by GitBox <gi...@apache.org> on 2022/02/25 18:05:56 UTC
[GitHub] [curator] martin-g opened a new pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
martin-g opened a new pull request #410:
URL: https://github.com/apache/curator/pull/410
Continuation of #407
I will rebase it to `master` once #407 is merged!
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] martin-g commented on a change in pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #410:
URL: https://github.com/apache/curator/pull/410#discussion_r815746393
##########
File path: curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java
##########
@@ -45,6 +45,23 @@
/** GZIP header magic number. */
private static final int GZIP_MAGIC = 0x8b1f;
+ private static final String JAVA_VERSION_STR = System.getProperty("java.version", "-1");
+ private static final int JAVA_VERSION;
+ static {
+ if ("1.8".equals(JAVA_VERSION_STR)) {
+ JAVA_VERSION = 8;
+ } else if (JAVA_VERSION_STR.contains("-")) {
+ // e.g. 18-ea, take 18
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR.split("-", 2)[0]);
+ } else if (JAVA_VERSION_STR.contains(".")) {
+ // e.g. 17.0.1, take 17
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR.split("\\.", 2)[0]);
+ } else {
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR, 10);
+ }
+ }
+ private static final byte OS_BIT = JAVA_VERSION >= 16 ? (byte) -1 : 0;
Review comment:
I will add it as a comment!
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] martin-g commented on pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #410:
URL: https://github.com/apache/curator/pull/410#issuecomment-1051309225
https://github.com/apache/curator/pull/410/commits/f6bac3372130882ee1991b087283842f542474ad "fixes" the problem in org.apache.curator.framework.imps.GzipCompressionProvider but it does not look nice.
I'd recommend a deeper look here!
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] eolivelli commented on pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #410:
URL: https://github.com/apache/curator/pull/410#issuecomment-1054126070
> Is it valueable ? Should I try to upgrade it or just delete it ?
if we can keep the existing tests it is better, and it would be great to convert the test
that said, I am not sure about the real value of that test
maybe @Randgalt or @cammckenzie know better than me.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] nicoloboschi commented on a change in pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on a change in pull request #410:
URL: https://github.com/apache/curator/pull/410#discussion_r815698375
##########
File path: curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java
##########
@@ -45,6 +45,23 @@
/** GZIP header magic number. */
private static final int GZIP_MAGIC = 0x8b1f;
+ private static final String JAVA_VERSION_STR = System.getProperty("java.version", "-1");
+ private static final int JAVA_VERSION;
+ static {
+ if ("1.8".equals(JAVA_VERSION_STR)) {
+ JAVA_VERSION = 8;
+ } else if (JAVA_VERSION_STR.contains("-")) {
+ // e.g. 18-ea, take 18
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR.split("-", 2)[0]);
+ } else if (JAVA_VERSION_STR.contains(".")) {
+ // e.g. 17.0.1, take 17
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR.split("\\.", 2)[0]);
+ } else {
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR, 10);
+ }
+ }
+ private static final byte OS_BIT = JAVA_VERSION >= 16 ? (byte) -1 : 0;
Review comment:
could you add a comment explaining why this is needed ?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] martin-g closed pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
martin-g closed pull request #410:
URL: https://github.com/apache/curator/pull/410
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] martin-g commented on pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #410:
URL: https://github.com/apache/curator/pull/410#issuecomment-1054096489
Now the build on JDK 17 fails due to Jersey.
https://github.com/apache/curator/blob/fe50da4904aa4ca4f51b473ad298ae664358ff3c/pom.xml#L84-L86 says that it is not worth it to upgrade.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] martin-g commented on a change in pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #410:
URL: https://github.com/apache/curator/pull/410#discussion_r815707779
##########
File path: curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java
##########
@@ -45,6 +45,23 @@
/** GZIP header magic number. */
private static final int GZIP_MAGIC = 0x8b1f;
+ private static final String JAVA_VERSION_STR = System.getProperty("java.version", "-1");
+ private static final int JAVA_VERSION;
+ static {
+ if ("1.8".equals(JAVA_VERSION_STR)) {
+ JAVA_VERSION = 8;
+ } else if (JAVA_VERSION_STR.contains("-")) {
+ // e.g. 18-ea, take 18
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR.split("-", 2)[0]);
+ } else if (JAVA_VERSION_STR.contains(".")) {
+ // e.g. 17.0.1, take 17
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR.split("\\.", 2)[0]);
+ } else {
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR, 10);
+ }
+ }
+ private static final byte OS_BIT = JAVA_VERSION >= 16 ? (byte) -1 : 0;
Review comment:
I just reverse engineered it.
I don't know what exactly has changed in JDK 16 that caused the breakage here.
This is the reason I said `I'd recommend a deeper look here!`. Someone who knows this code better may have better idea what is going on.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] martin-g commented on pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #410:
URL: https://github.com/apache/curator/pull/410#issuecomment-1054116019
> Also I am not sure about why we are using Jersey in Curator...
> do you want to try to upgrade please ?
It is used only in curator-x-discovery-server/src/test/java/org/apache/curator/x/discovery/server/jetty_jersey/TestMapsWithJersey.java
Is it valueable ? Should I try to upgrade it or just delete it ?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] nicoloboschi commented on a change in pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on a change in pull request #410:
URL: https://github.com/apache/curator/pull/410#discussion_r815724171
##########
File path: curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java
##########
@@ -45,6 +45,23 @@
/** GZIP header magic number. */
private static final int GZIP_MAGIC = 0x8b1f;
+ private static final String JAVA_VERSION_STR = System.getProperty("java.version", "-1");
+ private static final int JAVA_VERSION;
+ static {
+ if ("1.8".equals(JAVA_VERSION_STR)) {
+ JAVA_VERSION = 8;
+ } else if (JAVA_VERSION_STR.contains("-")) {
+ // e.g. 18-ea, take 18
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR.split("-", 2)[0]);
+ } else if (JAVA_VERSION_STR.contains(".")) {
+ // e.g. 17.0.1, take 17
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR.split("\\.", 2)[0]);
+ } else {
+ JAVA_VERSION = Integer.parseInt(JAVA_VERSION_STR, 10);
+ }
+ }
+ private static final byte OS_BIT = JAVA_VERSION >= 16 ? (byte) -1 : 0;
Review comment:
I think this is the related issue
https://bugs.openjdk.java.net/browse/JDK-8244706
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] eolivelli commented on pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #410:
URL: https://github.com/apache/curator/pull/410#issuecomment-1054098373
I am +1 to upgrading Jersey.
That comment (https://stackoverflow.com/questions/17098341#22033825) is very old.
Also I am not sure about why we are using Jersey in Curator...
do you want to try to upgrade please ?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] martin-g commented on pull request #410: Test with JDK 17. Update Mockito and JUnit 5.x
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #410:
URL: https://github.com/apache/curator/pull/410#issuecomment-1054294279
I've updated Jersey to it latest 2.x version but there is still some problem - the MapDiscoveryContext is not seen as a `@Provider` and `org.apache.curator.x.discovery.server.jetty_jersey.MapDiscoveryResource`'s constructor fails with a NPE. I'll try to debug it later.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org