Fix addition of unnecessary attributes in source-features and add Tests

Add tests for reduced feature attribute inference.

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,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<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(), //
+				"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<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(), //
+				"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<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 Matcher<String> isInferredSize() {
+		return not(anyOf(blankOrNullString(), equalTo("0")));
+	}
+
+	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,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

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