Skip to content

Commit

Permalink
Remove support for unused attributes in feature.xml
Browse files Browse the repository at this point in the history
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 eclipse-pde/eclipse.pde#730
  • Loading branch information
HannesWell committed Mar 29, 2024
1 parent b35d8e0 commit c6b6479
Show file tree
Hide file tree
Showing 14 changed files with 219 additions and 101 deletions.
Expand Up @@ -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);
}
}
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Expand Up @@ -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() {
Expand Down
@@ -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>
@@ -0,0 +1 @@
-Dtycho-version=4.0.4-SNAPSHOT
@@ -0,0 +1 @@
bin.includes = 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>
62 changes: 62 additions & 0 deletions 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>
@@ -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());
}

}
Expand Up @@ -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() {
Expand Down
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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;
}
}

0 comments on commit c6b6479

Please sign in to comment.