Skip to content

Commit

Permalink
Always sort BREE header values in ascending Java version order
Browse files Browse the repository at this point in the history 
Remove the ability to manually move required execution environments in
the Manifest Editor. Instead always sort the values in ascending Java
version order.

From a technical perspective the order of the values of the
'Bundle-RequiredExecutionEnvironment' header is irrelevant, similar to
for example the 'Import-Package' header.
Removing the buttons to reorder the values simplifies the UI.

Values of existing Plug-in projects are displayed in sorted order
regardless of if they are actually sorted in the MANIFEST.MF or not. On
the next write all values will be written in sorted order.
  • Loading branch information
@HannesWell
HannesWell committed Jul 7, 2024
1 parent 593f1d8 commit e69ca13
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 55 deletions.
34 changes: 29 additions & 5 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/VMUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,17 @@
package org.eclipse.pde.internal.core.util;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.Map;
import java.util.Properties;
import java.util.stream.Collectors;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.launching.IVMInstall;
import org.eclipse.jdt.launching.IVMInstallType;
import org.eclipse.jdt.launching.JavaRuntime;
Expand Down Expand Up @@ -96,12 +101,31 @@ public static String getVMInstallName(IExecutionEnvironment ee) throws CoreExcep
IPath containerPath = JavaRuntime.newJREContainerPath(ee);
IVMInstall vmi = JavaRuntime.getVMInstall(containerPath);
if (vmi == null) {
throw new CoreException(createErrorStatus(NLS.bind(UtilMessages.VMHelper_noJreForExecEnv, ee.getId())));
throw new CoreException(Status.error(NLS.bind(UtilMessages.VMHelper_noJreForExecEnv, ee.getId())));
}
return vmi.getName();
}

public static IStatus createErrorStatus(String message) {
return Status.error(message, null);
}
private static final Map<String, Double> JAVA_VERSION_OF_EE = Arrays.stream(getExecutionEnvironments())
.collect(Collectors.toUnmodifiableMap(IExecutionEnvironment::getId, ee -> {
return switch (ee.getId()) {
// The EE's handled explicitly have unexpected java targets.
// See https://github.com/eclipse-equinox/equinox/pull/655
case "J2SE-1.2" -> 1.2; //$NON-NLS-1$
case "J2SE-1.3" -> 1.3; //$NON-NLS-1$
case "J2SE-1.4" -> 1.4; //$NON-NLS-1$
default -> {
Properties properties = ee.getProfileProperties();
Object target = properties != null //
? properties.get(JavaCore.COMPILER_CODEGEN_TARGET_PLATFORM)
: null;
yield target instanceof String version ? Double.parseDouble(version) : 0;
}
};
}));

public static final Comparator<String> ASCENDING_EE_JAVA_VERSION = Comparator
.comparingDouble((String ee) -> JAVA_VERSION_OF_EE.getOrDefault(ee, 0.0))
.thenComparing(Comparator.naturalOrder());

}
4 changes: 3 additions & 1 deletion ...rg.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/BasePackageHeader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*******************************************************************************/
package org.eclipse.pde.internal.core.text.bundle;

import java.util.Comparator;

