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

Review resolver #349

Closed
wants to merge 9 commits into from
Closed

Review resolver #349

wants to merge 9 commits into from

Conversation

nicolasb29
Copy link
Contributor

@nicolasb29 nicolasb29 commented Nov 28, 2024

@nicolasb29 nicolasb29 changed the title Add jackson-annotations version before resolving Review resolver Nov 29, 2024
@timtebeek
Copy link
Contributor

Thanks @nicolasb29 ! @arlaneenalra Would you care to weigh in here? Since you were involved with

See also this problem now seen with "Spring Dependency Management plugin" on older Spring Boot versions

@timtebeek timtebeek added the bug Something isn't working label Nov 29, 2024
@arlaneenalra
Copy link
Contributor

Sorry for the delay. Based on what I'm reading, these two could be related. I'd be willing to bet the Spring Plugin is doing something similar to what our plugin is doing. You've got me wondering if it would make sense to revert my change and potentially resubmit it behind a flag to disable copying explicitly.

// By using a detached configuration, we separate this dependency resolution from the rest of the project's
// configuration. This also means that Gradle has no criteria with which to select between variants of
// dependencies which expose differing capabilities. So those must be manually configured
Configuration detachedConf = project.getConfigurations().detachedConfiguration(dependencies);
Copy link
Contributor

@arlaneenalra arlaneenalra Dec 3, 2024

Choose a reason for hiding this comment

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

Would it be possible to add a flag to the extension to disable the detached configurations behavior?

Or detect the spring plugin and use only use the detached configuration if it's present?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Potentially? The unfortunate part of doing that would mean moving further away from being compatible with the configuration cache feature (all tasks are marked as not compatible presently).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be possible to add a flag to the extension to disable the detached configurations behavior?

Or detect the spring plugin and use only use the detached configuration if it's present?

I will check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I add a Gradle parameter so you can add -Prewrite.detachResolve to detach the resolver.

Copy link
Collaborator

@shanman190 shanman190 Dec 5, 2024

Choose a reason for hiding this comment

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

@arlaneenalra, I'm trying to wrap my head around this, but is there a reason for your all's plugin to try to bother with central dependency version management for the rewrite configuration specifically? It seems like the rewrite configuration not being a detached configuration in all cases is causing some pretty wide spread problems.

The rewrite configuration is also really meant to be isolated just for the purposes of this plugin, so other plugins manipulating it's versions has shown to be detrimental (case in point from @nicolasb29's issue).

I'm really wondering if just going back to always being a detached configuration may be the right thing to do rather than trying to have a toggle to go one way or the other...

Copy link
Contributor

Choose a reason for hiding this comment

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

@arlaneenalra, I'm trying to wrap my head around this, but is there a reason for your all's plugin to try to bother with central dependency version management for the rewrite configuration specifically? It seems like the rewrite configuration not being a detached configuration in all cases is causing some pretty wide spread problems.

For our use case, we have a local publishing process that redirects some dependencies when doing development. That process doesn't work for an detached configuration like this without special handling (i.e. a custom configuration class). Our work arounds wound up injecting conflicting dependency jars and causing random failures because of the conflicts. If this is really not viable, I'll have to do some more digging to see if there are other alternatives .. hmm

Copy link
Collaborator

Choose a reason for hiding this comment

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

For what it's worth, what I typically do is use Gradle's publishToMavenLocal, then bring in the local snapshot jar via Gradle's native dependency resolution.

@timtebeek
Copy link
Contributor

Thanks all! Given the volume of issues reported with the earlier change we've decided to rollback instead:

I hope that gets most of the folks going again; apologies for the disruption here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants