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

[MRESOLVER-397] Deprecate Guice integration #328

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Sep 5, 2023

Lowering the provided instantiation support from 3 to 2, to lower resources for maintenance all these.

SL is about to be dropped, Guice same. Users should either use Sisu or use the new Supplier.


https://issues.apache.org/jira/browse/MRESOLVER-397

Lowering the provided instantiation support from 3
to 2, to lower resources for maintenance all these.

SL is about to be dropped, Guice same. Users should
either use Sisu or use the new Supplier.

---

https://issues.apache.org/jira/browse/MRESOLVER-397
@cstamas cstamas added this to the 1.9.16 milestone Sep 5, 2023
@cstamas cstamas requested a review from gnodet September 5, 2023 14:37
@cstamas cstamas self-assigned this Sep 5, 2023
@cstamas cstamas marked this pull request as ready for review September 5, 2023 14:37
@gnodet
Copy link
Contributor

gnodet commented Sep 6, 2023

Lowering the provided instantiation support from 3 to 2, to lower resources for maintenance all these.

SL is about to be dropped, Guice same. Users should either use Sisu or use the new Supplier.

https://issues.apache.org/jira/browse/MRESOLVER-397

Sisu is using guice anyway... Are there any Sisu specific things that would prevent the resolver to be used by any jsr-330 implementation ?

@cstamas
Copy link
Member Author

cstamas commented Sep 6, 2023

Yes, Sisu is built on "top" of Guice, and offers two-way passage, basically whoever uses Guice should be able to use Resolver by just throwing Sisu JAR in there as well.

As for "any jsr-330 implementation", am really unsure.

Anyway, the point of whole effort is:

We have no resources to maintain several integrations that we are not even sure, are they used or not. We should offer only 2, one DI/Sisu and one "pure java" (supplier) and done.

@cstamas cstamas merged commit f4c27eb into apache:master Sep 6, 2023
7 checks passed
@cstamas cstamas deleted the MRESOLVER-397 branch September 6, 2023 12:47
@basil
Copy link

basil commented Sep 24, 2024

Hi @cstamas, we are currently relying on this deprecated functionality in the Jenkins Acceptance Test Harness (ATH), a Guice-based project that also uses Maven Resolver to fetch Jenkins plugins for tesitng purposes. The relevant functionality is here: https://github.com/jenkinsci/acceptance-test-harness/blob/9b7b87da2b1d5873876d156756f8849b26fcbc16/src/main/java/org/jenkinsci/test/acceptance/utils/aether/AetherModule.java

Thank you very much for including this note:

This class is about to be dropped in 2.0.0 release. Use Sisu or use Maven Resolver Supplier to get Resolver instances.

However, I am not sure whether Sisu or Maven Resolver Supplier would be better for our use case. Since we are already using Guice, would Sisu be the preferred migration path? And if so, are there any examples you could point to?

@cstamas
Copy link
Member Author

cstamas commented Sep 24, 2024

Given you use Guice already, Sisu is "just" +300KB (if using no_asm and you provide ASM), or +500KB extra dependency. If you can live with that, I'd say "go with Sisu".

@basil can you point me at some building instructions where I can test some ideas for migration?

@basil
Copy link

basil commented Sep 24, 2024

@cstamas This is a test framework, so the added Sisu dependency would be fine for us. Thanks for confirming that Sisu is the preferred migration path for heavy Guice users. As far as building instructions, https://github.com/jenkinsci/acceptance-test-harness/blob/master/docs/DOCKER.md is the best place to start. For testing Guice integration, you could probably even avoid Docker and simply use e.g. run.sh firefox latest -Dtest=plugins.AntPluginTest#autoInstallAnt.

@cstamas
Copy link
Member Author

cstamas commented Sep 24, 2024

@basil something along these lines? jenkinsci/acceptance-test-harness#1733

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.

3 participants