Update to Guava 33

[cdietrich]

Closes #2876

[cdietrich]

needs to be checked for completeness / functionality

[LorenzoBettini]

To me, it looks fine. I'd tend to merge it ASAP, since it blocks the build ;)

[cdietrich]

we also need to check the mwe side (see pr) and build a mwe M0 and bootstrap against it.
then build Xtext M0 and bootstrap against it in mwe

how @szarnekow can have a look

[szarnekow]

we also need to check the mwe side (see pr) and build a mwe M0 and bootstrap against it.
then build Xtext M0 and bootstrap against it in mwe

Meaning the following sequence?

• Merge MWE
• Build MWE M0
• Bootstrap Xtext
• Build Xtext M0
• Update MWE and Bootstrap MWE

[cdietrich]

am not sure about the order. i assume we can start with Xtext
as the build seems to run through.
did not run any mwe workflows though.
mwe build also seems to run

[szarnekow]

@cdietrich I pushed an additional commit to your branch with two missing version constraints for Guava (xtext.ide and codemining). Also added an upper bound for failureaccess.

Do you remember why we need to add the dependency to failureaccess even if we don't use it directly? Shouldn't that be available by transitivity?

[LorenzoBettini]

Do you remember why we need to add the dependency to failureaccess even if we don't use it directly? Shouldn't that be available by transitivity?

@szarnekow maybe this is not the case but for other bundles created from Maven artifacts in Orbit transitive dependencies are to be listed explicitly since they are not really transitive (not required bundles); you might want to check from Eclipse the MANIFEST of google.inject to verify that. For example, I'm using Mockito from Orbit and I have to add as required bundles byte-buddy and other Mockito's dependencies (or I get NoClassDefFound errors during tests).

[cdietrich]

I don’t remember

[szarnekow]

Hmm I removed that require-bundle locally and could run xtext.tests and xtext.xtext.ui.tests without trouble. I'll make a branch and give it a try.

Note that failureaccess exports a package that imported in guava. In that sense, the dependency is declared.

[LorenzoBettini]

Great! Maybe it was just a problem of the Mockito bundle then.

[szarnekow]

Let's see if Jenkins agrees

[LorenzoBettini]

The CI is happy! :)

[cdietrich]

am getting

Caused by: java.lang.NoClassDefFoundError: com/google/common/util/concurrent/internal/InternalFutureFailureAccess
	at com.google.common.cache.LocalCache$LoadingValueReference.<init>(LocalCache.java:3527)
	at com.google.common.cache.LocalCache$LoadingValueReference.<init>(LocalCache.java:3533)
	at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2170)
	at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2081)
	at com.google.common.cache.LocalCache.get(LocalCache.java:4036)
	at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:4059)
	at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:5041)
	at com.google.common.cache.LocalCache$LocalLoadingCache.getUnchecked(LocalCache.java:5048)
	at com.google.inject.internal.Annotations$AnnotationChecker.hasAnnotations(Annotations.java:342)
	at com.google.inject.internal.Annotations.isBindingAnnotation(Annotations.java:414)
	at com.google.inject.Key.ensureIsBindingAnnotation(Key.java:383)
	at com.google.inject.Key.strategyFor(Key.java:370)
	at com.google.inject.Key.withAnnotation(Key.java:304)
	at com.google.inject.internal.AbstractBindingBuilder.annotatedWithInternal(AbstractBindingBuilder.java:88)
	at com.google.inject.internal.BindingBuilder.annotatedWith(BindingBuilder.java:51)
	at com.google.inject.internal.BindingBuilder.annotatedWith(BindingBuilder.java:42)

when create new xtext project and launch mwe2.
need to check why

[cdietrich]

=> something changed in orbit? with the new guava?
(am still on 2023-09 on this machine)
will also try latest

[cdietrich]

explicit readd works. but why did it work before?

[cdietrich]

=> we have no tests that launch mwe2

[szarnekow]

Ok, let’s drop the last commit. I’m only on m y phone right now. @LorenzoBettini can you please revert the last attempt I made?

[cdietrich]

ahhhh you did experiment here

[LorenzoBettini]

@szarnekow I've just reverted it; so we can merge right?

@cdietrich now that you reported the exception, I remember a similar problem in EMF Parsley without failureaccess; I had forgotten about that, and only remembered about troubles with Mockito.

[szarnekow]

LGTM

[LorenzoBettini]

OK, I'm merging

[ewillink]

A few observations from a distance.

a) Tycho is very tightly coupled to the platform, so really weird things can happen until you have the Tycho version that tracks the latest platform dependency change. "NoClassDefFoundError: com/google/common/util/concurrent/internal/InternalFutureFailureAccess" looks a bit like this.

b) Guava versions keep changing and have good compatibility. For my Xtext-based projects, I leave the Guava bounds unspecified until I have a good reason to specify; I therefore indirectly use the Xtext version specification and avoid appearing on Ed M's naughty list. So far I have only needed to repair a couple of deprecated-then-removed incompatibilities by creating my own utility version. My weekly forced rebuild alerts me to an incompatibility. My weekly compatibility builds check the preferred Guava version for each platform verson since Oxygen. (Provided my JUnit coverage is adequate.)

Wouldn't it be easier for Xtext to leave Guava bounds unspecified so that once the next Java/platform/Orbit/Tycho configuration gives you a new Guava, your 'weekly' full rebuild fails tell you what to work on?

Rather than "we also need to check the mwe side", why not improve the JUnit coverage?