-
Notifications
You must be signed in to change notification settings - Fork 136
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
Extract ECJ compiler code from jdt.core to ecj bundle and make ecj a real plug-in #181
Comments
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one should be renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.ecj) and be a proper maven library. It is actually the ecj compiler library without any dependencies, that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: org.eclipse.jdt.internal.compiler org.eclipse.jdt.internal.compiler.parser Not sure what's the best way to deal with it, may be rename those smaller packages in in jdt.core to avoid split packages issue, they are internal, so it shouldn't break many clients (but most likely will, like Xtext), but I haven't tried it and don't know if they use package protected API. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they would be fragments of org.eclipse.jdt.core.ecj, but this is problematic. This breaks tests/clients that required jdt.core before, it seems that by re-exporting the ecj host API, jdt core doesn't re-export the fragments of ecj. Not sure why both fragments were fragments before, might be we could just put them into the ecj bundle directly. So now rg.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool are just bundles. We could merge them together into ecj, I honestly don't know why they were fragments. What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. But this is just an idea how the "real ecj" could look like. See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
@stephan-herrmann , @jarthana : WDYT about #182 ? Do you have any idea why Would there be any issue if we just merge all three code bases together (ecj + org.eclipse.jdt.compiler.apt + org.eclipse.jdt.compiler.tool) in one real bundle? At the end that is what ecj.jar does? |
I can only guess / infer from what I see in our history. Both fragments implement additional capabilities connected to the compiler that were introduced around Java 6 (JSR 269 and JSR 199), right? As such the had to set BREE 1.6, while the rest of JDT/Core was still at 1.4 (!). That's why the implementation had to be kept separate. If that was the only reason, then the reason is gone in times where JDT/Core requires 11. For authoritative answers you'd have to ask Walter Harley or Olivier Thomann, I believe. I haven't heard from Walter for ages, but Olivier might still respond? |
+1 for merging the fragments into a new ECJ bundle. They make ECJ a better offering in the Java ecosystem. My hypothesis is that they are separate because they have been created long time ago while JDT was much more strict with who and what goes into jdt.core. |
The JDK 6 dependency was a sufficient reason to keep those fragments separate (see my previous comment). I just wasn't sure if that was the only reason. As of today I cannot think of any reason against merging. |
I am not sure I understand the changes in
I agree with Stephan's reasoning.
I can't think of a reason why this will be a big problem. Except that org.eclipse.jdt.core, when shipped with IDE, will contain some code that won't be useful. Or are you suggesting that we continue to make two bundles? |
Yes. One "ecj" bundle for batch compiler (and apt) and one "core" bundle for the rest of jdt, that depends on ecj. |
I'm not sure this is the same split that @jarthana had in mind, see:
This seems to refer to code from source folder |
I had that and "batch" in my mind. But just checked the jdt.core bundle and to my surprise found all those classes already in there. We really don't need them, do we? Perhaps there's some history I don't remember. |
Well, who remembers the decisions from 2001 or earlier :) Just two small observations:
|
Thanks Stephan. So, that looks like it's only the antadapter then. In that case, then my previous concern of adding redundant code to jdt.core is no longer a big deal. |
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one should be renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and be a proper maven library. It is actually the ecj compiler library without any dependencies, that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: org.eclipse.jdt.internal.compiler org.eclipse.jdt.internal.compiler.parser Not sure what's the best way to deal with it, may be rename those smaller packages in in jdt.core to avoid split packages issue, they are internal, so it shouldn't break many clients (but most likely will, like Xtext), but I haven't tried it and don't know if they use package protected API. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they would be inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one should be renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and be a proper maven library. It is actually the ecj compiler library without any dependencies, that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: org.eclipse.jdt.internal.compiler org.eclipse.jdt.internal.compiler.parser Not sure what's the best way to deal with it, may be rename those smaller packages in in jdt.core to avoid split packages issue, they are internal, so it shouldn't break many clients (but most likely will, like Xtext), but I haven't tried it and don't know if they use package protected API. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they would be inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one should be renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and be a proper maven library. It is actually the ecj compiler library without any dependencies, that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: org.eclipse.jdt.internal.compiler org.eclipse.jdt.internal.compiler.parser Not sure what's the best way to deal with it, may be rename those smaller packages in in jdt.core to avoid split packages issue, they are internal, so it shouldn't break many clients (but most likely will, like Xtext), but I haven't tried it and don't know if they use package protected API. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they would be inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one should be renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and be a proper maven library. It is actually the ecj compiler library without any dependencies, that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: org.eclipse.jdt.internal.compiler org.eclipse.jdt.internal.compiler.parser Not sure what's the best way to deal with it, may be rename those smaller packages in in jdt.core to avoid split packages issue, they are internal, so it shouldn't break many clients (but most likely will, like Xtext), but I haven't tried it and don't know if they use package protected API. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they would be inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
Why? It can be re-exported from JDT core. Do I miss something? I've updated draft patch to actually move all the code to the final place, rename bundle etc. Main point: we don't have 3 different jars now (ecj.jar / org.eclipse.jdt.compiler.apt / org.eclipse.jdt.compiler.tool), we don't need to "merge" them, we have everything inside org.eclipse.jdt.core.compiler.batch bundle. jdt.core reexports API from org.eclipse.jdt.core.compiler.batch. Note: I've used "org.eclipse.jdt.core.compiler.batch" name because that was used in MANIFEST.MF of the ecj.jar. See #182 With this code I have no compile errors in my SDK workspace (with PDE,JDT UI etc) and I can start Eclipse from Eclipse and everything still works. Here are current TODO's:
@akurtakov , @sravanlakkimsetti , @jarthana , @mpalat : I need your help:
Ideally we could get that in 4.26 M1. |
No objection. My comment was only trying to argue that clients of jdt.core must be able to use BatchCompiler (vs. Jay's worry that we're stuffing jdt.core with classes that don't belong there). This doesn't affect the question whether jdt.core directly contains that class, or pulls it in (and re-exports) from a new ecj plugin. |
Frankly, I don't know who needs that plugin name. If we keep it, then publish-to-maven-central will have to continue to translate this into the GA org.eclipse.jdt:ecj. But since 'ecj' is an exceptional artifactName anyway, that translation is probably OK / unavoidable. |
I am not sure if we still use build_ecj.xml. Of course, the ecj.1 is being built into the ecj*.jar. So, that one is needed. If I had to take a wild guess, could something be wrong with the version of the compiler.batch bundle? I see that we replace the placeholders with actual version on the fly from the "prepare-package" phase in the jdt.core's pom.xml. |
To check we're on the same page: This is about patching the version into Are you saying that part is still missing in #182? Or is your point about the confusion / duplication(?) between |
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one should be renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and be a proper maven library. It is actually the ecj compiler library without any dependencies, that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: org.eclipse.jdt.internal.compiler org.eclipse.jdt.internal.compiler.parser Not sure what's the best way to deal with it, may be rename those smaller packages in in jdt.core to avoid split packages issue, they are internal, so it shouldn't break many clients (but most likely will, like Xtext), but I haven't tried it and don't know if they use package protected API. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they would be inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
@laeubi : see my comment above where I was stuck last time. I've just rebased on master, see #182 build state where maven build will cry for sure.
The |
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one should be renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and be a proper maven library. It is actually the ecj compiler library without any dependencies, that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: org.eclipse.jdt.internal.compiler org.eclipse.jdt.internal.compiler.parser Not sure what's the best way to deal with it, may be rename those smaller packages in in jdt.core to avoid split packages issue, they are internal, so it shouldn't break many clients (but most likely will, like Xtext), but I haven't tried it and don't know if they use package protected API. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they would be inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one should be renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and be a proper maven library. It is actually the ecj compiler library without any dependencies, that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: org.eclipse.jdt.internal.compiler org.eclipse.jdt.internal.compiler.parser Not sure what's the best way to deal with it, may be rename those smaller packages in in jdt.core to avoid split packages issue, they are internal, so it shouldn't break many clients (but most likely will, like Xtext), but I haven't tried it and don't know if they use package protected API. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they would be inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one should be renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and be a proper maven library. It is actually the ecj compiler library without any dependencies, that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: org.eclipse.jdt.internal.compiler org.eclipse.jdt.internal.compiler.parser Not sure what's the best way to deal with it, may be rename those smaller packages in in jdt.core to avoid split packages issue, they are internal, so it shouldn't break many clients (but most likely will, like Xtext), but I haven't tried it and don't know if they use package protected API. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they would be inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
@iloveeclipse I tried to incorporate your changes in the wiki. Do you have a minute to review my changes? In particular, I wasn't able to explain how jdtCompilerAdapter.jar is created in maven builds. Perhaps you can fill in this detail? |
Will do. And yes, maven is a PITA for me. |
@stephan-herrmann : please check. I hope this is a bit more clear now. |
Thanks @iloveeclipse . This helps (and I still hope, I'll never have to edit pom.xml in this regard ...). Still I added one change on top as I felt confusion between creation and use of jdtCompilerAdapter. Well, this happens in true dogfooding situations, like exporting jdt.core w/ jdtCompilerAdapter using jdt.core and jdtCompilerAdapter :) |
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one is renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and is a proper maven library now. It is actually the ecj compiler library without any dependencies (except optional ant), that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: - org.eclipse.jdt.internal.compiler - org.eclipse.jdt.internal.compiler.parser So the new bundle exports them to jdt.core and jdt.core re-exports. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they are inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
- org.eclipse.jdt.compiler.apt - org.eclipse.jdt.compiler.tool See eclipse-jdt#181
…e-jdt#181) - Grammar file belongs to compiler, build scripts for it too. - Added grammar to sources. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=562044 for original contribution motivation for parser build scripts. No idea if the code still works and no idea where to get the *right* native JikesPG executable, so I can't test if that works or not. See eclipse-jdt#181
) Cleaning up org.eclipse.jdt.core and trying to sort out old unused scripts from still used but requiring an update after move. See eclipse-jdt#181
…e-jdt#181) jdt.core can't be used anymore as self containing Java compiler library, it is now org.eclipse.jdt.core.compiler.batch. See eclipse-jdt#181
…t#181) Add moved classes located now in org.eclipse.jdt.core.compiler.batch to the jdtCompilerAdapter.jar used by pde.build & Ant (see org.eclipse.ant.internal.ui.datatransfer.BuildFileCreator). See eclipse-pde/eclipse.pde#419 eclipse-jdt#181
…ipse-jdt#181) Add moved classes located now in org.eclipse.jdt.core.compiler.batch to the jdtCompilerAdapter.jar used by pde.build & Ant (see org.eclipse.ant.internal.ui.datatransfer.BuildFileCreator). See eclipse-pde/eclipse.pde#419 eclipse-jdt#181
The org.eclipse.jdt.core.ecj.validation was a "dummy" bundle so far, used only to validate compilation issues in IDE. That one is renamed (org.eclipse.jdt.core.ecj.validation -> org.eclipse.jdt.core.compiler.batch) and is a proper maven library now. It is actually the ecj compiler library without any dependencies (except optional ant), that could be consumed by JDT and the rest of the world. It must be required and re-exported by JDT core. Unfortunately, there are two split packages: - org.eclipse.jdt.internal.compiler - org.eclipse.jdt.internal.compiler.parser So the new bundle exports them to jdt.core and jdt.core re-exports. org.eclipse.jdt.compiler.apt and org.eclipse.jdt.compiler.tool were fragments of jdt.core, now they are inside org.eclipse.jdt.core.compiler.batch. TODO: 1) What I did NOT tried is to re-write all the magic scripts that build and package separated ecj library out of jdt.core. 2) The 3 antadapter classes are now split over ecj and jdt core, because BuildJarIndex.java and CheckDebugAttributes.java depend on JDT core code. 3) ecj.1 and build_ecj.xml aren't touched yet. ecj.1 was touched last time 2017, it seem to be used for man pages. 4) pom from jdt core will need an adoption. 5) org.eclipse.jdt-feature need to be updated 6) TBC See - eclipse-jdt#181 - eclipse-platform/eclipse.platform.ua#18
- org.eclipse.jdt.compiler.apt - org.eclipse.jdt.compiler.tool See eclipse-jdt#181
…e-jdt#181) - Grammar file belongs to compiler, build scripts for it too. - Added grammar to sources. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=562044 for original contribution motivation for parser build scripts. No idea if the code still works and no idea where to get the *right* native JikesPG executable, so I can't test if that works or not. See eclipse-jdt#181
) Cleaning up org.eclipse.jdt.core and trying to sort out old unused scripts from still used but requiring an update after move. See eclipse-jdt#181
…e-jdt#181) jdt.core can't be used anymore as self containing Java compiler library, it is now org.eclipse.jdt.core.compiler.batch. See eclipse-jdt#181
…t#181) Add moved classes located now in org.eclipse.jdt.core.compiler.batch to the jdtCompilerAdapter.jar used by pde.build & Ant (see org.eclipse.ant.internal.ui.datatransfer.BuildFileCreator). See eclipse-pde/eclipse.pde#419 eclipse-jdt#181
…ipse-jdt#181) Add moved classes located now in org.eclipse.jdt.core.compiler.batch to the jdtCompilerAdapter.jar used by pde.build & Ant (see org.eclipse.ant.internal.ui.datatransfer.BuildFileCreator). See eclipse-pde/eclipse.pde#419 eclipse-jdt#181
Follow up on eclipse-platform/eclipse.platform.ua#18 (comment).
Currently JDT has LOT of hacks around compiler part (dedicated source folders, dedicated build files, dedicated deployment steps, dedicated validation bundle) - all because the standalone Eclipse Compiler for Java (ECJ) code is located inside the jdt.core project that contains additionally all the IDE specific extensions.
Ideally ECJ could be just extracted from the jdt.core bundle so it can be used by jdt.core and other bundles as a regular bundle.
This would remove most of the custom hacks around building, testing and shipping ecj we have today.
I will provide a draft idea, but I have no enough time/knowledge to perform all the build/maven magic required to get that working.
Technically I believe this should work without breaking existing jdt.core clients.
The text was updated successfully, but these errors were encountered: