From c6b647983508b202e58835cbcb023534091d8c9e Mon Sep 17 00:00:00 2001 From: Hannes Wellmann Date: Thu, 12 Oct 2023 23:19:59 +0200 Subject: [PATCH] Remove support for unused attributes in feature.xml This removes support for the following attributes of plugin elements in feature.xml: - install/download-size - unpack - fragment The written values are not relevant anymore and therefore can be removed. In the context of https://github.com/eclipse-pde/eclipse.pde/issues/730 --- .../AbstractArtifactDependencyWalker.java | 1 - .../tycho/p2/resolver/FeatureGenerator.java | 6 - .../tycho/p2/resolver/WrappedArtifact.java | 2 +- .../.mvn/extensions.xml | 8 ++ .../.mvn/maven.config | 1 + .../feature/build.properties | 1 + .../feature/feature.xml | 23 ++++ .../feature.attributes.inference/pom.xml | 62 ++++++++++ .../FeatureAttributesInferenceTest.java | 110 ++++++++++++++++++ .../org/eclipse/tycho/model/PluginRef.java | 36 +----- .../packaging/FeatureXmlTransformer.java | 55 ++------- .../packaging/FeatureXmlTransformerTest.java | 6 - .../featureXmlVersionExpansion/feature.xml | 5 +- .../tycho/source/SourceFeatureMojo.java | 4 - 14 files changed, 219 insertions(+), 101 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-core/src/main/java/org/eclipse/tycho/core/osgitools/AbstractArtifactDependencyWalker.java b/tycho-core/src/main/java/org/eclipse/tycho/core/osgitools/AbstractArtifactDependencyWalker.java index d8272da98d..40cc85e87f 100644 --- a/tycho-core/src/main/java/org/eclipse/tycho/core/osgitools/AbstractArtifactDependencyWalker.java +++ b/tycho-core/src/main/java/org/eclipse/tycho/core/osgitools/AbstractArtifactDependencyWalker.java @@ -147,7 +147,6 @@ protected void traverseProduct(ProductConfiguration product, ArtifactDependencyV ref.setOs(os); ref.setWs(ws); ref.setArch(arch); - ref.setUnpack(true); traversePlugin(ref, visitor, visited); } } diff --git a/tycho-core/src/main/java/org/eclipse/tycho/p2/resolver/FeatureGenerator.java b/tycho-core/src/main/java/org/eclipse/tycho/p2/resolver/FeatureGenerator.java index b9a0c459cf..23decd6f3f 100644 --- a/tycho-core/src/main/java/org/eclipse/tycho/p2/resolver/FeatureGenerator.java +++ b/tycho-core/src/main/java/org/eclipse/tycho/p2/resolver/FeatureGenerator.java @@ -45,7 +45,6 @@ import org.eclipse.equinox.p2.metadata.IInstallableUnit; import org.eclipse.equinox.p2.metadata.IVersionedId; import org.eclipse.equinox.p2.metadata.VersionedId; -import org.eclipse.tycho.p2maven.tmp.BundlesAction; import org.eclipse.equinox.p2.publisher.eclipse.Feature; import org.eclipse.tycho.core.MavenModelFacade; import org.eclipse.tycho.core.MavenModelFacade.MavenLicense; @@ -93,14 +92,9 @@ private static Feature createFeature(Element featureElement, List capability.getNamespace().equals(BundlesAction.CAPABILITY_NS_OSGI_FRAGMENT)); Element pluginElement = doc.createElement(ELEMENT_PLUGIN); pluginElement.setAttribute(ATTR_ID, bundle.getId()); pluginElement.setAttribute(ATTR_VERSION, bundle.getVersion().toString()); - if (isFragment) { - pluginElement.setAttribute(ATTR_FRAGMENT, String.valueOf(true)); - } //TODO can we check form the IU if we need to unpack? Or is the bundle info required here? Does it actually matter at all? pluginElement.setAttribute(ATTR_UNPACK, String.valueOf(false)); featureElement.appendChild(pluginElement); diff --git a/tycho-core/src/main/java/org/eclipse/tycho/p2/resolver/WrappedArtifact.java b/tycho-core/src/main/java/org/eclipse/tycho/p2/resolver/WrappedArtifact.java index 6ecc5b68cb..4c9484d8a7 100644 --- a/tycho-core/src/main/java/org/eclipse/tycho/p2/resolver/WrappedArtifact.java +++ b/tycho-core/src/main/java/org/eclipse/tycho/p2/resolver/WrappedArtifact.java @@ -76,7 +76,7 @@ public String getWrappedVersion() { public String getReferenceHint() { return "The artifact can be referenced in feature files with the following data: "; + + "\" version=\"" + wrappedVersion + "\"/>"; } public String getGeneratedManifest() { 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..9680006bc0 --- /dev/null +++ b/tycho-its/src/test/java/org/eclipse/tycho/test/feature/FeatureAttributesInferenceTest.java @@ -0,0 +1,110 @@ +/******************************************************************************* + * 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() // + ), pluginNodes.get(1)); + + assertAttributesOnlyElementWith(Map.of(// + "id", equalTo("org.apiguardian.api"), // + "version", isSpecificVersion() // + ), 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() // + ), pluginSourceNodes.get(1)); + + assertAttributesOnlyElementWith(Map.of(// + "id", equalTo("org.apiguardian.api.source"), // + "version", isSpecificVersion() // + ), 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 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 1ade5c933d..a875a61bac 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,40 +85,8 @@ public void setArch(String arch) { dom.setAttribute("arch", arch); } - /** - * @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 - * be supported in a future version of Tycho. - */ - @Deprecated - public boolean isUnpack() { - return Boolean.parseBoolean(dom.getAttributeValue("unpack")); - } - - /** - * @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 - * be supported in a future version of Tycho. - */ - @Deprecated - public void setUnpack(boolean unpack) { - dom.setAttribute("unpack", Boolean.toString(unpack)); - } - - public long getDownloadSize() { - return Long.parseLong(dom.getAttributeValue("download-size")); - } - - public void setDownloadSize(long size) { - dom.setAttribute("download-size", Long.toString(size)); - } - - public long getInstallSize() { - return Long.parseLong(dom.getAttributeValue("install-size")); - } - - public void setInstallSize(long size) { - dom.setAttribute("install-size", Long.toString(size)); + public void removeAttribute(String attributeName) { + dom.removeAttribute(attributeName); } Element getDom() { diff --git a/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/FeatureXmlTransformer.java b/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/FeatureXmlTransformer.java index ef56b2192e..5645af4892 100644 --- a/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/FeatureXmlTransformer.java +++ b/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/FeatureXmlTransformer.java @@ -13,15 +13,11 @@ *******************************************************************************/ package org.eclipse.tycho.packaging; -import java.io.File; -import java.io.IOException; -import java.util.Enumeration; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.function.BinaryOperator; import java.util.function.Function; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; import java.util.stream.Collectors; import org.apache.maven.plugin.MojoFailureException; @@ -41,7 +37,12 @@ @Component(role = FeatureXmlTransformer.class) public class FeatureXmlTransformer { - private static final int KBYTE = 1024; + /** + * Obsolete attributes that are to remove. + * + * @see https://github.com/eclipse-pde/eclipse.pde/issues/730 + */ + private static final List OBSOLETE_PLUGIN_ATTRIBUTES = List.of("unpack", "download-size", "install-size"); @Requirement private Logger log; @@ -84,12 +85,7 @@ public Feature expandReferences(Feature feature, TargetPlatform targetPlatform) } ArtifactKey plugin = resolvePluginReference(targetPlatform, pluginRef, version); pluginRef.setVersion(plugin.getVersion()); - - File location = targetPlatform.getArtifactLocation(plugin); - if (location == null) { - throw new MojoFailureException("location is missing for plugin " + plugin); - } - setDownloadAndInstallSize(pluginRef, location); + OBSOLETE_PLUGIN_ATTRIBUTES.forEach(pluginRef::removeAttribute); } for (FeatureRef featureRef : feature.getIncludedFeatures()) { @@ -150,42 +146,11 @@ private ArtifactKey resolveFeatureReference(TargetPlatform targetPlatform, Featu } private static String quote(String nullableString) { - if (nullableString == null) + if (nullableString == null) { return null; - else - return "\"" + nullableString + "\""; - } - - private void setDownloadAndInstallSize(PluginRef pluginRefToEdit, File artifact) { - // TODO 375111 optionally disable this? - long downloadSize = 0; - long installSize = 0; - if (artifact.isFile()) { - installSize = getInstallSize(artifact); - downloadSize = artifact.length(); } else { - log.info("Download/install size is not calculated for directory based bundle " + pluginRefToEdit.getId()); + return "\"" + nullableString + "\""; } - - pluginRefToEdit.setDownloadSize(downloadSize / KBYTE); - pluginRefToEdit.setInstallSize(installSize / KBYTE); } - protected long getInstallSize(File location) { - long installSize = 0; - try (var locked = fileLockService.lock(location); // - JarFile jar = new JarFile(location);) { - Enumeration entries = jar.entries(); - while (entries.hasMoreElements()) { - JarEntry entry = entries.nextElement(); - long entrySize = entry.getSize(); - if (entrySize > 0) { - installSize += entrySize; - } - } - } catch (IOException e) { - throw new RuntimeException("Could not determine installation size of file " + location, e); - } - return installSize; - } } diff --git a/tycho-packaging-plugin/src/test/java/org/eclipse/tycho/packaging/FeatureXmlTransformerTest.java b/tycho-packaging-plugin/src/test/java/org/eclipse/tycho/packaging/FeatureXmlTransformerTest.java index dc260d1335..d6d9ac7662 100644 --- a/tycho-packaging-plugin/src/test/java/org/eclipse/tycho/packaging/FeatureXmlTransformerTest.java +++ b/tycho-packaging-plugin/src/test/java/org/eclipse/tycho/packaging/FeatureXmlTransformerTest.java @@ -15,7 +15,6 @@ import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -50,7 +49,6 @@ public static void initTestResources() throws Exception { junit4JarLocation = TestUtil.getTestResourceLocation("eclipse/plugins/org.junit4_4.8.1.v20100302.jar"); } - @SuppressWarnings("deprecation") @Test public void testExpandReferences() throws Exception { subject = new FeatureXmlTransformer(new SilentLog(), new NoopFileLockService()); @@ -69,10 +67,6 @@ public void testExpandReferences() throws Exception { assertThat(feature.getPlugins(), hasItem(plugin("org.junit4", "4.8.1.v20100302"))); PluginRef plugin = feature.getPlugins().get(0); assertEquals("org.junit4", plugin.getId()); - assertEquals(1L, plugin.getDownloadSize()); // 1720 bytes rounded to kiB - assertEquals(2L, plugin.getInstallSize()); // 2419 bytes rounded to kiB // TODO shouldn't - // installSize=downloadSize for unpack=false? - assertFalse(plugin.isUnpack()); } private static Matcher feature(final String id, final String version) { diff --git a/tycho-packaging-plugin/src/test/resources/projects/featureXmlVersionExpansion/feature.xml b/tycho-packaging-plugin/src/test/resources/projects/featureXmlVersionExpansion/feature.xml index eeed7534f3..490aa91e40 100644 --- a/tycho-packaging-plugin/src/test/resources/projects/featureXmlVersionExpansion/feature.xml +++ b/tycho-packaging-plugin/src/test/resources/projects/featureXmlVersionExpansion/feature.xml @@ -10,9 +10,6 @@ + version="4.8.1.qualifier"/> 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 e9695eef62..fdd408ecb8 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 @@ -568,8 +568,6 @@ 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.getOs() != null) { sourceRef.setOs(pluginRef.getOs()); } @@ -579,8 +577,6 @@ protected void addPlugin(Feature sourceFeature, P2ResolutionResult result, Plugi if (pluginRef.getArch() != null) { sourceRef.setArch(pluginRef.getArch()); } - sourceRef.setUnpack(false); - sourceFeature.addPlugin(sourceRef); }