Migrate build system from Maven to Gradle #1121

cliu123 · 2021-04-13T22:39:31Z

opendistro-for-elasticsearch/security pull request intake form

Please provide as much details as possible to get feedback/acceptance on your PR quickly

Category: (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)
Maintenance
Github Issue # or road-map entry, if available:
Description of changes:
Why these changes are required?
Security plugin has 2 build systems: Maven and Gradle(building .rpm and .deb packages). It's necessary to combine them into a single one(Gradle).
What is the old behavior before changes and new behavior after changes? (Please add any example/logs/screen-shot if available)
Testing done: (Please provide details of testing done: Unit testing, integration testing and manual testing)

Unit Tests
Manual Testing:
- Ran ./gradlew clean build --no-daemon -Dbuild.snapshot=false -x test, artifacts are generated in gradle-build/distributions/
- The following artifacts are generated:
  - opendistro_security-1.13.1.0.jar
  - opendistro-security-1.13.1.0.zip
  - opendistro-security-1.13.1.0-securityadmin-standalone.zip
  - opendistro-security-1.13.1.0-securityadmin-standalone.tar.gz
- Installed security plugin generated by Gradle on ES. Securify configurations are loaded into elasticsearch-7.10.2/plugins/opendistro_security/securityconfig
- Ran install_demo_configuration.sh, security settings are added into elasticsearch.yml file.
- The plugin folder name in elasticsearch-7.10.2/plugins is opendistro_security

