Skip to content

Commit

Permalink
Remove unnecessary attributes of plugin element in feature.xml
Browse files Browse the repository at this point in the history 
This commit removes some of the unused attributes of plugin element,
 which are redundant and are never read from a feature.xml file:
 install-size, fragment, download size and unpack attributes.
  • Loading branch information
@alshamams @HannesWell
alshamams authored and HannesWell committed Nov 1, 2023
1 parent 6465a65 commit 67a7a83
Show file tree
Hide file tree
Showing 16 changed files with 16 additions and 379 deletions.
18 changes: 5 additions & 13 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/builders/FeatureErrorReporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.eclipse.pde.internal.core.ibundle.IBundlePluginModel;
import org.eclipse.pde.internal.core.ibundle.IManifestHeader;
import org.eclipse.pde.internal.core.ifeature.IFeatureModel;
import org.eclipse.pde.internal.core.util.CoreUtility;
import org.eclipse.pde.internal.core.util.IdUtil;
import org.w3c.dom.Attr;
import org.w3c.dom.Element;
Expand Down Expand Up @@ -110,12 +109,11 @@ private void validatePlugins(Element parent) {
assertAttributeDefined(plugin, "id", CompilerFlags.ERROR); //$NON-NLS-1$
assertAttributeDefined(plugin, "version", CompilerFlags.ERROR); //$NON-NLS-1$
NamedNodeMap attributes = plugin.getAttributes();
boolean isFragment = plugin.getAttribute("fragment").equals("true"); //$NON-NLS-1$ //$NON-NLS-2$
for (int j = 0; j < attributes.getLength(); j++) {
Attr attr = (Attr) attributes.item(j);
String name = attr.getName();
if (name.equals("id")) { //$NON-NLS-1$
validatePluginExists(plugin, attr, isFragment);
validatePluginExists(plugin, attr);
} else if (name.equals("version")) { //$NON-NLS-1$
validateVersionAttribute(plugin, attr);
validateVersion(plugin, attr);
Expand Down Expand Up @@ -153,7 +151,7 @@ private void validateImports(Element parent) {
} else if (plugin != null && feature != null) {
reportExclusiveAttributes(element, "plugin", "feature", CompilerFlags.ERROR); //$NON-NLS-1$//$NON-NLS-2$
} else if (plugin != null) {
validatePluginExists(element, plugin, false);
validatePluginExists(element, plugin);
} else if (feature != null) {
validateFeatureExists(element, feature);
}
Expand Down Expand Up @@ -382,7 +380,7 @@ private void validateFeatureAttributes(Element element) {
if (name.equals("primary")) { //$NON-NLS-1$
reportDeprecatedAttribute(element, (Attr) attributes.item(i));
} else if (name.equals("plugin")) { //$NON-NLS-1$
validatePluginExists(element, (Attr) attributes.item(i), false);
validatePluginExists(element, (Attr) attributes.item(i));
}
}
}
Expand All @@ -405,12 +403,12 @@ protected boolean validateFeatureID(Element element, Attr attr) {
return true;
}

private void validatePluginExists(Element element, Attr attr, boolean isFragment) {
private void validatePluginExists(Element element, Attr attr) {
String id = attr.getValue();
int severity = CompilerFlags.getFlag(fProject, CompilerFlags.F_UNRESOLVED_PLUGINS);
if (severity != CompilerFlags.IGNORE) {
IPluginModelBase model = PluginRegistry.findModel(id);
if (model == null || !model.isEnabled() || (isFragment && !model.isFragmentModel()) || (!isFragment && model.isFragmentModel())) {
if (model == null || !model.isEnabled() || model.isFragmentModel()) {
VirtualMarker marker = report(NLS.bind(PDECoreMessages.Builders_Feature_reference, id), getLine(element, attr.getName()), severity, PDEMarkerFactory.CAT_OTHER);
addMarkerAttribute(marker, PDEMarkerFactory.compilerKey, CompilerFlags.F_UNRESOLVED_PLUGINS);
}
Expand Down Expand Up @@ -460,12 +458,6 @@ private void validateUnpack(Element parent) {
}
}
}

if ("true".equals(unpack) && !CoreUtility.guessUnpack(pModel.getBundleDescription())) {//$NON-NLS-1$
String message = NLS.bind(PDECoreMessages.Builders_Feature_missingUnpackFalse, (new String[] {parent.getAttribute("id"), "unpack=\"false\""})); //$NON-NLS-1$ //$NON-NLS-2$
VirtualMarker marker = report(message, getLine(parent), severity, PDEMarkerFactory.CAT_OTHER);
addMarkerAttribute(marker, PDEMarkerFactory.compilerKey, CompilerFlags.F_UNRESOLVED_PLUGINS);
}
}

/**
Expand Down
10 changes: 2 additions & 8 deletions ...rg.eclipse.pde.core/src/org/eclipse/pde/internal/core/exports/FeatureExportOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ public class FeatureExportOperation extends Job {

protected State fStateCopy;

protected static String FEATURE_POST_PROCESSING = "features.postProcessingSteps.properties"; //$NON-NLS-1$
protected static String PLUGIN_POST_PROCESSING = "plugins.postProcessingSteps.properties"; //$NON-NLS-1$
protected static final String FEATURE_POST_PROCESSING = "features.postProcessingSteps.properties"; //$NON-NLS-1$
protected static final String PLUGIN_POST_PROCESSING = "plugins.postProcessingSteps.properties"; //$NON-NLS-1$

private static final String[] GENERIC_CONFIG = new String[] {"*", "*", "*", ""}; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
protected FeatureExportInfo fInfo;
Expand Down Expand Up @@ -1159,7 +1159,6 @@ protected void createFeature(String featureID, String featureLocation, String[][
plugin.setAttribute("id", bundle.getSymbolicName()); //$NON-NLS-1$
plugin.setAttribute("version", bundle.getVersion().toString()); //$NON-NLS-1$
setFilterAttributes(plugin, currentConfig);
setAdditionalAttributes(plugin, bundle);
root.appendChild(plugin);

if (fInfo.exportSource && fInfo.exportSourceBundle) {
Expand All @@ -1168,7 +1167,6 @@ protected void createFeature(String featureID, String featureLocation, String[][
plugin.setAttribute("id", bundle.getSymbolicName() + ".source"); //$NON-NLS-1$ //$NON-NLS-2$
plugin.setAttribute("version", bundle.getVersion().toString()); //$NON-NLS-1$
setFilterAttributes(plugin, currentConfig);
setAdditionalAttributes(plugin, bundle);
root.appendChild(plugin);
} else // include the .source plugin, if available
{
Expand All @@ -1179,7 +1177,6 @@ protected void createFeature(String featureID, String featureLocation, String[][
plugin.setAttribute("id", bundle.getSymbolicName()); //$NON-NLS-1$
plugin.setAttribute("version", bundle.getVersion().toString()); //$NON-NLS-1$
setFilterAttributes(plugin, currentConfig);
setAdditionalAttributes(plugin, bundle);
root.appendChild(plugin);
}
}
Expand All @@ -1203,9 +1200,6 @@ static boolean isWorkspacePlugin(BundleDescription bundle) {
return entry != null && Arrays.asList(entry.getWorkspaceModels()).contains(PluginRegistry.findModel(bundle));
}

protected void setAdditionalAttributes(Element plugin, BundleDescription bundle) {
}

public static void errorFound() {
fHasErrors = true;
}
Expand Down
8 changes: 0 additions & 8 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/exports/PluginExportOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
import org.eclipse.osgi.service.resolver.State;
import org.eclipse.pde.core.plugin.TargetPlatform;
import org.eclipse.pde.internal.core.TargetPlatformHelper;
import org.eclipse.pde.internal.core.util.CoreUtility;
import org.w3c.dom.Element;

public class PluginExportOperation extends FeatureBasedExportOperation {

Expand Down Expand Up @@ -61,10 +59,4 @@ protected boolean shouldAddPlugin(BundleDescription bundle, Dictionary<String, S
// always include plug-ins, even ones with environment conflicts
return true;
}

@Override
protected void setAdditionalAttributes(Element plugin, BundleDescription bundle) {
plugin.setAttribute("unpack", Boolean.toString(CoreUtility.guessUnpack(bundle))); //$NON-NLS-1$
}

}
6 changes: 0 additions & 6 deletions ...rg.eclipse.pde.core/src/org/eclipse/pde/internal/core/exports/ProductExportOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import org.eclipse.pde.internal.core.iproduct.ILauncherInfo;
import org.eclipse.pde.internal.core.iproduct.IProduct;
import org.eclipse.pde.internal.core.util.CoreUtility;
import org.w3c.dom.Element;

public class ProductExportOperation extends FeatureExportOperation {
private static final String STATUS_MESSAGE = "!MESSAGE"; //$NON-NLS-1$
Expand Down Expand Up @@ -489,9 +488,4 @@ private String createMacInfoPList() {
}
return null;
}

@Override
protected void setAdditionalAttributes(Element plugin, BundleDescription bundle) {
plugin.setAttribute("unpack", Boolean.toString(CoreUtility.guessUnpack(bundle))); //$NON-NLS-1$
}
}
58 changes: 0 additions & 58 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/feature/FeatureData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ public class FeatureData extends IdentifiableObject implements IFeatureData {
private String nl;
private String arch;
private String filter;
private long downloadSize;
private long installSize;

public FeatureData() {
}
Expand All @@ -42,8 +40,6 @@ protected void reset() {
ws = null;
nl = null;
arch = null;
downloadSize = 0;
installSize = 0;
}

@Override
Expand All @@ -64,8 +60,6 @@ protected void parse(Node node) {
nl = getNodeAttribute(node, "nl"); //$NON-NLS-1$
arch = getNodeAttribute(node, "arch"); //$NON-NLS-1$
filter = getNodeAttribute(node, "filter"); //$NON-NLS-1$
downloadSize = getIntegerAttribute(node, "download-size"); //$NON-NLS-1$
installSize = getIntegerAttribute(node, "install-size"); //$NON-NLS-1$
}

protected void writeAttributes(String indent2, PrintWriter writer) {
Expand All @@ -75,10 +69,6 @@ protected void writeAttributes(String indent2, PrintWriter writer) {
writeAttribute("nl", getNL(), indent2, writer); //$NON-NLS-1$
writeAttribute("arch", getArch(), indent2, writer); //$NON-NLS-1$
writeAttribute("filter", getFilter(), indent2, writer); //$NON-NLS-1$
writer.println();
writer.print(indent2 + "download-size=\"" + getDownloadSize() + "\""); //$NON-NLS-1$ //$NON-NLS-2$
writer.println();
writer.print(indent2 + "install-size=\"" + getInstallSize() + "\""); //$NON-NLS-1$ //$NON-NLS-2$
}

private void writeAttribute(String attribute, String value, String indent2, PrintWriter writer) {
Expand Down Expand Up @@ -205,48 +195,6 @@ public void setFilter(String filter) throws CoreException {
firePropertyChanged(P_FILTER, oldValue, filter);
}

/**
* Gets the downloadSize.
* @return Returns a int
*/
@Override
public long getDownloadSize() {
return downloadSize;
}

/**
* Sets the downloadSize.
* @param downloadSize The downloadSize to set
*/
@Override
public void setDownloadSize(long downloadSize) throws CoreException {
ensureModelEditable();
Object oldValue = Long.valueOf(this.downloadSize);
this.downloadSize = downloadSize;
firePropertyChanged(P_DOWNLOAD_SIZE, oldValue, Long.valueOf(downloadSize));
}

/**
* Gets the installSize.
* @return Returns a int
*/
@Override
public long getInstallSize() {
return installSize;
}

/**
* Sets the installSize.
* @param installSize The installSize to set
*/
@Override
public void setInstallSize(long installSize) throws CoreException {
ensureModelEditable();
Object oldValue = Long.valueOf(this.installSize);
this.installSize = installSize;
firePropertyChanged(P_INSTALL_SIZE, oldValue, Long.valueOf(installSize));
}

@Override
public void restoreProperty(String name, Object oldValue, Object newValue) throws CoreException {
switch (name) {
Expand All @@ -262,12 +210,6 @@ public void restoreProperty(String name, Object oldValue, Object newValue) throw
case P_ARCH:
setArch((String) newValue);
break;
case P_DOWNLOAD_SIZE:
setDownloadSize(newValue != null ? ((Integer) newValue).intValue() : 0);
break;
case P_INSTALL_SIZE:
setInstallSize(newValue != null ? ((Integer) newValue).intValue() : 0);
break;
default:
super.restoreProperty(name, oldValue, newValue);
break;
Expand Down
62 changes: 6 additions & 56 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/feature/FeaturePlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@
package org.eclipse.pde.internal.core.feature;

import java.io.PrintWriter;
import java.util.Arrays;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.pde.core.plugin.IFragment;
import org.eclipse.pde.core.plugin.IFragmentModel;
import org.eclipse.pde.core.plugin.IPluginBase;
import org.eclipse.pde.core.plugin.IPluginModel;
import org.eclipse.pde.core.plugin.IPluginModelBase;
import org.eclipse.pde.core.plugin.ModelEntry;
import org.eclipse.pde.core.plugin.PluginRegistry;
Expand All @@ -30,23 +29,17 @@

public class FeaturePlugin extends FeatureData implements IFeaturePlugin {
private static final long serialVersionUID = 1L;
private boolean fFragment;
private String fVersion;
private boolean fUnpack = true;

public FeaturePlugin() {
}

@Override
protected void reset() {
super.reset();
fVersion = null;
fFragment = false;
}

@Override
public boolean isFragment() {
return fFragment;
return getPluginBase() instanceof IFragment;
}

public IPluginBase getPluginBase() {
Expand All @@ -61,19 +54,12 @@ public IPluginBase getPluginBase() {
ModelEntry entry = PluginRegistry.findEntry(id);
// if no plug-ins match the id, entry == null
if (entry != null) {
IPluginModelBase bases[] = entry.getActiveModels();
for (IPluginModelBase base : bases) {
if (base.getPluginBase().getVersion().equals(version)) {
model = base;
break;
}
}
model = Arrays.stream(entry.getActiveModels())
.filter(base -> base.getPluginBase().getVersion().equals(version))
.findFirst().orElse(null);
}
}
if (fFragment && model instanceof IFragmentModel) {
return model.getPluginBase();
}
if (!fFragment && model instanceof IPluginModel) {
if (model != null) {
return model.getPluginBase();
}
return null;
Expand All @@ -84,11 +70,6 @@ public String getVersion() {
return fVersion;
}

@Override
public boolean isUnpack() {
return fUnpack;
}

@Override
public void setVersion(String version) throws CoreException {
ensureModelEditable();
Expand All @@ -97,14 +78,6 @@ public void setVersion(String version) throws CoreException {
firePropertyChanged(this, P_VERSION, oldValue, version);
}

@Override
public void setUnpack(boolean unpack) throws CoreException {
ensureModelEditable();
boolean oldValue = fUnpack;
this.fUnpack = unpack;
firePropertyChanged(this, P_UNPACK, Boolean.valueOf(oldValue), Boolean.valueOf(unpack));
}

@Override
public void restoreProperty(String name, Object oldValue, Object newValue) throws CoreException {
if (name.equals(P_VERSION)) {
Expand All @@ -114,30 +87,16 @@ public void restoreProperty(String name, Object oldValue, Object newValue) throw
}
}

public void setFragment(boolean fragment) throws CoreException {
ensureModelEditable();
this.fFragment = fragment;
}

@Override
protected void parse(Node node) {
super.parse(node);
fVersion = getNodeAttribute(node, "version"); //$NON-NLS-1$
String f = getNodeAttribute(node, "fragment"); //$NON-NLS-1$
if (f != null && f.equalsIgnoreCase("true")) { //$NON-NLS-1$
fFragment = true;
}
String unpack = getNodeAttribute(node, "unpack"); //$NON-NLS-1$
if (unpack != null && unpack.equalsIgnoreCase("false")) { //$NON-NLS-1$
fUnpack = false;
}
}

public void loadFrom(IPluginBase plugin) {
id = plugin.getId();
label = plugin.getTranslatedName();
fVersion = plugin.getVersion();
fFragment = plugin instanceof IFragment;
}

@Override
Expand All @@ -149,16 +108,7 @@ public void write(String indent, PrintWriter writer) {
writer.println();
writer.print(indent2 + "version=\"" + getVersion() + "\""); //$NON-NLS-1$ //$NON-NLS-2$
}
if (isFragment()) {
writer.println();
writer.print(indent2 + "fragment=\"true\""); //$NON-NLS-1$
}
if (!isUnpack()) {
writer.println();
writer.print(indent2 + "unpack=\"false\""); //$NON-NLS-1$
}
writer.println("/>"); //$NON-NLS-1$
//writer.println(indent + "</plugin>");
}

@Override
Expand Down
Loading

0 comments on commit 67a7a83

Please sign in to comment.