Improving Eclipse dropins - What P2 and polish politcs had in common

This week started for me with sounds of well known tune:

Hello console, my old friend,

I've come to work with you again,

Because the reconciler softly creeping,

Left its staff while I was dreaming,

And the bundles that were planted in my dropins

Still remain

Within the space of profilins*.

It's all about the situation where certain operations,  described here in length, could break P2 profile in such a way, that P2 thought that physically removed bundle was still installed. The reproduction steps in the bug are really lengthy, but RPM version is short:

sudo yum install eclipse
sudo yum install eclipse-cdt
eclipse
sudo yum remove eclipse-cdt
sudo yum install eclipse-changelog 
eclipse #what? CDT is still in the profile?

Further investigation has revealed that it was changelog bug (in the first place), because it did not required cdt although it should. During the reconciliation phase, cdt was not yet removed from the profile, so P2 believed it was able to prevent its uninstallation, and during debug, explained its plan with friendly messages:

[p2] Mon May 21 13:07:14 CEST 2012 - [Start Level Event Dispatcher]
[reconciler] [plan] Some units will not be uninstalled:
[p2] Mon May 21 13:07:14 CEST 2012 - [Start Level Event Dispatcher]
[reconciler] [plan] org.eclipse.cdt.ui 5.4.0.201203191015
[p2] Mon May 21 13:07:14 CEST 2012 - [Start Level Event Dispatcher]
[reconciler] [plan] org.eclipse.cdt.core 5.4.0.201203191015

So, my first attempt to fix the issue was rather naive - uninstall removed bundles first, then perform all the reconciliation that has left. Result:  proper behaviour in this test case, but all other dropins tests broken. Not good.

But thanks to @dj_houghton and @prapicault, I was able to came up with something much easier, that not only fixes my unit tests case, but also manages to not break any other reconciler test:

@@ -123,6 +126,15 @@ public class ProfileSynchronizer {
   if (moveResult.getSeverity() == IStatus.ERROR || moveResult.getSeverity() == IStatus.CANCEL)
    return moveResult;
 
+  if (!request.getRemovals().isEmpty()) {
+   Collection requirements = new ArrayList();
+   for (IInstallableUnit unit : request.getRemovals()) {
+    RequiredCapability req = new RequiredCapability(IInstallableUnit.NAMESPACE_IU_ID, unit.getId(), new VersionRange(unit.getVersion(), true, unit.getVersion(), true), null, 0, 0, false, null);
+    requirements.add(req);
+   }
+   request.addExtraRequirements(requirements);
+  }
+
   // now create a plan for the rest of the work and execute it
   IStatus addRemoveResult = performAddRemove(request, context, monitor);
   if (addRemoveResult.getSeverity() == IStatus.ERROR || addRemoveResult.getSeverity() == IStatus.CANCEL)

The patch should be read: for each removed bundle add a negative requirement to the request, so P2 will remove those bundles no matter what. That's so simple. I do not know why it took me 3 days :-(.

And now the explanation of the title. The whole situation with P2 refusing to uninstall deleted bundles reminds me of a polish health care. It's a well know fact that medical help should reach people needing it in 12 minutes because even seriously wounded usually do not pass away in that time. So the norm for an ambulance to reach its target was 12 minutes, but the ratio of timely arrivals was to low (< 85%). So the parliament extended the time to 16 minutes :-).

If all goes well, this fix may hit Fedora as yet another 0 day update.

And a small bonus - have you noticed that (at least on Linux) you are finally informed what will be the action invoked? Check the screenshot below:

For a long time it was almost a matter of luck - you never knew if you hit the button or drop down menu.

* I will buy** a beer to a person that finds a good rhyme here.
** Applicable when we meet in person.

EDIT: Added proper twitter link to LeNettoyeur which turned out to be prapicault.