-
Notifications
You must be signed in to change notification settings - Fork 0
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
Remove strict requirement to bundle javax.inject #69
Remove strict requirement to bundle javax.inject #69
Conversation
@HannesWell thanks! I think that was inherited from xtext long time ago. Let's see whether the builds succeed. |
The builds fail because some of those classes (wrongly, I'd say) use javax.inject.Provider instead of the guice type. |
Thanks @LorenzoBettini for the quick response.
The class actually uses already the guice type, but
I added javax.inject as import so now hopefully the build passes. |
I was suggesting something else: instead of using Provider from javax.inject, use Provider from Google Guice |
It already uses the provider from Guice. But So there is nothing much that can be done in the code. :/ |
The build still fails though.. I don't remember how xtext dealt with the same situation cc @cdietrich |
Yes. But what was because |
which guice do you use in tp? |
@HannesWell the tests wrongly use javax.inject.Inject instead of guice inject @cdietrich I'm still at 2022-12, I'll update deps next week |
Ah, didn't noticed before that there are different build setups.
I changed the tests to use com.google.inject.Inject and adjusted the Manifests of the E4 examples (I assume that you don't want to/can migrate them to jakarta yet). |
Thanks @HannesWell it works now. I'd like to avoid having to specify javax.inject imports though: the clients projects would not work anymore now. If I moved to Jakarta would that solve the problem? Or, @cdietrich , moving to the new version of xtext/guice would solve the problem? |
Great!
That's right if they relied on the reexport of But yes, if parsley would require the |
Guide 7 has no dep or support for Javax.inject anymore |
@HannesWell thanks for the contribution! I'll merge it for now, and when I switch to new Xtext and Guice (#71 ) I'll remove all the import of javax.inject from MANIFESTs. @cdietrich thanks for the confirmation about the new Guice. |
@HannesWell did you try to setup the development environment with Oomph? I'm getting errors on the Xtend file import org.eclipse.emf.parsley.tests.util.GlobalAdapterFactoryEditingDomainModule
import org.eclipse.emf.parsley.tests.util.SingletonAdapterFactoryEditingDomainModule I suspect that's something to do with
|
Great, thanks!
Sorry I didn't. I had to adjust multiple projects and therefore only cloned the repos and modified them 'raw' and relied on the CI to check it. My first guess is that the guice annotations don't work here and therefore weren't applied before. |
@HannesWell I don't know if it's worthwhile to try to fix that. As I said, as part of #71 I'll upgrade to Guice 7 and I'll be able to remove |
OK, then please just revert the commit. I have to admit that I'm too busy and not eager to investigate it, if the plan is to revert it anyways the next week. 😅 |
OK! |
That would be great, thanks! |
@HannesWell just one more question: what about javax.inject.Inject annotations used in e4 applications? |
The E4 injector supports both Jakarta and javax annotations, you can use either one (probably can even them mix or apply both): And it will support Javax for at least two more years: |
@HannesWell, I've just contributed EMF Parsley 1.15.0 milestone to simrel: I removed all references to |
Perfect. Thanks! |
EMF-Parsly is one of few remaining projects that still pull the javax.inject bundles originating from Orbit into the Eclipse SimRel and it would therefore be good to get rid of that strict requirement.
Since the runtime Plugin is empty I assume it is just used to pull in some bundles into a (propably non-OSGi?) runtime.
If the required-bundle should be replaced by a
Import-Package
forjavax.inject
please let me know.Context: eclipse-simrel/simrel.build#49