Bugzilla – Full Text Bug Listing
| Summary: | [QVTo] Be able to add blackbox java libraries in standalone environment | ||
|---|---|---|---|
| Product: | [Modeling] QVTo | Reporter: | Simon harrer <simon.harrer> |
| Component: | Engine | Assignee: | Christopher Gerking <christopher.gerking> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | christopher.gerking, denis.nikif, ed, marius.groeger, michael.heerklotz, rolf.theunissen, serg.boyko2011 |
| Version: | unspecified | Flags: | serg.boyko2011:
indigo+
|
| Target Milestone: | 3.4.0 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | Extensibility | ||
| Bug Depends on: | |||
| Bug Blocks: | 453909 | ||
| Attachments: | |||
|
Description
Simon harrer
*** Bug 388293 has been marked as a duplicate of this bug. *** From Bug 388293: See forum post http://www.eclipse.org/forums/index.php/m/922941/ for a developer that made a possible patch. I'm not sure whether the proposed fix and this bug report actually address the same issue. On the one hand, the fix by Thomas Legrand seems to enable blackbox libraries in a JVM outside Eclipse. Is there a point in running Eclipse QVTo outside Eclipse at all? On the other hand, this report focuses on blackbox libraries that are located as Java projects in the Eclipse workspace, which is the bigger challenge because the code is not part of the running VM. The latter issue results in blackbox libraries that raise compile errors in the workspace, but work perfectly when the transformation runs as a plugin. To address this issue, would it be worth an integration with JDT to obtain the class files that make up the blackbox library? (In reply to comment #3) > Is there a point in running Eclipse QVTo outside Eclipse at all? Yes. Running a transformation from an Ant/MWE script is halfway to ooutside EClipse. > The latter issue results in blackbox libraries that raise compile errors in > the workspace, but work perfectly when the transformation runs as a plugin. What errors. If it is just the usual failure to make platform: relative URIs work, then the usual solutions can be used. (In reply to comment #4) > What errors. If it is just the usual failure to make platform: relative URIs > work, then the usual solutions can be used. The problem arises if a QVTo transformation in the workspace tries to resolve a Java blackbox library as part of a plugin project in the same workspace. If there is a usual solution please let me know. (In reply to comment #5) Come on. What problem? Does your machine catch fire? (In reply to comment #3) > Is there a point in running Eclipse QVTo outside Eclipse at all? We are currently running QVTo transformations standalone (outside Eclipse), in an embedded industrial application. To be able to add blackbox transformations could improve the performance of some transformations drastically. (In reply to comment #6) > (In reply to comment #5) > Come on. What problem? Does your machine catch fire? I gave this report from a user perspective, where 'false positives' are actually problematic. It's very painful to see error messages telling that something is wrong, whereas everything will actually work just fine once you try it. However, bug 289982 seems to provide the proper solution. (In reply to comment #8) > (In reply to comment #6) > > (In reply to comment #5) > > Come on. What problem? Does your machine catch fire? > > I gave this report from a user perspective, You miss my point. There is no repro. There is no cut and paste of some error messages. There is no stack trace. The problem may be genuine but the problem description is not available to the developers. An API to add Blackbox Java Libraries in Standalone mode would be really great. For now I solved this problem with calling: JavaBlackboxProvider jbp = new JavaBlackboxProvider(); JavaBlackboxProvider.Descriptor descriptor = jbp.createDescriptor(unit); jbp.fDescriptorMap.put(id, descriptor); BlackboxRegistry.INSTANCE.fProviders.add(jbp); through reflection, but an API would of course be way better (And it could be added in less than 15min very easily). Support for Blackbox Java Libraries in Eclipse plugin projects sounds interesting, but is also way more complicated. Hi Michael I try to use your code, but I don't understand how to create unit (instance of IConfigurationElement) in a standalone application. Platform.getExtensionRegistry() returns null (In reply to Michael Heerklotz from comment #10) > JavaBlackboxProvider jbp = new JavaBlackboxProvider(); > JavaBlackboxProvider.Descriptor descriptor = jbp.createDescriptor(unit); > jbp.fDescriptorMap.put(id, descriptor); > BlackboxRegistry.INSTANCE.fProviders.add(jbp); Hi Michael I tried to follow your approach: http://www.eclipse.org/forums/index.php/t/628356/ But blackbox still doesn't work ;( (In reply to Michael Heerklotz from comment #10) > An API to add Blackbox Java Libraries in Standalone mode would be really > great. > > For now I solved this problem with calling: > > JavaBlackboxProvider jbp = new JavaBlackboxProvider(); > JavaBlackboxProvider.Descriptor descriptor = jbp.createDescriptor(unit); > jbp.fDescriptorMap.put(id, descriptor); > BlackboxRegistry.INSTANCE.fProviders.add(jbp); Created attachment 243184 [details] org.eclipse.qvto.patch The patch should enable blackbox libraries in standalone mode. It is now possible to import a Java class by means of it fully qualified name, and call its methods directly from QVTo. To make this work, there is a new Annotation type called 'Module' which enables annotating a Java class with the set of nsURIs that it requires to resolve types: @Module(packageURIs={"http://www.eclipse.org/emf/2002/Ecore"}) public class Test { ... } Please note that this does not work in concrete syntax blackbox mode without importing the required class. Unfortunately, that's precisely what the QVT specification suggests. It would require searching all available Java classes for a specific method signature, which is impossible. In particular, that's also why StandaloneBlackboxProvider.getModuleDescriptors() can only provide an empty collection. According to my patch, the new functionality is also available in non-standalone mode. If this is not desired, we just have to remove the respective extension from plugin.xml file. (In reply to Michael Heerklotz from comment #10) > An API to add Blackbox Java Libraries in Standalone mode would be really > great. I think there is no need for an API, because adding blackboxes manually should not be required at all. My patch should enable offhand importing of specific Java classes on the classpath. Another slight extension to my approach would be to let users announce every existing blackbox class manually by creating an AbstractCompilationUnitDescriptor instance and registering it with a StandaloneBlackboxProvider singleton. This way, we could even implement StandaloneBlackboxProvider.getModuleDescriptors() by iterating over all registered descriptors. However, this would require manual coding effort for standalone users. (In reply to Christopher Gerking from comment #13) > Created attachment 243184 [details] > org.eclipse.qvto.patch > > The patch should enable blackbox libraries in standalone mode. It is now > possible to import a Java class by means of it fully qualified name, and > call its methods directly from QVTo. To make this work, there is a new > Annotation type called 'Module' which enables annotating a Java class with > the set of nsURIs that it requires to resolve types: > > @Module(packageURIs={"http://www.eclipse.org/emf/2002/Ecore"}) > public class Test { > ... > } > > Please note that this does not work in concrete syntax blackbox mode without > importing the required class. Unfortunately, that's precisely what the QVT > specification suggests. It would require searching all available Java > classes for a specific method signature, which is impossible. In particular, > that's also why StandaloneBlackboxProvider.getModuleDescriptors() can only > provide an empty collection. Such restriction looks reasonable. > > According to my patch, the new functionality is also available in > non-standalone mode. If this is not desired, we just have to remove the > respective extension from plugin.xml file. I think that for testing purpose it's sensible to leave all providers (bundle and standalone) registered. Pushed to master for RC1. commit 04469e67b366e2dafd4714c6181c3f6ab5d7c5de (In reply to Sergey Boyko from comment #16) > (In reply to Christopher Gerking from comment #13) > > Please note that this does not work in concrete syntax blackbox mode without > > importing the required class. Unfortunately, that's precisely what the QVT > > specification suggests. It would require searching all available Java > > classes for a specific method signature, which is impossible. In particular, > > that's also why StandaloneBlackboxProvider.getModuleDescriptors() can only > > provide an empty collection. > > Such restriction looks reasonable. So you don't like the idea of a "standalone setup"? This would require users to register a StandaloneDescriptor manually before invoking QVTo. I could imagine an API similar to the following: StandaloneBlackboxProvider.INSTANCE.registerDescriptor(new StandaloneDescriptor(MyBlackbox.class)); This would remediate the above restriction, but implies some user effort. (In reply to Christopher Gerking from comment #17) > > Such restriction looks reasonable. ... > This would remediate the above restriction, but implies some user effort. Please raise an OMG issue to cover the deficiencies in the specification and suggested remedies. Remember that remedies must not be Eclipse specific; Eclipse QVTo is potentially just one implementation of a shared specification. When considering how the specification should evolve to resolve this, and thinking about perhaps a C++ based QVTo tool, it may become clearer whether the lookup help should be within the QVTo language or in the surrounding implementatiioon setup. (In reply to Christopher Gerking from comment #17) > (In reply to Sergey Boyko from comment #16) > > (In reply to Christopher Gerking from comment #13) > > > Please note that this does not work in concrete syntax blackbox mode without > > > importing the required class. Unfortunately, that's precisely what the QVT > > > specification suggests. It would require searching all available Java > > > classes for a specific method signature, which is impossible. In particular, > > > that's also why StandaloneBlackboxProvider.getModuleDescriptors() can only > > > provide an empty collection. > > > > Such restriction looks reasonable. > > So you don't like the idea of a "standalone setup"? This would require users > to register a StandaloneDescriptor manually before invoking QVTo. I could > imagine an API similar to the following: > > StandaloneBlackboxProvider.INSTANCE.registerDescriptor(new > StandaloneDescriptor(MyBlackbox.class)); > > This would remediate the above restriction, but implies some user effort. I meant that current implementation (which requires to explicitly import Java class of blackbox module) does not impose a big restriction. Bbox library may simultaneously be registered via extenstion point (org.eclipse.m2m.qvt.oml.javaBlackboxUnits) thus providing sensible list of module descriptors. And also has @Module annotation thus be available in standalone mode. However I also think that some kind of "standalone setup" is helpful. Actually it will substitute the registration via extension point. Created attachment 243886 [details]
org.eclipse.m2m.qvt.oml patch
Created attachment 243887 [details]
org.eclipse.m2m.qvt.oml.emf.util patch
The two new patches comprise some more slight optimizations. Created attachment 243931 [details]
org.eclipse.m2m.qvt.oml.emf.util patch
Created attachment 244957 [details]
org.eclipse.m2m.qvt.oml.emf.util patch
Created attachment 244962 [details]
org.eclipse.m2m.qvt.oml patch
Created attachment 244963 [details]
org.eclipse.m2m.qvt.oml.common patch
Created attachment 244964 [details]
org.eclipse.m2m.qvt.oml.emf.util patch
Created attachment 244966 [details]
org.eclipse.m2m.qvt.oml patch
Created attachment 244967 [details]
org.eclipse.m2m.qvt.oml.emf.util patch
I think we have another problem here, because the conversion between MetamodelRegistry and EPackage.Registry does not cover the delegation mechanism inside EPackage.Registry. See the Java snippet below:
EPackage myEPackage = EcoreFactory.eINSTANCE.createEPackage();
EPackage.Registry delegateRegistry = new EPackageRegistryImpl();
delegateRegistry.put("xyz", myEPackage);
EPackage.Registry packageRegistry = new EPackageRegistryImpl(delegateRegistry);
IMetamodelProvider provider = new EmfStandaloneMetamodelProvider(packageRegistry);
MetamodelRegistry metaRegistry = new MetamodelRegistry(provider);
EPackage result = metaRegistry.toEPackageRegistry().getEPackage("xyz");
assert (result == myEPackage);
(In reply to Christopher Gerking from comment #30) > I think we have another problem here, because the conversion between > MetamodelRegistry and EPackage.Registry does not cover the delegation > mechanism inside EPackage.Registry. Forked as bug 441094. Why was there no documentation update? Attempting to open /org.eclipse.m2m.tests.qvt.oml/parserTestData/models/bug326871/bug326871.qvto with the QVTo editor gives many errors. Whereas opening /org.eclipse.m2m.tests.qvt.oml/parserTestData/models/bug325192/bug325192.qvto is fine. Consequently attempting to Run As... for bug326871.qvto also fails. (In reply to Ed Willink from comment #33) > Attempting to open > > /org.eclipse.m2m.tests.qvt.oml/parserTestData/models/bug326871/bug326871.qvto > > with the QVTo editor gives many errors. Whereas opening > > /org.eclipse.m2m.tests.qvt.oml/parserTestData/models/bug325192/bug325192.qvto > > is fine. > > Consequently attempting to Run As... for bug326871.qvto also fails. Verified 'bug326871.qvto' in Eclipse instance launched from the local workspace - no problems were found. Also this script is included in JUnit test suite and it is passed locally and on Hudson. Note: mentioned workspace contains all QVTo plug-ins from http://git.eclipse.org/c/mmt/org.eclipse.qvto.git/plain/releng/org.eclipse.qvto.releng/psfs/qvto-all.psf (In reply to Sergey Boyko from comment #34) > Verified 'bug326871.qvto' in Eclipse instance launched from the local > workspace - no problems were found. I expect it to work in the local workspace. a) the editor can see the plugin.xml from the IProject. b) a launch provokes the Java compile so a locallaunch can use the local blackbox. No need for a nested Eclipse. (In reply to Ed Willink from comment #35) > (In reply to Sergey Boyko from comment #34) > > Verified 'bug326871.qvto' in Eclipse instance launched from the local > > workspace - no problems were found. > > I expect it to work in the local workspace. > > a) the editor can see the plugin.xml from the IProject. > > b) a launch provokes the Java compile so a locallaunch can use the local > blackbox. > > No need for a nested Eclipse. Didn't quite get your idea. QVTo needs the compiled class (.class file ) provided for BB implementation. In local workspace you have only .java file. (In reply to Sergey Boyko from comment #36) > Didn't quite get your idea. > QVTo needs the compiled class (.class file ) provided for BB implementation. > In local workspace you have only .java file. The editor does not need the *.class file. It is sufficient to analyze the *.java declarations. The ASM library is very helpful for this. A build can be performed automatically as part of the launch so that the class file is available to the executing transformation. cf you can create and run HelloWorld.java without a nested Eclipse. IMHO, anything that requires the user to start a nested Eclipse is very undesirable. For auto-generated editors it is perhaps necessary, but for interpreters and general modeling it should not be required. (In reply to Christopher Gerking from comment #3) > The latter issue results in blackbox libraries that raise compile errors in > the workspace, but work perfectly when the transformation runs as a plugin. > > To address this issue, would it be worth an integration with JDT to obtain > the class files that make up the blackbox library? You are referring exactly to the problem that I described above. My initial idea was hooking JDT to obtain and analyze the bytecode. Ed, try to execute the transformation in Debug mode using a 'QVTO Application Launch Delegate' configuration instead of an 'Operational QVT Interpreter'. No idea what happens internally but that seems to work. I don't know why this option is only available for Debug not for Run. Sergey already described some more features that would be helpful to support blackboxes in the local workspace: (In reply to Sergey Boyko from bug 289982, comment 4) > Further development might include linking of QVTo unit with Java source for > blackboxes mapping at development time: > - hyperlink navigation to Java sources > - quick fixes for creating appropriate stub in Java file > - Java-debugger interoperability By the way: the workaround for the missing blackbox support at development time is often to import the deployed transformation plugin as runtime workspace project. This results in metamodel mappings that provide illegal overrides, as discussed in bug 444453. (In reply to Christopher Gerking from comment #38) > Ed, try to execute the transformation in Debug mode using a 'QVTO > Application Launch Delegate' configuration instead of an 'Operational QVT > Interpreter'. This doesn't help. Since the editor reports errors on /org.eclipse.m2m.tests.qvt.oml/parserTestData/models/bug326871/bug326871.qvto the launch configuration also reports errors (to the Error Log too) and so no launch can be created. As it stands I cannot find the guidance on using a new-style blackbox standalone, so I'll resort to a Java post processing transformation, which I have to have anyway since ensuring that Ecore references are to Ecore.ecore for EClasses but http://... for EDataTypes as required by Xtext is too hard for a clean modeling tool to do anyway. (In reply to Ed Willink from comment #40) > (In reply to Christopher Gerking from comment #38) > > Ed, try to execute the transformation in Debug mode using a 'QVTO > > Application Launch Delegate' configuration instead of an 'Operational QVT > > Interpreter'. > > This doesn't help. > > Since the editor reports errors on > > /org.eclipse.m2m.tests.qvt.oml/parserTestData/models/bug326871/bug326871.qvto > > the launch configuration also reports errors (to the Error Log too) and so > no launch can be created. > This is handled by means of pure blackbox declarations. Please look at http://git.eclipse.org/c/mmt/org.eclipse.qvto.git/tree/tests/org.eclipse.m2m.tests.qvt.oml/parserTestData/models/bug326871a_standalone There library 'bug326871_lib.qvto' contains blackbox declarations (note that there are no 'import' statements). In main script 'bug326871a_standalone.qvto' this library is imported and so it's possible to reference those methods. This is standard way to solve problems with missing blackbox methods in workspace. And 'QVTO Application Launch Delegate' allows to connect and debug actual Java implementation of BB (by means of launching Eclipse application behind the scene). (In reply to Ed Willink from comment #40) > This doesn't help. > > Since the editor reports errors on > > /org.eclipse.m2m.tests.qvt.oml/parserTestData/models/bug326871/bug326871.qvto > > the launch configuration also reports errors (to the Error Log too) and so > no launch can be created. Works for me straight away on the very same transformation. The errors shown by the editor seem to be ignored by the QVTO Launch Application Delegate. Created attachment 249128 [details]
Screenshot of unsuccessful launcgh
Using M3 for nearly everything the attached screenshot shows my unsuccessfgul attempt to launch. The errors do seem to matter.
(In reply to Sergey Boyko from comment #41) > There library 'bug326871_lib.qvto' contains blackbox declarations (note that > there are no 'import' statements). > In main script 'bug326871a_standalone.qvto' this library is imported and so > it's possible to reference those methods. Thanks. These are really useful examples. Publicize them. Plus the following missing information. You make the blackboxes available at runtime using e.g. TransformationExecutorBlackboxRegistry.INSTANCE.registerModules(BlackBoxLibrary.class); (This is unfortunately internal API. Since TransformationExecutorBlackboxRegistry wraps BlackboxRegistry, perhaps TransformationExecutorBlackboxRegistry can be promoted to non-internal.) (It will be even easier once the intermediate library.qvto can be eliminated in favour of Java analysis of a Java class.) (In reply to Ed Willink from comment #43) > Using M3 for nearly everything the attached screenshot shows my > unsuccessfgul attempt to launch. The errors do seem to matter. Confirmed. Seems as if the Application Launch Delegate just reuses the default UI which has an instant validation. Instead, the validation should take place later in the context of the nested Eclipse. (In reply to Christopher Gerking from comment #38) > try to execute the transformation in Debug mode using a 'QVTO > Application Launch Delegate' configuration instead of an 'Operational QVT > Interpreter'. No idea what happens internally but that seems to work. I > don't know why this option is only available for Debug not for Run. Further discussed as bug 459750. (In reply to Ed Willink from comment #44) > You make the blackboxes available at runtime using e.g. > > TransformationExecutorBlackboxRegistry.INSTANCE. > registerModules(BlackBoxLibrary.class); > > (This is unfortunately internal API. Using TransformationExecutor.BlackboxRegistry.INSTANCE avoids the API problem. Sergey, I can't really understand your measures to allow registration of Java classes. To me, it would be enough to declare a StandaloneMetamodelProvider singleton INSTANCE, and then add the following static method to TransformationExecutor:
public static void registerBlackboxModules(Class<?>... classes) {
for(Class<?> c : classes) {
StandaloneBlackboxProvider.INSTANCE.registerDescriptor(c);
}
}
Normally, it would be preferrable to allow one BlackboxRegistry per TransformationExecutor (similar to EPackage.Registry) in order to avoid interference between multiple QVTo transformations running at the same time.
(In reply to Christopher Gerking from comment #48) > Sergey, I can't really understand your measures to allow registration of > Java classes. To me, it would be enough to declare a > StandaloneMetamodelProvider singleton INSTANCE, and then add the following > static method to TransformationExecutor: > > public static void registerBlackboxModules(Class<?>... classes) { > for(Class<?> c : classes) { > StandaloneBlackboxProvider.INSTANCE.registerDescriptor(c); > } > } My intent was to encapsulate BlackBox-related functionality since I expected that the list of such functions is not limited to a single registerDescriptor(). I didn't want to introduce yet another singleton (in StandaloneMetamodelProvider) and tried to reuse the already existing BlackboxRegistry.INSTANCE. > > Normally, it would be preferrable to allow one BlackboxRegistry per > TransformationExecutor (similar to EPackage.Registry) in order to avoid > interference between multiple QVTo transformations running at the same time. Agreed. That is most convenient way. Thus registerBlackboxModules() should be moved to org.eclipse.m2m.qvt.oml.ExecutionContext class. (In reply to Sergey Boyko from comment #49) > My intent was to encapsulate BlackBox-related functionality since I expected > that the list of such functions is not limited to a single > registerDescriptor(). > I didn't want to introduce yet another singleton (in > StandaloneMetamodelProvider) and tried to reuse the already existing > BlackboxRegistry.INSTANCE. You are right. At the latest when standalone users become able to create their own custom instances of TransformationExecutor.BlackboxRegistry, the singleton (internal) BlackboxRegistry will not be sufficient anymore as well. Therefore, declaring StandaloneMetamodelProvider as singleton is also counterproductive in the long run. I prepared the internal BlackboxRegistry for non-singleton usage. Based on that, custom blackbox registries could be enabled for Mars+1 ? Commit ID: 1eaa7da7fd6be5edc44235258991eb34ca0d1db1 > Thus registerBlackboxModules() should be moved to > org.eclipse.m2m.qvt.oml.ExecutionContext class. Moving to ExecutionContext implies that blackboxes are never accessible at compile time. When using concrete syntax blackbox libraries, that's not strictly required (but still produces a warning). However, the "old" way of providing blackboxes is to generate libraries from Java classes at compile time, so it requires to inject Java classes already at compile time. (In reply to Christopher Gerking from comment #50) > > I prepared the internal BlackboxRegistry for non-singleton usage. Based on > that, custom blackbox registries could be enabled for Mars+1 ? > I think it's of course feasible. > > > Thus registerBlackboxModules() should be moved to > > org.eclipse.m2m.qvt.oml.ExecutionContext class. > > Moving to ExecutionContext implies that blackboxes are never accessible at > compile time. When using concrete syntax blackbox libraries, that's not > strictly required (but still produces a warning). However, the "old" way of > providing blackboxes is to generate libraries from Java classes at compile > time, so it requires to inject Java classes already at compile time. Agreed, my assumption was wrong. BB libraries should be available during compilation also. (In reply to Sergey Boyko from comment #49) > I expected that the list of such functions is not limited to a single > registerDescriptor(). Yes, we need more. That's because blackbox registration using the extension point is highly flexible. It allows to specify multiple modules per unit, custom names, and also the namespace URIs needed by that library. Since it is desirable to reuse every QVTo script as is in standalone mode, the standalone API needs to be flexible as well. I propose something similar to: Diagnostic registerModule(String unitQualifiedName, String moduleName, Class<?> cls, String[] packageURIs); In particular, this allows to register multiple modules under the same unit. The existing registerModules(Class<?>... classes) becomes a facade only, registering each class as a one-module unit, and deriving the registered names from the Java classes. It is arguable whether the URIs are actually required here, as they can also be annotated to the Java class itself. However, there might be situations where third-party classes must be registered, and adding that annotation is impossible. I prepared a patch and a new standalone test suite that runs all existing test cases. Still requires some cleanup to allow specific blackbox registration per test case (currently all blackboxes are registered also if the test case doesn't require them). The only remaining problems are with legacy blackbox libraries, for which a method without an @Operation annotation is still interpreted as a contextual operation. For example, there is a public List<String> split(String self, String regex) method in class StringLibrary, for which there is no such context annotation, but is nevertheless used as a contextual operation in some test scripts. The problem here is that the current blackbox loading mechanism only creates contextual operations if the annotation indicates that there is a context. Therefore, contextual operation calls cannot be resolved. (In reply to Christopher Gerking from comment #52) > (In reply to Sergey Boyko from comment #49) > > I expected that the list of such functions is not limited to a single > > registerDescriptor(). > > Yes, we need more. That's because blackbox registration using the extension > point is highly flexible. It allows to specify multiple modules per unit, > custom names, and also the namespace URIs needed by that library. > > Since it is desirable to reuse every QVTo script as is in standalone mode, > the standalone API needs to be flexible as well. I propose something similar > to: > > Diagnostic registerModule(String unitQualifiedName, String moduleName, > Class<?> cls, String[] packageURIs); > > In particular, this allows to register multiple modules under the same unit. > The existing registerModules(Class<?>... classes) becomes a facade only, > registering each class as a one-module unit, and deriving the registered > names from the Java classes. It is arguable whether the URIs are actually > required here, as they can also be annotated to the Java class itself. > However, there might be situations where third-party classes must be > registered, and adding that annotation is impossible. > > I prepared a patch and a new standalone test suite that runs all existing > test cases. Pushed to cgerking/standalone. Commit ID: cd578bf9a9bc54e6f97d068f39bf6faba8c1161e There is now a StandaloneTests.launch file which contains the appropriate configuration to run the test suite in standalone mode. In particular, it adds the parserTestData folder to the classpath in order to allow ClassPathUnitResolver to handle imports (cf. bug 433937). Can these tests be added to the continuous integration? > The only remaining problems are with legacy blackbox libraries, for which a > method without an @Operation annotation is still interpreted as a contextual > operation. There are six test cases failing due to this problem. (In reply to Christopher Gerking from comment #53) > (In reply to Christopher Gerking from comment #52) > > > > Yes, we need more. That's because blackbox registration using the extension > > point is highly flexible. It allows to specify multiple modules per unit, > > custom names, and also the namespace URIs needed by that library. > > > > Since it is desirable to reuse every QVTo script as is in standalone mode, > > the standalone API needs to be flexible as well. I propose something similar > > to: > > > > Diagnostic registerModule(String unitQualifiedName, String moduleName, > > Class<?> cls, String[] packageURIs); > > > > In particular, this allows to register multiple modules under the same unit. > > The existing registerModules(Class<?>... classes) becomes a facade only, > > registering each class as a one-module unit, and deriving the registered > > names from the Java classes. It is arguable whether the URIs are actually > > required here, as they can also be annotated to the Java class itself. > > However, there might be situations where third-party classes must be > > registered, and adding that annotation is impossible. > > > > I prepared a patch and a new standalone test suite that runs all existing > > test cases. Excellent patch! Now we have the whole set of registerModule(..) functions in API. > > Pushed to cgerking/standalone. > > Commit ID: cd578bf9a9bc54e6f97d068f39bf6faba8c1161e Pushed to master for M6. Commit id: 5c3e1366d96f7b3a391c940b27557365edd2c487 > There is now a StandaloneTests.launch file which contains the appropriate > configuration to run the test suite in standalone mode. In particular, it > adds the parserTestData folder to the classpath in order to allow > ClassPathUnitResolver to handle imports (cf. bug 433937). Can these tests be > added to the continuous integration? I think it should be there. I'll look at this after M6. > > > The only remaining problems are with legacy blackbox libraries, for which a > > method without an @Operation annotation is still interpreted as a contextual > operation. > > There are six test cases failing due to this problem. Yes, legacy libraries are handled by means of nested 'static Metaclass' class. I think it's reasonable to filter them out of standalone test suit just like some tests are filtered from javaless execution. (In reply to Sergey Boyko from comment #54) > Yes, legacy libraries are handled by means of nested 'static Metaclass' > class. Why don't we deprecate the org.eclipse.m2m.qvt.oml.ocl.libraries extension point, and extend the Metaclass declarations by means of equivalent @Operation annotations? I tried that, and now the only remaining problem is that continue_perf acts as a context for the contextless currentTimeMillis() operation in EmfToolsLibrary. However, that seems to be a bug in the test scripts, because using an extended module as context for one of its contextless operations is illegal. So would you agree on changing the affected test scripts from this.currentTimeMillis() to currentTimeMillis() ? Commit ID: 22af71458c7dc2a372d3b7ab7c350ac5e5d95f20 Please cherry-pick to master. > I think it's reasonable to filter them out of standalone test suit just like > some tests are filtered from javaless execution. With the above changes there is no need to do so. I noticed that the extension point for blackboxes treats both unit name and module name as sort of optional (that's because specifying a unit is optional itself). Hence, a single registerModule(Class, String) method is inappropriate, as it is unclear whether its default handling applies to the unit name or the module name. Of course we could assist by means of explicit documentation. But since it is also unclear which one is the preferred case, I suggest to remove the above method and add a default null handling for both cases. The most convenient registerModule(Class) is still there. Commit ID: dd8f8fc746cc813a47ae97666d5b44b7ed9361d1 (In reply to Christopher Gerking from comment #55) > (In reply to Sergey Boyko from comment #54) > > Yes, legacy libraries are handled by means of nested 'static Metaclass' > > class. > > Why don't we deprecate the org.eclipse.m2m.qvt.oml.ocl.libraries extension > point, and extend the Metaclass declarations by means of equivalent > @Operation annotations? Extension point 'org.eclipse.m2m.qvt.oml.ocl.libraries' is of course depricated. Deprecation message is the following: "The extension point 'org.eclipse.m2m.qvt.oml.ocl.libraries' is deprecated, it has been replaced by 'org.eclipse.m2m.qvt.oml.javaBlackboxUnits'" I see that you removed registration of 'LegacyNativeLibraryProviderAdapter'. I don't think it's sensible solution since we should not break existing code even those use legacy solutions. > I tried that, and now the only remaining problem is > that continue_perf acts as a context for the contextless currentTimeMillis() > operation in EmfToolsLibrary. However, that seems to be a bug in the test > scripts, because using an extended module as context for one of its > contextless operations is illegal. It's not a bug but rather correct usage. Just like in Java in QVTo one can declare: helper name() : String { return ""; } helper name2() : String { return this.name(); } > So would you agree on changing the > affected test scripts from this.currentTimeMillis() to currentTimeMillis() ? > > Commit ID: 22af71458c7dc2a372d3b7ab7c350ac5e5d95f20 > > Please cherry-pick to master. > > > > I think it's reasonable to filter them out of standalone test suit just like > > some tests are filtered from javaless execution. > > With the above changes there is no need to do so. (In reply to Sergey Boyko from comment #57) > I see that you removed registration of 'LegacyNativeLibraryProviderAdapter'. I added equivalent registrations for javaBlackboxUnits. I don't see any problems, because the old extension point is still there and clients are registerded. It is just not used by QVTo itself anymore. > It's not a bug but rather correct usage. Just like in Java in QVTo one can > declare: > > helper name() : String { > return ""; > } > helper name2() : String { > return this.name(); > } But if name() resides in an imported library, it doesn't work. Is that a bug? (In reply to Christopher Gerking from comment #58) > (In reply to Sergey Boyko from comment #57) > > I see that you removed registration of 'LegacyNativeLibraryProviderAdapter'. > > I added equivalent registrations for javaBlackboxUnits. I don't see any > problems, because the old extension point is still there and clients are > registerded. It is just not used by QVTo itself anymore. LegacyNativeLibraryProviderAdapter handles legacy extension point 'org.eclipse.m2m.qvt.oml.ocl.libraries'. Removing this adapter makes it impossible to use legacy libraries. If I understand correctly you have provided an alternative registration for legacy libraries in JUnit tests. This works for QVTo tests but I don't think it's an option for other users. > > > > It's not a bug but rather correct usage. Just like in Java in QVTo one can > > declare: > > > > helper name() : String { > > return ""; > > } > > helper name2() : String { > > return this.name(); > > } > > But if name() resides in an imported library, it doesn't work. Is that a bug? Yes, to make it works one should use qualifier: import NewTransformation1; import NewLibrary; transformation NewTransformation(); main() { this.NewLibrary::name(); this.NewTransformation1::name2(); } I'm not sure whether it's bug or not. Looks like that according to specification this is correct: "The variable this represents the transformation instance being defined. It can be used implicitly to refer to the properties of the transformation class or to the operations owned by him. " (In reply to Christopher Gerking from comment #56) > I noticed that the extension point for blackboxes treats both unit name and > module name as sort of optional (that's because specifying a unit is > optional itself). Hence, a single registerModule(Class, String) method is > inappropriate, as it is unclear whether its default handling applies to the > unit name or the module name. > > Of course we could assist by means of explicit documentation. But since it > is also unclear which one is the preferred case, I suggest to remove the > above method and add a default null handling for both cases. The most > convenient registerModule(Class) is still there. > > Commit ID: dd8f8fc746cc813a47ae97666d5b44b7ed9361d1 Pushed to master for M7. Commit id: 22d26c859e111d6108fb06e4f0d3cbd33d443076 Closing as fixed. |