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

Bump up dependency analyze to 1.10.0 #2643

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

iamsanjay
Copy link
Contributor

Summary

This PR upgrades the Gradle dependency version to 1.10.0 to facilitate the ongoing Java upgrade.

Background

We initially planned to upgrade the Gradle dependency to version 1.9.2. However, after implementing this upgrade, we encountered "unusedDeclaredArtifacts" errors. Further investigation revealed that these errors were due to a known bug (gradle-dependency-analyze/gradle-dependency-analyze#527) in version 1.9.2. This bug has been resolved in version 1.10.0.

Changes Introduced

Upon upgrading to version 1.10.0, several build.gradle files were found to be missing dependencies that are now required due to changes introduced in this newer version (as per commit 2bc05cf9df5690c4). This PR addresses these errors by adding the necessary dependencies to the relevant build.gradle files. Additionally, some of these dependencies were not needed for Java 11. However, when running analyzeDependencies for Java 21, new "usedUndeclaredArtifacts" errors were encountered. To resolve these issues, the necessary dependencies were added to the relevant build.gradle files.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

@iamsanjay
Copy link
Contributor Author

* What went wrong:
Execution failed for task ':solr:prometheus-exporter:analyzeTestClassesDependencies'.
> Dependency analysis found issues.
  usedUndeclaredArtifacts
   - org.apache.solr:solrj-zookeeper:10.0.0-SNAPSHOT@


* What went wrong:
Execution failed for task ':solr:modules:gcs-repository:analyzeClassesDependencies'.
> Dependency analysis found issues.
  usedUndeclaredArtifacts
   - com.google.auth:google-auth-library-credentials:1.23.0@jar

* What went wrong:
Execution failed for task ':solr:modules:hdfs:analyzeClassesDependencies'.
> Dependency analysis found issues.
  usedUndeclaredArtifacts
   - io.dropwizard.metrics:metrics-core:4.2.26@jar


* What went wrong:
Execution failed for task ':solr:modules:s3-repository:analyzeClassesDependencies'.
> Dependency analysis found issues.
  usedUndeclaredArtifacts
   - software.amazon.awssdk:http-client-spi:2.26.19@jar

I was curious about why these new errors appeared immediately after the upgrade. From the community, I learned that the new changes in gradle-dependency-analyze (as detailed in gradle-dependency-analyze/gradle-dependency-analyze#527) have introduced these warnings. While some of these dependencies seem to be transitive, I cannot see them being directly used in the project. I wonder if there might be another reason for these warnings beyond them being transitive dependencies.

@risdenk
Copy link
Contributor

risdenk commented Aug 13, 2024

These I think seem reasonable from a quick glance.

@iamsanjay
Copy link
Contributor Author

@dsmiley Should I create JIRA for this one?

@dsmiley
Copy link
Contributor

dsmiley commented Aug 15, 2024

@risdenk I know you introduced this plugin but IMO it's more annoying & burdensome than helpful. Maybe we should back out this plugin? I was somewhat appreciative of the Maven equivalent, which had some imperfections but mostly worked. The Gradle one seems more troublesome, requiring that we explicitly list dependencies that are not in fact directly used. Nowadays, I just want the build to be simpler.

JIRA issues aren't required for build improvements in general.

@iamsanjay
Copy link
Contributor Author

Planning to merge this shortly!

@iamsanjay iamsanjay merged commit 3deb0a4 into apache:main Aug 22, 2024
5 checks passed
@iamsanjay
Copy link
Contributor Author

@risdenk Should I backport this change to 9_x?

@risdenk
Copy link
Contributor

risdenk commented Aug 22, 2024

@iamsanjay yes

@risdenk
Copy link
Contributor

risdenk commented Aug 22, 2024

@risdenk I know you introduced this plugin but IMO it's more annoying & burdensome than helpful. Maybe we should back out this plugin? I was somewhat appreciative of the Maven equivalent, which had some imperfections but mostly worked. The Gradle one seems more troublesome, requiring that we explicitly list dependencies that are not in fact directly used. Nowadays, I just want the build to be simpler.

JIRA issues aren't required for build improvements in general.

since we talked about this during the community meeting - I agree the verbosity is a pain but this basically moves runtime issues into build time issues. Every case I've looked into the classes/dependencies in question were used in the code somewhere. There could be bugs in the plugin but they seem to get fixed overtime. It usually is the case the plugin seems to miss things that needed to be included (which is what this upgrade addresses). I haven't seen a case where a dependency needed to be declared but then wasn't actually used.

iamsanjay added a commit to iamsanjay/solr that referenced this pull request Aug 23, 2024
Upgrades the ca.cutterslade.analyze version to 1.10.0 to facilitate the ongoing Java upgrade (SOLR-17321)

(cherry picked from commit 3deb0a4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants