Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove animal sniffer from low-level REST client #29646

Merged
merged 3 commits into from
Apr 26, 2018

Conversation

jasontedor
Copy link
Member

The low-level REST client targets JDK 7. To avoid compiling against JDK functionality not available in JDK 7, we use animal sniffer. However, when we switched to using the JDK 9 and now the JDK 10 compiler which has built-in support for targeting previous JDKs, we no longer need to use animal sniffer. This is because the JDK is now packaged with the signatures needed to ensure that when we target JDK 7 at compile-time it is detected that we are only using JDK 7 functionality. This commit removes the use of animal sniffer from the low-level REST client build.

The low-level REST client targets JDK 7. To avoid compiling against JDK
functionality not available in JDK 7, we use animal sniffer. However,
when we switched to using the JDK 9 and now the JDK 10 compiler which
has built-in support for targeting previous JDKs, we no longer need to
use animal sniffer. This is because the JDK is now packaged with the
signatures needed to ensure that when we target JDK 7 at compile-time it
is detected that we are only using JDK 7 functionality. This commit
removes the use of animal sniffer from the low-level REST client build.
@jasontedor
Copy link
Member Author

Here are the compiler arguments that we use today when building the low-level REST client:

-d
/home/jason/src/elastic/elasticsearch/client/rest/build/classes/java/main
-encoding
UTF-8
-g
-sourcepath
""
-proc:none
-XDuseUnsharedTable=true
-classpath
/home/jason/.gradle/caches/modules-2/files-2.1/org.apache.httpcomponents/httpclient/4.5.2/733db77aa8d9b2d68015189df76ab06304406e50/httpclient-4.5.2.jar:/home/jason/.gradle/caches/modules-2/files-2.1/org.apache.httpcomponents/httpcore/4.4.5/e7501a1b34325abb00d17dde96150604a0658b54/httpcore-4.4.5.jar:/home/jason/.gradle/caches/modules-2/files-2.1/org.apache.httpcomponents/httpasyncclient/4.1.2/95aa3e6fb520191a0970a73cf09f62948ee614be/httpasyncclient-4.1.2.jar:/home/jason/.gradle/caches/modules-2/files-2.1/org.apache.httpcomponents/httpcore-nio/4.4.5/f4be009e7505f6ceddf21e7960c759f413f15056/httpcore-nio-4.4.5.jar:/home/jason/.gradle/caches/modules-2/files-2.1/commons-codec/commons-codec/1.10/4b95f4897fa13f2cd904aee711aeafc0c5295cd8/commons-codec-1.10.jar:/home/jason/.gradle/caches/modules-2/files-2.1/commons-logging/commons-logging/1.1.3/f6f66e966c70a83ffbdb6f17a0919eaf7c8aca7f/commons-logging-1.1.3.jar
-Werror
-Xlint:all,-path,-serial,-options,-deprecation
-Xdoclint:all
-Xdoclint:-missing
-proc:none
--release
7
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/HttpDeleteWithEntity.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/ResponseListener.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/HttpGetWithEntity.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/Response.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/HeapBufferedAsyncResponseConsumer.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/HttpAsyncResponseConsumerFactory.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/DeadHostState.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/RequestLogger.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/RestClient.java
/home/jason/src/elastic/elasticsearch/client/rest/src/main/java/org/elasticsearch/client/ResponseException.java

The relevant component here is the --release 7.

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks @jasontedor

@colings86 colings86 added >test Issues or PRs that are addressing/adding tests :Core/Java High Level REST Client labels Apr 24, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

