Bugzilla – Full Text Bug Listing
| Summary: | Post M1 Problem with ClasspathTypeProviderFactory evolution | ||
|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Ed Willink <ed> |
| Component: | Xtext | Assignee: | Project Inbox <tmf.xtext-inbox> |
| Status: | RESOLVED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | christian.dietrich.opensource, karsten.thoms, sven.efftinge |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows NT | ||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 445697 | ||
(In reply to Ed Willink from comment #0) > ? any chance of restoring the old constructor ? Doh! Injection allows me to just add the extra argument. Need to investigate backwards compatibility. > ? better: any chance of using resourceSet.getClass().getClassLoader() for > non-XtextResourceSets avoiding the need for an overload in the first place ? Not sure I understand. The old constructor has just been deprecated and is still available, no? No.
The old constructor:
@Inject
public ClasspathTypeProviderFactory(ClassLoader classLoader) {
this.classLoader = classLoader;
}
has been replaced by:
@Inject
public ClasspathTypeProviderFactory(ClassLoader classLoader, TypeResourceServices services) {
this.classLoader = classLoader;
this.services = services;
}
so my derived constructor fails and I need some Class.forName trickery to be compatible.
The specific Eclipse OCL problem has gone away because of recoding to avoid all usage of xtext.common.types. Xtext may wish to morph this bug into an enhancement request to make at least part of xtext.common.types.access public API and therefore extensible. Xtext may also review the API to enable the user classpath to be specified. Recent newsgroup postings suggest that the classpathURI context doesn't really work. I suggest that the user be encouraged to load their own class in their own context so that this class can determine what the user context is. Fortunately FileEditorInput is not final, so a user-defined FileEditorInput derivation makes for a good way of passing context through the IEditorInput open editor parameter in JUnit tests. i dont think it still makes sense to do anything about this besides being more careful about such changes in future If you are 'more careful' about changing internal API, you will never be able to improve Xtext ever again. The eventual issue was the need for the user's class loader/path to be used. This could be addressed by a better API. i dont understand. you can specify the classloader to be used. you can bind e.g. a urlclassloader there (In reply to Ed Willink from comment #0) > For Eclipse OCL I have a problem that I am using a plain ResourceSetImpl and > require the user's class loader to be used for the JVM path. Enhanced facilities are only available with the non-standard XtextResourceSet. Please understand that we can't deal all issues and decide to not work on certain items. Christian decided to set this to WONTFIX, or we could just keep this here open forever. If you think that this is important for you and there is a way to solve your issue in a way acceptable for the project, we are willing to accept PRs and review them. |
ClasspathTypeProviderFactory is not API so I have no right to complain but: For Eclipse OCL I have a problem that I am using a plain ResourceSetImpl and require the user's class loader to be used for the JVM path. I therefore have a ResourceSetClasspathTypeProviderFactory that has the following overload: @Override protected ClasspathTypeProvider createClasspathTypeProvider(ResourceSet resourceSet) { return new ClasspathTypeProvider(resourceSet.getClass().getClassLoader(), resourceSet, getIndexedJvmTypeAccess()); } to ensure that resourceSet.getClass().getClassLoader() is used. The recent constructor change causes my build to fail. ? any chance of restoring the old constructor ? ? better: any chance of using resourceSet.getClass().getClassLoader() for non-XtextResourceSets avoiding the need for an overload in the first place ? [Since XtextResourceSet.getClasspathURIContext() provides an important functionality change, shouldn't it be available to everyone via a ResourceSet adapter rather than a non-standard ResourceSet?]