Thanks to https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437,
we've run into a situation where Spotless is incorrectly formatting
a particular piece of syntax (due the underlying Eclipse bug). We
were able to turn off formatting of this syntax using `// @formatter:off`
and `// @formatter:on`, but there was a further problem. We configure
IntelliJ to use the Eclipse formatter plugin, but this doesn't
respect the `@formatter` tags since these are set at the Spotless
level, not the Eclipse formatter level. Note that these tags aren't
set in the Eclipse formatter config, because there we use `// tag::`
and `// end::` in order to avoid reformatting docs snippets, which
have a much narrower line width.
What a mess.
So, to get around all this, drop the `@formatter` tags and tweak
our custom `SnippetLengthCheck` Checkstyle rule so that
`// tag:noformat` regions are not subject to the narrower line length
check, but are still exempt from formatting.
Our spotless configuration wasn't being applied to `build-tools`
and `build-tools-internal`. Move the Spotless configuration to a
Java plugin in `build-conventions`, and apply it everywhere.
This resulted in a lot more Java files being subject to formatting,
so I added more exclusions to the list.
Also remove the `paddedCell` stuff, we've never needed it.
This introduces a general document to track guidelines for working on and with
the elasticsearch gradle build
Co-authored-by: Rory Hunter <pugnascotia@users.noreply.github.com>
Co-authored-by: Rory Hunter <pugnascotia@users.noreply.github.com>
Change the formatter config to sort / order imports, and reformat the
codebase. We already had a config file for Eclipse users, so Spotless now
uses that.
The "Eclipse Code Formatter" plugin ought to be able to use this file as
well for import ordering, but in my experiments the results were poor.
Instead, use IntelliJ's `.editorconfig` support to configure import
ordering.
I've also added a config file for the formatter plugin.
Other changes:
* I've quietly enabled the `toggleOnOff` option for Spotless. It was
already possible to disable formatting for sections using the markers
for docs snippets, so enabling this option just accepts this reality
and makes it possible via `formatter:off` and `formatter:on` without
the restrictions around line length. It should still only be used as
a very last resort and with good reason.
* I've removed mention of the `paddedCell` option from the contributing
guide, since I haven't had to use that option for a very long time. I
moved the docs to the spotless config.
This brings CONTRIBUTING.md in line with master, mostly picking up the
javadoc discussion I just applied in #71924, but also picking up some
left overs that we'd never backported.
The build has been updated to require JDK15 for compilation but the
contributing docs were out of date. This commit updates the docs to
have the correct JDK requirement.
Backport of #69824
It turns out you can configure custom Checkstyle rules using the plugin
for IntelliJ, you just have to tell the plugin where to find the code.
Therefore, update the contributing docs with the necessary process.
Also fork the Checkstyle rule `MissingJavadocTypeCheck` so that we can
disable it for test classes. While it's useful to describe test cases in
test methods' Javadoc, this is somewhat less true for test classes
themselves.
As per the new licensing change for Elasticsearch and Kibana this commit
moves existing Apache 2.0 licensed source code to the new dual license
SSPL+Elastic license 2.0. In addition, existing x-pack code now uses
the new version 2.0 of the Elastic license. Full changes include:
- Updating LICENSE and NOTICE files throughout the code base, as well
as those packaged in our published artifacts
- Update IDE integration to now use the new license header on newly
created source files
- Remove references to the "OSS" distribution from our documentation
- Update build time verification checks to no longer allow Apache 2.0
license header in Elasticsearch source code
- Replace all existing Apache 2.0 license headers for non-xpack code
with updated header (vendored code with Apache 2.0 headers obviously
remains the same).
- Replace all Elastic license 1.0 headers with new 2.0 header in xpack.
Storing the generated config in the `build` dir meant that it could be
removed for various reasons, which causes e.g. IntelliJ to pop up
dialog, and is a mild annoyance. Instead, write the file to the project
root.
Also inline the path to the suppressions config, which removes a step
from the setup instructions.
Backport of #66109.
With a few changes, our checkstyle config can be used by e.g. IntelliJ's
Checkstyle plugin. Add a task to generate this file automatically, and
create the necessary IntelliJ config to use it.
Also add a line-length setting to .editorconfig.
The review phase is an important part of contributing to Elasticsearch, but we
do not mention it in our instructions to contributors. This commit adds some
notes on how contributions are reviewed and describes some common reasons for
rejection.
Co-authored-by: Colin Goodheart-Smithe <colings86@users.noreply.github.com>
* Remove usage of deprecated testCompile configuration
* Replace testCompile usage by testImplementation
* Make testImplementation non transitive by default (as we did for testCompile)
* Update CONTRIBUTING about using testImplementation for test dependencies
* Fail on testCompile configuration usage
This commit removes the configuration time vs execution time distinction
with regards to certain BuildParms properties. Because of the cost of
determining Java versions for configuration JDK locations we deferred
this until execution time. This had two main downsides. First, we had
to implement all this build logic in tasks, which required a bunch of
additional plumbing and complexity. Second, because some information
wasn't known during configuration time, we had to nest any build logic
that depended on this in awkward callbacks.
We now defer to the JavaInstallationRegistry recently added in Gradle.
This utility uses a much more efficient method for probing Java
installations vs our jrunscript implementation. This, combined with some
optimizations to avoid probing the current JVM as well as deferring
some evaluation via Providers when probing installations for BWC builds
we can maintain effectively the same configuration time performance
while removing a bunch of complexity and runtime cost (snapshotting
inputs for the GenerateGlobalBuildInfoTask was very expensive). The end
result should be a much more responsive build execution in almost all
scenarios.
(cherry picked from commit ecdbd37f2e)
This commit makes a number of improvements when importing the
Elasticsearch project into IntelliJ IDEA. Specifically:
- Contributing documentation has been updated to reflect that the
'idea' task should no long be used and Gradle project import is
instead the officially supported way of setting up the project.
- Attempts to run the 'idea' task will result in a failure with a
message directing folks to our CONTRIBUTING.md document.
- The project JDK is explicit set rather that using whatever JAVA_HOME
is.
- Gradle build operation delegation is disabled, and test execution is
configured to 'choose per test'.
- Gradle is configured to inherit the project JDK.
- Some code style conventions are automatically configured.
- File encoding is explicitly set to UTF-8.
- Parallel module compilation is enabled and deprecated feature
warnings are disabled.
- A remote debug run configuration using listen mode is created.
- JUnit runner is configured with required system properties.
- License headers are configured such that Apache 2 is the default
notice added to all source files with exception of source in /x-pack
which will use the Elastic license.
We have a long history of advancing the required compiler to the newest
JDK. JDK 13 has been with us for awhile, but we were blocked from
upgrading since Gradle was not compatible with JDK 13. With the
advancement in our project to Gradle 6 which supports JDK 13, we can now
advance our minimum compiler version. This commit updates the minimum
compiler version to JDK 13.
Some unit test checking locale sensitive functionality require the
-Djava.locale.providers=SPI,COMPAT flag to be set. When running tests though
gradle we pass this already to the BuildPlugin, but running from the IDE this
might need to be set manually. Adding a note explaining this to the
CONTRIBUTING.md doc and leaving a note in the test comment of
SearchQueryIT.testRangeQueryWithLocaleMapping which is a test we know
that suffers from this issue.
With this commit we add a clarifying note in the contribution guidelines
that our examples show the usage on Unix and also explain how to invoke
the Gradle wrapper script on Windows.
Closes#49521
The team sometimes get questions around the use of `!foo` vs. `foo == false` in
PRs and reviews (e.g. #48615). This change adds a bullet point to CONTRIBUTING.md
to make expectations here clearer and gives us something to point to in case of
discussion.
Backport of #48446. Update the Eclipse formatter settings as follows:
* Split arguments for methods, constructors etc into separate lines when
they wrap
* Enable "joined wrapped lines", which basically means the formatter
will ignore existing line breaks and join and re-wrap statements. I
had originally hesitated about enabling this, but without it the
codebase's style won't be as consistent as it should be, defeating the
point of all this formatting business.
* Disable formatting between `// tag::` and `// end::` comments, which
are the code snippet markers for the docs. They need a different line
length, which isn't possible to configure separately.
* Detect and preserve aligned line comments at the end of consecutive
lines.
* And a bunch of other tweaks.
Also update the contribution guidelines.
Today our instructions in `CONTRIBUTING.md` regarding license headers are a
little unclear. This commit clarifies the exact headers that are required for
contributions.
* Remove eclipse conditionals
We used to have some meta projects with a `-test` prefix because
historically eclipse could not distinguish between test and main
source-sets and could only use a single classpath.
This is no longer the case for the past few Eclipse versions.
This PR adds the necessary configuration to correctly categorize source
folders and libraries.
With this change eclipse can import projects, and the visibility rules
are correct e.x. auto compete doesn't offer classes from test code or
`testCompile` dependencies when editing classes in `main`.
Unfortunately the cyclic dependency detection in Eclipse doesn't seem to
take the difference between test and non test source sets into account,
but since we are checking this in Gradle anyhow, it's safe to set to
`warning` in the settings. Unfortunately there is no setting to ignore
it.
This might cause problems when building since Eclipse will probably not
know the right order to build things in so more wirk might be necesarry.
This commit adds a Java source formatter and checker into the build process.
This is not yet enabled for any sub-projects - to format and check a
sub-project, add its Gradle path into `build.gradle` and run:
./gradlew spotlessApply
to format, and:
./gradlew spotlessJavaCheck
# or:
./gradlew precommit
to verify formatting.
After we bumped the JDK compiler minimum requirements to JDK 11, we
missed updating the contributing docs to point to JDK 11 instead of JDK
10. This commit fixes that.
Additionally, when we stopped requiring JDK 7 to run the reindex from
old tests, we did not update the contributing docs to reflect this. This
commit also fixes that.
This commit removes the suggestions to send a PR with instructions for
more IDEs on disabling wildcard imports. This is not explicitly needed
both because the main IDEs instructions already exist, and any part of
the docs are welcome for PRs to add more clarity.
closes#34415
This reworks how we configure the `shadow` plugin in the build. The major
change is that we no longer bundle dependencies in the `compile` configuration,
instead we bundle dependencies in the new `bundle` configuration. This feels
more right because it is a little more "opt in" rather than "opt out" and the
name of the `bundle` configuration is a little more obvious.
As an neat side effect of this, the `runtimeElements` configuration used when
one project depends on another now contains exactly the dependencies needed
to run the project so you no longer need to reference projects that use the
shadow plugin like this:
```
testCompile project(path: ':client:rest-high-level', configuration: 'shadow')
```
You can instead use the much more normal:
```
testCompile "org.elasticsearch.client:elasticsearch-rest-high-level-client:${version}"
```
This bundles the x-pack:protocol project into the x-pack:plugin:core
project because we'd like folks to consider it an implementation detail
of our build rather than a separate artifact to be managed and depended
on. It is now bundled into both x-pack:plugin:core and
client:rest-high-level. To make this work I had to fix a few things.
Firstly, I had to make PluginBuildPlugin work with the shadow plugin.
In that case we have to bundle only the `shadow` dependencies and the
shadow jar.
Secondly, every reference to x-pack:plugin:core has to use the `shadow`
configuration. Without that the reference is missing all of the
un-shadowed dependencies. I tried to make it so that applying the shadow
plugin automatically redefines the `default` configuration to mirror the
`shadow` configuration which would allow us to use bare project references
to the x-pack:plugin:core project but I couldn't make it work. It'd *look*
like it works but then fail for transitive dependencies anyway. I think
it is still a good thing to do but I don't have the willpower to do it
now.
Finally, I had to fix an issue where Eclipse and IntelliJ didn't properly
reference shadowed transitive dependencies. Neither IDE supports shadowing
natively so they have to reference the shadowed projects. We fix this by
detecting `shadow` dependencies when in "Intellij mode" or "Eclipse mode"
and adding `runtime` dependencies to the same target. This convinces
IntelliJ and Eclipse to play nice.
Adds a description of the most important subdirectories to
`CONTRIBUTING.md` to help folks that are less familiar with the project
get their bearings. It reflects that state of the project right now so
it will inevitably go out of date. But I'll try and keep it up to date.
* add copyright/scope configuration for intellij
This commit introduces a section discussing the ability to define
copyright rules in intellij for inserting the appropriate copyright
headers into files across both apache2-Elasticsearch, as well as the
commercial code under `x-pack`.
Some other re-organization was made to create more sub-structure
* update to reflect reviewer comments
