Make Xtext compatible with Guava 31 #2671
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Widen the version ranges in all MANIFEST.MF where the guava bundle is required to "[30.1.0,32.0.0)" and test Xtext with Guava 31 in the xtext-latest.target. Guice still requires Guava 30 so that version is still required, but this change should at least allow to use Xtext in conjunction with Guava 31.1. At the moment that is very problematic because Xtext plugins reexport the guava bundle. Because Guice uses Guava only internally and not in its API Guava classes of version 31 cannot 'leak' into Xtext and class-space consistency is ensured.
|
i have zero clue what could go wrong and thus cannot assess impact :( |
|
pushed to https://github.com/eclipse/xtext/tree/HannesWell-guava31compatibility so that jenkins can build repos to test with. |
|
as also mwe uses xtext i have no idea what happens. do i need to install something extra to get this running still? |
|
running mwe fails with |
|
so users would have to adapt build.properties |
|
same problem when running unit tests, this time from guice => is this a guava 31 specific problem? or caused by the mix? |
|
i also dont get this target resolved. @HannesWell any idea why jakarta is not found? |
|
|
is this cause the artifact is also packaged to https://ci.eclipse.org/xtext/job/xtext/job/cd_guice6x_lsp4j0210/lastSuccessfulBuild/artifact/build/p2-repository/plugins/ |
|
=> this is also why i totally dislike the m2e pde feature. gives no clue where the problem is |
|
of course consuming with pure p2 works but i still have to add transitive deps of guava directly. i assume this is why we do the reexport of the dependency to javax.inject in xtext |
|
Thanks for pushing and building the branch, will try it out with our product later this day. In your target in #2671 (comment), you have specified
This is because you pull in the com.google.inject 6.0.0 bundle from https://ci.eclipse.org/xtext/job/xtext/job/cd_guice6x_lsp4j0210/lastSuccessfulBuild/artifact/build/p2-repository, but this repo does not include jakarta.inject 2. Unfortunately it is currently not possible to resolve units from a So a solution would be to include jakarta-inject-2 in the repo that contains guice 6. |
|
@HannesWell yes i fixed the first manifest error by splitting it, but it still does not work. |
Unfortunately I cannot reproduce that failure. But failureaccess is there and already a valid OSGi bundle. |
|
So you can run Workflows? How does you manifest and build.properties look like |
Sorry I just checked if the target resolves, assumed this was the issue. What should I try exactly? In a new workspace use the Target from your previous comment and try the Xtext examples you mentioned? |
|
Use the target, create new Xtext project, run workflow. Run generated init test if workflow succeeds |
OK, with that I get the same errors like you in #2671 (comment). 😞 I assume, MWE is not running in a OSGi runtime and has a flat classpath, doesn't it? |
|
Yes it is just java . But even if it succeeds pure java unit tests show same problem. |
|
OK, too bad. I still wonder why this happens, since plain Junit launches (and I assume also MWE) uses the dependency closure of the plugin. And if the plugin is resolved against guava 31, then the separate failure-access artifact should be included too, and if it is resolved against guava 30 from Orbit, those classes are in the guava-jar. Anyway, I'm closing this then since I assume it will not make it into the upcoming release. |
This was referenced Apr 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As suggested in #2056 (comment), this PR aims to widen the version ranges in all MANIFEST.MF where the guava bundle is required to "[30.1.0,32.0.0)".
Test Xtext with Guava 31 in the
xtext-latest.targetonly and test it with Guava 30 in the older targets.Guice 5.0.1 requires Guava 30 so that version is still required, but this change should at least allow to use Xtext in conjunction with Guava 31.1 (altough this means that one needs two Guava versions if one wants to use Guava 31). At the moment one can not use Guava 31 when Xtext is used without errors because Xtext plugins reexport the guava bundle.
Because Guice uses Guava only internally and not in its API Guava classes of version 31 cannot 'leak' into Xtext and class-space consistency is ensured. If MWE does not reexport Guava and also only use it internally the same applies for that and a update in MWE is not required.