Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 368238 - "eclipse p2.directorApp -uninstallIU ${IU} -installIU ${IU}" trashes the installation based on repo state
Summary: "eclipse p2.directorApp -uninstallIU ${IU} -installIU ${IU}" trashes the inst...
Status: CLOSED WONTFIX
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: 3.7.1   Edit
Hardware: PC Linux
: P3 normal with 4 votes (vote)
Target Milestone: ---   Edit
Assignee: P2 Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-10 07:43 EST by Manuel Maier CLA
Modified: 2022-02-09 09:57 EST (History)
10 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Manuel Maier CLA 2012-01-10 07:43:42 EST
Reproducible: every time.
Hosts: any


Here is what we wish to do:

- We have an installation of Eclipse at /dest.
- Installed there we have for example the IU org.eclipse.sdk.feature.group installed from the integration builds repository http://download.eclipse.org/eclipse/updates/3.8-I-builds.
- Now we want to check on a daily basis if there is a new version of the IU available and if so, update it.
(similiar to the Help -> Check for Updates in the Eclipse UI).
- This has to happen on command line level as we want to include this in a batch job.
- Further we only want to download changes from eclipse.org(not the whole drops every day). Because at first we tried to do an -uninstallIU and then in a 2nd command -installIU. But this will produce too much network overhead in my opinion.

- For now I tried to use the p2 director the following way:
eclipse -application org.eclipse.equinox.p2.director -vm /opt/java/linux_64/jdk1.6.0_21/bin -nosplash -consoleLog -purgeHistory -repository http://download.eclipse.org/eclipse/updates/3.8-I-builds/ -uninstallIU org.eclipse.sdk.feature.group -installIU org.eclipse.sdk.feature.group -destination /tmp/test/dest -bundlePool /tmp/test/bundlepool/softice/dest -profileProperties org.eclipse.update.install.features=true -roaming -profile SDKProfile.win32.x86_64 -p2.os win32 -p2.arch x86_64 -p2.ws win32

The command does exactly what we want if there really is an update (say from one Integration build to another). Old files get uninstalled, the new one get installed and what stays the same is untouched.
But if nothing changes in the repository only the -uninstallIU operation gets executed. And everything is removed.
Comment 1 DJ Houghton CLA 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 CLA 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 CLA 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 CLA 2012-02-14 12:15:12 EST
This seems related to bug# 360414.
Comment 5 Paul Webster CLA 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 CLA 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 CLA 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 CLA 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.