Remove support for unused attributes in feature.xml

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);
                 }
             }

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<IInstallableUn
         for (IInstallableUnit bundle : bundles) {
             //TODO can a feature contain the same id in different versions? PDE Editor seems not to support this...
             if (versionedIds.add(new VersionedId(bundle.getId(), bundle.getVersion()))) {
-                boolean isFragment = bundle.getProvidedCapabilities().stream().anyMatch(
-                        capability -> 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);

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: <plugin id=\"" + wrappedBsn
-                + "\" version=\"" + wrappedVersion + "\" download-size=\"0\" install-size=\"0\" unpack=\"false\"/>";
+                + "\" version=\"" + wrappedVersion + "\"/>";
     }
 
     public String getGeneratedManifest() {

tycho-its/projects/feature.attributes.inference/.mvn/extensions.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<extensions>
+	<extension>
+		<groupId>org.eclipse.tycho</groupId>
+		<artifactId>tycho-build</artifactId>
+		<version>${tycho-version}</version>
+	</extension>
+</extensions>

tycho-its/projects/feature.attributes.inference/.mvn/maven.config

@@ -0,0 +1 @@
+-Dtycho-version=4.0.4-SNAPSHOT

tycho-its/projects/feature.attributes.inference/feature/build.properties

@@ -0,0 +1 @@
+bin.includes = feature.xml

tycho-its/projects/feature.attributes.inference/feature/feature.xml

@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<feature
+      id="feature.attributes.inference.test"
+      version="1.0.0">
+
+   <plugin
+         id="junit-jupiter-api"
+         version="0.0.0"/>
+
+   <plugin
+         id="junit-platform-suite-api"
+         download-size="0"
+         install-size="0"
+         version="0.0.0"
+         unpack="false"/>
+
+   <plugin
+         id="org.apiguardian.api"
+         download-size="0"
+         install-size="0"
+         version="0.0.0"/>
+
+</feature>

tycho-its/projects/feature.attributes.inference/pom.xml

@@ -0,0 +1,62 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+	<modelVersion>4.0.0</modelVersion>
+
+	<groupId>org.eclipse.tycho.it</groupId>
+	<artifactId>feature-attributes-inference</artifactId>
+	<version>1.0.0</version>
+	<packaging>pom</packaging>
+	<properties>
+		<tycho-version>4.0.4-SNAPSHOT</tycho-version>
+		<target-platform>http://download.eclipse.org/releases/latest/</target-platform>
+	</properties>
+
+	<modules>
+		<module>feature</module>
+	</modules>
+	<repositories>
+		<repository>
+			<id>platform</id>
+			<url>${target-platform}</url>
+			<layout>p2</layout>
+		</repository>
+	</repositories>
+
+	<build>
+		<plugins>
+			<plugin>
+				<groupId>org.eclipse.tycho</groupId>
+				<artifactId>tycho-maven-plugin</artifactId>
+				<version>${tycho-version}</version>
+				<extensions>true</extensions>
+			</plugin>
+			<plugin>
+				<groupId>org.eclipse.tycho</groupId>
+				<artifactId>tycho-source-plugin</artifactId>
+				<version>${tycho-version}</version>
+				<executions>
+					<execution>
+						<id>feature-source</id>
+						<goals>
+							<goal>feature-source</goal>
+						</goals>
+					</execution>
+				</executions>
+			</plugin>
+            <plugin>
+                <groupId>org.eclipse.tycho</groupId>
+                <artifactId>tycho-p2-plugin</artifactId>
+                <version>${tycho-version}</version>
+                <executions>
+                    <execution>
+                        <id>attach-p2-metadata</id>
+                        <phase>package</phase>
+                        <goals>
+                            <goal>p2-metadata</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+		</plugins>
+	</build>
+</project>

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<Element> 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<Element> 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<Element> 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<String> isSpecificVersion() {
+		return not(anyOf(blankOrNullString(), equalTo("0.0.0")));
+	}
+
+	private void assertAttributesOnlyElementWith(Map<String, Matcher<String>> expectedAttributes, Element element) {
+		assertEquals(0, element.getChildNodes().getLength());
+		NamedNodeMap attributes = element.getAttributes();
+		Map<String, String> 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());
+	}
+
+}

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() {

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<String> 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<JarEntry> 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;
-	}
 }