From a6d52841710d5fb8b114920ed8596256a3c654ee Mon Sep 17 00:00:00 2001 From: Hannes Wellmann Date: Fri, 13 Dec 2024 23:08:11 +0100 Subject: [PATCH] [Launching] Improve addition of default VM arguments for Eclipse apps Simplify the code and add arguments only if they are not even added with another value. --- .../internal/launching/launcher/VMHelper.java | 6 ++ .../AbstractPDELaunchConfiguration.java | 84 +++++-------------- .../JUnitLaunchConfigurationDelegate.java | 4 +- ...UnitPluginLaunchConfigurationDelegate.java | 4 +- 4 files changed, 30 insertions(+), 68 deletions(-) diff --git a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/VMHelper.java b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/VMHelper.java index 2a801c2ee8..1252b6b3f8 100644 --- a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/VMHelper.java +++ b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/VMHelper.java @@ -232,4 +232,10 @@ public static IRuntimeClasspathEntry getJREEntry(ILaunchConfiguration configurat return JavaRuntime.newRuntimeContainerClasspathEntry(containerPath, IRuntimeClasspathEntry.BOOTSTRAP_CLASSES); } + public static void addNewArgument(List arguments, String key, String value) { + if (arguments.stream().noneMatch(a -> a.startsWith(key + "="))) { //$NON-NLS-1$ + arguments.add(key + "=" + value); //$NON-NLS-1$ + } + } + } diff --git a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/AbstractPDELaunchConfiguration.java b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/AbstractPDELaunchConfiguration.java index 9ebb0321c7..04920b916c 100644 --- a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/AbstractPDELaunchConfiguration.java +++ b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/AbstractPDELaunchConfiguration.java @@ -17,6 +17,7 @@ import java.io.File; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -43,6 +44,7 @@ import org.eclipse.jdt.launching.IVMRunner; import org.eclipse.jdt.launching.JavaRuntime; import org.eclipse.jdt.launching.VMRunnerConfiguration; +import org.eclipse.osgi.service.resolver.BundleDescription; import org.eclipse.pde.core.plugin.IPluginModelBase; import org.eclipse.pde.core.plugin.TargetPlatform; import org.eclipse.pde.internal.core.ICoreConstants; @@ -112,8 +114,7 @@ public String showCommandLine(ILaunchConfiguration configuration, String mode, I VMRunnerConfiguration runnerConfig = new VMRunnerConfiguration(getMainClass(), getClasspath(configuration)); IVMInstall launcher = VMHelper.createLauncher(configuration); - boolean isModular = JavaRuntime.isModularJava(launcher); - runnerConfig.setVMArguments(updateVMArgumentWithAdditionalArguments(getVMArguments(configuration), isModular, configuration)); + runnerConfig.setVMArguments(updateVMArgumentWithAdditionalArguments(getVMArguments(configuration), launcher)); runnerConfig.setProgramArguments(getProgramArguments(configuration)); runnerConfig.setWorkingDirectory(getWorkingDirectory(configuration).getAbsolutePath()); runnerConfig.setEnvironment(getEnvironment(configuration)); @@ -148,8 +149,7 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun VMRunnerConfiguration runnerConfig = new VMRunnerConfiguration(getMainClass(), getClasspath(configuration)); IVMInstall launcher = VMHelper.createLauncher(configuration); - boolean isModular = JavaRuntime.isModularJava(launcher); - runnerConfig.setVMArguments(updateVMArgumentWithAdditionalArguments(getVMArguments(configuration), isModular, configuration)); + runnerConfig.setVMArguments(updateVMArgumentWithAdditionalArguments(getVMArguments(configuration), launcher)); runnerConfig.setProgramArguments(getProgramArguments(configuration)); runnerConfig.setWorkingDirectory(getWorkingDirectory(configuration).getAbsolutePath()); runnerConfig.setEnvironment(getEnvironment(configuration)); @@ -167,50 +167,23 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun } - private String[] updateVMArgumentWithAdditionalArguments(String[] args, boolean isModular, ILaunchConfiguration configuration) { - String modAllSystem= "--add-modules=ALL-SYSTEM"; //$NON-NLS-1$ - String allowSecurityManager = "-Djava.security.manager=allow"; //$NON-NLS-1$ - boolean addModuleSystem = false; - boolean addAllowSecurityManager = false; - int argLength = args.length; - if (isModular && !argumentContainsAttribute(args, modAllSystem)) { - addModuleSystem = true; - argLength++; // Need to add the argument + private String[] updateVMArgumentWithAdditionalArguments(String[] args, IVMInstall vmInstall) { + List arguments = new ArrayList<>(Arrays.asList(args)); + boolean isModular = JavaRuntime.isModularJava(vmInstall); + if (isModular) { + VMHelper.addNewArgument(arguments, "--add-modules", "ALL-SYSTEM"); //$NON-NLS-1$//$NON-NLS-2$ } - - if (isEclipseBundleGreaterThanVersion(4, 24)) { // Don't add allow flags for eclipse before 4.24 - try { - IVMInstall vmInstall = VMHelper.getVMInstall(configuration); - if (vmInstall instanceof AbstractVMInstall) { - AbstractVMInstall install = (AbstractVMInstall) vmInstall; - String vmver = install.getJavaVersion(); - if (vmver != null && JavaCore.compareJavaVersions(vmver, JavaCore.VERSION_17) >= 0) { - if (!argumentContainsAttribute(args, allowSecurityManager)) { - addAllowSecurityManager = true; - argLength++; // Need to add the argument - } - } - } - } catch (CoreException e) { - PDELaunchingPlugin.log(e); - } - } - if (addModuleSystem || addAllowSecurityManager) { - args = Arrays.copyOf(args, argLength); - if (addAllowSecurityManager) { - args[--argLength] = allowSecurityManager; - } - if (addModuleSystem) { - args[--argLength] = modAllSystem; + if (isEclipseBundleGreaterThanVersion(4, 24) // Don't add allow flags for eclipse before 4.24 + && vmInstall instanceof AbstractVMInstall install) { + String vmver = install.getJavaVersion(); + if (vmver != null && JavaCore.compareJavaVersions(vmver, JavaCore.VERSION_17) >= 0) { + VMHelper.addNewArgument(arguments, "-Djava.security.manager", "allow"); //$NON-NLS-1$ //$NON-NLS-2$ } } if (!isModular) { - ArrayList arrayList = new ArrayList<>(Arrays.asList(args)); - arrayList.remove(modAllSystem); - arrayList.trimToSize(); - args = arrayList.toArray(new String[arrayList.size()]); + arguments.remove("--add-modules=ALL-SYSTEM"); //$NON-NLS-1$ } - return args; + return arguments.toArray(String[]::new); } @@ -218,31 +191,18 @@ private boolean isEclipseBundleGreaterThanVersion(int major, int minor) { PDEState pdeState = TargetPlatformHelper.getPDEState(); if (pdeState != null) { try { - Optional platformBaseModel = Arrays.stream(pdeState.getTargetModels()).filter(x -> Objects.nonNull(x.getBundleDescription())).filter(x -> ("org.eclipse.platform").equals(x.getBundleDescription().getSymbolicName()))//$NON-NLS-1$ - .findFirst(); - if (platformBaseModel.isPresent()) { - Version version = platformBaseModel.get().getBundleDescription().getVersion(); - Version comparedVersion = new Version(major, minor, 0); - if (version != null && version.compareTo(comparedVersion) >= 0) { - return true; - } - } - } - catch (Exception ex) { + Optional model = Arrays.stream(pdeState.getTargetModels()) // + .map(IPluginModelBase::getBundleDescription).filter(Objects::nonNull) // + .filter(x -> "org.eclipse.platform".equals(x.getSymbolicName())).findFirst(); //$NON-NLS-1$ + return model.map(BundleDescription::getVersion).filter(v -> v.compareTo(new Version(major, minor, 0)) >= 0).isPresent(); + } catch (Exception ex) { PDELaunchingPlugin.log(ex); } } return false; - } - private boolean argumentContainsAttribute(String[] args, String modAllSystem) { - for (String string : args) { - if (string.equals(modAllSystem)) - return true; - } - return false; - } + /** * Returns the VM runner for the given launch mode to use when launching the diff --git a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/JUnitLaunchConfigurationDelegate.java b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/JUnitLaunchConfigurationDelegate.java index a65f291495..dbfcc3aa56 100644 --- a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/JUnitLaunchConfigurationDelegate.java +++ b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/JUnitLaunchConfigurationDelegate.java @@ -280,9 +280,7 @@ protected void collectExecutionArguments(ILaunchConfiguration configuration, Lis IVMInstall launcher = VMHelper.createLauncher(configuration); boolean isModular = JavaRuntime.isModularJava(launcher); if (isModular) { - String modAllSystem = "--add-modules=ALL-SYSTEM"; //$NON-NLS-1$ - if (!vmArguments.contains(modAllSystem)) - vmArguments.add(modAllSystem); + VMHelper.addNewArgument(vmArguments, "--add-modules", "ALL-SYSTEM"); //$NON-NLS-1$//$NON-NLS-2$ } // if element is a test class annotated with @RunWith(JUnitPlatform.class, we add this in program arguments @SuppressWarnings("restriction") diff --git a/ui/org.eclipse.pde.unittest.junit/src/org/eclipse/pde/unittest/junit/launcher/JUnitPluginLaunchConfigurationDelegate.java b/ui/org.eclipse.pde.unittest.junit/src/org/eclipse/pde/unittest/junit/launcher/JUnitPluginLaunchConfigurationDelegate.java index d94361b9a6..1d93ac197f 100644 --- a/ui/org.eclipse.pde.unittest.junit/src/org/eclipse/pde/unittest/junit/launcher/JUnitPluginLaunchConfigurationDelegate.java +++ b/ui/org.eclipse.pde.unittest.junit/src/org/eclipse/pde/unittest/junit/launcher/JUnitPluginLaunchConfigurationDelegate.java @@ -586,9 +586,7 @@ protected void collectExecutionArguments(ILaunchConfiguration configuration, Lis IVMInstall launcher = VMHelper.createLauncher(configuration); boolean isModular = JavaRuntime.isModularJava(launcher); if (isModular) { - String modAllSystem = "--add-modules=ALL-SYSTEM"; //$NON-NLS-1$ - if (!vmArguments.contains(modAllSystem)) - vmArguments.add(modAllSystem); + VMHelper.addNewArgument(vmArguments, "--add-modules", "ALL-SYSTEM"); //$NON-NLS-1$//$NON-NLS-2$ } // if element is a test class annotated with @RunWith(JUnitPlatform.class, we // add this in program arguments