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 Oct 12, 2023
1 parent 16eba00 commit cfd6f72
Show file tree
Hide file tree
Showing 9 changed files with 7 additions and 117 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,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.equinox.p2.publisher.eclipse.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
Expand Up @@ -85,42 +85,6 @@ 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() {
return dom;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,10 @@
*******************************************************************************/
package org.eclipse.tycho.packaging;

import java.io.File;
import java.io.IOException;
import java.util.Enumeration;
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 Down Expand Up @@ -84,12 +79,6 @@ 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);
}

for (FeatureRef featureRef : feature.getIncludedFeatures()) {
Expand Down Expand Up @@ -156,36 +145,4 @@ private static String quote(String nullableString) {
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());
}

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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@
*******************************************************************************/
package org.eclipse.tycho.packaging;

import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.StandardCopyOption;
import java.util.Map;

import org.codehaus.plexus.PlexusContainer;
Expand All @@ -32,7 +31,6 @@
import org.eclipse.tycho.core.ArtifactDependencyVisitor;
import org.eclipse.tycho.core.FeatureDescription;
import org.eclipse.tycho.core.PluginDescription;
import org.eclipse.tycho.model.PluginRef;

/**
* Assembles standard eclipse update site directory structure on local filesystem.
Expand Down Expand Up @@ -154,7 +152,7 @@ public void visitPlugin(PluginDescription plugin) {
location = plugin.getLocation(true);
}

if (unpackPlugins && isDirectoryShape(plugin, location)) {
if (unpackPlugins && location.isDirectory()) {
// need a directory
File outputJar = getOutputFile(PLUGINS_DIR, bundleId, version, null);

Expand All @@ -175,11 +173,6 @@ public void visitPlugin(PluginDescription plugin) {
}
}

protected boolean isDirectoryShape(PluginDescription plugin, File location) {
PluginRef pluginRef = plugin.getPluginRef();
return ((pluginRef != null && pluginRef.isUnpack()) || location.isDirectory());
}

private void unpackJar(File location, File outputJar) {
ZipUnArchiver unzip;
FileLockService fileLockService;
Expand Down Expand Up @@ -214,12 +207,8 @@ private void copyDir(File location, File outputJar) {
}

private void copyUrl(String source, File destination) {
try {
URL url = new URL(source);
try (InputStream is = url.openStream();
OutputStream os = new BufferedOutputStream(new FileOutputStream(destination))) {
is.transferTo(os);
}
try (InputStream is = new URL(source).openStream()) {
Files.copy(is, destination.toPath(), StandardCopyOption.REPLACE_EXISTING);
} catch (IOException e) {
throw new RuntimeException("Could not copy URL contents", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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());
Expand All @@ -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<FeatureRef> feature(final String id, final String version) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@

<plugin
id="org.junit4"
download-size="0"
install-size="0"
version="4.8.1.qualifier"
unpack="false"/>
version="4.8.1.qualifier"/>

</feature>
Original file line number Diff line number Diff line change
Expand Up @@ -564,8 +564,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());
}
Expand All @@ -575,8 +573,6 @@ protected void addPlugin(Feature sourceFeature, P2ResolutionResult result, Plugi
if (pluginRef.getArch() != null) {
sourceRef.setArch(pluginRef.getArch());
}
sourceRef.setUnpack(false);

sourceFeature.addPlugin(sourceRef);
}

Expand Down

0 comments on commit cfd6f72

Please sign in to comment.