Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies, support Java 11 #659

Merged
merged 4 commits into from
Dec 14, 2022
Merged

Conversation

elevenfive
Copy link
Contributor

@elevenfive elevenfive commented Jul 4, 2022

  • All tests passed. If this feature is not already covered by the tests, I added new tests.

Replacement for #653 with Gradle 6 instead of Gradle 7 cc @yahavi

@elevenfive elevenfive mentioned this pull request Jul 4, 2022
1 task
@elevenfive elevenfive changed the title Update dependencies, fix build Update dependencies, support Java 11 Jul 4, 2022
@yahavi
Copy link
Member

yahavi commented Dec 5, 2022

Thanks for this contribution, @elevenfive!
One of the Gradle tests is failing:

Gradle suite > Gradle test > org.jfrog.gradle.plugin.artifactory.GradlePluginTest.publicationsTestKotlinDsl0 FAILED
org.gradle.testkit.runner.UnexpectedBuildFailure: Unexpected build execution failure in /var/folders/dn/3smrq2yj2qddjccqlky47mpw0000gq/T/gradle_tests_space with arguments [clean, artifactoryPublish, --stacktrace]

Output:

FAILURE: Build failed with an exception.

* What went wrong:
A problem occurred configuring root project 'gradle_tests_space'.
> Could not open cache directory 55ogsh56r3ls2qvvelop96jkt (/private/var/folders/dn/3smrq2yj2qddjccqlky47mpw0000gq/T/.gradle-test-kit-yahavi/caches/4.10.3/gradle-kotlin-dsl-accessors/55ogsh56r3ls2qvvelop96jkt).
   > org.jetbrains.kotlin.protobuf.InvalidProtocolBufferException: Protocol message contained an invalid tag (zero).

Steps to reproduce:

  • Run ./gradlew build-info-extractor-gradle:test

Would you consider taking a look?

@elevenfive
Copy link
Contributor Author

Hi @yahavi, thank you for looking! I just updated my branch to resolve conflicts with main branch and also to pull new updates as applicable.

  • We can upgrade to Java 11, I'm not sure if the project is ready for that (builder env must have Java 11 of course), so I put notes for that regarding updating the Testng library

  • All tests that don't require network connection to the test instance pass for me, can you retest with the updates I posted?

@yahavi
Copy link
Member

yahavi commented Dec 6, 2022

Thanks @elevenfive,

  1. This library is a dependency for many old clients and we prefer migrating to Java 11 only if there is no other choice. We had some issues after the migration from Java 7 to Java 8. AFAIK Java 8 should receive security updates until 2030.
  2. Unfortunately publicationsTestKotlinDsl failed again.
    You can start an OSS server to run the Gradle tests. See https://github.com/jfrog/build-info#testing-on-artifactory-oss.
    I don't know what is the root cause, but it looks like upgrading Gradle from 5 to 6 caused this issue. I'm not sure whether Gradle 4 with Kotlin is in use by somebody, maybe we can drop Gradle 4 support on the Gradle plugin and skip the test on Gradle 4 - @eyalbe4 WDYT?

@elevenfive
Copy link
Contributor Author

elevenfive commented Dec 6, 2022

Thank you I will try to repro locally and see if there is a fix.

AFAICT Gradle 4 & 5 are both EOL, with 5 last receiving an update in 2019 and 4 last receiving an update in 2018; I would personally suggest dropping Gradle 4 support at a minimum.

https://gradle.org/releases/

@yahavi
Copy link
Member

yahavi commented Dec 13, 2022

Hi @elevenfive,
We had a team conversation about it. Dropping Gradle 4's support requires a new major (5).
We do have plans in the near future to separate the Gradle Artifactory plugin code from the build-info repository and release a new major. After the separation, it will be easier to make structural changes and dependency upgrades in the build-info repository.
In the meanwhile, we prefer to use Gradle 5 to build this project.

@elevenfive
Copy link
Contributor Author

Gotcha - I have reverted back to Gradle 5 and added a comment. Thanks for looking into it team!

Copy link
Member

@yahavi yahavi left a comment

Choose a reason for hiding this comment

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

It looks good now.
Thanks for your contribution, @elevenfive!

@yahavi yahavi merged commit 594376f into jfrog:master Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants