search

eclipse-equinox / p2

Eclipse Public License 2.0
15 stars 41 forks source link

Refresh a subset of bundles if they provide missing requirements #422

Closed laeubi closed 8 months ago

laeubi commented 9 months ago

When refreshing large sets of bundles the resolver sometimes take a choice where a requirement can't be bound even though it is there and resolvable.

This adds some additional code to detect this situation and then refresh a smaller set of bundles that provide these until a max retry is reached, all bundles are resolved, or all providers have been tried to refresh already.

github-actions[bot] commented 9 months ago

Test Results

    9 files  ±0      9 suites  ±0   34m 36s :stopwatch: + 2m 43s 2 183 tests ±0  2 179 :white_check_mark: ±0   4 :zzz: ±0  0 :x: ±0  6 639 runs  ±0  6 628 :white_check_mark: ±0  11 :zzz: ±0  0 :x: ±0 

Results for commit d3c8dc61. ± Comparison against base commit 6f0466c7.

:recycle: This comment has been updated with latest results.

laeubi commented 9 months ago

The code is fine given what it is trying to do. But I question if we can have a testcase that exhibits the behavior so we can add a test for this code path.

I'm not yet sure in wich form such a test-case would have to be provided, can I somehow "export" a bundle-set (metadata-only) and use it as an input for a test with simple-configurator?

tjwatson commented 8 months ago

@laeubi you may not like this, but to move this forward I would like to have an option that defaults to the current behavior so we can enable this for the problematic scenarios. I would rather do that now so that the ones impacted by this issue can try out the solution without risking others. Once we get more confident in the approach then we can flip the default or get rid of the option.

laeubi commented 8 months ago

@tjwatson do you like to suggest how this should be implemented? Maybe as simple as a system property to check?

tjwatson commented 8 months ago

@tjwatson do you like to suggest how this should be implemented? Maybe as simple as a system property to check?

Yes, but use BundleContext.getProperty instead of System.getProperty.

laeubi commented 8 months ago

@tjwatson I now added this inside a block that is disabled by default and can be enabled with equinox.simpleconfigurator.deeprefresh=true