You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2017/06/22 01:23:05 UTC
[kudu-CR] WIP: Add gradle build
Grant Henke has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7258
Change subject: WIP: Add gradle build
......................................................................
WIP: Add gradle build
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
Remaining Work:
- Partially shade kudu-client (async and slf4j)
- Validate and adjust pom contents
- Investigate test failures
- Other inline TODO comments
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.jar
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/gradlew.bat
A java/interface-annotations/build.gradle
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
A java/kudu-jepsen/build.gradle
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ITBigLinkedList.scala
A java/kudu-spark/build.gradle
A java/settings.gradle
28 files changed, 984 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/1
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#8).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper/gradle-wrapper.jar
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/gradlew.bat
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
34 files changed, 1,442 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/8
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 14:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7258/17/java/build.gradle
File java/build.gradle:
Line 19: buildscript { apply from: file('gradle/buildscript.gradle'), to: buildscript }
Got trailing whitespace here.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/dependencies.gradle
File java/gradle/dependencies.gradle:
Line 60: versions["scalaBase"] = versions.scala.substring(0, versions.scala.lastIndexOf("."))
> Oh sorry meant to respond. If the Scala version doesn't match this expected
Sure.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add gradle build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: WIP: Add gradle build
......................................................................
Patch Set 2:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradle/dependencies.gradle
File java/gradle/dependencies.gradle:
Line 8: avro : "1.8.1",
> ?
This is used in the kudu-flume-sink test
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradle/protobuf.gradle
File java/gradle/protobuf.gradle:
Line 14: // Configure Intellij to see the generated classes
> do we need the same for eclipse?
yes, we would. There is a decent amount of polish missing from this patch. I didn't want to spend too much time unless we were going to go this route and have this committed. If this initial patch seams reasonable I can touch up the details and remaining todo's.
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradlew
File java/gradlew:
Line 1: #!/usr/bin/env sh
> license
This is a generated file by running 'gradle wrapper'. Do generated files have different requirements for license headers?
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradlew.bat
File java/gradlew.bat:
Line 1: @if "%DEBUG%" == "" @echo off
> license
This is a generated file by running 'gradle wrapper'. Do generated files have different requirements for license headers?
http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-flume-sink/build.gradle
File java/kudu-flume-sink/build.gradle:
Line 1: apply plugin: "com.commercehub.gradle.plugin.avro"
> we don't use avro do we?
Yes, its used in the AvroKuduOperationsProducer and the tests.
http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-jepsen/build.gradle
File java/kudu-jepsen/build.gradle:
Line 38: def masterNodes = project.hasProperty('masterNodes') ? masterNodes : "m0"
> does this second masterNodes somehow become equivalent to project.property(
you mean using the ext block? That can work too, though this logic may still need to exist to handle passing in parameters that override. I can double check that.
http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala:
Line 43: object ITBigLinkedList {
> why this rename? should we also rename the file then?
This is a remnant from accidentally renaming this file and not the test file. Will fix.
http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-spark/build.gradle
File java/kudu-spark/build.gradle:
Line 43: }
> can we add an 'else' here that fails the build to prevent people from makin
Yeah, thats a good idea.This logic may change a bit when I wrap up generating all variations of this artifact in the main build call instead of depending on parameters.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add gradle build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#3).
Change subject: WIP: Add gradle build
......................................................................
WIP: Add gradle build
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
Remaining Work:
- Partially shade kudu-client (async and slf4j)
- Validate and adjust pom contents
- Investigate test failures
- Other inline TODO comments
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.jar
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradle/yetus-repo/org/apache/yetus/yetus-project/0.4.0/yetus-project-0.4.0.pom
A java/gradlew
A java/gradlew.bat
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
A java/kudu-jepsen/build.gradle
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
A java/settings.gradle
29 files changed, 1,021 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/3
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradle/wrapper/gradle-wrapper.jar
File java/gradle/wrapper/gradle-wrapper.jar:
> My understanding is that this is okay as long as they are not included in d
hrm, then we'd need to change our 'build_source_release.py' script to remove this from the resulting archive. We currently do our source releases by just using 'git archive', so this would be a change.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#18).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKuduOperationsProducer.java
A java/kudu-jepsen/build.gradle
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
M java/kudu-spark-tools/pom.xml
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/kudu-spark/pom.xml
A java/settings.gradle
34 files changed, 1,453 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/18
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Reviewed-on: http://gerrit.cloudera.org:8080/7258
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKuduOperationsProducer.java
A java/kudu-jepsen/build.gradle
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
M java/kudu-spark-tools/pom.xml
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/kudu-spark/pom.xml
A java/settings.gradle
34 files changed, 1,453 insertions(+), 6 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 13: Code-Review+1
lgtm as a starting point. seems to be some issue still with running tests in kudu-spark-tools but I think it's OK to iterate from here
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 7:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradle/shadow.gradle
File java/gradle/shadow.gradle:
Line 21: tasks.remove(knows) // Remove "easter egg" knows task.
> what's this? tried googling but couldn't find it
The person who wrote the gradle shadow plugin added a task that just outputs ascii art. I didn't want it cluttering `gradle tasks` output and resulting in questions.
Line 49: )
> not understanding this part at all
This plugin assumes you are shading all dependencies. In the kudu-client we only shade a few so we need a way to signify that. I added a configuration called "compileUnshaded" to track this information, but need to map it back to a maven scope for publishing. Otherwise these dependencies would not show up in the published maven poms.
I can definitely add more docs around this and link to issue on the plugin tracking improvements so this isn't necessary.
http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradle/wrapper/gradle-wrapper.jar
File java/gradle/wrapper/gradle-wrapper.jar:
> the ASF is against checking in binary jars
My understanding is that this is okay as long as they are not included in distributions. The samza project also doing this: https://github.com/apache/samza/tree/master/gradle/wrapper
There is some discussion on it on SAMZA-283 too.
We could consider some other method of downloading this as a sort of "custom bootstrap".
http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradlew.bat
File java/gradlew.bat:
Line 1: @if "%DEBUG%" == "" @echo off
> can we add these files to the RAT excludes file?
Done
http://gerrit.cloudera.org:8080/#/c/7258/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java:
Line 35: @Ignore
> why is this change in here? if you want to disable a test please do it in a
This test is super flaky and failed my build almost all the time. I didn't mean to check this in, but its probably something worth fixing.
http://gerrit.cloudera.org:8080/#/c/7258/7/java/kudu-spark/build.gradle
File java/kudu-spark/build.gradle:
Line 40: } else {
> is it possible to be explicit about == "2" and then give a build error if i
Done
Line 50: } else {
> same
I think the pattern would likely stay the same for any major spark build going forward. The only reason this exists is because spark 1 uses "spark" instead of "spark1" in the artifact name.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 18: Code-Review+2
Thanks for addressing all of my concerns. We can decide whether or not to shade kudu-mapreduce later.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7258/11/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 29: def wrapperJarPath = "gradle/wrapper/gradle-wrapper.jar"
> I think this should be prefixed with APP_HOME. Otherwise gradlew doesn't wo
Good catch. Done.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 10:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/7258/10/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 1: // Ensure the wrapper script is generated based on the version defined in the project
License header?
http://gerrit.cloudera.org:8080/#/c/7258/10/java/gradlew
File java/gradlew:
Line 1: #!/usr/bin/env sh
Does this need a copyright/license header?
Line 69: curl -o gradle/wrapper/gradle-wrapper.jar https://raw.githubusercontent.com/gradle/gradle/master/gradle/wrapper/gradle-wrapper.jar
This should probably point to a specific tag instead of master.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#6).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper/gradle-wrapper.jar
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradle/yetus-repo/org/apache/yetus/yetus-project/0.4.0/yetus-project-0.4.0.pom
A java/gradlew
A java/gradlew.bat
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
34 files changed, 1,494 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/6
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 10:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/7258/10/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 1: // Ensure the wrapper script is generated based on the version defined in the project
> License header?
Done
http://gerrit.cloudera.org:8080/#/c/7258/10/java/gradlew
File java/gradlew:
Line 1: #!/usr/bin/env sh
> Does this need a copyright/license header?
It's a good question. It's auto-generated by gradle except for a small snippet of code that is injected. That small snippet of code comes from wrapper.gradle which does have a license header. So I think this falls under the "generated" code exception.
Line 69: curl -o gradle/wrapper/gradle-wrapper.jar https://raw.githubusercontent.com/gradle/gradle/master/gradle/wrapper/gradle-wrapper.jar
> This should probably point to a specific tag instead of master.
Done
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add gradle build
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: WIP: Add gradle build
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7258/1/java/gradle/protobuf.gradle
File java/gradle/protobuf.gradle:
Line 1: apply plugin: "com.google.protobuf"
> Yes, this continues to work the same way the maven build does. Specifically
nice
http://gerrit.cloudera.org:8080/#/c/7258/1/java/kudu-jepsen/build.gradle
File java/kudu-jepsen/build.gradle:
Line 30: // Gradle's Clojure support is fairly limited.
> I don't think inter-dependencies is a big deal here. I can look into other
OK sounds good - I don't want to bog the gradle transition down with having to evaluate two new build systems :)
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#14).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
32 files changed, 1,422 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/14
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradle/wrapper/gradle-wrapper.jar
File java/gradle/wrapper/gradle-wrapper.jar:
> hrm, then we'd need to change our 'build_source_release.py' script to remov
My latest revision removed this jar and created a workaround to "bootstrap" it in the gradlew file if it were missing. Sorry I didn't update this comment.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 14:
(46 comments)
http://gerrit.cloudera.org:8080/#/c/7258/14/build-support/release/rat_exclude_files.txt
File build-support/release/rat_exclude_files.txt:
Line 14: java/gradlew.bat
> Isn't this deleted by wrapper.gradle?
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/.gitignore
File java/.gitignore:
Line 31: bin
> Hmm, what does gradle put in here? I thought all the build output was isola
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/build.gradle
File java/build.gradle:
Line 30: // they are used to ensure there are no dependency issues
> It'd also be good to note that individual subprojects may include other gra
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/artifacts.gradle
File java/gradle/artifacts.gradle:
Line 18: task testJar(type: Jar, dependsOn: testClasses, group: "Build") {
> Why doesn't this one need extension "jar"?
Done
Line 21: from sourceSets.test.output
> Nit: to be consistent with the other definitions, put this before classifie
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/buildscript.gradle
File java/gradle/buildscript.gradle:
Line 18: repositories {
> If any of these are used for just 1-2 plugins, it'd be good to precede thei
Done
Line 26: // Manage plugin dependencies since the plugin block can't be used in included build scripts yet.
> Might be good to link to the online docs that can contextualize "yet".
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/compile.gradle
File java/gradle/compile.gradle:
Line 18: // Set compilation settings for the project
> Can you beef this up a little? For one, this isn't just one "project", righ
Done
Line 19: allprojects {
> Why allprojects and not subprojects?
This actually isn't required since we apply it in the subprojects block in the main build. I copied this script from an older project of mine and will simplify it a bit.
Line 24: options.fork = true // enable compilation in a separate daemon process.
> Hmm, why do this?
Done
Line 31: }
> This doesn't seem specific to Gradle; should it be ported to the Maven buil
I am not sure I know how to do this in the maven build. Mainly the ability to detect jav 8 vs java7
Line 38: scalaCompileOptions.additionalParameters = ["-Xlint", "-feature"] // turn on compiler warnings for potential mistakes
> Maybe this belongs in Maven too?
Happy to do this in a separate patch. Its only command line build output changes, so not critical.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/dependencies.gradle
File java/gradle/dependencies.gradle:
Line 28: commonsIo : "2.5",
> Nit: not alphabetically sorted. Check the rest too.
Done
Line 64: def spark2Version = '2.1.1'
> Nit: these are only used once on L65-68; may be clearer to just define them
I thought this made the below logic more explicit and readable.
Line 81: flumeConfiguration: "org.apache.flume:flume-ng-configuration:$versions.flume",
> Not alphabetically sorted.
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/properties.gradle
File java/gradle/properties.gradle:
Line 18: // A Common method to handle loading gradle properties with a default when
> Nit: should be lower-case unless it's referring to something with the prope
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/publishing.gradle
File java/gradle/publishing.gradle:
Line 26: def mavenUrl = project.hasProperty('mavenUrl') ? project.mavenUrl : ''
> I thought by defining hasProperty inside ext, you wouldn't need to prefix i
We actually defined propertyExists since propertyExists.hasProperty doesn't check system properties. I will switch to using that.
Line 35: // To test locally, replace mavenUrl in gradle.properties to file://localhost/tmp/myRepo/
> Would it be useful to use propertyWithDefault() so that mavenUrl could be s
Yeah, I will do that. I missed converting the file once they were added.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/quality.gradle
File java/gradle/quality.gradle:
Line 20: project.apply plugin: 'pmd'
> I think it'd be nice to add a short one-line description explaining what ea
Done
Line 21: apply plugin: "com.github.ben-manes.versions"
> Add a comment explaining what this does, since it's not obvious from the na
Done
Line 32: // Create an aggregate checkstyle task
> I don't really understand what these "aggregate" tasks do. It seems all the
Done
Line 65: // Configure the versions plugin to only show updates for released versions.
> Not understanding this; in what context does the build "show updates" for a
This is configuring the dependencyUpdates plugin from the versions plugin. It has nothing to do with checkstyle/findbugs/pmd, but it does have to do with code quality imho. I think staying up to date on project dependencies is important.
This task will tell you if there are newer dependencies available for your project. Try running `gradle dependencyUpdates` for an example.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/scopes.gradle
File java/gradle/scopes.gradle:
Line 19: apply plugin: 'propdeps'
> Nit: sort alphabetically. Or does this order matter?
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/shadow.gradle
File java/gradle/shadow.gradle:
Line 32: }
> I expected to see this in artifacts.gradle, since that's where artifacts {.
This is here since only modules that use shading would have this artifact. This is just replacing the default jar from artifacts as commented below.
Line 45: project.conf2ScopeMappings.addMapping(
> What does this blurb mean?
I will add a comment.
Line 68: // Handle install and deploy the same.
> Nit: "in the same way" (when I see "the same" it's usually referring to a p
Done
Line 79: // Remove the shaded Dependencies from the pom
> Nit: either "shadedDependencies" (to refer directly to the variable referen
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/tests.gradle
File java/gradle/tests.gradle:
Line 18: // Support parallel unit test execution
> Nit: for comments written as sentences in English, terminate with a period.
Done
Line 20: maxParallelForks = project.hasProperty('maxParallelForks') ? maxParallelForks.toInteger() : 1
> Not propertyWithDefault here?
Done
Line 27: exceptionFormat = 'full'
> Nit: it's a little jarring to see both double quotes and single quotes used
Done
Line 31: // Adds pattern based integration test support.
> I imagine this was done so the Gradle build feels more like the Maven build
I do think it is desirable. This allows "fast" unit tests to be run first and then slower integration tests to be run next. Running `gradle check` will run all tests and quality checks.
Line 78: check.dependsOn(integrationTest)
> What is 'check'?
This is the default gradle task for all project validations/checks. In the case of this project it runs all tests and all quality checks.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 28: doLast {
> Why is this necessary?
Otherwise this code is run during project configuration and not when the task itself is run.
Line 29: def wrapperJarPath = "\$APP_HOME/gradle/wrapper/gradle-wrapper.jar"
> What is $APP_HOME and where is it defined?
It's defined in the wrapper script (gradlew) that we are injecting this into.
Line 61: wrapper.finalizedBy removeWindowScript
> What does it mean to have two finalizedBy statements? Are they additive or
They are additive.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-client-tools/build.gradle
File java/kudu-client-tools/build.gradle:
Line 31: testCompile project(path: ":kudu-client", configuration: 'shadowTest')
> A little surprised that we don't need "compile project(":kudu-client")" too
It's pulled in via kudu-mapreduce transitively. Running the following on this module shows the dependency tree:
`gradle dependencies --configuration=compile`
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-client/build.gradle
File java/kudu-client/build.gradle:
Line 19: import org.gradle.api.internal.artifacts.publish.ArchivePublishArtifact
> Not used?
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-flume-sink/build.gradle
File java/kudu-flume-sink/build.gradle:
Line 18: apply plugin: "com.commercehub.gradle.plugin.avro"
> What does this plugin do? It's not for "provided libs.avro"; that just refe
It runs the code generation from the schema files. Will add a comment.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-flume-sink/pom.xml
File java/kudu-flume-sink/pom.xml:
Line 24: <avro.version>1.8.2</avro.version>
> Why did you need to make this change as part of the Gradle build patch?
I don't need to do it as a part of this patch. I can do it in a different patch.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-jepsen/build.gradle
File java/kudu-jepsen/build.gradle:
Line 34: compile project(":kudu-client")
> Shouldn't this depend on the shadow JAR?
Done
Line 35: compile project(":kudu-client").sourceSets.test.output
> Is this a dependency on kudu-client's test JAR? If so, is there any way we
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-jepsen/pom.xml
File java/kudu-jepsen/pom.xml:
Line 34: <jepsen.version>0.1.5</jepsen.version>
> Why does this need to be bumped as part of this patch?
It doesn't I can make it a separate patch.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-mapreduce/build.gradle
File java/kudu-mapreduce/build.gradle:
Line 17:
> Why don't we need shadow.gradle here?
It could be a bug, but in the maven build we don't shade these jars. Happy to add it to both if you think we should be shading them.
Line 23: }
> This pattern repeats itself quite often; can we define it somewhere once?
There isn't a super clean way to do this. What I did instead was create a pull request against async so that we can remove this all together on the next version update:
https://github.com/OpenTSDB/async/pull/8
http://gerrit.cloudera.org:8080/#/c/7258/14/java/pom.xml
File java/pom.xml:
Line 80: <scala-2.11.version>2.11.11</scala-2.11.version>
> Why does this need to change?
It doesn't need to be in this patch. I can update it in a different patch.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/settings.gradle
File java/settings.gradle:
Line 19: include "kudu-client"
> Nit: alphabetical sorting
Done
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 16:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/7258/16/java/build.gradle
File java/build.gradle:
File-level comment for the purpose of this file ("This is the entry-point for the gradle build, yada yada yada")?
http://gerrit.cloudera.org:8080/#/c/7258/16/java/gradle.properties
File java/gradle.properties:
File-level comment here.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/compile.gradle
File java/gradle/compile.gradle:
Line 24: options.incremental = true // enable incremental compilation.
> Done
I didn't mean to suggest it should be removed. I just didn't know whether it's good or bad; there was no information to help me understand why it's a good thing.
Line 31: }
> I am not sure I know how to do this in the maven build. Mainly the ability
You'd have to add a Maven profile. See https://stackoverflow.com/a/16743137.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/dependencies.gradle
File java/gradle/dependencies.gradle:
PS14, Line 60: .
> Perhaps add some checking just beforehand to ensure that the scalaVersions
Missed this?
Line 64: versions["scalaBase"] = versions.scala.substring(0, versions.scala.lastIndexOf("."))
> I thought this made the below logic more explicit and readable.
No problem, I can buy that.
http://gerrit.cloudera.org:8080/#/c/7258/16/java/gradle/publishing.gradle
File java/gradle/publishing.gradle:
Line 37: // To test locally, replace mavenUrl in gradle.properties to file://localhost/tmp/myRepo/
Now that mavenUrl uses propertyWithDefault(), is this guidance necessary? I think -D or -P is better than modifying gradle.properties.
http://gerrit.cloudera.org:8080/#/c/7258/16/java/gradle/quality.gradle
File java/gradle/quality.gradle:
PS16, Line 22: fro
for
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 28: // This allows us to avoid checking in the jar to our repository.
> Otherwise this code is run during project configuration and not when the ta
Could you add a comment explaining that?
http://gerrit.cloudera.org:8080/#/c/7258/16/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
PS16, Line 27: doesn"t
Single quote was better here.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-mapreduce/build.gradle
File java/kudu-mapreduce/build.gradle:
Line 17:
> It could be a bug, but in the maven build we don't shade these jars. Happy
Hmm, ask Dan?
Line 23: }
> There isn't a super clean way to do this. What I did instead was create a p
Nice. Can you add a link to the pull request in each place where you apply this pattern? That way we'll be more likely to notice and remove it once async merges the pull request.
http://gerrit.cloudera.org:8080/#/c/7258/16/java/kudu-spark/pom.xml
File java/kudu-spark/pom.xml:
PS16, Line 104: <args>
: <arg>-unchecked</arg>
: <arg>-deprecation</arg>
: <arg>-explaintypes</arg>
: </args>
I didn't see this in the gradle build. Is it built-in to gradle's scala plugin?
http://gerrit.cloudera.org:8080/#/c/7258/16/java/settings.gradle
File java/settings.gradle:
File-level comment.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add gradle build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#2).
Change subject: WIP: Add gradle build
......................................................................
WIP: Add gradle build
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
Remaining Work:
- Partially shade kudu-client (async and slf4j)
- Validate and adjust pom contents
- Investigate test failures
- Other inline TODO comments
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.jar
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/gradlew.bat
A java/interface-annotations/build.gradle
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
A java/kudu-jepsen/build.gradle
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
A java/settings.gradle
29 files changed, 980 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/2
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] WIP: Add gradle build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: WIP: Add gradle build
......................................................................
Patch Set 2:
(3 comments)
I am happy to add extremely thorough comments to the build.
http://gerrit.cloudera.org:8080/#/c/7258/2/java/build.gradle
File java/build.gradle:
Line 9: subprojects {
> What motivated this decomposition? Is it idiomatic for gradle? Or just some
Its a bit of both. Its fairly common practice. You can see that pattern used quite a bit in the gradle project itself: https://github.com/gradle/gradle/tree/master/gradle
Line 11: apply from: "$rootDir/gradle/scopes.gradle"
> Can these be sorted alphabetically? If not, could you add comments to expla
I can add a comment on the order. I load them in the order they are "needed" or used for a build. I find its a nice way to order the scripts. It also helps avoid any configuration or plugin dependency issues.
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradlew
File java/gradlew:
Line 1: #!/usr/bin/env sh
> If it's a generated file, why are we checking it in? Shouldn't it just be r
The point of this file is to prevent the need for a gradle install on the box running the build. Without it a gradle install is needed to generate all the necessary files to run the wrapper. And if you have gradle installed much of the benefit of the wrapper goes away.
This would be re-generated anytime we upgrade the gradle version the project is built with.
I am happy to answer more questions on this and you can read more about the wrapper here: https://docs.gradle.org/4.0/userguide/gradle_wrapper.html
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add gradle build
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: WIP: Add gradle build
......................................................................
Patch Set 2:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradle/dependencies.gradle
File java/gradle/dependencies.gradle:
Line 8: avro : "1.8.1",
?
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradle/protobuf.gradle
File java/gradle/protobuf.gradle:
Line 14: // Configure Intellij to see the generated classes
do we need the same for eclipse?
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradlew
File java/gradlew:
Line 1: #!/usr/bin/env sh
license
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradlew.bat
File java/gradlew.bat:
Line 1: @if "%DEBUG%" == "" @echo off
license
http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-flume-sink/build.gradle
File java/kudu-flume-sink/build.gradle:
Line 1: apply plugin: "com.commercehub.gradle.plugin.avro"
we don't use avro do we?
http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-jepsen/build.gradle
File java/kudu-jepsen/build.gradle:
Line 1: apply plugin: "nebula.clojure"
add license header
PS2, Line 7: if (it.project == project) {
: it.onlyIf { false }
: }
is this equivalent to:
it.onlyIf { it.project != project }
?
PS2, Line 38: masterNodes
does this second masterNodes somehow become equivalent to project.property("masterNodes")?
should we be adding 'extra properties' as described in this doc? https://docs.gradle.org/3.3/userguide/writing_build_scripts.html
It looks like that could handle the setting of defaults and also prevent against typos?
http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-mapreduce/build.gradle
File java/kudu-mapreduce/build.gradle:
Line 1: dependencies {
add license header. same in all other files
http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala:
Line 43: object ITBigLinkedList {
why this rename? should we also rename the file then?
is this incompatible if we expect users to be able to run ITBLL via submitting the kudu-spark-tools jar?
http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-spark/build.gradle
File java/kudu-spark/build.gradle:
Line 43: }
can we add an 'else' here that fails the build to prevent people from making innocent mistakes like setting sparkBase=2.2?
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add gradle build
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: WIP: Add gradle build
......................................................................
Patch Set 2:
(3 comments)
Just getting the lay of the land. I would appreciate many more code comments, even on very basic and perhaps obvious gradle-y stuff. That will help the rest of us gradle newbs understand how it works.
Put another way, imagine that you're writing a gradle tutorial, and you've chosen to use the Kudu Java client as the basis for the tutorial. If you wouldn't mind going into that level of detail and explaining how everything works in code comments and text blocks, I think the transition to gradle could happen faster as we'd learn how to use it really quickly.
http://gerrit.cloudera.org:8080/#/c/7258/2/java/build.gradle
File java/build.gradle:
Line 9: subprojects {
What motivated this decomposition? Is it idiomatic for gradle? Or just something that felt right to you?
Line 11: apply from: "$rootDir/gradle/scopes.gradle"
Can these be sorted alphabetically? If not, could you add comments to explain why they're in this order?
http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradlew
File java/gradlew:
Line 1: #!/usr/bin/env sh
> This is a generated file by running 'gradle wrapper'. Do generated files ha
If it's a generated file, why are we checking it in? Shouldn't it just be regenerated by each developer before using gradle?
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#13).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
32 files changed, 1,419 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/13
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#4).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper/gradle-wrapper.jar
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradle/yetus-repo/org/apache/yetus/yetus-project/0.4.0/yetus-project-0.4.0.pom
A java/gradlew
A java/gradlew.bat
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
34 files changed, 1,119 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/4
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 10: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 7:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradle/shadow.gradle
File java/gradle/shadow.gradle:
Line 21: tasks.remove(knows) // Remove "easter egg" knows task.
what's this? tried googling but couldn't find it
PS7, Line 45: project.conf2ScopeMappings.addMapping(
: MavenPlugin.COMPILE_PRIORITY + 1,
: configurations.compileUnshaded,
: Conf2ScopeMappingContainer.COMPILE
: )
not understanding this part at all
http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradle/wrapper/gradle-wrapper.jar
File java/gradle/wrapper/gradle-wrapper.jar:
the ASF is against checking in binary jars
http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradlew.bat
File java/gradlew.bat:
Line 1: @if "%DEBUG%" == "" @echo off
can we add these files to the RAT excludes file?
http://gerrit.cloudera.org:8080/#/c/7258/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java:
Line 35: @Ignore
why is this change in here? if you want to disable a test please do it in a separate unrelated commit.
http://gerrit.cloudera.org:8080/#/c/7258/7/java/kudu-spark/build.gradle
File java/kudu-spark/build.gradle:
Line 40: } else {
is it possible to be explicit about == "2" and then give a build error if it is anything else?
Line 50: } else {
same
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#17).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKuduOperationsProducer.java
A java/kudu-jepsen/build.gradle
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
M java/kudu-spark-tools/pom.xml
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/kudu-spark/pom.xml
A java/settings.gradle
34 files changed, 1,453 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/17
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7258/4/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala:
Line 43: object ITBigLinkedList {
I think we decided this shouldn't be renamed?
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#5).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper/gradle-wrapper.jar
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradle/yetus-repo/org/apache/yetus/yetus-project/0.4.0/yetus-project-0.4.0.pom
A java/gradlew
A java/gradlew.bat
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
34 files changed, 1,495 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/5
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 14:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/compile.gradle
File java/gradle/compile.gradle:
Line 24: options.fork = true // enable compilation in a separate daemon process.
> I didn't mean to suggest it should be removed. I just didn't know whether i
I didn't have a great reason either. So removing it keeps things simple.
Line 31: }
> You'd have to add a Maven profile. See https://stackoverflow.com/a/16743137
After spending more time on this and making the maven profile work. It turns out Kudu doesn't have too many errors to overcome like other projects I have worked on. I think I will drop this and fix the lint errors. It should make for cleaner javadoc in the long run.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/dependencies.gradle
File java/gradle/dependencies.gradle:
Line 60: versions["scalaBase"] = versions.scala.substring(0, versions.scala.lastIndexOf("."))
> Missed this?
Oh sorry meant to respond. If the Scala version doesn't match this expected pattern, the build would fail and it would show that the "malformed" version couldn't be resolved. Perhaps that is good enough?
http://gerrit.cloudera.org:8080/#/c/7258/16/java/gradle/quality.gradle
File java/gradle/quality.gradle:
Line 22:
> for
Done
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 28: doLast {
> Could you add a comment explaining that?
Done
http://gerrit.cloudera.org:8080/#/c/7258/16/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 27: task bootstrapWrapper() {
> Single quote was better here.
oops....a little trigger happy on the find and replace.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-mapreduce/build.gradle
File java/kudu-mapreduce/build.gradle:
Line 17:
> Hmm, ask Dan?
Will ask.
Line 23: }
> Nice. Can you add a link to the pull request in each place where you apply
Done
http://gerrit.cloudera.org:8080/#/c/7258/16/java/kudu-spark/pom.xml
File java/kudu-spark/pom.xml:
Line 108: <phase>process-resources</phase>
> I didn't see this in the gradle build. Is it built-in to gradle's scala plu
I was playing with the args and left these in. Fixed to match the gradle build in compile.gradle. I also added docs for the flags.
http://gerrit.cloudera.org:8080/#/c/7258/16/java/settings.gradle
File java/settings.gradle:
> File-level comment.
Done
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#16).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
A java/kudu-jepsen/build.gradle
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/kudu-spark/pom.xml
A java/settings.gradle
29 files changed, 1,431 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/16
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#11).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
33 files changed, 1,420 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/11
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#12).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
32 files changed, 1,419 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/12
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 14:
(48 comments)
For the non-generated code, could you add a file level comment to each file explaining its use and purpose?
http://gerrit.cloudera.org:8080/#/c/7258/14/build-support/release/rat_exclude_files.txt
File build-support/release/rat_exclude_files.txt:
Line 14: java/gradlew.bat
Isn't this deleted by wrapper.gradle?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/.gitignore
File java/.gitignore:
Line 31: bin
Hmm, what does gradle put in here? I thought all the build output was isolated to build/
http://gerrit.cloudera.org:8080/#/c/7258/14/java/build.gradle
File java/build.gradle:
PS14, Line 29: // Plugins and scripts are applied in the natural "build order"
: // they are used to ensure there are no dependency issues
It'd also be good to note that individual subprojects may include other gradle/foo.gradle files; these are included here because they apply to _all_ subprojects.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/artifacts.gradle
File java/gradle/artifacts.gradle:
Line 18: task testJar(type: Jar, dependsOn: testClasses, group: "Build") {
Why doesn't this one need extension "jar"?
Line 21: from sourceSets.test.output
Nit: to be consistent with the other definitions, put this before classifier.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/buildscript.gradle
File java/gradle/buildscript.gradle:
Line 18: repositories {
If any of these are used for just 1-2 plugins, it'd be good to precede their definitions with a comment listing the derived plugins. That way we know how to prune this list if/when we were to remove a plugin.
Line 26: // Manage plugin dependencies since the plugin block can't be used in included build scripts yet.
Might be good to link to the online docs that can contextualize "yet".
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/compile.gradle
File java/gradle/compile.gradle:
Line 18: // Set compilation settings for the project
Can you beef this up a little? For one, this isn't just one "project", right?
Line 19: allprojects {
Why allprojects and not subprojects?
Line 24: options.fork = true // enable compilation in a separate daemon process.
Hmm, why do this?
PS14, Line 26: if (JavaVersion.current().isJava8Compatible()) {
: tasks.withType(Javadoc) {
: // disable the crazy super-strict doclint tool in Java 8
: options.addStringOption('Xdoclint:none', '-quiet')
: }
: }
This doesn't seem specific to Gradle; should it be ported to the Maven build too?
Line 37: scalaCompileOptions.encoding = 'UTF-8' // make sure the encoding is utf-8 regardless of system default.
Since this is repeated for the Java configuration, perhaps it should be defined in properties.gradle?
Line 38: scalaCompileOptions.additionalParameters = ["-Xlint", "-feature"] // turn on compiler warnings for potential mistakes
Maybe this belongs in Maven too?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/dependencies.gradle
File java/gradle/dependencies.gradle:
Line 28: commonsIo : "2.5",
Nit: not alphabetically sorted. Check the rest too.
PS14, Line 60: versions.scala.substring(0, versions.scala.lastIndexOf("."))
Perhaps add some checking just beforehand to ensure that the scalaVersions property meets this expectation? The default value is obviously fine but it can be set by the user to be anything, right?
PS14, Line 63: def spark1Version = '1.6.3'
: def spark2Version = '2.1.1'
Nit: these are only used once on L65-68; may be clearer to just define them inline in the versions["spark"] calls.
Line 81: flumeConfiguration: "org.apache.flume:flume-ng-configuration:$versions.flume",
Not alphabetically sorted.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/properties.gradle
File java/gradle/properties.gradle:
PS14, Line 18: Common
Nit: should be lower-case unless it's referring to something with the proper name Common.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/publishing.gradle
File java/gradle/publishing.gradle:
PS14, Line 26: project.hasProperty
I thought by defining hasProperty inside ext, you wouldn't need to prefix it with "project." when referring to it?
Line 35: // To test locally, replace mavenUrl in gradle.properties to file://localhost/tmp/myRepo/
Would it be useful to use propertyWithDefault() so that mavenUrl could be specified via -D or -P?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/quality.gradle
File java/gradle/quality.gradle:
PS14, Line 18: project.apply plugin: 'checkstyle'
: project.apply plugin: 'findbugs'
: project.apply plugin: 'pmd'
I think it'd be nice to add a short one-line description explaining what each of these tools does. I know what checkstyle/findbugs do, but I'm not familiar with pmd.
Line 21: apply plugin: "com.github.ben-manes.versions"
Add a comment explaining what this does, since it's not obvious from the name.
Line 32: // Create an aggregate checkstyle task
I don't really understand what these "aggregate" tasks do. It seems all they do is add a task to a group and provide a definition; how is that enough to do something of value?
I can take my answer in the form of a code comment.
Line 65: // Configure the versions plugin to only show updates for released versions.
Not understanding this; in what context does the build "show updates" for anything? How does this relate to checkstyle/findbugs/pmd?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/scopes.gradle
File java/gradle/scopes.gradle:
Line 19: apply plugin: 'propdeps'
Nit: sort alphabetically. Or does this order matter?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/shadow.gradle
File java/gradle/shadow.gradle:
PS14, Line 29: // Add the shadowJar to the published artifacts
: artifacts {
: archives shadowJar
: }
I expected to see this in artifacts.gradle, since that's where artifacts {...} was defined. I imagine it has to be here due to ordering constraints. If that's the case, can you note it?
Line 45: project.conf2ScopeMappings.addMapping(
What does this blurb mean?
Line 68: // Handle install and deploy the same.
Nit: "in the same way" (when I see "the same" it's usually referring to a previous part of the phrase, like "I went to the store and Grant did the same").
PS14, Line 79: shaded Dependencies
Nit: either "shadedDependencies" (to refer directly to the variable referenced below) or "shaded dependencies" (to refer to the common concept).
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/tests.gradle
File java/gradle/tests.gradle:
Line 18: // Support parallel unit test execution
Nit: for comments written as sentences in English, terminate with a period. Below too.
Line 20: maxParallelForks = project.hasProperty('maxParallelForks') ? maxParallelForks.toInteger() : 1
Not propertyWithDefault here?
Line 27: exceptionFormat = 'full'
Nit: it's a little jarring to see both double quotes and single quotes used in the same code blurb when it isn't necessary to switch between the two (i.e. to switch to one or the other for interpolation reasons).
Line 31: // Adds pattern based integration test support.
I imagine this was done so the Gradle build feels more like the Maven build, but is it actually a desirable feature? Do we want to treat IT tests differently?
What you've done here makes sense for now; the question I'm posing is more long term, I think.
Line 78: check.dependsOn(integrationTest)
What is 'check'?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 28: doLast {
Why is this necessary?
Line 29: def wrapperJarPath = "\$APP_HOME/gradle/wrapper/gradle-wrapper.jar"
What is $APP_HOME and where is it defined?
Line 61: wrapper.finalizedBy removeWindowScript
What does it mean to have two finalizedBy statements? Are they additive or does the last one "win"?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-client-tools/build.gradle
File java/kudu-client-tools/build.gradle:
Line 31: testCompile project(path: ":kudu-client", configuration: 'shadowTest')
A little surprised that we don't need "compile project(":kudu-client")" too.
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-client/build.gradle
File java/kudu-client/build.gradle:
Line 19: import org.gradle.api.internal.artifacts.publish.ArchivePublishArtifact
Not used?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-flume-sink/build.gradle
File java/kudu-flume-sink/build.gradle:
Line 18: apply plugin: "com.commercehub.gradle.plugin.avro"
What does this plugin do? It's not for "provided libs.avro"; that just references a library version, right?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-flume-sink/pom.xml
File java/kudu-flume-sink/pom.xml:
Line 24: <avro.version>1.8.2</avro.version>
Why did you need to make this change as part of the Gradle build patch?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-jepsen/build.gradle
File java/kudu-jepsen/build.gradle:
Line 34: compile project(":kudu-client")
Shouldn't this depend on the shadow JAR?
Line 35: compile project(":kudu-client").sourceSets.test.output
Is this a dependency on kudu-client's test JAR? If so, is there any way we can alias it so the dependency is a little more clear (and perhaps less verbose)?
Also, shouldn't this depend on the shadow version?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-jepsen/pom.xml
File java/kudu-jepsen/pom.xml:
Line 34: <jepsen.version>0.1.5</jepsen.version>
Why does this need to be bumped as part of this patch?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-mapreduce/build.gradle
File java/kudu-mapreduce/build.gradle:
Line 17:
Why don't we need shadow.gradle here?
PS14, Line 20: compile(libs.async) {
: // async uses versions ranges for slf4j making builds non-deterministic
: exclude group: "org.slf4j", module: "slf4j-api"
: }
This pattern repeats itself quite often; can we define it somewhere once?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/pom.xml
File java/pom.xml:
Line 80: <scala-2.11.version>2.11.11</scala-2.11.version>
Why does this need to change?
http://gerrit.cloudera.org:8080/#/c/7258/14/java/settings.gradle
File java/settings.gradle:
Line 19: include "kudu-client"
Nit: alphabetical sorting
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add gradle build
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: WIP: Add gradle build
......................................................................
Patch Set 1:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/7258/1//COMMIT_MSG
Commit Message:
Line 22: - Reporting available dependency updates
Nice!
Line 23: - Protobuf and Avro code generation
Do we use Avro for anything?
http://gerrit.cloudera.org:8080/#/c/7258/1/java/README.md
File java/README.md:
Line 171: $ gradle :kudu-spark
Why does the client build command have an additional :assemble?
Line 174: to build against Spark 1 and/or Scala 2.10
Do these parameters work independently? I'm not sure we want to add Spark2/2.10 or Spark1/2.11 to our testing matrix.
http://gerrit.cloudera.org:8080/#/c/7258/1/java/gradle/buildscript.gradle
File java/gradle/buildscript.gradle:
PS1, Line 10: cant
typo: can't
http://gerrit.cloudera.org:8080/#/c/7258/1/java/gradle/protobuf.gradle
File java/gradle/protobuf.gradle:
Line 1: apply plugin: "com.google.protobuf"
Will this download a protoc for the platform? We recently added that, and it's really nice because we no longer have to do the thirdparty build just to build java.
http://gerrit.cloudera.org:8080/#/c/7258/1/java/gradle/quality.gradle
File java/gradle/quality.gradle:
Line 13: // Configure the versions plugin to only show updates for released versions
end with period
http://gerrit.cloudera.org:8080/#/c/7258/1/java/kudu-flume-sink/build.gradle
File java/kudu-flume-sink/build.gradle:
Line 17:
whitespace
http://gerrit.cloudera.org:8080/#/c/7258/1/java/kudu-jepsen/build.gradle
File java/kudu-jepsen/build.gradle:
Line 30: // Gradle's Clojure support is fairly limited.
Maybe we should consider just using lein for the clojure stuff? Since it's more-or-less disabled by default I don't think it'd be a big deal. Might make having inter-dependencies hard though.
http://gerrit.cloudera.org:8080/#/c/7258/1/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ITBigLinkedList.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ITBigLinkedList.scala:
Line 43: object ITBigLinkedList {
This isn't actually a test class, so the rename shouldn't be necessary, right? The test class is https://github.com/apache/kudu/blob/master/java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedListTest.scala
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 17:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7258/17/java/build.gradle
File java/build.gradle:
Line 19: // common logic for the various subprojects in the build.
> Got trailing whitespace here.
Done
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#15).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
A java/kudu-jepsen/build.gradle
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/kudu-spark/pom.xml
M java/pom.xml
A java/settings.gradle
30 files changed, 1,432 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/15
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#7).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper/gradle-wrapper.jar
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/gradlew.bat
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
33 files changed, 1,437 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/7
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7258/4/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala:
Line 43: object IntegrationTestBigLinkedList {
> I think we decided this shouldn't be renamed?
Done
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 7: Code-Review+1
LGTM as a first cut, especially since we're going to have them run in parallel for a time.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] WIP: Add gradle build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: WIP: Add gradle build
......................................................................
Patch Set 3:
No need to review yet. Just pushing a rebase so Todd can test.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 13:
Pushed a fix to unblock Todds iterative testing. Will follow up for a fix to the multibuild support.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7258/11/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 29: def wrapperJarPath = "gradle/wrapper/gradle-wrapper.jar"
I think this should be prefixed with APP_HOME. Otherwise gradlew doesn't work from other cwds
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#10).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
33 files changed, 1,402 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/10
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP: Add gradle build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: WIP: Add gradle build
......................................................................
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/7258/1//COMMIT_MSG
Commit Message:
Line 23: - Protobuf and Avro code generation
> Do we use Avro for anything?
The kudu-flume-sink leverages avro in the unit tests.
http://gerrit.cloudera.org:8080/#/c/7258/1/java/README.md
File java/README.md:
Line 171: $ gradle :kudu-spark:assemble
> Why does the client build command have an additional :assemble?
Ah I just left it off of this command on accident. This is the syntax to reference a task for a submodule only instead of the entire build even when at the parent directory.
Line 174: to build against Spark 1 and/or Scala 2.10
> Do these parameters work independently? I'm not sure we want to add Spark2
Currently they do work independently.
http://gerrit.cloudera.org:8080/#/c/7258/1/java/gradle/buildscript.gradle
File java/gradle/buildscript.gradle:
Line 10: // Manage plugin dependencies since the plugin block can't be used in included build scripts yet.
> typo: can't
Done
http://gerrit.cloudera.org:8080/#/c/7258/1/java/gradle/protobuf.gradle
File java/gradle/protobuf.gradle:
Line 1: apply plugin: "com.google.protobuf"
> Will this download a protoc for the platform? We recently added that, and
Yes, this continues to work the same way the maven build does. Specifically because of the "artifact =" line below that instructs the plugin to use maven for protoc. All of the platform detection is included by default.
http://gerrit.cloudera.org:8080/#/c/7258/1/java/gradle/quality.gradle
File java/gradle/quality.gradle:
Line 13: // Configure the versions plugin to only show updates for released versions.
> end with period
Done
http://gerrit.cloudera.org:8080/#/c/7258/1/java/kudu-flume-sink/build.gradle
File java/kudu-flume-sink/build.gradle:
Line 17
> whitespace
Done
http://gerrit.cloudera.org:8080/#/c/7258/1/java/kudu-jepsen/build.gradle
File java/kudu-jepsen/build.gradle:
Line 30: // Gradle's Clojure support is fairly limited.
> Maybe we should consider just using lein for the clojure stuff? Since it's
I don't think inter-dependencies is a big deal here. I can look into other options like lein. But from initial searches its just a language that has got less attention on Gradle.
Note: This does still work as is. Its just a bit weird since it doesn't follow usual gradle conventions.
http://gerrit.cloudera.org:8080/#/c/7258/1/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ITBigLinkedList.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ITBigLinkedList.scala:
Line 43
> This isn't actually a test class, so the rename shouldn't be necessary, rig
My mistake. I was changing it back and forth and changed the wrong one in the end.
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2066. Add experimental Gradle build support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7258
to look at the new patch set (#9).
Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................
KUDU-2066. Add experimental Gradle build support
Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.
The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.
Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories
- Cross compiling scala in a single build command
- Code quality checks (checkstyle, findbugs, pmd)
Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M build-support/release/rat_exclude_files.txt
M java/.gitignore
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/multibuild.gradle
A java/gradle/properties.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
A java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/pom.xml
A java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
A java/kudu-spark/build.gradle
M java/pom.xml
A java/settings.gradle
33 files changed, 1,407 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/7258/9
--
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>