Bug 368238

Comment 1 DJ Houghton 2012-01-10 16:37:43 EST

This sounds like another good argument for allowing the director command-line app to perform an update rather than have users perform the "uninstall then install" hack.  

In the UI we do some plan analysis and modify the request, could we do something like that in the director? Or at least offer access to the UpdateOperation class?

Comment 2 Helmut J. Haigermoser 2012-01-11 07:16:51 EST

(In reply to comment #1)
> This sounds like another good argument for allowing the director command-line
> app to perform an update rather than have users perform the "uninstall then
> install" hack.  
> 
> In the UI we do some plan analysis and modify the request, could we do
> something like that in the director? Or at least offer access to the
> UpdateOperation class?

Hi DJ :)
Since in p2 an update operation really is "uninstall then install" the director seems to expose that concept to the users, which can be confusing. So maybe instead of making users specify -installIU iu -uninstallIU iu we could add a flag -updateIU iu to avoid confusion?

As for the bug at hand, I looked at the code and found out what happens, looks like a change request that has the same IU in "iusToAdd" and "iusToRemove" will result in a provplan with 2 operands: add IU property "STRICT", remove IU property "STRICT" . These operands will be executed in that order and lead to the property ending up removed in the profile. That will cause p2 to think the whole IU is obsolete (not explictly installed, and not needed by anything else), so it will be cleaned out.

One fix I came up with is very central to p2, in 
SimplePlanner::planPropertyOperations. 

That method calculates the operations needed for any given operation, and what it currently does is first add IU properties, then remove IU properties. So, if like above the same IU property is "updated(added/removed)" we end up with the property being removed. The fix to this is simple: change the order to "first remove props, then add props". This method already uses that order for profile properties, so doing the same for IU properties seems like a safe fix. Still, it's a huge change for a bug in the director..

Another fix could be to fix the provplan calculation, looks like we find out that no IU needs changing for this kind of request, but the IU properties seem to be missed in that check, fixing that might be an even better option to fix the behavior..

Let me know what you think! :)
Helmut

Comment 3 Helmut J. Haigermoser 2012-01-23 10:09:15 EST

(In reply to comment #1)
> This sounds like another good argument for allowing the director command-line
> app to perform an update rather than have users perform the "uninstall then
> install" hack.  
> 
> In the UI we do some plan analysis and modify the request, could we do
> something like that in the director? 
DJ,
I've added a few lines of code to the DirectorApplication class, method performProvisioningActions (insert this code right after the installs and uninstalls arrays are filled):

// in update operations the uninstall/install can sometimes point at 
// the same IU+Version, remove those no-ops from the lists
// @see bug 368238: https://bugs.eclipse.org/bugs/show_bug.cgi?id=368238

Collection<IInstallableUnit> overLaps = new ArrayList<IInstallableUnit>(installs);
overLaps.retainAll(uninstalls);
for (IInstallableUnit unit : overLaps) {
 System.out.println("WARNING: IInstallable Unit " + unit.getId() + "(" + unit.getVersion() + ") cannot be updated, no new content found");
  installs.remove(unit);
  uninstalls.remove(unit);
}


These lines will check the IUs put into the "add" and "remove" areas of a provplan, and, if the same IU is in both we throw them out again, since a remove/install operation of the same IU should be a null operation anyway..

Let me know what you think
Helmut

Comment 4 Ian Bull 2012-02-14 12:15:12 EST

This seems related to bug# 360414.

Comment 5 Paul Webster 2012-02-28 14:45:34 EST

Ian, can Helmut's fix be applied with little risk?  We left the bug as normal, but really if it trashes our install it's a little hard to revert the change.

PW

Comment 6 Alain Picard 2017-03-17 11:22:28 EDT

(In reply to Paul Webster from comment #5)
> Ian, can Helmut's fix be applied with little risk?  We left the bug as
> normal, but really if it trashes our install it's a little hard to revert
> the change.
> 
> PW

Is there any reason why this patch was never applied and can it be safe to apply it?

Comment 7 Ed Merks 2020-02-19 03:37:58 EST

(In reply to Alain Picard from comment #6)
> 
> Is there any reason why this patch was never applied and can it be safe to
> apply it?

Yes, because no one is paid to work on p2.

Comment 8 Eclipse Genie 2022-02-09 09:57:30 EST

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.