You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2022/05/07 12:58:01 UTC
[GitHub] [thrift] Jimexist opened a new pull request, #2601: java lib to upgrade to gradle 7 and move off maven
Jimexist opened a new pull request, #2601:
URL: https://github.com/apache/thrift/pull/2601
<!-- Explain the changes in the pull request below: -->
<!-- We recommend you review the checklist/tips before submitting a pull request. -->
- [ ] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? (not required for trivial changes)
- [ ] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
- [ ] Did you squash your changes to a single commit? (not required, but preferred)
- [ ] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
- [ ] If your change does not involve any code, include `[skip ci]` anywhere in the commit message to free up build resources.
<!--
The Contributing Guide at:
https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
has more details and tips for committing properly.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120617459
> > confirmed that the build failure in Java 8 was due to kolin's ktfmt which depends on gradle api that's Java 11+ only:
>
> Maybe using that plugin can be wrapped around a conditional? If so, then could go ahead and revert the Xenial changes here. it'd be nice to make the choice to require JDK 11 a conscious decision for later, rather than as a side-effect of the Gradle update now.
yes let me try to put a if guard around ktfmt and revert the changes of upgrading java to xenial docker image
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120351733
> > move off maven
>
> because ...
@Jens-G "maven" is the name of the Gradle plugin used for the publishing of the libthrift.jar. That Gradle plugin was deprecated and replaced by a plugin called "maven-publish". Gradle 7 does not support the old plugin, so this transition needs to happen in order to support Gradle 7.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on a diff in pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2601:
URL: https://github.com/apache/thrift/pull/2601#discussion_r867596253
##########
build/docker/ubuntu-xenial/Dockerfile:
##########
@@ -155,17 +155,22 @@ RUN apt-get install -y --no-install-recommends \
haxelib install --always hxcpp 3.4.64 2>&1 > /dev/null
# note: hxcpp 3.4.185 (latest) no longer ships static libraries, and caused a build failure
-ENV GRADLE_VERSION="6.9.2"
+ENV GRADLE_VERSION="7.4.2"
+# This is where we install and put the JDK, `java` is under $JAVA_HOME/bin
+ENV JAVA_HOME="/usr/local/jdk-11"
RUN apt-get install -y --no-install-recommends \
`# Java dependencies` \
ant \
ant-optional \
- openjdk-8-jdk \
maven \
unzip && \
+`# Open JDK 11 - we cannot install openjdk-11-jdk using apt-get because xenial has only openjdk-8-jdk` \
+ wget https://download.java.net/openjdk/jdk11/ri/openjdk-11+28_linux-x64_bin.tar.gz -O /tmp/openjdk-11+28_linux-x64_bin.tar.gz && \
+ (echo "3784cfc4670f0d4c5482604c7c513beb1a92b005f569df9bf100e8bef6610f2e /tmp/openjdk-11+28_linux-x64_bin.tar.gz" | sha256sum -c -) && \
+ tar -C /usr/local -xzf /tmp/openjdk-11+28_linux-x64_bin.tar.gz && \
Review Comment:
Xenial is old. It's probably time to drop it soon, but this is a reasonable temporary workaround.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120237477
This is put to draft state because on xenial JDK 8 is used but somehow gradle 7.4+ does not quite work with Java 8. It'll at least require Java 11, which makes sense because it's the LTS version that's not at its end of life now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120603420
> FYI i think JDK 8 support is still with Gradle 7:
>
> build log
the need for java 8 is possibly due to kotlin's unspecified dependencies, for which i've pinned down in https://github.com/apache/thrift/pull/2602/files#diff-fac297e377f3c9d2ebbc3e82419e0c9d7a6d15e741acdfaf62c4485b8e228b83R37-R42
maybe after merging that i'll try to revert the upgrade of xenial's java 8 => 11 changes (maybe in later PR), or not, since xenial is a bit old.
i'm trying to add focal in:
- #2528 or even jammy but maybe not today
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on a diff in pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on code in PR #2601:
URL: https://github.com/apache/thrift/pull/2601#discussion_r867483724
##########
build/docker/ubuntu-xenial/Dockerfile:
##########
@@ -155,17 +155,22 @@ RUN apt-get install -y --no-install-recommends \
haxelib install --always hxcpp 3.4.64 2>&1 > /dev/null
# note: hxcpp 3.4.185 (latest) no longer ships static libraries, and caused a build failure
-ENV GRADLE_VERSION="6.9.2"
+ENV GRADLE_VERSION="7.4.2"
+# This is where we install and put the JDK, `java` is under $JAVA_HOME/bin
+ENV JAVA_HOME="/usr/local/jdk-11"
RUN apt-get install -y --no-install-recommends \
`# Java dependencies` \
ant \
ant-optional \
- openjdk-8-jdk \
maven \
unzip && \
+`# Open JDK 11 - we cannot install openjdk-11-jdk using apt-get because xenial has only openjdk-8-jdk` \
+ wget https://download.java.net/openjdk/jdk11/ri/openjdk-11+28_linux-x64_bin.tar.gz -O /tmp/openjdk-11+28_linux-x64_bin.tar.gz && \
+ (echo "3784cfc4670f0d4c5482604c7c513beb1a92b005f569df9bf100e8bef6610f2e /tmp/openjdk-11+28_linux-x64_bin.tar.g" | sha256sum -c -) && \
Review Comment:
```suggestion
(echo "3784cfc4670f0d4c5482604c7c513beb1a92b005f569df9bf100e8bef6610f2e /tmp/openjdk-11+28_linux-x64_bin.tar.gz" | sha256sum -c -) && \
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120619502
> @Jimexist I fixed the merge conflicts, and also bumped the spotless version, and formatted the kotlin and java files using Gradle 7.4.2. I'll let CI finish its checks, but I did notice that:
>
> 1. there are some deprecated stuff in use here that will be removed in Gradle 8 (which is not out yet, but will be nice to be prepared for) -- could be fixed as part of this PR,
> 2. some Gradle plugins use banned APIs that are removed/hidden in Java 17 (this might require waiting on upstream plugin developers, or disabling some plugins if Java 17 is used) -- could be a follow-on PR, and
> 3. there's a lot of warnings spamming the console for both lib/java and lib/kotlin -- could be a follow-on PR.
>
> As a side note: I'm really happy to be helping review your changes. I'm learning a lot from the exercise, and I'm impressed with the quality and pace of your contributions. I've been feeling guilty about being added as a committer, but not really having much time to contribute. But, all your work has kept me interested, and contributing to the project, even if it's limited to the Java and CI changes. So, thanks for contributing so that I can contribute. 😺
thanks for reviewing my pull request and helping with formatting!
1. i've fixed the deprecations
2. i guess we can wait a bit for gradle 7.5 to be out for which they will possibly fix them
3. i think you meant javadoc warnings?
thanks for the kind words as well and i'm also learning a lot along the way. i think there's no shame and pressure on being a committer and the amount of actual contributions you make, as long as it's net positive then it's all good, also i totally agree that reviewing code and commenting is also very good part of contributions. also a side note - i don't even know how to become a committer (there's no documented process yet) but that won't stop me from committing code either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on a diff in pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2601:
URL: https://github.com/apache/thrift/pull/2601#discussion_r867436695
##########
lib/java/gradle/environment.gradle:
##########
@@ -64,14 +64,14 @@ repositories {
}
dependencies {
- compile "org.slf4j:slf4j-api:${slf4jVersion}"
- compile "org.apache.httpcomponents:httpclient:${httpclientVersion}"
- compile "org.apache.httpcomponents:httpcore:${httpcoreVersion}"
- compile "javax.servlet:javax.servlet-api:${servletVersion}"
- compile "javax.annotation:javax.annotation-api:${javaxAnnotationVersion}"
- compile "org.apache.commons:commons-lang3:3.12.0"
+ implementation "org.slf4j:slf4j-api:${slf4jVersion}"
+ implementation "org.apache.httpcomponents:httpclient:${httpclientVersion}"
+ implementation "org.apache.httpcomponents:httpcore:${httpcoreVersion}"
Review Comment:
This can be rebased onto master, it should fix these differences. I think there's a merge conflict, though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120754441
> i don't even know how to become a committer (there's no documented process yet) but that won't stop me from committing code either way.
At the ASF, each project's PMC nominates and votes on new committers (and new PMC members) in secret on their private mailing lists. Each project is different, and each PMC member might be different in their criteria for nominating somebody. I'm not a PMC member for this project, so I don't know how it typically happens here. Personally, I think you're probably doing all the right things to warrant consideration, but it's up to the PMC.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2601: java lib to upgrade to gradle 7 and move off maven
Posted by GitBox <gi...@apache.org>.
Jens-G commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120214710
> move off maven
because ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1121698513
> It will be much easier to re-run only the failed CI tasks that way, and in only a few minutes, instead of having it queued for hours and hours. All the relevant CI checks related to these checks seem to be passing just fine, though.
for anyone interested in reviewing GitHub action related pull requests:
- #2595
- #2594
- #2596
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on a diff in pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on code in PR #2601:
URL: https://github.com/apache/thrift/pull/2601#discussion_r867438962
##########
lib/java/gradle/publishing.gradle:
##########
@@ -38,84 +38,66 @@ task installJavadoc(type: Copy, group: 'Install', dependsOn: javadoc) {
from javadoc.destinationDir
}
-// This is not needed by Gradle builds but the remaining Ant builds seem to
-// need access to the generated test classes for Thrift unit tests so we
-// assist them to use it this way.
-task copyDependencies(type: Copy, group: 'Build') {
- description = 'Copy runtime dependencies in a common location for other Ant based projects'
- project.assemble.dependsOn it
-
- destinationDir = file("$buildDir/deps")
- from configurations.testRuntime
- // exclude some very specific unit test dependencies
- exclude '**/junit*.jar', '**/mockito*.jar', '**/hamcrest*.jar'
+java {
+ withJavadocJar()
+ withSourcesJar()
}
-// ----------------------------------------------------------------------------
-// Allow this configuration to be shared between install and uploadArchives tasks
-def configurePom(pom) {
- pom.project {
- name 'Apache Thrift'
- description 'Thrift is a software framework for scalable cross-language services development.'
- packaging 'jar'
- url 'http://thrift.apache.org'
-
- scm {
- url 'https://github.com/apache/thrift'
- connection 'scm:git:https://github.com/apache/thrift.git'
- developerConnection 'scm:git:git@github.com:apache/thrift.git'
- }
-
- licenses {
- license {
- name 'The Apache Software License, Version 2.0'
- url "${project.license}"
- distribution 'repo'
- }
- }
-
- developers {
- developer {
- id 'dev'
- name 'Apache Thrift Developers'
- email 'dev@thrift.apache.org'
+publishing {
+ publications {
+ mavenJava(MavenPublication) {
+ artifactId = "libthrift"
+ // explicitly set 3 jars because "from components.jar" will include shadow jar which isn't what we want
Review Comment:
```suggestion
// explicitly set 3 jars because calling "from components.java" will include shadow jar which isn't what we want
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on a diff in pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2601:
URL: https://github.com/apache/thrift/pull/2601#discussion_r867436633
##########
lib/java/Makefile.am:
##########
@@ -26,7 +26,7 @@ all-local:
--console=plain
install-exec-hook:
- $(GRADLE) $(GRADLE_OPTS) install \
+ $(GRADLE) $(GRADLE_OPTS) publishToMavenLocal \
Review Comment:
Are the names `publishToMavenLocal` and `publish` defined in the plugin? Or are these defined somewhere in this build configuration?
I ask only because it would be kind of nice to keep the same terminology as "maven" when performing maven operations. Maven calls publishing to the local repository "install", and publishing to an external repository "deploy". If we can use those names, that'd be great. If the plugin defines the names, then I guess our hands are tied.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120353270
Looking at this changeset makes me just hate Gradle so much. There's just so much tooling that I don't understand, and some of it seems pretty low-level. I prefer Maven over Gradle. I know XML sucks, but I think its lifecycle concept is simpler, and the conventions are more concise. The direction these changes are going is fine, though. I'm learning Gradle by reviewing these. Maybe I won't hate it as much as I understand it more. But, I understand Makefiles, and I still hate them... so maybe I still will. :shrug: :smiley_cat:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120857532
It looks like the conditional configuration isn't working. When the plugin isn't applied, then the type safe `ktfmt` configuration block that calls `kotlinLangStyle()` fails with a compilation error. I can't figure out how to conditionally apply the configuration... I don't think any type-safe way will work, and I don't understand the non-type-safe ways discussed in the docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120237325
> > move off maven
>
> because ...
I had to correct my typing, sorry for the confusion... @Jens-G
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120567275
I saw Travis fail with:
```
BUILD SUCCESSFUL in 45s
6 actionable tasks: 6 executed
ninja: build stopped: subcommand failed.
The command "build/docker/run.sh" exited with 1.
```
No idea what the problem is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120762056
> 3\. i think you meant javadoc warnings?
Mostly, yes. Most of the javadoc warnings can probably be taken care of with `-Xdoclint:all,-missing` (it's not really a problem if an unimportant method is missing a javadoc, but if one exists, then all checks should be enabled).
I thought I saw a few warnings on the kotlin side, but I don't remember what they were.
In any case, these don't have to be part of this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120604901
confirmed that the build failure in Java 8 was due to kolin:
<details>
<summary>build log</summary>
```
root@5ec15a799389:/opt/thrift/src/lib/kotlin# ../java/gradlew assemble
FAILURE: Build failed with an exception.
* What went wrong:
A problem occurred configuring root project 'libthrift-kotlin'.
> Could not determine the dependencies of null.
> Could not resolve all task dependencies for configuration ':classpath'.
> Could not resolve com.ncorti.ktfmt.gradle:plugin:0.8.0.
Required by:
project : > com.ncorti.ktfmt.gradle:com.ncorti.ktfmt.gradle.gradle.plugin:0.8.0
> No matching variant of com.ncorti.ktfmt.gradle:plugin:0.8.0 was found. The consumer was configured to find a runtime of a library compatible with Java 8, packaged as a jar, and its dependencies declared externally, as well as attribute 'org.gradle.plugin.api-version' with value '7.4.2' but:
- Variant 'apiElements' capability com.ncorti.ktfmt.gradle:plugin:0.8.0 declares a library, packaged as a jar, and its dependencies declared externally:
- Incompatible because this component declares an API of a component compatible with Java 11 and the consumer needed a runtime of a component compatible with Java 8
- Other compatible attribute:
- Doesn't say anything about org.gradle.plugin.api-version (required '7.4.2')
- Variant 'runtimeElements' capability com.ncorti.ktfmt.gradle:plugin:0.8.0 declares a runtime of a library, packaged as a jar, and its dependencies declared externally:
- Incompatible because this component declares a component compatible with Java 11 and the consumer needed a component compatible with Java 8
- Other compatible attribute:
- Doesn't say anything about org.gradle.plugin.api-version (required '7.4.2')
* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
* Get more help at https://help.gradle.org
Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
See https://docs.gradle.org/7.4.2/userguide/command_line_interface.html#sec:command_line_warnings
BUILD FAILED in 37s
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1121697053
Another Travis thing is failing. This time, it's because it couldn't download something from NuGet, unrelated to these changes. I look forward to more things being done in GH Actions. It will be much easier to re-run only the failed CI tasks that way, and in only a few minutes, instead of having it queued for hours and hours. All the relevant CI checks related to these checks seem to be passing just fine, though.
So, I think this is ready to merge.
@Jens-G since you made a comment earlier, did you want to take another look, or express any comments or objections before this is merged?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1121417897
> for now i'm just configuring the plugin in both versions but only conditionally apply it with Java 11+ so there's no conflict. hopefully this can solve the issue.
This seems to work for the CI tests (`gradle assemble` seems to work fine), but manually running `gradle build` or `gradle ktfmtFormat` using JDK 8 won't work still. That's probably okay.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1123820264
> #2601
hi @mitumaruto i'm not sure i understand your comment nor suggested changes, can you elaborate?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120913333
> It looks like the conditional configuration isn't working. When the plugin isn't applied, then the type safe `ktfmt` configuration block that calls `kotlinLangStyle()` fails with a compilation error. I can't figure out how to conditionally apply the configuration... I don't think any type-safe way will work, and I don't understand the non-type-safe ways discussed in the docs.
for now i'm just configuring the plugin in both versions but only conditionally apply it with Java 11+ so there's no conflict. hopefully this can solve the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120586366
> @Jimexist This looks good to me, but I'm curious what the user experience is when trying to build with JDK 8. Will it fail in incomprehensible ways, or will the requirement to use JDK 11 be made obvious? For Maven, the maven-enforcer-plugin can provide a meaningful error message when the minimum Java version is not used.
In that case gradle will complain saying that gradle 7.4.2 needs Java 11.
I also put a step in https://github.com/apache/thrift/pull/2602/files where the Java 11 built artifact is run using Java 8 during cross test to make sure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120352415
> This is put to draft state because on xenial JDK 8 is used but somehow gradle 7.4+ does not quite work with Java 8. It'll at least require Java 11, which makes sense because it's the LTS version that's not at its end of life now.
My personal preference would be to migrate the build to use JDK 11 at a minimum. The resulting libthrift.jar could still be used with Java 8, if the appropriate `--release 8` flag is placed on `javac`. In my experience, JDK 11 is even better than JDK 8 at enforcing the Java 8 language rules and results in byte code that more reliably runs on a variety of Java 8 runtimes anyway. But, I don't know if you'd get resistance to this, because some people don't like building and testing with a different JDK version than the Java runtime the jar is targeting. I don't know how to set `--release 8` for Gradle, but it can't be too hard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120607478
> confirmed that the build failure in Java 8 was due to kolin's ktfmt which depends on gradle api that's Java 11+ only:
Maybe using that plugin can be wrapped around a conditional? If so, then could go ahead and revert the Xenial changes here.
it'd be nice to make the choice to require JDK 11 a conscious decision for later, rather than as a side-effect of the Gradle update now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii merged pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii merged PR #2601:
URL: https://github.com/apache/thrift/pull/2601
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1121699185
> did you want to take another look, or express any comments or objections before this is merged?
one thing i didn't test myself, or on CI, is _actual_ publishing, because my lack of access. but i've tested publishing locally and the content seems fine:
- `.jar`
- `-sources.jar`
- `-javadoc.jar`
- pom file
- without the shadow jar
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120355087
> Looking at this changeset makes me just hate Gradle so much. There's just so much tooling that I don't understand, and some of it seems pretty low-level. I prefer Maven over Gradle. I know XML sucks, but I think its lifecycle concept is simpler, and the conventions are more concise. The direction these changes are going is fine, though. I'm learning Gradle by reviewing these. Maybe I won't hate it as much as I understand it more. But, I understand Makefiles, and I still hate them... so maybe I still will. 🤷 😺
I'm totally with you on e.g. the unencessity of breaking changes needed to upgrade from 6.x to 7.x and all the migration from `maven` to `maven-publish` are just friction with no obvious gain. on the other hand there are good chances happening in gradle land, e.g. using `.kts` i.e. kotlin which has better tooling and typing support than groovy. Nowadays i don't use maven as much but it's a more mature and popular choice so i'm okay with learning and using either one. currently libthrift uses gradle so be it (unless someone wants to make a chance).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on a diff in pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on code in PR #2601:
URL: https://github.com/apache/thrift/pull/2601#discussion_r867438931
##########
lib/java/build.gradle:
##########
@@ -60,7 +60,6 @@ if (Boolean.parseBoolean(project.release)) {
// Keeping the rest of the build logic in functional named scripts for clarity
apply from: 'gradle/environment.gradle'
apply from: 'gradle/sourceConfiguration.gradle'
-apply from: 'gradle/additionalArtifacts.gradle'
Review Comment:
this file is removed and not needed and superseded by https://github.com/apache/thrift/pull/2601/files#diff-da5bde3bdf21a82cc6725ecd18f6f9f7d7c1dbcf7789e264892b51053a330a36R41-R43
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on a diff in pull request #2601: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on code in PR #2601:
URL: https://github.com/apache/thrift/pull/2601#discussion_r867438354
##########
lib/java/Makefile.am:
##########
@@ -26,7 +26,7 @@ all-local:
--console=plain
install-exec-hook:
- $(GRADLE) $(GRADLE_OPTS) install \
+ $(GRADLE) $(GRADLE_OPTS) publishToMavenLocal \
Review Comment:
i agree with your preference on consistency.
here our hands are tied because `publishToMavenLocal` is provided by the `maven-publish` plugin:
https://docs.gradle.org/current/userguide/publishing_maven.html#publishing_maven:tasks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120602254
FYI i think JDK 8 support is still with Gradle 7:
<details>
<summary>build log</summary>
```bash
root@5ec15a799389:/opt/thrift/src/lib/java# ./gradlew
bash: ./gradlew: No such file or directory
root@5ec15a799389:/opt/thrift/src/lib/java# java -version
openjdk version "1.8.0_332"
OpenJDK Runtime Environment (build 1.8.0_332-b09)
OpenJDK 64-Bit Server VM (build 25.332-b09, mixed mode)
root@5ec15a799389:/opt/thrift/src/lib/java# ./gradlew assemble
Downloading https://services.gradle.org/distributions/gradle-7.4.2-bin.zip
...........10%...........20%...........30%...........40%...........50%...........60%...........70%...........80%...........90%...........100%
Welcome to Gradle 7.4.2!
Here are the highlights of this release:
- Aggregated test and JaCoCo reports
- Marking additional test source directories as tests in IntelliJ
- Support for Adoptium JDKs in Java toolchains
For more details see https://docs.gradle.org/7.4.2/release-notes.html
Starting a Gradle Daemon (subsequent builds will be faster)
> Task :compileJava
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
> Task :javadoc
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/EnumCache.java:51: warning: no @return
public TEnum get(Class<? extends TEnum> enumClass, int value) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/ThriftMetadata.java:112: warning: no @param for level
protected String getIndent(int level) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/ThriftMetadata.java:112: warning: no @return
protected String getIndent(int level) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/ThriftMetadata.java:117: warning: no @param for sb
protected void append(StringBuilder sb, String format, Object... args) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/ThriftMetadata.java:117: warning: no @param for format
protected void append(StringBuilder sb, String format, Object... args) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/ThriftMetadata.java:117: warning: no @param for args
protected void append(StringBuilder sb, String format, Object... args) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/ThriftMetadata.java:122: warning: no @return
protected String getName() {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:37: warning: no @param for obj
public static void checkNotNull(Object obj, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:37: warning: no @param for argName
public static void checkNotNull(Object obj, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:42: warning: no @param for value
public static void checkPositiveInteger(long value, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:42: warning: no @param for argName
public static void checkPositiveInteger(long value, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:47: warning: no @param for value
public static void checkNotNegative(long value, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:47: warning: no @param for argName
public static void checkNotNegative(long value, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:59: warning: no @param for isValid
public static void checkValid(boolean isValid, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:59: warning: no @param for argName
public static void checkValid(boolean isValid, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:64: warning: no @param for isValid
public static void checkValid(boolean isValid, String argName, String validValues) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:64: warning: no @param for argName
public static void checkValid(boolean isValid, String argName, String validValues) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:64: warning: no @param for validValues
public static void checkValid(boolean isValid, String argName, String validValues) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:69: warning: no @param for arg
public static void checkNotNullAndNotEmpty(String arg, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:69: warning: no @param for argName
public static void checkNotNullAndNotEmpty(String arg, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:75: warning: no @param for <T>
public static <T> void checkNotNullAndNotEmpty(T[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:75: warning: no @param for array
public static <T> void checkNotNullAndNotEmpty(T[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:75: warning: no @param for argName
public static <T> void checkNotNullAndNotEmpty(T[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:81: warning: no @param for array
public static void checkNotNullAndNotEmpty(byte[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:81: warning: no @param for argName
public static void checkNotNullAndNotEmpty(byte[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:87: warning: no @param for array
public static void checkNotNullAndNotEmpty(short[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:87: warning: no @param for argName
public static void checkNotNullAndNotEmpty(short[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:93: warning: no @param for array
public static void checkNotNullAndNotEmpty(int[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:93: warning: no @param for argName
public static void checkNotNullAndNotEmpty(int[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:99: warning: no @param for array
public static void checkNotNullAndNotEmpty(long[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:99: warning: no @param for argName
public static void checkNotNullAndNotEmpty(long[] array, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:105: warning: no @param for <T>
public static <T> void checkNotNullAndNotEmpty(Iterable<T> iter, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:105: warning: no @param for iter
public static <T> void checkNotNullAndNotEmpty(Iterable<T> iter, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:105: warning: no @param for argName
public static <T> void checkNotNullAndNotEmpty(Iterable<T> iter, String argName) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:112: warning: no @param for <T>
public static <T> void checkNotNullAndNumberOfElements(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:112: warning: no @param for collection
public static <T> void checkNotNullAndNumberOfElements(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:112: warning: no @param for numElements
public static <T> void checkNotNullAndNumberOfElements(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:112: warning: no @param for argName
public static <T> void checkNotNullAndNumberOfElements(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:124: warning: no @param for value1
public static void checkValuesEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:124: warning: no @param for value1Name
public static void checkValuesEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:124: warning: no @param for value2
public static void checkValuesEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:124: warning: no @param for value2Name
public static void checkValuesEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:136: warning: no @param for value1
public static void checkIntegerMultiple(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:136: warning: no @param for value1Name
public static void checkIntegerMultiple(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:136: warning: no @param for value2
public static void checkIntegerMultiple(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:136: warning: no @param for value2Name
public static void checkIntegerMultiple(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:148: warning: no @param for value1
public static void checkGreater(long value1, String value1Name, long value2, String value2Name) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:148: warning: no @param for value1Name
public static void checkGreater(long value1, String value1Name, long value2, String value2Name) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:148: warning: no @param for value2
public static void checkGreater(long value1, String value1Name, long value2, String value2Name) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:148: warning: no @param for value2Name
public static void checkGreater(long value1, String value1Name, long value2, String value2Name) {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:159: warning: no @param for value1
public static void checkGreaterOrEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:159: warning: no @param for value1Name
public static void checkGreaterOrEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:159: warning: no @param for value2
public static void checkGreaterOrEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:159: warning: no @param for value2Name
public static void checkGreaterOrEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:171: warning: no @param for value1
public static void checkLessOrEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:171: warning: no @param for value1Name
public static void checkLessOrEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:171: warning: no @param for value2
public static void checkLessOrEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:171: warning: no @param for value2Name
public static void checkLessOrEqual(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:183: warning: no @param for value
public static void checkWithinRange(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:183: warning: no @param for valueName
public static void checkWithinRange(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:183: warning: no @param for minValueInclusive
public static void checkWithinRange(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:183: warning: no @param for maxValueInclusive
public static void checkWithinRange(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:195: warning: no @param for value
public static void checkWithinRange(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:195: warning: no @param for valueName
public static void checkWithinRange(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:195: warning: no @param for minValueInclusive
public static void checkWithinRange(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/partial/Validate.java:195: warning: no @param for maxValueInclusive
public static void checkWithinRange(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/TDeserializer.java:85: warning: no @throws for org.apache.thrift.transport.TTransportException
public TDeserializer(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/TDeserializer.java:109: warning: no @throws for org.apache.thrift.transport.TTransportException
public TDeserializer(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TEndpointTransport.java:56: warning: no description for @param
* @param newSize
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TEndpointTransport.java:58: warning: no @throws for org.apache.thrift.transport.TTransportException
protected void resetConsumedMessageSize(long newSize) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TEndpointTransport.java:78: warning: no description for @param
* @param size
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TEndpointTransport.java:90: warning: no description for @param
* @param numBytes
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TEndpointTransport.java:100: warning: no description for @param
* @param numBytes
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TEndpointTransport.java:102: warning: no @throws for org.apache.thrift.transport.TTransportException
protected void countConsumedMessageBytes(long numBytes) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/AutoExpandingBufferWriteTransport.java:42: warning: no @throws for org.apache.thrift.transport.TTransportException
public AutoExpandingBufferWriteTransport(
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TFileTransport.java:572: warning: no @param for args
public static void main(String[] args) throws Exception {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TFileTransport.java:572: warning: no @throws for java.lang.Exception
public static void main(String[] args) throws Exception {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TTransportFactory.java:35: warning: no @throws for org.apache.thrift.transport.TTransportException
public TTransport getTransport(TTransport trans) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:57: warning: no @throws for org.apache.thrift.transport.TTransportException
protected TIOStreamTransport() throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:76: warning: no @throws for org.apache.thrift.transport.TTransportException
public TIOStreamTransport(InputStream is) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:122: warning: no @throws for org.apache.thrift.transport.TTransportException
public TIOStreamTransport(InputStream is, OutputStream os) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:97: warning: no @throws for org.apache.thrift.transport.TTransportException
public TIOStreamTransport(OutputStream os) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:49: warning: no @param for config
protected TIOStreamTransport(TConfiguration config) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:49: warning: no @throws for org.apache.thrift.transport.TTransportException
protected TIOStreamTransport(TConfiguration config) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:64: warning: no description for @param
* @param config
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:67: warning: no @throws for org.apache.thrift.transport.TTransportException
public TIOStreamTransport(TConfiguration config, InputStream is) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:105: warning: no description for @param
* @param config
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:109: warning: no @throws for org.apache.thrift.transport.TTransportException
public TIOStreamTransport(TConfiguration config, InputStream is, OutputStream os)
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:84: warning: no description for @param
* @param config
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TIOStreamTransport.java:87: warning: no @throws for org.apache.thrift.transport.TTransportException
public TIOStreamTransport(TConfiguration config, OutputStream os) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingServerSocket.java:56: warning: no @param for port
public TNonblockingServerSocket(int port) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingServerSocket.java:56: warning: no @throws for org.apache.thrift.transport.TTransportException
public TNonblockingServerSocket(int port) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingServerSocket.java:61: warning: no @param for port
public TNonblockingServerSocket(int port, int clientTimeout) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingServerSocket.java:61: warning: no @param for clientTimeout
public TNonblockingServerSocket(int port, int clientTimeout) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingServerSocket.java:61: warning: no @throws for org.apache.thrift.transport.TTransportException
public TNonblockingServerSocket(int port, int clientTimeout) throws TTransportException {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingSocket.java:99: warning: no description for @param
* @param selector
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingSocket.java:121: warning: no @return
public SocketChannel getSocketChannel() {
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingTransport.java:39: warning: no @return
public abstract boolean startConnect() throws IOException;
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingTransport.java:39: warning: no @throws for java.io.IOException
public abstract boolean startConnect() throws IOException;
^
/opt/thrift/src/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingTransport.java:46: warning: no @return
public abstract boolean finishConnect() throws IOException;
^
100 warnings
Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
See https://docs.gradle.org/7.4.2/userguide/command_line_interface.html#sec:command_line_warnings
BUILD SUCCESSFUL in 1m 27s
6 actionable tasks: 6 executed
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120605978
@Jimexist I fixed the merge conflicts, and also bumped the spotless version, and formatted the kotlin and java files using Gradle 7.4.2. I'll let CI finish its checks, but I did notice that:
1. there are some deprecated stuff in use here that will be removed in Gradle 8 (which is not out yet, but will be nice to be prepared for) -- could be fixed as part of this PR,
2. some Gradle plugins use banned APIs that are removed/hidden in Java 17 (this might require waiting on upstream plugin developers, or disabling some plugins if Java 17 is used) -- could be a follow-on PR, and
3. there's a lot of warnings spamming the console for both lib/java and lib/kotlin -- could be a follow-on PR.
As a side note: I'm really happy to be helping review your changes. I'm learning a lot from the exercise, and I'm impressed with the quality and pace of your contributions. I've been feeling guilty about being added as a committer, but not really having much time to contribute. But, all your work has kept me interested, and contributing to the project, even if it's limited to the Java and CI changes. So, thanks for contributing so that I can contribute. :smiley_cat:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120570306
> I saw Travis fail with:
>
> ```
> BUILD SUCCESSFUL in 45s
> 6 actionable tasks: 6 executed
> ninja: build stopped: subcommand failed.
> The command "build/docker/run.sh" exited with 1.
> ```
>
> No idea what the problem is.
the error was further above. search for `ECONNRESET` - it's a flaky download connection reset, i guess rebuilding would help. but that's only limited to nodejs which i haven't touched in this pull request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2601: THRIFT-5581: java lib to upgrade to gradle 7 and migrate from `maven` to `maven-publish` plugin
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2601:
URL: https://github.com/apache/thrift/pull/2601#issuecomment-1120584810
@Jimexist This looks good to me, but I'm curious what the user experience is when trying to build with JDK 8. Will it fail in incomprehensible ways, or will the requirement to use JDK 11 be made obvious? For Maven, the maven-enforcer-plugin can provide a meaningful error message when the minimum Java version is not used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org