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 28, 2024
1 parent b35d8e0 commit bc1f509
Show file tree
Hide file tree
Showing 14 changed files with 218 additions and 104 deletions.
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-Dtycho-version=4.0.4-SNAPSHOT
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bin.includes = feature.xml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -85,43 +85,7 @@ 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));
}

Element getDom() {
public Element getDom() {
return dom;
}
}
Original file line number Diff line number Diff line change
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.getDom()::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 bc1f509

Please sign in to comment.