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.

Sign up for GitHub

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

Jump to bottom

disable annotation processor for docs #30610

Merged
merged 6 commits into from
May 21, 2018
Merged

disable annotation processor for docs #30610

merged 6 commits into from
May 21, 2018

Conversation

alpar-t
Copy link
Contributor

@alpar-t alpar-t commented May 15, 2018 •
edited
Loading

Could not find evidence that the log4j annotation processor is used.
The compiler flag enables the Gradle 5.0 behavior

Closes #30476.

@alpar-t alpar-t added >non-issue :Delivery/Build Build or test infrastructure labels May 15, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@alpar-t
disable annotation processor for docs
54ab648 
Could not find evidence that the log4j annotation processor is used.
The compiler flag enables the Gradle 5.0 behavior

Closes elastic#30476
nik9000
nik9000 reviewed May 15, 2018
View reviewed changes
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

I wonder if we should apply the build standard build plugin here instead. It brings that along for free.

@alpar-t
Copy link
Contributor Author

alpar-t commented May 16, 2018

@nik9000 I'm not sure which plugin you mean

@rjernst
Copy link
Member

rjernst commented May 16, 2018

@atorok Nik is talking about the BuildPlugin in buildSrc, which is applied to all java projects in elasticsearch. Are you sure this PR fixes the entire project, or only :docs? The log4j depdendency is through core, and basically everything depends on that, so I would expect all compile tasks to have this warning. I would add this flag to BuildPlugin.configureCompile(...)

@alpar-t
Copy link
Contributor Author

alpar-t commented May 16, 2018

I would have expected it to pop up for other gradle projects as well, but it only triggered for :docs just tested it again. I'm not sure why that is, but don't see any harm in moving it to the BuildPlugin.

@alpar-t
Copy link
Contributor Author

alpar-t commented May 16, 2018 •
edited
Loading

Oh I see what is going on now. The build plugin already takes care of this, but it's not applied to docs because it's mutually exclusive with elasticsearch.docs-test. I'll ad the config to the other plugins too.

rjernst
rjernst reviewed May 16, 2018
View reviewed changes
buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestTestPlugin.groovy Outdated
@@ -48,5 +49,12 @@ public class RestTestPlugin implements Plugin<Project> {
integTest.group = JavaBasePlugin.VERIFICATION_GROUP
integTest.mustRunAfter(project.precommit)
project.check.dependsOn(integTest)

project.tasks.withType(JavaCompile) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is actually needed, because StandaloneTestPlugin is required, which also has this. In fact, I think what is here now will cause -proc:none to be in the compile args twice?

rjernst
rjernst approved these changes May 18, 2018
View reviewed changes
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM, one last nit

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestTestPlugin.groovy Outdated
@@ -23,6 +23,7 @@ import org.gradle.api.InvalidUserDataException
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.plugins.JavaBasePlugin
import org.gradle.api.tasks.compile.JavaCompile
Copy link
Member

Choose a reason for hiding this comment

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

Looks like the changes in this file are extraneous and should be reverted

@alpar-t alpar-t merged commit 638383d into elastic:master May 21, 2018
@alpar-t alpar-t deleted the fix/30476 branch May 21, 2018 06:43
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 21, 2018
@jasontedor
Merge remote-tracking branch 'elastic/master' into packaging-script-o…
bd65664 
…ne-liners

* elastic/master:
  [TEST] Enable DEBUG logging on testAutoQueueSizingWithMax
  [ML] Don't install empty ML metadata on startup (elastic#30751)
  Add assertion on removing copy_settings (elastic#30748)
  bump lucene version for 6_3_0
  [DOCS] Mark painless execute api as experimental (elastic#30710)
  disable annotation processor for docs (elastic#30610)
  Add more script contexts (elastic#30721)
  Fix default shards count in create index docs (elastic#30747)
dnhatn added a commit that referenced this pull request May 21, 2018
@dnhatn
Merge branch 'master' into ccr
3f6434c 
* master:
  Reduce CLI scripts to one-liners (#30759)
  SQL: Preserve scoring in bool queries (#30730)
  QA: Switch rolling upgrade to 3 nodes (#30728)
  [TEST] Enable DEBUG logging on testAutoQueueSizingWithMax
  [ML] Don't install empty ML metadata on startup (#30751)
  Add assertion on removing copy_settings (#30748)
  bump lucene version for 6_3_0
  [DOCS] Mark painless execute api as experimental (#30710)
  disable annotation processor for docs (#30610)
  Add more script contexts (#30721)
  Fix default shards count in create index docs (#30747)
  Mute testCorruptFileThenSnapshotAndRestore
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request May 22, 2018
@martijnvg
Merge remote-tracking branch 'es/ccr' into ccr_create_and_follow
139977e 
* es/ccr: (50 commits)
  Reduce CLI scripts to one-liners (elastic#30759)
  SQL: Preserve scoring in bool queries (elastic#30730)
  QA: Switch rolling upgrade to 3 nodes (elastic#30728)
  [TEST] Enable DEBUG logging on testAutoQueueSizingWithMax
  [ML] Don't install empty ML metadata on startup (elastic#30751)
  Add assertion on removing copy_settings (elastic#30748)
  bump lucene version for 6_3_0
  [DOCS] Mark painless execute api as experimental (elastic#30710)
  disable annotation processor for docs (elastic#30610)
  Add more script contexts (elastic#30721)
  Fix default shards count in create index docs (elastic#30747)
  Mute testCorruptFileThenSnapshotAndRestore
  Scripting: Remove getDate methods from ScriptDocValues (elastic#30690)
  Upgrade to Lucene-7.4.0-snapshot-59f2b7aec2 (elastic#30726)
  [Docs] Fix single page :docs:check invocation (elastic#30725)
  Docs: Add uptasticsearch to list of clients (elastic#30738)
  [DOCS] Removes out-dated x-pack/docs/en/index.asciidoc
  [DOCS] Removes redundant index.asciidoc files (elastic#30707)
  [TEST] Reduce forecast overflow to disk test memory limit (elastic#30727)
  Plugins: Remove meta plugins (elastic#30670)
  ...
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nik9000 nik9000 nik9000 left review comments

@rjernst rjernst rjernst approved these changes

Assignees
No one assigned
Labels
:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Gradle deprecation warnings: detecting annotation processors on the compile classpath
5 participants
@alpar-t @elasticmachine @rjernst @nik9000 @mark-vieira