TO-DOs, if any: (Please describe pending items and provide Github issues# for each of them)
Is it backport from main branch? (If yes, please add backport PR # and commits #)

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

vrozov

Please limit scope of the PR just to migration from maven to gradle build system. Anything not directly related to maven->gradle migration such as UT, java doc, JDK version upgrade should be part of a separate PR(s).

vrozov · 2021-04-14T17:40:37Z

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.2-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-6.8-bin.zip


If gradle version upgrade is necessary, please move that to a separate PR as well.

sujithvm · 2021-04-14T17:45:43Z

build.gradle

+}
+
+dependencies {
+    compile('org.elasticsearch.plugin:transport-netty4-client:7.10.2') {


compile is deprecated in favor of implementation. Can we use that instead?

Replacing all compile with implementation fails compileJava task. Some of them indeed can be replaced. I didn't change them for consistency.

sujithvm · 2021-04-14T17:47:05Z

build.gradle

+    compile('org.apache.httpcomponents:httpclient-cache:4.5.3') {
+        exclude group: 'org.apache.httpcomponents', module: 'httpcore'
+    }
+    compile('org.elasticsearch.client:elasticsearch-rest-high-level-client:7.10.2') {


Please define variable and reuse for same versions. Eg: 7.10.2 is used for a couple of dependencies.

sujithvm · 2021-04-14T17:47:45Z

build.gradle

+    testImplementation 'org.elasticsearch:elasticsearch-ssl-config:7.10.2'
+    testImplementation 'org.elasticsearch.plugin:percolator-client:7.10.2'
+    compile 'org.elasticsearch.plugin:lang-mustache-client:7.10.2'
+    compile 'org.elasticsearch.plugin:parent-join-client:7.10.2'


Group all testImplementation, implementation, compileOnly, runtimeOnly dependencies

By group, do you mean put dependencies with same scope together?

sujithvm · 2021-04-14T17:50:30Z

build.gradle

+configurations.all {
+    if (it.state != Configuration.State.UNRESOLVED) return
+    resolutionStrategy {
+        force 'commons-codec:commons-codec:1.14'


Do all the dependencies need force?

Yes, all these need force because otherwise some dependency conflicts come out.

cliu123 · 2021-04-14T18:04:31Z

Please limit scope of the PR just to migration from maven to gradle build system. Anything not directly related to maven->gradle migration such as UT, java doc, JDK version upgrade should be part of a separate PR(s).

I've excluded java doc from compilation, I can remove the change in java doc.
But all other changes are necessary. If we move all these changes to separate PRs, we'd have to create quite a few PRs and merge them before this PR.
Would that help if I modify the title of the PR and add some description for those changes in the PR description to clarify the scope of the PR?

vrozov · 2021-04-14T20:00:37Z

Change PR and commit title to something like "migrate build system from maven to gradle"

vrozov · 2021-04-14T20:03:55Z

Changing PR title is necessary to avoid confusion regarding the goal of the PR, but it will not help with other issues. If version of gradle needs to be upgraded, it should be a separate PR and commit as in one large PR it is impossible to distinguish changes required for gradle upgrade and changes required for migrating from maven to gradle. The same applies to all other unrelated to maven -> gradle changes.

cliu123 · 2021-07-13T22:18:21Z

Will re-open later if needed.

Signed-off-by: Peter Nied <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]>

cliu123 requested a review from a team April 13, 2021 22:39

cliu123 force-pushed the migrate_build_system branch from 328b85d to e21acbe Compare April 14, 2021 15:48

vrozov suggested changes Apr 14, 2021

View reviewed changes

cliu123 force-pushed the migrate_build_system branch from e21acbe to 08a3f69 Compare April 14, 2021 17:15

vrozov reviewed Apr 14, 2021

View reviewed changes

sujithvm reviewed Apr 14, 2021

View reviewed changes

Chang Liu (cgliu) added 2 commits April 14, 2021 13:56

Migrate build system from Maven to Gradle

9e16da8

Exclude Java Doc from Java compilation

ff003b2

cliu123 force-pushed the migrate_build_system branch from 08a3f69 to ff003b2 Compare April 15, 2021 04:41

cliu123 changed the title ~~Introduce Gradle build system~~ Migrate build system from Maven to Gradle Apr 15, 2021

Chang Liu (cgliu) added 2 commits April 14, 2021 22:25

Revert changes in Java Doc

b9cc0dd

Disable javadoc task

241ab7d

cliu123 force-pushed the migrate_build_system branch from 3184226 to c9b98cd Compare April 15, 2021 22:22

Define variables for dependency version numbers and group dependencies.

3c360dc

cliu123 force-pushed the migrate_build_system branch from c9b98cd to 3c360dc Compare April 15, 2021 22:26

Add jacoco coverage

1860012

cliu123 closed this Jul 13, 2021

davidlago mentioned this pull request Dec 23, 2021

Replace maven by gradle #1533

Closed

rs-eliatra mentioned this pull request Jan 31, 2022

Migrate build system to gradle #1592

Merged

gaobinlong pushed a commit to gaobinlong/security that referenced this pull request Aug 30, 2023

Move to OpenSearch 3.0.0 (opensearch-project#1121)

0e68427

Signed-off-by: Peter Nied <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate build system from Maven to Gradle #1121

Migrate build system from Maven to Gradle #1121

cliu123 commented Apr 13, 2021 •

edited

Loading

vrozov left a comment

vrozov Apr 14, 2021

sujithvm Apr 14, 2021

cliu123 Apr 15, 2021

sujithvm Apr 14, 2021

cliu123 Apr 15, 2021

sujithvm Apr 14, 2021

cliu123 Apr 15, 2021

cliu123 Apr 15, 2021

sujithvm Apr 14, 2021

cliu123 Apr 15, 2021

cliu123 commented Apr 14, 2021 •

edited

Loading

vrozov commented Apr 14, 2021

vrozov commented Apr 14, 2021

cliu123 commented Jul 13, 2021

Migrate build system from Maven to Gradle #1121

Migrate build system from Maven to Gradle #1121

Conversation

cliu123 commented Apr 13, 2021 • edited Loading

opendistro-for-elasticsearch/security pull request intake form

vrozov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cliu123 commented Apr 14, 2021 • edited Loading

vrozov commented Apr 14, 2021

vrozov commented Apr 14, 2021

cliu123 commented Jul 13, 2021

cliu123 commented Apr 13, 2021 •

edited

Loading

cliu123 commented Apr 14, 2021 •

edited

Loading