Community
Participate
Working Groups
The separate ecj download jar is great. It would be interesting/useful if it also included the Ant compiler adapter (and the relevant extension contribution) so that it could be directly used in Ant / Eclipse scenarios (e.g., PDE Build). Currently AFAICT users have to use JDT core which pulls in quite a number of other things.
Created attachment 181349 [details] new JDT compiler adapter bundle project The original comment was a little unclear. ECJ already includes the Ant compiler adapter classes but it does not contribute these to Ant running in Eclipse. That is, it does not have a plugin.xml that identifies "extra classpath entries" for Ant. <plugin> <extension point="org.eclipse.ant.core.extraClasspathEntries"> <extraClasspathEntry library="jdtCompilerAdapter.jar"> </extraClasspathEntry> </extension> </plugin> Unfortunately, simply adding this markup to the ecj jar does not work because the compiler adapter classes would be loaded by the ecj bundle classloader whereas they need to be loaded by the Ant runtime classloader. The OSGi centric approach would be to have ECJ *move* the compiler adapter classes to a nested jar (e.g., jdtCompilerAdapter.jar as in jdt.core). This would likely upset non-OSGi users of ecj. Turns out that we can work around this by creating a small compiler adapter bundle that includes the adapter in a nested JAR, contributes this jar to Ant via the extension registry and imports particular packages from ecj. I'll attach the a project for this new bundle here. Note that the bundle includes the jdtCompilerAdapter.jar so there would need to be a bit o build magic to make this build. In theory we could make the jdtCompilerAdapter.jar itself be a bundle and fold this wrapper inside. A little warping but it might make build easier... While JDT could replace the jdt.core compiler adapter distribution with this new bundle that may break people consuming jdt.core and assuming that all the ant stuff will just be there. jdt.core could depend on the new thing but that would introduce a circularity that will technically not a problem might cause build issues. A less impactful approach would be to simply ship the new adapter bundle alongside ecj and frame it as "OSGi integration for ecj" Either way, this is an important inprovement as it allows ecj to be used from ant within OSGi-based systems.
>A less impactful approach would be to simply ship the new adapter bundle >alongside ecj and frame it as "OSGi integration for ecj" Olivier, what do you think about this?
Thoughts? We would like to do the PDE build product. If you'd rather not do this, let us know and we'll provide our own JDT compiler adapter.
I would like to see a change which minimize the impact on existing users and doesn't make the build more complex. How do you build the jar inside this new bundle from the source inside org.eclipse.jdt.core ?
I suspect that we'll need a touch of custom build code regardless of what form is chosen. We might be "saved" somewhat by the (hopefully true) compiler adapter not changing much. I see three choices. 1) check a compiled adapter .jar into the new compiler adapter project in CVS and build normally 2) Do some snazzy custom Ant to have the build of the compiler adpater bundle grab a built jdeCompilerAdapter.jar from jdt.core 3) setup the build of jdt.core (and jdtCompilerAdapter.jar) to produce jdtCompilerAdapter.jar that is a bundle and then have a step in the overall build that pulls it out and makes it available. In all cases the result would be an additional, compiler adapter for using ECJ in OSGi scenarios. jdt.core would continue to ship what it does today as would ECJ so users would be completely unaffected. #1 is simple but has obvious drawbacks. This is what PDE Build would do if there is no JDT solution. #2 is interesting but I'm not sure what that custom Ant code would look like #3 would actually work reasonably well but (had to be one) the compiler adapter bundle version would have to be manually updated. This is because PDE Build would just be compiling it as a JAR and the JAR would just happen to include some hand authored files that just happen to be OSGi markup. PDE build would not be constructing the bundle but rather the JAR. I have to work out one thing with the Ant classloading but otherwise this should work. What appeals to you?
If jdt.core continues to provide a jar that contains the ant adapter, how this will cohabits with the new bundle ? What happens if a user requires both bundles ? I'll think about the three options.
Cohabitation should work (have not tested). Either the jdt.core adapter or the other will be found and loaded by the Ant classloader. Either way, this is just a small wrapper that adapts to the real compiler code of which there is only one copy. User requirements don't play into this here because in neither jdt.core or this new bundle export any of the adapter code. It is all setup to go to the Ant classloader.
Created attachment 182675 [details] patch to make jdtCompilerAdapter into a bundle This patch scopes out approach #3, turning the compiler adpater JAR into a bundle. Then consumers can simply take it and use it in an OSGi context. As mentioned, the only real drawback here is that the version number has to be manually incremented when the compiler adapter code changes. Assuming that this is infrequent, it should be tractable. On the plus side, no changes to the JDT build and no changes for any users.
Created attachment 183025 [details] slightly updated patch to make jdtCompilerAdapter a bundle Andrew pointed out that the patch is missing the Bundle-ClassPath entry. Olivier, what are your thoughts on getting this in?
(In reply to comment #9) > Olivier, what are your thoughts on getting this in? I will discuss this topic next Monday at the JDT/Core call.
(In reply to comment #9) > Olivier, what are your thoughts on getting this in? A few question. 1) Why the classpath is "bogus"? Bundle-ClassPath: bogus 2) Should not we have an entry with ? Bundle-Localization: plugin Without it, I don't see how the internationalization of names (pluginName and providerName) is done. Things I don't like: ecj.jar already has a MANIFEST.MF file. The proposed change would override it. We could change it so that it provides the extension point for extra classpath entries. I also don't like to put the files inside a source folder which means that they end up inside the output folder when they are not needed in the output folder. They are only needed at build time.
(In reply to comment #11) > 1) Why the classpath is "bogus"? > Bundle-ClassPath: bogus "bogus" is used because there has to be a classpath entry but it should not point to any code. If you don't have one then OSGi defaults to "." which is the bundle itself. The code in this bundle (the compiler adapter bundle) is NOT supposed to be on the OSGi class path, rather it iwll be on the Ant classpath. > 2) Should not we have an entry with ? > Bundle-Localization: plugin > Without it, I don't see how the internationalization of names (pluginName and > providerName) is done. Sure. That was likely just an oversight on my part. > Things I don't like: > ecj.jar already has a MANIFEST.MF file. The proposed change would override it. The MANIFEST is not or the ecj.jar but rather the jdtCompilerAdapter.jar. There should be no change in ecj manifest. > We could change it so that it provides the extension point for extra classpath > entries. Unfortunately that will not work as the compiler adatper class itself is in the normal code for the bundle. See para 3 in comment 1. the adapter has to be only on the Ant classpath. > I also don't like to put the files inside a source folder which means that they > end up inside the output folder when they are not needed in the output folder. > They are only needed at build time. Understood but this manifest is "special". It is only needed at runtime and only in the case where we are running ECJ in OSGi with Eclipse Ant support. The manifest needs to be in the source / output folders as that is the only way I can see to get it in the final JAR.
(In reply to comment #12) > The MANIFEST is not or the ecj.jar but rather the jdtCompilerAdapter.jar. > There should be no change in ecj manifest. Then I need to make some adjustment. As it looks, it overrides the wrong MANIFEST.MF file. > Understood but this manifest is "special". It is only needed at runtime and > only in the case where we are running ECJ in OSGi with Eclipse Ant support. > The manifest needs to be in the source / output folders as that is the only way > I can see to get it in the final JAR. If you only care about getting it inside the jdtCompilerAdapter.jar after the build is done, the files can be located where we want since we are using custom build callbacks. I'll take a look at the way to organize the file in order to get them in the right jars.
Thanks for looking at this. Please let me know if there is anything I can do and I'd be happy to test.
(In reply to comment #14) > Thanks for looking at this. Please let me know if there is anything I can do > and I'd be happy to test. So just to be sure I understand well what you are trying to do. You want these files to end up inside the jdtCompilerAdapter.jar file ? But the jdtCompilerAdapter.jar file would still be delivered as part of the org.eclipse.jdt.core jar file ? If this is what you want, I think it is quite easy to achieve this goal.
yeah, this is the best we can think of. We'd then go and get jdtCompilerAdapter.jar and ecj.jar both of which would be valid bundles and use them in an OSGi environment. Ideally we would be able to have ecj.jar just do "the right thing" for the OSGi case but that would likely make it harder for folks to use in the non-OSGi case. If you want to go that path it would be preferred but as long as this new jdt compiler adapter bundle is available we can manage the usecase.
Created attachment 183472 [details] Proposed fix
This seems to put the right files at the right location on plugin export. Released for 3.7M4. Jeff, please verify that this works as expected with the next N-build. Reopen if this is not working as expected.
In the current nightly builds the jdtCompilerAdapter.jar in the jdt.core bundle does not have an OSGi manifest. The plugin.properties and plugin.xml are there but the MANIFEST.MF is a standard JAR manifest with no OSGi information. It looks like so where in the process the manifest from the scripts folder has been skipped or overwritten.
When I export or run the build.xml script from inside the org.eclipse.jdt.core project, I do get the right contents for the MANIFEST.MF file inside the jdtCompilerAdapter.jar. Andrew, would you have any idea what could be wrong ?
Created attachment 184492 [details] patch The only thing I can think of is that the ${jar.Location} zip already exists and is just not getting updated. The attached patch deletes the zip before recreating it. The patch also includes two other changes: 1) Jars should have the manifest as the first entry, the patch does this by listing it in a separate fileset. (Although unzip -l shows the manifest second behind the META-INF/ directory entry) 2) Versioning the compiler adapter, depending on how this jar is getting used, it is not good to always have the same version if it is going to end up in p2 repos. The patch changes the version to be the same as the main jdt jar. This is done by turning on inheritAll for the callbacks, which could have some affect on the existing post.build.jars target, this should be reviewed to make sure the the export-ecj.xml script still behaves as expected.
So this supposed that the version inside the antadapter manifest is: 1.0.0.qualifier ?
The <eclipse.versionReplace/> task replaces the manifest version without regard to the existing value so the .qualifier is not required here. You will end up with a version "3.7.0...." exactly the same as the jdt.core version. The ".qualifier" interpretation is something that normally happens inside pde.build, the result of which is the eclipse.versionReplacer task generated with the new version to use.
Thanks Andrew. I released that patch and I'll check next N-build to see if it works as expected.
We were not happy with the current solution but it was workable given the compiler adapter changes very infrequently. Updating the version number as described is a great improvement. Thanks.
(In reply to comment #25) > We were not happy with the current solution but it was workable given the > compiler adapter changes very infrequently. Updating the version number as > described is a great improvement. Thanks. Why did you reopen the bug ? Something wrong with the last patch ?
uhh, I certainly didn't mean to. Not sure how that would have happened. I'll close it.
Jeff, I20101205-2000 looks ok to me. Please verify that this is what you want. Thanks.
Verified for 3.7M4 using build I20101205-2000
Yup., the one in M4 looks good. Thanks
