Skip to content

Commit

Permalink
ExpandedProduct.getFeatures(ROOT_FEATURES) returns over-qualified IDs
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 9920425)
  • Loading branch information
merks committed Mar 16, 2024
1 parent dcbceb4 commit bf417c0
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ class ExpandedProduct implements IProductDescriptor {
private final String expandedVersion;
private List<IVersionedId> expandedBundles = null;
private List<IVersionedId> expandedFeatures = null;
private List<IInstallableUnit> expandedRootFeatures = Collections.emptyList();
private List<IVersionedId> expandedRootFeatures = Collections.emptyList();
private List<IInstallableUnit> expandedRootFeatureIUs = Collections.emptyList();

private final MultiLineLogger logger;

Expand Down Expand Up @@ -93,7 +94,7 @@ public List<IVersionedId> getFeatures(int options) {
}

public List<IInstallableUnit> getRootFeatures() {
return expandedRootFeatures;
return expandedRootFeatureIUs;
}

private void expandVersions() {
Expand All @@ -106,7 +107,9 @@ private void expandVersions() {
if (contentType != ProductContentType.BUNDLES) {
expandedFeatures = resolver.resolveReferences("feature", ArtifactType.TYPE_ECLIPSE_FEATURE,
defaults.getFeatures(INCLUDED_FEATURES));
expandedRootFeatures = resolver.resolveReferencesToIUs("feature", ArtifactType.TYPE_ECLIPSE_FEATURE,
expandedRootFeatures = resolver.resolveReferences("feature", ArtifactType.TYPE_ECLIPSE_FEATURE,
defaults.getFeatures(ROOT_FEATURES));
expandedRootFeatureIUs = resolver.resolveReferencesToIUs("feature", ArtifactType.TYPE_ECLIPSE_FEATURE,
defaults.getFeatures(ROOT_FEATURES));
}
resolver.reportErrors(logger);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import static org.hamcrest.CoreMatchers.endsWith;
import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
Expand Down Expand Up @@ -272,9 +271,11 @@ public void testPublishingWithRootFeatures() {
assertThat(productUnit.getRequirements(),
hasItem(requirement("org.eclipse.rcp.feature.group", "4.4.0.v20140128")));
assertThat(productUnit.getRequirements(), hasItem(requirement("org.eclipse.e4.rcp.feature.group", "1.0")));
assertThat(productUnit.getRequirements(),
not(hasItem(requirement("org.eclipse.help.feature.group", "2.0.102.v20140128"))));
assertThat(productUnit.getRequirements(), not(hasItem(requirement("org.eclipse.egit.feature.group", "2.0"))));
assertThat(productUnit.getRequirements(), hasItem(requirement("org.eclipse.help.feature.group",
"2.0.102.v20140128",
"(|(org.eclipse.equinox.p2.install.mode.root=true)(product.with.root.features.install.mode.root=true))")));
assertThat(productUnit.getRequirements(), hasItem(requirement("org.eclipse.egit.feature.group", "2.0",
"(|(org.eclipse.equinox.p2.install.mode.root=true)(product.with.root.features.install.mode.root=true))")));

assertEquals("org.eclipse.help", seeds.get(1).getId());
assertThat(seeds.get(1).getInstallableUnit(), is(unitWithId("org.eclipse.help.feature.group")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@
import static org.eclipse.tycho.test.util.InstallableUnitUtil.PRODUCT_TYPE_PROPERTY;

import java.util.Map;
import java.util.Objects;

import org.eclipse.equinox.internal.p2.metadata.InstallableUnit;
import org.eclipse.equinox.p2.metadata.IInstallableUnit;
import org.eclipse.equinox.p2.metadata.IProvidedCapability;
import org.eclipse.equinox.p2.metadata.IRequirement;
import org.eclipse.equinox.p2.metadata.ITouchpointData;
import org.eclipse.equinox.p2.metadata.IVersionedId;
import org.eclipse.equinox.p2.metadata.Version;
import org.eclipse.equinox.p2.metadata.expression.IMatchExpression;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeMatcher;
Expand Down Expand Up @@ -167,6 +170,26 @@ public void describeTo(Description description) {
};
}

public static Matcher<? super IRequirement> requirement(final String id, final String version,
final String filter) {
IMatchExpression<IInstallableUnit> filterEpxression = InstallableUnit.parseFilter(filter);
final IInstallableUnit unit = InstallableUnitUtil.createIU(id, version);
return new TypeSafeMatcher<>() {

@Override
public void describeTo(Description description) {
description.appendText("a require of unit ").appendText(id).appendText(":").appendText(version)
.appendText(";filter=").appendText(filter);
}

@Override
protected boolean matchesSafely(IRequirement item) {
IMatchExpression<IInstallableUnit> requirementFilter = item.getFilter();
return item.isMatch(unit) && Objects.equals(filterEpxression, requirementFilter);
}
};
}

public static Matcher<? super IRequirement> requirement(final String id, final String version) {
final IInstallableUnit unit = InstallableUnitUtil.createIU(id, version);
return new TypeSafeMatcher<>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,12 @@ public void testRootLevelFeaturesAreIncludedInP2Repository() throws Exception {
Optional<File> rootFeatureInRepo = p2Repository.findFeatureArtifact("pi.root-level-installed-feature");
assertTrue(rootFeatureInRepo.isPresent());

// ... although there is no dependency from the product IU.
assertThat(p2Repository.getUniqueIU("main.product.id").getRequiredIds(),
not(hasItem("pi.root-level-installed-feature.feature.group")));
// ... although there is no unfiltered dependency from the product IU.
var unfilteredRequiredIds = p2Repository.getUniqueIU("main.product.id").getUnfilteredRequiredIds();
assertThat(unfilteredRequiredIds, not(hasItem("pi.root-level-installed-feature.feature.group")));

// There are the expected unfiltered requirement such as this one.
assertThat(unfilteredRequiredIds, hasItem("pi.example.feature.feature.group"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,17 @@ public List<String> getRequiredIds() throws Exception {
return getValues(unitElement, "requires/required/@name");
}

public List<String> getUnfilteredRequiredIds() throws Exception {
return XMLTool.getMatchingNodes(unitElement, "requires/required").stream().filter(node -> {
try {
var nodes = getNodes(node, "filter");
return nodes.isEmpty();
} catch (XPathExpressionException e) {
throw new RuntimeException(e);
}
}).map(node -> node.getAttributes().getNamedItem("name")).map(Node::getNodeValue).toList();
}

/**
* Returns the IDs of IUs required with strict version range.
*/
Expand Down

0 comments on commit bf417c0

Please sign in to comment.