Bugzilla – Full Text Bug Listing
| Summary: | [ui] Visualization/handling of patched versions in a composite repository | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Gunnar Wagenknecht <gunnar> | ||||||
| Component: | p2 | Assignee: | DJ Houghton <dj.houghton> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | enhancement | ||||||||
| Priority: | P3 | CC: | aniefer, daniel_megert, david_williams, irbull, jeffmcaffer, john.arthorne, pascal, stephan.herrmann | ||||||
| Version: | unspecified | ||||||||
| Target Milestone: | 3.7 M6 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 316702, 330534 | ||||||||
| Attachments: |
|
||||||||
|
Description
Gunnar Wagenknecht
(In reply to comment #0) > When updating X, X must not be replaced with X_patched if not explicitly > confirmed (eg. warning dialog). This overlaps with bug 316702. (In reply to comment #1) > This overlaps with bug 316702. From looking at bug 316702 I would consider bug 316702 a "meta" bug and this a concrete work item out of it. But this may depend on who is doing the work. (In reply to comment #2) > From looking at bug 316702 I would consider bug 316702 a "meta" bug and this a > concrete work item out of it. But this may depend on who is doing the work. Clearly bug 316702 is more general, yes. By making the connection between both bugs I'm trying to avoid that this current bug would add yet one more specific warning dialog. That would actually work against the spirit of the other bug, which aims at unifying the UI rather than adding more clutter. It's not clear to me how this could be done. There are several IU's in play in this scenario: - The original feature F being patched - An original bundle B in feature F - The patch feature P - The bundle B' specified by patch P Now the problem case arises when installing some other feature that requires bundle B: p2 may select B' instead of B if it otherwise satisfies the dependencies. In this situation the fact that P exists, and that P includes B', is completely irrelevant to the computation that p2 is doing. Ie., there is no markup on B' indicating that it "belongs to some patch", so there is no way to detect and indicate this to the user. It's not the patch that gets installed in the problematic scenario - it is a bundle that happens to be included in a patch. (In reply to comment #4) > It's not clear to me how this could be done. There are several IU's in play in > this scenario: > > - The original feature F being patched > - An original bundle B in feature F > - The patch feature P > - The bundle B' specified by patch P > > Now the problem case arises when installing some other feature that requires > bundle B: p2 may select B' instead of B if it otherwise satisfies the > dependencies. In this situation the fact that P exists, and that P includes B', > is completely irrelevant to the computation that p2 is doing. Ie., there is no > markup on B' indicating that it "belongs to some patch", so there is no way to > detect and indicate this to the user. It's not the patch that gets installed in > the problematic scenario - it is a bundle that happens to be included in a > patch. Generally speaking you're right of course. It's a core feature of p2 to update an installed bundle to a newer version, and this can't be flagged with a warning. However, in the solution we agreed upon in bug 330534 comment 8 ff the bundle B' cannot be installed without P. Thus, for this scenario it should suffice to flag the installation of P. my 2c. In other bugs it was decided that patching (i.e. using another bundle's namespace) requires explicit approval. Once it's approved that bundle could flag itself through a (to be defined) attribute (on the plug-in and/or feature). The UI would simply have to display that attribute. It seems like we should just display all patch IUs differently in the p2 UI (different icon). Any time you are installing a patch, it will replace the implementation of something you already have installed, which the user should probably be aware of. This seems quite reasonable, in fact DJ tells me the old Update Manager UI had a different icon for patches as well. However, in the Object Teams case, this won't help much. The patch is just referenced by the "Object Teams Development Tooling" feature. You would have to select to install that, click Next, and then expand the tree to find the patch. I'm all in favour of doing this but it's not going to give a strong warning to the user. Another option is that the "Install details" page has a warning or info line in the summary at the top that says "you are installing a patch that replaces existing functionality" or something like that. > However, in the Object Teams case, this won't help much. The patch is just
> referenced by the "Object Teams Development Tooling" feature. You would have to
> select to install that, click Next, and then expand the tree to find the patch.
Couldn't the patch info be propagated up to the feature and decorate it with a "patch" image?
(In reply to comment #8) > Couldn't the patch info be propagated up to the feature and decorate it with a > "patch" image? I think the patch info only exists at the feature and needs to be pushed down into the plug-in metadata when building the feature using PDE build. Or could it be "discoverd" at runtime? We can surely change the icon but I would not go any further (for example I would not bubble up the changes in the feature when the thing is brought in by a regular IU) Created attachment 188182 [details]
initial patch
Here is an initial patch which changes the IU icon for patch IUs. It appears in the Install New Software dialog as well as in the Installed Software dialog.
Created attachment 188267 [details]
updated patch
Updated patch to handle npe and also add new icon for a disabled (grey'd out) patch IU when viewing in a repo.
Released latest patch. Interested parties please take a look in the latest builds (starting in tonight's nightly) and re-open or open a new bug if it doesn't match your requirements. I agree with Pascal's comment 10 in that we don't want to do a deep recursive search in the UI when trying to install an IU. This patch contained references to the 1.5 method Boolean.parseBoolean and the bundle is still marked as J2SE-1.4 in its manifest. I have updated these references to use Boolean.valueOf instead. |