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

Bug 359544

Summary:

[target] Versions are improperly compared when resolving to the latest version

Product:

[Eclipse Project] PDE

Reporter:

Nigel Westbury <nigelipse>

Component:

Assignee:

Curtis Windatt <curtis.windatt.public>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

curtis.windatt.public

Version:

4.2

Keywords:

contributed

Target Milestone:

3.8 M4

Hardware:

OS:

Windows XP

Whiteboard:

Bug Depends on:

347695

Bug Blocks:

Attachments:

Description	Flags
patch to use correct version comparisons	curtis.windatt.public: iplog+

Description Nigel Westbury

2011-09-30 06:33:44 EDT

Build Identifier: 3.7.0

The code is doing a compare of the String value of the versions.  It should be comparing the Version objects.  The first three components of a version are non-negative integers and must be compared as such.

Our plug-ins have moved on to a minor version of 10 and the target definition is resolving to an older version, causing chaos in our development team.

A patch is attached.  I would have prefered to add getOsgiVersion/setOsgiVersion to BundleInfo and deprecated the getVersion/setVersion methods.  The author of this comparator is PDE probably assumed getVersion was returning the Version object so this change to BundleInfo would help avoid this error.  However that would involve another patch for the Equinox team so may not be worth doing.

I considered catching the IllegalArgumentException and falling back to the string compare, just to make sure that we don't break anyone who might have some invalid version strings out there.  However I think such users would have other problems so giving them an exception is beter.   

Reproducible: Always

Comment 1 Nigel Westbury

2011-09-30 06:37:16 EDT

Created attachment 204368 [details]
patch to use correct version comparisons

Comment 2 Curtis Windatt

2011-09-30 09:33:31 EDT

We are working at converting the target code from provisional to a real API (bug 347695).  We will make this change once that fix has been committed.  Thanks for the patch.

Comment 3 Curtis Windatt

2011-10-25 11:33:51 EDT

Target API is waiting for M4

Comment 4 Curtis Windatt

2011-11-03 16:13:38 EDT

Applied the fix, but added a catch for the illegal argument exception (logs the error).  While it is true that the user will have other problems with target bundles, an uncaught exception will not be handled nicely.

http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=9568a385e731ab3a727589517dc697262e936ba8

Comment 5 Curtis Windatt

2011-12-07 12:14:49 EST

Verified