Community
Participate
Working Groups
Created attachment 200613 [details] proposed new API Some tools, like maven-bundle-plugin from Apache Felix project [1], assemble OSGi bundle entries from external sources dynamically as part of the build process. Inside eclipse workspace such dynamic bundle entries are usually not present as workspace resources but are linked to using JDT classpath containers or variables. I would like to propose new bundle classpath resolution API that will allow thirdparty tools like m2e [2] participate in PDE bundle classpath and source lookup calculation when launching Eclipse Applications and JUnit Plug-in Tests (see attached patch). I am happy to answer questions about this proposal either here or on pde-dev mailing list. [1] http://felix.apache.org/site/apache-felix-maven-bundle-plugin-bnd.html [2] http://eclipse.org/m2e/
This would be of benefit to the community, but it will need some serious review before making any parts of the classpath resolution API. I am concerned that there will not be committer time to provide a complete review right now and provide ongoing support in the future.
When approximately can we expect more specific answer? We are trying to orchestrate changes to several tools (pde, maven-bundle-plugin, bnd, and m2e/pde integration logic of course) to allow easier coexistence of Maven and PDE OSGi projects inside the same workspace. We specifically timed this work at the beginning of Eclipse release cycle to make it less risky for PDE developers to accept this change, but if you ultimately decide to reject this proposal we would like to know about it earlier. Is there anything we can do to make it easier for PDE developers to accept this new API? For example, would it help if we marked the new API as "provisional" or moved it to internal packages so you won't fully commit to this API until you are more comfortable with it? I would also like to point out that proposed patch is pretty small at ~430 diff lines (and actual code change is probably 30 lines, if we ignore comments and xml metadata). It does not add anything fundamentally new to PDE but rather makes existing functionality customizable via an extension point and has virtually no affect on PDE behaviour unless there are providers of the new extension. I believe overall risk of this change is manageable given that we are still pre-M1.
Please move forward with this change request, it would really help downstream consumers of tycho and m2e-tycho
I spent some additional time looking at the patch and the PDE team discussed this at our latest status call. We will support the addition of this API in 3.8. The code is well documented and should not be difficult to maintain. There may be some refactoring/renaming required.
Created attachment 202544 [details] Updated API I've gone through the patch, adding some additional comments, fixing some potential CCEs, and refactoring the names/locations. Let me know what you think. We need to test out a simple example implementation. Adding tests for this would also be important to prevent classpath changes from breaking this. If you have spare cycles any help here would be appreciated.
Created attachment 202545 [details] mylyn/context/zip
Created attachment 202577 [details] basic unit tests Your changes look reasonable. Attached find two basic unit tests, one for dev.properties generation and one for source lookup path. I could not find any helpers I could use to import existing projects, so I just wrote the needed plumbing code directly in the test class. I am not 100% sure, but the tests may have problems with filesystem locking on windows, at least in m2e we needed to synchronize test tearDown with background jobs to make tests work reliably on Windows. Feel free to refactor or throw this away. I've also update the real client [1] to use the updated API and everything appears to work. [1] https://github.com/sonatype/m2eclipse-tycho/blob/mbp-embed-dependency/org.sonatype.tycho.m2e/src/org/sonatype/tycho/m2e/internal/launching/PDEBundleClasspathResolver.java
(In reply to comment #7) > Created attachment 202577 [details] > basic unit tests > > Your changes look reasonable. > > Attached find two basic unit tests, one for dev.properties generation and one > for source lookup path. I could not find any helpers I could use to import > existing projects, so I just wrote the needed plumbing code directly in the > test class. I am not 100% sure, but the tests may have problems with filesystem > locking on windows, at least in m2e we needed to synchronize test tearDown with > background jobs to make tests work reliably on Windows. Feel free to refactor > or throw this away. > > I've also update the real client [1] to use the updated API and everything > appears to work. > > [1] > https://github.com/sonatype/m2eclipse-tycho/blob/mbp-embed-dependency/org.sonatype.tycho.m2e/src/org/sonatype/tycho/m2e/internal/launching/PDEBundleClasspathResolver.java FYI, I am having difficulties applying your patch because the path to the changed files is prefixed with a/ui and b/ui.
(In reply to comment #8) > > FYI, I am having difficulties applying your patch because the path to the > changed files is prefixed with a/ui and b/ui. The patch was produced using git-format-patch on command line. You should be able to apply the patch using either git-am or git-apply from the root of your local repository clone. For example, I just tried the following on my macbook and it worked without problems curl https://bugs.eclipse.org/bugs/attachment.cgi?id=202577 | git apply Let me know if you want me to recreate the patch using some other tools and I'll reattach.
I have the patch applying, however it appears to be missing cpe/some.properties and classpathresolver.launch. What information was in those files?
cpe/some.properties is an empty file needed to keep git from deleting 'cpe' directory. git-apply does create this file for me, please create it manually if this does not work for you. classpathresolver.launch is a leftover from some experiments I did, please remove corresponding copyFile(...) statement. sorry for the confusion.
Created attachment 203009 [details] Complete patch Had to wrangle git/eGit into doing what I needed, but this git patch should contain all of the changes.
Pushed the changes to master. I will tag for the M2 warmup builds. Thanks for your quick responses Igor. We'll need a new and noteworthy entry. I will write it early next week unless Igor wants to take a shot at it.
There's no doc for it. Filed bug 378647.