* master: (7067 commits)
  Add 6.4.0 version to master (elastic#29684)
  Add comments inadvertently removed during migrate
  [ML] Wait for updates to established memory usage
  Add build time checks for package licenses
  Build: Fix License attribute to be written in deb control data
  Build: Use templated copyright file for deb distributions
  Fix the dashes in license names
  Only enable modules to have native controllers
  Fix SQL CLI on Windows
  Add distribution type to startup scripts
  Fix classpath for X-Pack scripts on Windows
  Rename syskeygen
  Rename users
  Rename migrate
  Rename saml-metadata
  Rename sql-cli
  Rename croneval
  Rename setup-passwords
  Rename certutil
  Rename certgen
  ...
@jasontedor
Copy link
Member Author

test this please

@jasontedor
Copy link
Member Author

run gradle build tests

@jasontedor
Copy link
Member Author

test this please

* master:
  [Test] Fix docs check for DEB package in packaging tests (elastic#30126)
  Painless: Docs Clean Up (elastic#29592)
  Fixes Eclipse build for sql jdbc project (elastic#30114)
  Remove reference to `not_analyzed`.
  [Docs] Add community analysis plugin (elastic#29612)
  Remove the suggest metric from stats APIs (elastic#29635)
  Fix byte size value equals/hash code test (elastic#29643)
  Upgrade to Gradle 4.7 (elastic#29644)
@jasontedor jasontedor merged commit 7a74b19 into elastic:master Apr 26, 2018
@jasontedor jasontedor deleted the remove-animal-sniffer branch April 26, 2018 02:41
jasontedor added a commit that referenced this pull request Apr 26, 2018
The low-level REST client targets JDK 7. To avoid compiling against JDK
functionality not available in JDK 7, we use animal sniffer. However,
when we switched to using the JDK 9 and now the JDK 10 compiler which
has built-in support for targeting previous JDKs, we no longer need to
use animal sniffer. This is because the JDK is now packaged with the
signatures needed to ensure that when we target JDK 7 at compile-time it
is detected that we are only using JDK 7 functionality. This commit
removes the use of animal sniffer from the low-level REST client build.
@jasontedor
Copy link
Member Author

Thanks @javanna and @nik9000.

martijnvg added a commit that referenced this pull request Apr 26, 2018
* es/master:
  Watcher: Fold two smoke test projects into smoke-test-watcher (#30137)
  In the field capabilities API, deprecate support for providing fields in the request body. (#30157)
  Set JAVA_HOME before forking setup commands (#29647)
  Remove animal sniffer from low-level REST client (#29646)
  Cleanup .gitignore (#30145)
  Do not add noop from local translog to translog again (#29637)
  Build: Assert jar LICENSE and NOTICE files match
  Correct transport compression algorithm in docs (#29645)
  [Test] Fix docs check for DEB package in packaging tests (#30126)
  Painless: Docs Clean Up (#29592)
  Fixes Eclipse build for sql jdbc project (#30114)
  Remove reference to `not_analyzed`.
  [Docs] Add community analysis plugin (#29612)
martijnvg added a commit that referenced this pull request Apr 26, 2018
* es/6.x:
  In the field capabilities API, deprecate support for providing fields in the request body. (#30157)
  Set JAVA_HOME before forking setup commands (#29647)
  Remove animal sniffer from low-level REST client (#29646)
  Cleanup .gitignore (#30145)
  Do not add noop from local translog to translog again (#29637)
  Painless: Docs Clean Up (#29592)
  Build: Assert jar LICENSE and NOTICE files match
  Correct transport compression algorithm in docs (#29645)
  AwaitsFix for testGradleVersionsMatchVersionUtils
  [Test] Fix docs check for DEB package in packaging tests (#30126)
  Remove reference to `not_analyzed`.
  [Docs] Add community analysis plugin (#29612)
  [DOCS] Removed differencies between text and code (#27993)
javanna added a commit to javanna/elasticsearch that referenced this pull request Apr 30, 2018
Animal sniffer is no longer needed, we can remove it for sniffer like
we did for the low-level REST client with elastic#29646
javanna added a commit that referenced this pull request May 1, 2018
Animal sniffer is no longer needed, we can remove it for sniffer like
we did for the low-level REST client with #29646
javanna added a commit that referenced this pull request May 1, 2018
Animal sniffer is no longer needed, we can remove it for sniffer like
we did for the low-level REST client with #29646
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>test Issues or PRs that are addressing/adding tests v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants