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

Dependencies are resolved from appropriate repositories #13582

Closed
wants to merge 1 commit into from

Conversation

erichaagdev
Copy link
Contributor

This change applies repository content filtering to configured repositories, reducing time spent during dependency resolution.

Previously, attempts would be made to resolve dependencies for org.opensaml.*, net.shibboleth.utilities.*, and net.minidev.* from the Spring releases repository, resulting in numerous failed requests during dependency resolution and prolonged resolution times.

This was particularly noticeable during IDE sync times. With these changes, my IDE sync time is ~3s whereas previously it was consistently above 40s.

This change applies repository content filtering to configured
repositories, reducing the time spent during dependency resolution.

This fixes an issue where requests for 'org.opensaml',
'net.shibboleth.utilities' and 'net.minidev' dependencies were being
made in the Spring releases repositories, resulting in many failed
requests during dependency resolution and increased resolution times.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 24, 2023
@rwinch rwinch self-assigned this Aug 7, 2023
@rwinch rwinch changed the base branch from main to 5.8.x August 7, 2023 14:47
@rwinch rwinch changed the base branch from 5.8.x to 6.0.x August 7, 2023 14:47
@rwinch rwinch changed the base branch from 6.0.x to main August 7, 2023 14:48
@rwinch rwinch added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 7, 2023
@rwinch rwinch closed this in 30bc263 Aug 7, 2023
@rwinch rwinch added this to the 5.8.6 milestone Aug 7, 2023
@rwinch
Copy link
Member

rwinch commented Aug 7, 2023

Thanks for the PR @erichaagdev I've merged this into 5.8.x via 30bc263 and then merged into 6.0.x 6.1.x and main (6.2.0-M2)

rwinch added a commit that referenced this pull request Aug 7, 2023
This makes the include more robust

Issue gh-13582
rwinch added a commit that referenced this pull request Aug 7, 2023
Use includeGroupByRegex

This makes the include more robust

Issue gh-13582
rwinch added a commit that referenced this pull request Aug 7, 2023
Use includeGroupByRegex

This makes the include more robust

Issue gh-13582
rwinch added a commit that referenced this pull request Aug 7, 2023
Use includeGroupByRegex

This makes the include more robust

Issue gh-13582
Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Thanks, @erichaagdev, this is really exciting. I've left some feedback inline.

@@ -1,6 +1,7 @@
plugins {
id 'org.antora' version '1.0.0'
id 'io.spring.antora.generate-antora-yml' version '0.0.1'
id 'io.spring.convention.repository'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this part of this commit? Is it necessary to prevent the unnecessary checks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Some of the failed calls were caused by resolving dependencies for this project. By using the repository conventions plugin we make sure this project uses the same resolution rules as the rest of the build. You will notice I also removed the now redundant repository declarations previously present at the end of this build file (lines 64-70).

@@ -34,6 +34,14 @@ class RepositoryConventionPlugin implements Plugin<Project> {
if (forceMavenRepositories?.contains('local')) {
mavenLocal()
}
maven {
Copy link
Contributor

Choose a reason for hiding this comment

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

While minor, could this remain in its original place in the file? If so, it's a little easier to follow the Git history if only the edits needed to affect the change are what gets committed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This move was intentional. Gradle will resolve dependencies from repositories in the order they are declared. This move ensures the shibboleth dependencies are not attempted to be resolved from any other repository. For example, if this was moved to be after mavenCentral(), then Gradle will attempt to resolve the shibboleth dependencies from Maven Central even though they are not available there. Then, the content inclusion (includeGroup) ensures no other dependencies are attempted to be resolved from the shibboleth repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants