| Summary: | Add an p2 RCP UI | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Marco Maccaferri <macca> | ||||
| Component: | p2 | Assignee: | P2 Inbox <equinox.p2-inbox> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | anthony.dahanne, antonel.pazargic, benjamin.dev, bsd, dj.houghton, irbull, jeffmcaffer, kim.moir, Lars.Vogel, matthew, mllong, pascal, pwebster, spektom, tjwatson, torsten | ||||
| Version: | unspecified | ||||||
| Target Milestone: | 3.7 M7 | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | 340063 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
Still broken with 3.6.2RC1. The short answer is that the p2.user.ui feature is not intended to be used in RCP scenarios. It explicitly includes bundles like p2.ui.sdk that are intended to be used in Eclipse IDE scenarios. The features provided by the p2 team will capture only a certain number of usecases and consuming teams may need to create their own features to suit their precise needs. The longer answer relates to compare and why it would be sucking in the world but that is for a different component. (In reply to comment #2) > The short answer is that the p2.user.ui feature is not intended to be used in > RCP scenarios. It explicitly includes bundles like p2.ui.sdk that are intended > to be used in Eclipse IDE scenarios. Sorry, but this page: http://wiki.eclipse.org/Equinox/p2/Adding_Self-Update_to_an_RCP_Application Seems to say exactly the opposite: "If your goal is to simply use the same update UI used in the SDK inside your RCP app, very few modifications are required ... You'll need to include the org.eclipse.equinox.p2.user.ui feature in your application. This will add the following UI bundles to your application (in addition to all of the p2 core and other required bundles)" Not to mention that the sample RCP Mail application is including the same p2.ui.sdk bundles included from the feature (so I'm expecting it to fail in the same manner if it wasn't for the ide bundle). Just for the sake of discussion, I tried to build my own feature to circumvent this problem but I wasn't able to make it work (the UI simply disappear without errors, it is very frustrating). So as an alternative, it would be good to provide instructions about what it is the intended way to include the p2 update ui in an RCP application. Not to pile on, Jeff, but I have to agree with Marco. The p2.ui.sdk has been explicitly advertised as designed for and usable by RCP applications for a number of years, including by your own Ian Bull as part of the "Provisioning RCP Applications with p2" webinar (14m50s). It also states it explicitly in the developer's guide (topic: /org.eclipse.platform.doc.isv/guide/p2_uireuse.htm). As near as I can make out, this dependency chain was first created in Helios and is being caused by a single class: "RevertProfilePageWithCompare". It should be easy enough to refactor this out into another bundle or rework this in such a way that this one class does not drag the entire IDE UI along with it. (In reply to comment #4) I am in complete agreement that the additional dependencies are goofy and should be eliminated. It is more than unfortunate that the addition of the revert button would drag in Resources, the IDE UI, ... RCP apps would likely want to have revert as well. In the end this lack of factoring can also be seen in the Compare bundle which has but one class, CompareWithOtherResourceDialog, that depends on Resources and ui.ide. In short, this problem would be averted and Compare made more useful if the org.eclipse.compare dependency on Resource and UI IDE was optional. I have opened Bug 338345 to capture this direction. Note that AFAICT, the p2 functionality does not actually depend on Resources and IDE UI etc so the optional fix should be enough. On the feature side I think we still differ. Despite the name, the p2.user.ui feature is intended for IDE scenarios. It has been advertised as being usable for RCP apps because it is the only thing we have! Its way easier to say "start with this feature" than "get these 45 bundles". The feature makes available preference pages, capabilities/activities, ... that we really have no evidence would/should be needed in widespread RCP applications. It includes support for update sites, dropins folders, repository tools, the director app and legacy metadata generation that would seldom if ever be needed in RCP scenarios. The real problem here is that we do not supply a base RCP p2 feature that makes it easy to add p2 in RCP scenarios. For Indigo we put in place a p2.core feature for people wanting to add p2 to headless apps (or wanting to do all the UI themselves). With a fix to the above goofy dependency problem we could put together a p2.rcp.ui feature that would have some amount of function by default. Going slightly meta, features have always been just one grouping of function. The referenced help/wiki/tutorial information talk about adding or removing bundles. What we as the producer need to do is provide a small number of easy to consume groupings that makes the 70-80% cases simple. We have failed on that front for RCP apps. See also http://dev.eclipse.org/mhonarc/lists/p2-dev/msg03225.html http://www.toedter.com/blog/?p=358 I did some more investigation and found that the compare dependencies on Resources in particular are much deeper than I thought. Bummer. So the only way to make p2 work in RCP environments right now is to remove the revert button as noted in the quoted posts (note though you have to remove the Installation History extension as well). For Indigo we should make a revert/history function that does not rely on the compare function as it is unlikely that Compare will be refactored any time soon. I opened Bug 338351 to capture that issue. With or without that bug addressed, to make a p2 for RCP that is at all directly useful we should create an analog to p2.ui.sdk *bundle* (say o.e.e.p2.ui.rcp) that be more suited to RCP scenarios. - remove the capabilities - remove/replace the Installation History page (revert) as discussed above - remove any references to legacy update function Other changes? Volunteers to make this new bundle? Then there should be a new feature, o.e.e.p2.rcp.ui (or some such), that points to the new bundle and trimmed down version of what is in the user.ui feature. In both the feature and the bundle case we should make it clear that these are starting points. There is no way we can capture all scenarios. p2 is a provisioning platform, not a solution for all situations. We want to lower the barrier for common scenarios. These two things should help considerably. Jeff -- Thank you for looking into this. I think we are on the same page and I fully support your approach. From the sounds of things, it seems like this bug, Bug 338351, and Bug 281226 may be merging. I created and committed org.eclipse.equinox.p2.rcp.feature and org.eclipse.equinox.p2.ui.rcp as starting points for this work. The feature looks pretty good (though there is a pending change to the p2.core.feature to bring in operations). The bundle is a straight copy of the p2.ui.sdk bundle with the manifest cleaned up and the revert button/Installation History page removed. I've also updated the releng project sets and the title of this bug to better reflect what is happening. There is no intention to change the original p2.user.ui feature rather the RCP problem will be addressed with a new feature. Updated the map files and opened bug 340063 to get the new feature in the build and the equinox target components feature. Also committed a change to the p2.core.feature to include the p2.operations bundle. From here the feature and bundle should be usable in RCP scenarios. Things to do - review the duplication of code in the ui.rcp and ui.sdk bundles. Perhaps the duplication makes sense given that we are configuring things for different scenarios. Dunno - Add back an Installation History page with a Revert button - any other changes that make sense for the RCP scenarios Currently the compare dependency is only brought in because we have the ability to compare install states in the About dialog. What if we create a new fragment for the p2.ui.sdk bundle and take the class which adds the Compare button and functionality of opening the compare editor there. The Installation History page could check (or even provide a new extension point, etc) before it renders the page. Then RCP users could just include the regular feature (without the fragment) in their product. Is this possible? Matt, what are your thoughts about this? fyi, there were compile errors in last night's nightly build because the p2.ui.tests bundle does an Import Package and the package from the new RCP bundle was bound rather than the one from the SDK bundle (which contains the extra class). I've updated the manifest for the p2.tests.ui bundle to do a RequireBundle to avoid this in tonight's build. thanks for looking at this DJ. Yes, we should rename the package if we are indeed maintaining a separate bundle (vs. the fragment approach that you mention). For now my strategy was to maintain the p2.ui.rcp bundle as a close copy of the original. I do NOT think that that is a long term strategy. Rather, it gets us something that works and can be consumed. In practice the p2.iu.rcp and p2.ui.sdk bundles should never be in the same install as they are both trying to define how p2 works so the name collision should not be a issue. As I say, if we are really going to craft a p2 RCP ui then it should stand alone and not have any particular relationship to the sdk ui. Changing assignee to Jeff since he is doing the work. Paul and I talked about this and potential ways to make a separation between the bundles and have something useful for RCP apps as well as the SDK. Here is what we would recommend: org.eclipse.equinox.p2.ui - create a new *internal* interface to be used as a service - modify RevertProfilePage to look for the service and if it exists, then use it to add the Compare button to the page org.eclipse.equinox.p2.ui.sdk.compare - create new bundle with dependency on org.eclipse.compare - move RevertProfilePageWithCompare.java to the new bundle - implement the service (use DS) and modify the code in RevertProfilePageWithCompare so it behaves correctly org.eclipse.equinox.p2.ui.sdk - remove the dependency from org.eclipse.equinox.p2.ui.sdk to org.eclipse.compare - change the plugin.xml so the Installation History page it contributes is org.eclipse.equinox.p2.ui.RevertProfilePage Thoughts? The direction looks interesting but I' concerned about changing bundle structure too much at this point in the release. This change would effectively meant that IDE users would have to include the new bundle to get equivalent function. We'd add it in the feature and all but feels questionable after the API freeze. I'm on the fence there. The current state is less than optimal (duplicate code, odd package naming, ...) but works. It is essentially what people on the web have been saying to do to make p2 work in RCP (see blogs from Lars and Kai). Pascal had volunteered Matt to get involved and do something more reasonable. Awesome. To help kick start that I committed what we already had and got it hooked into the build etc. (In reply to comment #15) > The direction looks interesting but I' concerned about changing bundle > structure too much at this point in the release. This change would effectively > meant that IDE users would have to include the new bundle to get equivalent > function. We'd add it in the feature and all but feels questionable after the > API freeze. I'm on the fence there. I think adding the bundle was already borderline anyway wrt API freeze :p The new bundle would be added in the feature, so it would be transparent for the user. Honestly I think that if we are continuing to fix this (which I think we should), we should do the right thing and be done with it. Tom has suggested that short term we modify the code in the p2.ui.sdk bundle such that it will work with an optional dependency on the org.eclipse.compare bundle. This would allow the bundle to work both with and without the presence of the compare bundle, and we would only have one copy of the code in the repository. (and wouldn't have to worry about duplicating fixes to 2 locations, etc) I'm all for doing the right thing and would very much appreciate Matt (anyone) looking at it further. Thanks. (In reply to comment #17) > Tom has suggested that short term we modify the code in the p2.ui.sdk bundle > such that it will work with an optional dependency on the org.eclipse.compare > bundle. This would allow the bundle to work both with and without the presence > of the compare bundle, and we would only have one copy of the code in the > repository. (and wouldn't have to worry about duplicating fixes to 2 locations, > etc) If we go down that road, we have to mark the dependency non-greedy so tjhe compare bundle is not brought in at install time. What would we do from a feature point of view? I suggest that we keep the feature. This would be the main way that people add p2 to their RCP configurations. Its a bit of a bummer that there would be "SDK" things in there but that's not the worst case we have currently. It may well be that Matt (or someone) can come up with a better way to configure/position the UI for RCP use. That would drive the retention of the p2.ui.rcp bundle. That is, I think, more work than DJ or I have the ability to do. (In reply to comment #20) > Its a bit of a bummer that there would be "SDK" things in there but that's not > the worst case we have currently. It may well be that Matt (or someone) can > come up with a better way to configure/position the UI for RCP use. That would > drive the retention of the p2.ui.rcp bundle. That is, I think, more work than > DJ or I have the ability to do. This is why I suggested to simply make the compare dependency optional and go with the p2.ui.sdk bundle for now for RCP scenarios. In the future we may come up a more divergent ui for RCP scenarios, but for now it seems 99.9% of the code is identical, so lets try to keep that as one bundle that does 100.1% of what we want :) it seems that we are going the direction of optional inclusions. As such I have: - removed the new p2.ui.rcp bundle - updated the user.ui feature to depend on p2.core.feature and p2.rcp.feature Created attachment 191550 [details]
org.eclipse.equinox.p2.ui.sdk patch
The patch changes org.eclipse.equinox.p2.ui.sdk to have an optional non-greedy dependency on org.eclipse.compare. If org.eclipse.compare is not available RevertProfilePage without the compare button is used, otherwise then RevertProfilePageWithCompare is used.
I've released Matt's patch in HEAD. *** Bug 338351 has been marked as a duplicate of this bug. *** Can we say that bug 322505 and bug 281226 are fixed as well in 3.7? Yes What are the steps to required to make this work in Eclipse 3.7? I added the feature to a new product configuration and after the start (from the Eclipse IDE) I receive the following error message Could not locate the running profile instance. The eclipse.p2.data.area and eclipse.p2.profile properties may not be set correctly in this application's config.ini file. From Jeffs Blog entry: All you have to do is add that feature to your RCP product and away you go. http://mcaffer.com/2011/03/p2-in-rcp-applications/ Did you check "Generate metadata repository" when exporting your product? (In reply to comment #28) > What are the steps to required to make this work in Eclipse 3.7? I added the > feature to a new product configuration and after the start (from the Eclipse > IDE) I receive the following error message > > Could not locate the running profile instance. The eclipse.p2.data.area and > eclipse.p2.profile properties may not be set correctly in this application's > config.ini file. > > From Jeffs Blog entry: All you have to do is add that feature to your RCP > product and away you go. http://mcaffer.com/2011/03/p2-in-rcp-applications/ @Michael: Thanks! I didn't realize that this still does not for if started from the IDE. Looking good now. Re-opening this bug as the critical change, making org.eclipse.core optional, was lost when bringing in the patch for bug 304594. http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/bundles/org.eclipse.equinox.p2.ui.sdk/META-INF/MANIFEST.MF?id=9177c55c6f5e400de365bf0c3af44011f529b4a5 (In reply to comment #31) > Re-opening this bug as the critical change, making org.eclipse.core optional, > was lost when bringing in the patch for bug 304594. > > http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/bundles/org.eclipse.equinox.p2.ui.sdk/META-INF/MANIFEST.MF?id=9177c55c6f5e400de365bf0c3af44011f529b4a5 I opened bug374915 to track this. Moving this bug back to fixed. |
Build Identifier: M20110105-0951 Starting from Eclipse 3.6 the org.eclipse.equinox.p2.user.ui feature pulls in an unwanted dependency to org.eclipse.compare which in turn depends to a number of other plugins including org.eclipse.ui.ide. This is blocking any non-IDE RCP application from building with any 3.6.x SDK. Here is the p2 director output: [java] Installation failed. [java] Cannot complete the install because one or more required items could not be found. [java] Software being installed: EclipseTrader 1.0.0.v201101091448 (org.eclipsetrader.platform.workbench 1.0.0.v201101091448) [java] Missing requirement: Equinox Provisioning Platform Update Support 1.0.100.v20100513 (org.eclipse.equinox.p2.ui.sdk 1.0.100.v20100513) requires 'bundle org.eclipse.compare 0.0.0' but it could not be found [java] Cannot satisfy dependency: [java] From: Equinox p2 Provisioning 2.0.1.r361_v20100903-897HFZNFXzxlyl0CrPS8t_228D9D (org.eclipse.equinox.p2.user.ui.feature.group 2.0.1.r361_v20100903-897HFZNFXzxlyl0CrPS8t_228D9D) [java] To: org.eclipse.equinox.p2.ui.sdk [1.0.100.v20100513] [java] Cannot satisfy dependency: [java] From: EclipseTrader 1.0.0.v201101091448 (org.eclipsetrader.platform.workbench 1.0.0.v201101091448) [java] To: org.eclipse.equinox.p2.user.ui.feature.group [2.0.1.r361_v20100903-897HFZNFXzxlyl0CrPS8t_228D9D] Previous versions do not have these dependencies and build without problems. Reproducible: Always