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

Bug 333071

Summary:

Resolver hook is not driven for StateHelper.getUnsatisfiedLeaves

Product:

[Eclipse Project] Equinox

Reporter:

Glyn Normington <glyn.normington>

Component:

Framework

Assignee:

Thomas Watson <tjwatson>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

tjwatson

Version:

3.7

Target Milestone:

3.7 M5

Hardware:

Macintosh

OS:

Mac OS X - Carbon (unsup.)

Whiteboard:

Bug Depends on:

Bug Blocks:

330776

Attachments:

Description	Flags
Possible fix	none
Possible fix	none

Description Glyn Normington

2010-12-22 06:23:12 EST

Virgo uses StateHelper.getUnsatisfiedLeaves to determine missing dependencies which it then provisions from a repository. The resolver hook is not driven by getUnsatisfiedLeaves and so bundles in the user region appear to have no unsatisfied leaves even though their dependencies are missing from the user region (but present in the kernel region). This results in a resolution failure later because Virgo's resolver hook removes the relevant candidate capabilities which are meant to be hidden from the user region.

The alternative of driving a full resolution to determine missing dependencies is not particularly attractive as it would need to be driven multiple times to provision transitive dependencies and is a relatively expensive operation since uses constraints are checked.

So I propose that the resolver hook should be driven for getUnsatisfiedLeaves so that it can remove any potential wires which are not allowed (in terms of region configuration).

Comment 1 Thomas Watson

2011-01-03 15:04:00 EST

In principle I agree, but this may not be strictly according to specification since an actual resolve operation will not be occurring.  Hooks are only supposed to be called during actual resolve operations and call the StateHelper does not cause a resolve operation.  So we may not have information such as the trigger bundles etc.  Hooks may not be prepared to be called outside of an actual resolve operation (although I cannot think of why this would really be an issue).

Comment 2 Thomas Watson

2011-01-04 15:17:08 EST

Created attachment 186043 [details]
Possible fix

Here is a possible fix.  There are many methods on StateHelper that need to drive resolver hooks.

isResolvable(BundleSpecification)
isResolvable(HostSpecification)
isResolvable(ImportPackageSpecification)
getUnsatisfiedLeaves(BundleDescription[])
getUnsatisfiedConstraints(BundleDescription)

This patch adds calls to the resolver hook for all of these.  This is not always simple because some of these methods were used as implementation details of the other methods.  In order to avoid nested hook calls from the same thread I had to add private methods that took a ResolverHook.

I also am not sure why getUnsatisfiedConstraints method needs to call the isResolvable methods for the constraints if finds as unresolved.  It seems to me that this method should return all unresolved constraints for a bundle regardless of the fact that there is a supplier available that we think should satisfy the constraint.

Comment 3 Thomas Watson

2011-01-04 15:37:58 EST

(In reply to comment #2)
> I also am not sure why getUnsatisfiedConstraints method needs to call the
> isResolvable methods for the constraints if finds as unresolved.  It seems to
> me that this method should return all unresolved constraints for a bundle
> regardless of the fact that there is a supplier available that we think should
> satisfy the constraint.

This has been the behavior since 3.0.  For now I think we should keep this behavior.

Comment 4 Thomas Watson

2011-01-04 16:36:53 EST

Created attachment 186053 [details]
Possible fix

The last patch did not correctly call out to the resolver hook in getUnsatisfiedLeaves after collecting the unsatisfied constraints.

Comment 5 Glyn Normington

2011-01-05 03:37:09 EST

(In reply to comment #3)
> (In reply to comment #2)
> > I also am not sure why getUnsatisfiedConstraints method needs to call the
> > isResolvable methods for the constraints if finds as unresolved.  It seems to
> > me that this method should return all unresolved constraints for a bundle
> > regardless of the fact that there is a supplier available that we think should
> > satisfy the constraint.
> 
> This has been the behavior since 3.0.  For now I think we should keep this
> behavior.

This behaviour sounds safer to me.

Comment 6 Glyn Normington

2011-01-05 03:38:57 EST

(In reply to comment #1)
> In principle I agree, but this may not be strictly according to specification
> since an actual resolve operation will not be occurring.  Hooks are only
> supposed to be called during actual resolve operations and call the StateHelper
> does not cause a resolve operation.  So we may not have information such as the
> trigger bundles etc.  Hooks may not be prepared to be called outside of an
> actual resolve operation (although I cannot think of why this would really be
> an issue).

I think hook writers will understand that these kinds of "partial resolve" operations drive the hook.

Comment 7 Glyn Normington

2011-01-05 03:40:05 EST

If you'd like to point me at a build that contains this fixes, I will try it out.

Comment 8 Glyn Normington

2011-01-06 05:39:55 EST

Thanks for supplying me a build. I confirm that this fixes the problem and all the kernel tests pass.

Note to self: the code to support this level of Equinox is on an equinox-updates branch of the bug330776-framework-hooks branch (in the Virgo kernel repository).

Comment 9 Thomas Watson

2011-01-06 09:54:03 EST

Thanks for confirming Glyn.  I released this fix to HEAD.