From cf3265a322ec6caf067bbdd1c217120c5b855fc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20L=C3=A4ubrich?= Date: Thu, 29 Feb 2024 09:26:54 +0100 Subject: [PATCH] Absence of an unpack attribute must not result in BundleShapeAdvice 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. --- .../p2/publisher/eclipse/FeaturesAction.java | 2 +- .../p2/tests/AbstractProvisioningTest.java | 2 +- .../publisher/actions/FeaturesActionTest.java | 46 +++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/FeaturesAction.java b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/FeaturesAction.java index 1a8e440d0f..9f20d2f662 100644 --- a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/FeaturesAction.java +++ b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/FeaturesAction.java @@ -186,7 +186,7 @@ private void createAdviceFileAdvice(Feature feature, IPublisherInfo publisherInf private void createBundleShapeAdvice(Feature feature, IPublisherInfo publisherInfo) { FeatureEntry entries[] = feature.getEntries(); for (FeatureEntry entry : entries) { - if (entry.isUnpack() && entry.isPlugin() && !entry.isRequires()) + if (entry.unpackSet() && entry.isUnpack() && entry.isPlugin() && !entry.isRequires()) publisherInfo.addAdvice(new BundleShapeAdvice(entry.getId(), Version.parseVersion(entry.getVersion()), IBundleShapeAdvice.DIR)); } } diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/AbstractProvisioningTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/AbstractProvisioningTest.java index e111ce9265..ddefec77b5 100644 --- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/AbstractProvisioningTest.java +++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/AbstractProvisioningTest.java @@ -739,7 +739,7 @@ private static void indent(OutputStream output, int indent) { } } - public static void writeBuffer(File outputFile, StringBuilder buffer) throws IOException { + public static void writeBuffer(File outputFile, CharSequence buffer) throws IOException { outputFile.getParentFile().mkdirs(); try (FileOutputStream stream = new FileOutputStream(outputFile)) { stream.write(buffer.toString().getBytes()); diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/publisher/actions/FeaturesActionTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/publisher/actions/FeaturesActionTest.java index 4572a8c7ed..f99459d9b9 100644 --- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/publisher/actions/FeaturesActionTest.java +++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/publisher/actions/FeaturesActionTest.java @@ -48,6 +48,7 @@ import org.eclipse.equinox.p2.publisher.IPublisherInfo; import org.eclipse.equinox.p2.publisher.IPublisherResult; import org.eclipse.equinox.p2.publisher.PublisherInfo; +import org.eclipse.equinox.p2.publisher.PublisherResult; import org.eclipse.equinox.p2.publisher.actions.IAdditionalInstallableUnitAdvice; import org.eclipse.equinox.p2.publisher.actions.ICapabilityAdvice; import org.eclipse.equinox.p2.publisher.actions.IFeatureRootAdvice; @@ -55,6 +56,7 @@ import org.eclipse.equinox.p2.publisher.actions.ITouchpointAdvice; import org.eclipse.equinox.p2.publisher.actions.IUpdateDescriptorAdvice; import org.eclipse.equinox.p2.publisher.eclipse.FeaturesAction; +import org.eclipse.equinox.p2.publisher.eclipse.IBundleShapeAdvice; import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor; import org.eclipse.equinox.p2.tests.TestActivator; import org.eclipse.equinox.p2.tests.TestData; @@ -191,6 +193,50 @@ public void testMatchGreaterOrEqual() throws Exception { } } + public void testUnpack() throws Exception { + File testFolder = getTestFolder("FeaturesAction.testUnpack"); + File featureXML = new File(testFolder, "feature.xml"); + writeBuffer(featureXML, """ + + + + + + + + + + + """); + PublisherInfo info = new PublisherInfo(); + FeaturesAction action = new FeaturesAction(new File[] { testFolder }); + PublisherResult results = new PublisherResult(); + action.perform(info, results, new NullProgressMonitor()); + // no unpack attribute --> jar + assertShapeAdvice(info, "org.plugin1", IBundleShapeAdvice.JAR); + // unpack=true --> dir + assertShapeAdvice(info, "org.plugin2", IBundleShapeAdvice.DIR); + // unpack=false --> JAR + assertShapeAdvice(info, "org.plugin3", IBundleShapeAdvice.JAR); + // imports should never unpack as attribute is invalid for them! + assertShapeAdvice(info, "org.import1", IBundleShapeAdvice.JAR); + assertShapeAdvice(info, "org.import2", IBundleShapeAdvice.JAR); + assertShapeAdvice(info, "org.import3", IBundleShapeAdvice.JAR); + } + + protected void assertShapeAdvice(PublisherInfo info, String bundle, String shape) { + for (IBundleShapeAdvice shapeAdvice : info.getAdvice(null, true, bundle, Version.create("1.0.0"), + IBundleShapeAdvice.class)) { + assertEquals(bundle + " has wrong advice", shape, shapeAdvice.getShape()); + return; + } + if (IBundleShapeAdvice.JAR.equals(shape)) { + return; + } + fail(shape + " was expected for " + bundle + " but no advice was found!"); + + } + public void testFilters() throws Exception { File testFolder = getTestFolder("FeaturesAction.testFilters"); StringBuilder buffer = new StringBuilder();