Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 387611

Summary:

When installing new bundles p2 always immediately updates the bundle

Product:

[Eclipse Project] Equinox

Reporter:

Thomas Watson <tjwatson>

Component:

Assignee:

P2 Inbox <equinox.p2-inbox>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

irbull, pascal

Version:

unspecified

Target Milestone:

Kepler M2

Hardware:

OS:

Mac OS X

Whiteboard:

Bug Depends on:

Bug Blocks:

388031

Attachments:

Description	Flags
Possible fix	none

Description Thomas Watson

2012-08-20 10:19:16 EDT

This got introduced with the "fix" in bug 326745.

The following code got added with that fix:


current = manipulatingContext.installBundle(bundleLocation);
if (!version.equals(current.getVersion()) || 
    !symbolicName.equals(current.getSymbolicName()))
	// can happen if, for example, the new version of the bundle is installed
	// to the same bundle location as the old version
	current.update();

The problem is that "version" here is a String and current.getVersion returns a Version object so the version.equals(current.getVersion()) call never returns true.  This means that every install is immediately followed by an unnecessary update.

Comment 1 Thomas Watson

2012-08-22 15:55:40 EDT

Created attachment 220164 [details]
Possible fix

Here is a possible fix.  I put some safe guards (null checks) because earlier code seems to indicate that the symbolicName and the version variables can be null.  Next I parsed the version string into a proper Version object to do the equals check.  I was being extra cautious to catch IllegalArgumentException if for some reason we could not parse the version string.

The patch also contains another small, unrelated fix in startBundles.  Previously an attempt was made to start the system bundle.  This is not needed since the system bundle is obviously already active since we are in the middle of activating the configurator.  I would like this released also.  Let me know if you want a separate bug report for that fix.

I am working on a new implementation of the framework and noticed these two issues.

Comment 2 Pascal Rapicault

2012-08-22 17:57:39 EDT

Don't bother with multiple bug reports, this one will suffice. I noticed that you just do a printstacktrace and it would probably be best to log instead.

Comment 3 Thomas Watson

2012-08-23 08:24:07 EDT

(In reply to comment #2)
> Don't bother with multiple bug reports, this one will suffice. I noticed
> that you just do a printstacktrace and it would probably be best to log
> instead.

I didn't log because other parts of that method did not seem to log when exceptions were caught.  If we decide not to log then the printStackTrace likely should be surrounded by a Activator.DEBUG check like in other parts of that method.

Comment 4 Ian Bull

2012-08-24 14:02:05 EDT

I've released this with the print stack trace protected by the Activator.DEBUG flag.

Thanks Tom.

Comment 5 Pascal Rapicault

2012-08-24 14:46:11 EDT

Should not we fix this for Juno as well?

Comment 6 Thomas Watson

2012-08-24 16:18:33 EDT

(In reply to comment #5)
> Should not we fix this for Juno as well?

+1, I think this is a safe thing to do and the current behavior is a bit wacky.  Also, this behavior was introduced in Juno M1 if I read the git log correctly.

Comment 7 Ian Bull

2012-08-24 17:35:31 EDT

(In reply to comment #6)
> (In reply to comment #5)
> > Should not we fix this for Juno as well?
> 
> +1, I think this is a safe thing to do and the current behavior is a bit
> wacky.  Also, this behavior was introduced in Juno M1 if I read the git log
> correctly.

Done. 

See bug#388031