import org.eclipse.osgi.util.ManifestElement;
import org.eclipse.pde.internal.core.ICoreConstants;
import org.eclipse.pde.internal.core.bundle.BundlePluginBase;
Expand All @@ -25,7 +27,7 @@ public abstract class BasePackageHeader extends CompositeManifestHeader {
private static final long serialVersionUID = 1L;

public BasePackageHeader(String name, String value, IBundle bundle, String lineDelimiter) {
super(name, value, bundle, lineDelimiter, true);
super(name, value, bundle, lineDelimiter, Comparator.naturalOrder());
}

protected String getVersionAttribute() {
Expand Down
12 changes: 8 additions & 4 deletions ...ipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/CompositeManifestHeader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
Expand All @@ -33,21 +34,24 @@ public class CompositeManifestHeader extends ManifestHeader {
private static final long serialVersionUID = 1L;

private final boolean fSort;
private final Comparator<String> fElementComparator;

protected List<PDEManifestElement> fManifestElements;

protected Map<String, PDEManifestElement> fElementMap;

public CompositeManifestHeader(String name, String value, IBundle bundle, String lineDelimiter) {
this(name, value, bundle, lineDelimiter, false);
this(name, value, bundle, lineDelimiter, null);
}

public CompositeManifestHeader(String name, String value, IBundle bundle, String lineDelimiter, boolean sort) {
public CompositeManifestHeader(String name, String value, IBundle bundle, String lineDelimiter,
Comparator<String> elementComparator) {
fName = name;
fBundle = bundle;
fLineDelimiter = lineDelimiter;
setModel(fBundle.getModel());
fSort = sort;
fSort = elementComparator != null;
fElementComparator = elementComparator;
fValue = value;
processValue(value);
}
Expand Down Expand Up @@ -120,7 +124,7 @@ protected void addManifestElement(PDEManifestElement element, boolean update) {
element.setHeader(this);
if (fSort) {
if (fElementMap == null) {
fElementMap = new TreeMap<>();
fElementMap = new TreeMap<>(fElementComparator);
}
fElementMap.put(element.getValue(), element);
} else {
Expand Down
3 changes: 2 additions & 1 deletion ...re/text/org/eclipse/pde/internal/core/text/bundle/RequiredExecutionEnvironmentHeader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@

import org.eclipse.osgi.util.ManifestElement;
import org.eclipse.pde.internal.core.ibundle.IBundle;
import org.eclipse.pde.internal.core.util.VMUtil;

public class RequiredExecutionEnvironmentHeader extends CompositeManifestHeader {

private static final long serialVersionUID = 1L;

public RequiredExecutionEnvironmentHeader(String name, String value, IBundle bundle, String lineDelimiter) {
super(name, value, bundle, lineDelimiter);
super(name, value, bundle, lineDelimiter, VMUtil.ASCENDING_EE_JAVA_VERSION);
}

@Override
Expand Down
25 changes: 11 additions & 14 deletions ....pde.ui.tests/src/org/eclipse/pde/ui/tests/model/bundle/ExecutionEnvironmentTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static org.junit.Assert.assertNotNull;

import java.util.HashSet;
import java.util.List;
import java.util.Set;

import org.eclipse.pde.internal.core.ibundle.IManifestHeader;
Expand Down Expand Up @@ -131,10 +130,10 @@ public void testAddMulitplieExecutionEnvironmnets() throws Exception {
int pos = fDocument.getLineOffset(3);
int length = fDocument.getLineOffset(7) - fDocument.getLineOffset(3);
assertEquals("""
Bundle-RequiredExecutionEnvironment: J2SE-1.4,
CDC-1.1/Foundation-1.1,
J2SE-1.5,
OSGi/Minimum-1.1
Bundle-RequiredExecutionEnvironment: CDC-1.1/Foundation-1.1,
OSGi/Minimum-1.1,
J2SE-1.4,
J2SE-1.5
""", fDocument.get(pos, length));
}

Expand All @@ -150,8 +149,7 @@ public void testRemoveExecutionEnvironment() throws Exception {
""");
load(true);
RequiredExecutionEnvironmentHeader header = getRequiredExecutionEnvironmentHeader();
String env = header.getEnvironments().get(1);
header.removeExecutionEnvironment(env);
header.removeExecutionEnvironment("CDC-1.1/Foundation-1.1");

TextEdit[] ops = fListener.getTextOperations();
assertEquals(1, ops.length);
Expand All @@ -163,8 +161,8 @@ public void testRemoveExecutionEnvironment() throws Exception {
int pos = fDocument.getLineOffset(3);
int length = fDocument.getLineOffset(5) - fDocument.getLineOffset(3);
assertEquals("""
Bundle-RequiredExecutionEnvironment: J2SE-1.4,
OSGi/Minimum-1.1
Bundle-RequiredExecutionEnvironment: OSGi/Minimum-1.1,
J2SE-1.4
""", fDocument.get(pos, length));
}

Expand All @@ -180,9 +178,8 @@ public void testRemoveMultipleExecutionEnvironments() throws Exception {
""");
load(true);
RequiredExecutionEnvironmentHeader header = getRequiredExecutionEnvironmentHeader();
List<String> envs = header.getEnvironments();
header.removeExecutionEnvironment(envs.get(1));
header.removeExecutionEnvironment(envs.get(0));
header.removeExecutionEnvironment("CDC-1.1/Foundation-1.1");
header.removeExecutionEnvironment("J2SE-1.4");

TextEdit[] ops = fListener.getTextOperations();
assertEquals(1, ops.length);
Expand Down Expand Up @@ -221,8 +218,8 @@ public void testPreserveSpacing() throws Exception {

assertEquals("""
Bundle-RequiredExecutionEnvironment:\s
J2SE-1.4,
OSGi/Minimum-1.1
OSGi/Minimum-1.1,
J2SE-1.4
""", fDocument.get(pos, length));
}

Expand Down
4 changes: 0 additions & 4 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2905,10 +2905,6 @@ public class PDEUIMessages extends NLS {

public static String RequiredExecutionEnvironmentSection_remove;

public static String RequiredExecutionEnvironmentSection_up;

public static String RequiredExecutionEnvironmentSection_down;

public static String RequiredExecutionEnvironmentSection_fragmentDesc;

public static String RequiredExecutionEnvironmentSection_pluginDesc;
Expand Down
27 changes: 3 additions & 24 deletions ...pse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/ExecutionEnvironmentSection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ public void dispose() {
}

public ExecutionEnvironmentSection(PDEFormPage page, Composite parent) {
super(page, parent, Section.DESCRIPTION, new String[] {PDEUIMessages.RequiredExecutionEnvironmentSection_add, PDEUIMessages.RequiredExecutionEnvironmentSection_remove, PDEUIMessages.RequiredExecutionEnvironmentSection_up, PDEUIMessages.RequiredExecutionEnvironmentSection_down});
super(page, parent, Section.DESCRIPTION, new String[] { //
PDEUIMessages.RequiredExecutionEnvironmentSection_add,
PDEUIMessages.RequiredExecutionEnvironmentSection_remove });
createClient(getSection(), page.getEditor().getToolkit());
}

Expand Down Expand Up @@ -210,8 +212,6 @@ protected void buttonSelected(int index) {
switch (index) {
case 0 -> handleAdd();
case 1 -> handleRemove();
case 2 -> handleUp();
case 3 -> handleDown();
}
}

Expand Down Expand Up @@ -245,30 +245,9 @@ public void run() {

private void updateButtons() {
Table table = fEETable.getTable();
int count = table.getItemCount();
boolean canMoveUp = count > 0 && table.getSelection().length == 1 && table.getSelectionIndex() > 0;
boolean canMoveDown = count > 0 && table.getSelection().length == 1 && table.getSelectionIndex() < count - 1;

TablePart tablePart = getTablePart();
tablePart.setButtonEnabled(0, isEditable());
tablePart.setButtonEnabled(1, isEditable() && table.getSelection().length > 0);
tablePart.setButtonEnabled(2, isEditable() && canMoveUp);
tablePart.setButtonEnabled(3, isEditable() && canMoveDown);
}

private void handleDown() {
int selection = fEETable.getTable().getSelectionIndex();
swap(selection, selection + 1);
}

private void handleUp() {
int selection = fEETable.getTable().getSelectionIndex();
swap(selection, selection - 1);
}

public void swap(int index1, int index2) {
RequiredExecutionEnvironmentHeader header = getHeader();
header.swap(index1, index2);
}

private void handleRemove() {
Expand Down
2 changes: 0 additions & 2 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2283,8 +2283,6 @@ GetNonExternalizedStringsAction_allExternalizedMessage=All strings in manifest f
RequiredExecutionEnvironmentSection_title=Execution Environments
RequiredExecutionEnvironmentSection_add=Add...
RequiredExecutionEnvironmentSection_remove=Remove
RequiredExecutionEnvironmentSection_up=Up
RequiredExecutionEnvironmentSection_down=Down
RequiredExecutionEnvironmentSection_fragmentDesc=Specify the minimum execution environments required to run this fragment.
RequiredExecutionEnvironmentSection_pluginDesc=Specify the minimum execution environments required to run this plug-in.
RequiredExecutionEnvironmentSection_dialog_title=Execution Environments
Expand Down

0 comments on commit e69ca13

Please sign in to comment.