From a1b7bed96a5a6498e27d9b2580de3f75aba52215 Mon Sep 17 00:00:00 2001 From: Hannes Wellmann Date: Sat, 14 Oct 2023 09:52:07 +0200 Subject: [PATCH] Fix addition of unnecessary attributes in source-features and add Tests Add tests for reduced feature attribute inference. --- .../.mvn/extensions.xml | 8 ++ .../.mvn/maven.config | 1 + .../feature/build.properties | 1 + .../feature/feature.xml | 23 ++++ .../feature.attributes.inference/pom.xml | 62 +++++++++ .../FeatureAttributesInferenceTest.java | 125 ++++++++++++++++++ .../org/eclipse/tycho/model/PluginRef.java | 6 - .../tycho/source/SourceFeatureMojo.java | 12 +- 8 files changed, 229 insertions(+), 9 deletions(-) create mode 100644 tycho-its/projects/feature.attributes.inference/.mvn/extensions.xml create mode 100644 tycho-its/projects/feature.attributes.inference/.mvn/maven.config create mode 100644 tycho-its/projects/feature.attributes.inference/feature/build.properties create mode 100644 tycho-its/projects/feature.attributes.inference/feature/feature.xml create mode 100644 tycho-its/projects/feature.attributes.inference/pom.xml create mode 100644 tycho-its/src/test/java/org/eclipse/tycho/test/feature/FeatureAttributesInferenceTest.java diff --git a/tycho-its/projects/feature.attributes.inference/.mvn/extensions.xml b/tycho-its/projects/feature.attributes.inference/.mvn/extensions.xml new file mode 100644 index 0000000000..64513b8578 --- /dev/null +++ b/tycho-its/projects/feature.attributes.inference/.mvn/extensions.xml @@ -0,0 +1,8 @@ + + + + org.eclipse.tycho + tycho-build + ${tycho-version} + + \ No newline at end of file diff --git a/tycho-its/projects/feature.attributes.inference/.mvn/maven.config b/tycho-its/projects/feature.attributes.inference/.mvn/maven.config new file mode 100644 index 0000000000..46e617dd07 --- /dev/null +++ b/tycho-its/projects/feature.attributes.inference/.mvn/maven.config @@ -0,0 +1 @@ +-Dtycho-version=4.0.4-SNAPSHOT diff --git a/tycho-its/projects/feature.attributes.inference/feature/build.properties b/tycho-its/projects/feature.attributes.inference/feature/build.properties new file mode 100644 index 0000000000..64f93a9f0b --- /dev/null +++ b/tycho-its/projects/feature.attributes.inference/feature/build.properties @@ -0,0 +1 @@ +bin.includes = feature.xml diff --git a/tycho-its/projects/feature.attributes.inference/feature/feature.xml b/tycho-its/projects/feature.attributes.inference/feature/feature.xml new file mode 100644 index 0000000000..001263be1a --- /dev/null +++ b/tycho-its/projects/feature.attributes.inference/feature/feature.xml @@ -0,0 +1,23 @@ + + + + + + + + + + diff --git a/tycho-its/projects/feature.attributes.inference/pom.xml b/tycho-its/projects/feature.attributes.inference/pom.xml new file mode 100644 index 0000000000..763e006c48 --- /dev/null +++ b/tycho-its/projects/feature.attributes.inference/pom.xml @@ -0,0 +1,62 @@ + + + 4.0.0 + + org.eclipse.tycho.it + feature-attributes-inference + 1.0.0 + pom + + 4.0.4-SNAPSHOT + http://download.eclipse.org/releases/latest/ + + + + feature + + + + platform + ${target-platform} + p2 + + + + + + + org.eclipse.tycho + tycho-maven-plugin + ${tycho-version} + true + + + org.eclipse.tycho + tycho-source-plugin + ${tycho-version} + + + feature-source + + feature-source + + + + + + org.eclipse.tycho + tycho-p2-plugin + ${tycho-version} + + + attach-p2-metadata + package + + p2-metadata + + + + + + + \ No newline at end of file diff --git a/tycho-its/src/test/java/org/eclipse/tycho/test/feature/FeatureAttributesInferenceTest.java b/tycho-its/src/test/java/org/eclipse/tycho/test/feature/FeatureAttributesInferenceTest.java new file mode 100644 index 0000000000..f7742efb61 --- /dev/null +++ b/tycho-its/src/test/java/org/eclipse/tycho/test/feature/FeatureAttributesInferenceTest.java @@ -0,0 +1,125 @@ +/******************************************************************************* + * Copyright (c) 2023, 2023 Hannes Wellmann and others. + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Hannes Wellmann - initial API and implementation + *******************************************************************************/ +package org.eclipse.tycho.test.feature; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.blankOrNullString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertEquals; + +import java.io.File; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import org.apache.maven.it.Verifier; +import org.eclipse.tycho.test.AbstractTychoIntegrationTest; +import org.eclipse.tycho.test.util.XMLTool; +import org.hamcrest.Matcher; +import org.junit.Assert; +import org.junit.Test; +import org.w3c.dom.Attr; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NamedNodeMap; + +public class FeatureAttributesInferenceTest extends AbstractTychoIntegrationTest { + + @Test + public void testFeatureAttributesInference() throws Exception { + Verifier verifier = getVerifier("feature.attributes.inference", true, true); + verifier.executeGoals(List.of("clean", "package")); + verifier.verifyErrorFreeLog(); + File featureTargetDir = new File(verifier.getBasedir(), "feature/target"); + File featureJar = assertFileExists(featureTargetDir, "feature.attributes.inference.test-1.0.0.jar")[0]; + File featureSourceJar = assertFileExists(featureTargetDir, + "feature.attributes.inference.test-1.0.0-sources-feature.jar")[0]; + + List pluginNodes = getPluginElements(featureJar); + Assert.assertEquals(3, pluginNodes.size()); + + // Check the feature.xml in the feature-jar + assertAttributesOnlyElementWith(Map.of(// + "id", equalTo("junit-jupiter-api"), // + "version", isSpecificVersion() // + ), pluginNodes.get(0)); + + assertAttributesOnlyElementWith(Map.of(// + "id", equalTo("junit-platform-suite-api"), // + "version", isSpecificVersion(), // + "download-size", isInferredSize(), // + "install-size", isInferredSize(), // + "unpack", equalTo("false") // + ), pluginNodes.get(1)); + + assertAttributesOnlyElementWith(Map.of(// + "id", equalTo("org.apiguardian.api"), // + "version", isSpecificVersion(), // + "download-size", isInferredSize(), // + "install-size", isInferredSize() // + ), pluginNodes.get(2)); + + // Check the feature.xml in the source-feature-jar + List pluginSourceNodes = getPluginElements(featureSourceJar); + Assert.assertEquals(3, pluginSourceNodes.size()); + + assertAttributesOnlyElementWith(Map.of(// + "id", equalTo("junit-jupiter-api.source"), // + "version", isSpecificVersion() // + ), pluginSourceNodes.get(0)); + + assertAttributesOnlyElementWith(Map.of(// + "id", equalTo("junit-platform-suite-api.source"), // + "version", isSpecificVersion(), // + "download-size", equalTo("0"), // + "install-size", equalTo("0"), // + "unpack", equalTo("false") // + ), pluginSourceNodes.get(1)); + + assertAttributesOnlyElementWith(Map.of(// + "id", equalTo("org.apiguardian.api.source"), // + "version", isSpecificVersion(), // + "download-size", equalTo("0"), // + "install-size", equalTo("0"), // + "unpack", equalTo("false") // + ), pluginSourceNodes.get(2)); + } + + private List getPluginElements(File featureJar) throws Exception { + Document document = XMLTool.parseXMLDocumentFromJar(featureJar, "feature.xml"); + return XMLTool.getMatchingNodes(document, "/feature/plugin").stream().filter(Element.class::isInstance) + .map(Element.class::cast).toList(); + } + + private Matcher isInferredSize() { + return not(anyOf(blankOrNullString(), equalTo("0"))); + } + + private static Matcher isSpecificVersion() { + return not(anyOf(blankOrNullString(), equalTo("0.0.0"))); + } + + private void assertAttributesOnlyElementWith(Map> expectedAttributes, Element element) { + assertEquals(0, element.getChildNodes().getLength()); + NamedNodeMap attributes = element.getAttributes(); + Map elementAttributes = IntStream.range(0, attributes.getLength()).mapToObj(attributes::item) + .map(Attr.class::cast).collect(Collectors.toMap(Attr::getName, Attr::getValue)); + + expectedAttributes.forEach((name, expectation) -> assertThat(elementAttributes.get(name), expectation)); + assertEquals(expectedAttributes.size(), elementAttributes.size()); + } + +} diff --git a/tycho-metadata-model/src/main/java/org/eclipse/tycho/model/PluginRef.java b/tycho-metadata-model/src/main/java/org/eclipse/tycho/model/PluginRef.java index 8afd13d9a1..d3fc7842fd 100644 --- a/tycho-metadata-model/src/main/java/org/eclipse/tycho/model/PluginRef.java +++ b/tycho-metadata-model/src/main/java/org/eclipse/tycho/model/PluginRef.java @@ -85,12 +85,6 @@ public void setArch(String arch) { dom.setAttribute("arch", arch); } - @Deprecated - public boolean hasUnpack() { - String value = dom.getAttributeValue("unpack"); - return value != null && !value.isBlank(); - } - /** * @deprecated The installation format (packed/unpacked) shall be specified through the bundle's * Eclipse-BundleShape manifest header. The feature.xml's unpack attribute may not diff --git a/tycho-source-plugin/src/main/java/org/eclipse/tycho/source/SourceFeatureMojo.java b/tycho-source-plugin/src/main/java/org/eclipse/tycho/source/SourceFeatureMojo.java index 1a5cd6e2e5..a8de90ce17 100644 --- a/tycho-source-plugin/src/main/java/org/eclipse/tycho/source/SourceFeatureMojo.java +++ b/tycho-source-plugin/src/main/java/org/eclipse/tycho/source/SourceFeatureMojo.java @@ -564,8 +564,12 @@ protected void addPlugin(Feature sourceFeature, P2ResolutionResult result, Plugi PluginRef sourceRef = new PluginRef("plugin"); sourceRef.setId(sourceBundle.getId()); sourceRef.setVersion(sourceBundle.getVersion()); - sourceRef.setDownloadSize(0); - sourceRef.setInstallSize(0); + if (pluginRef.hasDownloadSize()) { + sourceRef.setDownloadSize(0); + } + if (pluginRef.hasInstallSize()) { + sourceRef.setInstallSize(0); + } if (pluginRef.getOs() != null) { sourceRef.setOs(pluginRef.getOs()); } @@ -575,7 +579,9 @@ protected void addPlugin(Feature sourceFeature, P2ResolutionResult result, Plugi if (pluginRef.getArch() != null) { sourceRef.setArch(pluginRef.getArch()); } - if (pluginRef.hasUnpack()) { + if (pluginRef.hasDownloadSize() && pluginRef.hasInstallSize()) { + // PDE editor does not set unpack="true" but just removes the unpack attribute. + // Thus an absent attribute can also mean unpack="true" and therefore the presence of other attributes has to be checked in order to check if the unused attributes are still set. sourceRef.setUnpack(false); } sourceFeature.addPlugin(sourceRef);