Absence of an unpack attribute must not result in BundleShapeAdvice

laeubi commented 6 months ago

Currently the FeaturesAction assumes that the unpack attribute is always set, if that is not the case it leads to unexpected results because then unpack=true is assumed always. This is especially dangerous as PDE since 2023-12 release removes the attributes and Tycho assume they are false by default the same as PDE has done in the past when adding new items.

To mitigate this this adds an additional check to only evaluate the unpack if it is set, together with a testcase that covers all possible combinations.

I give +1 as project lead for this really late fix but I think it is urgent enough as it has unexpected side-effects for users using already released PDE that will likely be hard to discover and the previous state was to always specify this attribute so existing setup won't be affected. Waiting for one more release to fix this seems therefore not acceptable for me.

akurtakov commented 6 months ago

The change looks good to me. I would like to get @merks review here too as we are really late in the game.

github-actions[bot] commented 6 months ago

Test Results

9 files ±0 9 suites ±0 30m 20s :stopwatch: - 1m 3s 2 184 tests +1 2 180 :white_check_mark: +1 4 :zzz: ±0 0 :x: ±0 6 642 runs +3 6 631 :white_check_mark: +3 11 :zzz: ±0 0 :x: ±0

Results for commit cf3265a3. ± Comparison against base commit 151d95ba.

akurtakov commented 6 months ago

For the record - PMC approved

laeubi commented 6 months ago

I build started here: https://ci.eclipse.org/releng/job/Builds/job/I-build-4.31/162/

eclipse-equinox / p2

Absence of an unpack attribute must not result in BundleShapeAdvice #477

Test Results