Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExpandedProduct.getFeatures(ROOT_FEATURES) returns over-qualified IDs #3674

Merged
merged 1 commit into from
Mar 16, 2024

Conversation

merks
Copy link
Contributor

@merks merks commented Mar 16, 2024

It returns IVersionedId instances where the ID is that of the installable unit, i.e., with an extra .feature.group suffix, rather than the ID of the feature itself.
ExpandedProduct.getFeatures(INCLUDED_FEATURES) returns the correct result. A problem arises from this now because with the 4.31 changes to org.eclipse.equinox.p2.publisher.eclipse.ProductAction to generate filtered requirements for the installMode="root" features, this inconsistency produces invalid requirements, i.e., a requirement on xyz.feature.group.feature.group, which does not exist, rather than on xyz.feature.group.

Copy link

github-actions bot commented Mar 16, 2024

Test Results

  588 files  ±0    588 suites  ±0   4h 15m 30s ⏱️ + 34m 43s
  398 tests ±0    391 ✅ ±0   7 💤 ±0  0 ❌ ±0 
1 194 runs  ±0  1 172 ✅ ±0  22 💤 ±0  0 ❌ ±0 

Results for commit 6ca7af1. ± Comparison against base commit 5072628.

♻️ This comment has been updated with latest results.

@merks
Copy link
Contributor Author

merks commented Mar 16, 2024

@laeubi

The failing test suggests a good place to improve test for this change. Now that the correct feature ID is used the test fails. I'm just not sure the best way to author the improved test. The requirement is present now, as it should be,, but with a filter, so while we could just remove the not() call, we really should also test that the expected filter is present on the requirement. I'm not sure how best to express that so I'm looking for advice. Or maybe you can complete the changes? I can't get this one test to run in debug mode for me. :-(

It returns IVersionedId instances where the ID is that of the
installable unit, i.e., with an extra .feature.group suffix, rather than
the ID of the feature itself.
ExpandedProduct.getFeatures(INCLUDED_FEATURES) returns the correct
result. A problem arises from this now because with the 4.31 changes to
org.eclipse.equinox.p2.publisher.eclipse.ProductAction to generate
filtered requirements for the installMode="root" features, this
inconsistency produces invalid requirements, i.e., a requirement on
xyz.feature.group.feature.group, which does not exist, rather than on
xyz.feature.group.
@merks
Copy link
Contributor Author

merks commented Mar 16, 2024

@laeubi

Two failing tests are updated to check for the requirement that was formerly absent now being present though with a filter, including one test that checks the exact value of the filter.

@laeubi laeubi added the backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change label Mar 16, 2024
@laeubi
Copy link
Member

laeubi commented Mar 16, 2024

/request-license-review

Copy link

/request-license-review

License review requests:

After all reviews have concluded, re-run the license-vetting check from the Github Actions web-interface to update its status.

Workflow run (with attached summary files):
https://github.com/eclipse-tycho/tycho/actions/runs/8309449351

@laeubi laeubi merged commit 9920425 into eclipse-tycho:main Mar 16, 2024
11 of 12 checks passed
@eclipse-tycho-bot
Copy link

💚 All backports created successfully

Status Branch Result
tycho-4.0.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@merks merks deleted the pr-product-root-features branch March 23, 2024 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants