Skip to content

Commit 9bc4a18

Browse files
HannesWellHannesWell
committed
[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.
1 parent 51ef6fe commit 9bc4a18

File tree

4 files changed

+30
-68
lines changed

4 files changed

+30
-68
lines changed

‎ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/VMHelper.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,4 +232,10 @@ public static IRuntimeClasspathEntry getJREEntry(ILaunchConfiguration configurat
232232
return JavaRuntime.newRuntimeContainerClasspathEntry(containerPath, IRuntimeClasspathEntry.BOOTSTRAP_CLASSES);
233233
}
234234

235+
public static void addNewArgument(List<String> arguments, String key, String value) {
236+
if (arguments.stream().noneMatch(a -> a.startsWith(key + "="))) { //$NON-NLS-1$
237+
arguments.add(key + "=" + value); //$NON-NLS-1$
238+
}
239+
}
240+
235241
}

‎ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/AbstractPDELaunchConfiguration.java‎

Lines changed: 22 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.io.File;
1818
import java.util.ArrayList;
1919
import java.util.Arrays;
20+
import java.util.List;
2021
import java.util.Map;
2122
import java.util.Objects;
2223
import java.util.Optional;
@@ -43,6 +44,7 @@
4344
import org.eclipse.jdt.launching.IVMRunner;
4445
import org.eclipse.jdt.launching.JavaRuntime;
4546
import org.eclipse.jdt.launching.VMRunnerConfiguration;
47+
import org.eclipse.osgi.service.resolver.BundleDescription;
4648
import org.eclipse.pde.core.plugin.IPluginModelBase;
4749
import org.eclipse.pde.core.plugin.TargetPlatform;
4850
import org.eclipse.pde.internal.core.ICoreConstants;
@@ -112,8 +114,7 @@ public String showCommandLine(ILaunchConfiguration configuration, String mode, I
112114

113115
VMRunnerConfiguration runnerConfig = new VMRunnerConfiguration(getMainClass(), getClasspath(configuration));
114116
IVMInstall launcher = VMHelper.createLauncher(configuration);
115-
boolean isModular = JavaRuntime.isModularJava(launcher);
116-
runnerConfig.setVMArguments(updateVMArgumentWithAdditionalArguments(getVMArguments(configuration), isModular, configuration));
117+
runnerConfig.setVMArguments(updateVMArgumentWithAdditionalArguments(getVMArguments(configuration), launcher));
117118
runnerConfig.setProgramArguments(getProgramArguments(configuration));
118119
runnerConfig.setWorkingDirectory(getWorkingDirectory(configuration).getAbsolutePath());
119120
runnerConfig.setEnvironment(getEnvironment(configuration));
@@ -148,8 +149,7 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun
148149

149150
VMRunnerConfiguration runnerConfig = new VMRunnerConfiguration(getMainClass(), getClasspath(configuration));
150151
IVMInstall launcher = VMHelper.createLauncher(configuration);
151-
boolean isModular = JavaRuntime.isModularJava(launcher);
152-
runnerConfig.setVMArguments(updateVMArgumentWithAdditionalArguments(getVMArguments(configuration), isModular, configuration));
152+
runnerConfig.setVMArguments(updateVMArgumentWithAdditionalArguments(getVMArguments(configuration), launcher));
153153
runnerConfig.setProgramArguments(getProgramArguments(configuration));
154154
runnerConfig.setWorkingDirectory(getWorkingDirectory(configuration).getAbsolutePath());
155155
runnerConfig.setEnvironment(getEnvironment(configuration));
@@ -167,82 +167,42 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun
167167

168168
}
169169

170-
private String[] updateVMArgumentWithAdditionalArguments(String[] args, boolean isModular, ILaunchConfiguration configuration) {
171-
String modAllSystem= "--add-modules=ALL-SYSTEM"; //$NON-NLS-1$
172-
String allowSecurityManager = "-Djava.security.manager=allow"; //$NON-NLS-1$
173-
boolean addModuleSystem = false;
174-
boolean addAllowSecurityManager = false;
175-
int argLength = args.length;
176-
if (isModular && !argumentContainsAttribute(args, modAllSystem)) {
177-
addModuleSystem = true;
178-
argLength++; // Need to add the argument
170+
private String[] updateVMArgumentWithAdditionalArguments(String[] args, IVMInstall vmInstall) {
171+
List<String> arguments = new ArrayList<>(Arrays.asList(args));
172+
boolean isModular = JavaRuntime.isModularJava(vmInstall);
173+
if (isModular) {
174+
VMHelper.addNewArgument(arguments, "--add-modules", "ALL-SYSTEM"); //$NON-NLS-1$//$NON-NLS-2$
179175
}
180-
181-
if (isEclipseBundleGreaterThanVersion(4, 24)) { // Don't add allow flags for eclipse before 4.24
182-
try {
183-
IVMInstall vmInstall = VMHelper.getVMInstall(configuration);
184-
if (vmInstall instanceof AbstractVMInstall) {
185-
AbstractVMInstall install = (AbstractVMInstall) vmInstall;
186-
String vmver = install.getJavaVersion();
187-
if (vmver != null && JavaCore.compareJavaVersions(vmver, JavaCore.VERSION_17) >= 0) {
188-
if (!argumentContainsAttribute(args, allowSecurityManager)) {
189-
addAllowSecurityManager = true;
190-
argLength++; // Need to add the argument
191-
}
192-
}
193-
}
194-
} catch (CoreException e) {
195-
PDELaunchingPlugin.log(e);
196-
}
197-
}
198-
if (addModuleSystem || addAllowSecurityManager) {
199-
args = Arrays.copyOf(args, argLength);
200-
if (addAllowSecurityManager) {
201-
args[--argLength] = allowSecurityManager;
202-
}
203-
if (addModuleSystem) {
204-
args[--argLength] = modAllSystem;
176+
if (isEclipseBundleGreaterThanVersion(4, 24) // Don't add allow flags for eclipse before 4.24
177+
&& vmInstall instanceof AbstractVMInstall install) {
178+
String vmver = install.getJavaVersion();
179+
if (vmver != null && JavaCore.compareJavaVersions(vmver, JavaCore.VERSION_17) >= 0) {
180+
VMHelper.addNewArgument(arguments, "-Djava.security.manager", "allow"); //$NON-NLS-1$ //$NON-NLS-2$
205181
}
206182
}
207183
if (!isModular) {
208-
ArrayList<String> arrayList = new ArrayList<>(Arrays.asList(args));
209-
arrayList.remove(modAllSystem);
210-
arrayList.trimToSize();
211-
args = arrayList.toArray(new String[arrayList.size()]);
184+
arguments.remove("--add-modules=ALL-SYSTEM"); //$NON-NLS-1$
212185
}
213-
return args;
186+
return arguments.toArray(String[]::new);
214187
}
215188

216189

217190
private boolean isEclipseBundleGreaterThanVersion(int major, int minor) {
218191
PDEState pdeState = TargetPlatformHelper.getPDEState();
219192
if (pdeState != null) {
220193
try {
221-
Optional<IPluginModelBase> platformBaseModel = Arrays.stream(pdeState.getTargetModels()).filter(x -> Objects.nonNull(x.getBundleDescription())).filter(x -> ("org.eclipse.platform").equals(x.getBundleDescription().getSymbolicName()))//$NON-NLS-1$
222-
.findFirst();
223-
if (platformBaseModel.isPresent()) {
224-
Version version = platformBaseModel.get().getBundleDescription().getVersion();
225-
Version comparedVersion = new Version(major, minor, 0);
226-
if (version != null && version.compareTo(comparedVersion) >= 0) {
227-
return true;
228-
}
229-
}
230-
}
231-
catch (Exception ex) {
194+
Optional<BundleDescription> model = Arrays.stream(pdeState.getTargetModels()) //
195+
.map(IPluginModelBase::getBundleDescription).filter(Objects::nonNull) //
196+
.filter(x -> "org.eclipse.platform".equals(x.getSymbolicName())).findFirst(); //$NON-NLS-1$
197+
return model.map(BundleDescription::getVersion).filter(v -> v.compareTo(new Version(major, minor, 0)) >= 0).isPresent();
198+
} catch (Exception ex) {
232199
PDELaunchingPlugin.log(ex);
233200
}
234201
}
235202
return false;
236-
237203
}
238204

239-
private boolean argumentContainsAttribute(String[] args, String modAllSystem) {
240-
for (String string : args) {
241-
if (string.equals(modAllSystem))
242-
return true;
243-
}
244-
return false;
245-
}
205+
246206

247207
/**
248208
* Returns the VM runner for the given launch mode to use when launching the

‎ui/org.eclipse.pde.launching/src/org/eclipse/pde/launching/JUnitLaunchConfigurationDelegate.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -280,9 +280,7 @@ protected void collectExecutionArguments(ILaunchConfiguration configuration, Lis
280280
IVMInstall launcher = VMHelper.createLauncher(configuration);
281281
boolean isModular = JavaRuntime.isModularJava(launcher);
282282
if (isModular) {
283-
String modAllSystem = "--add-modules=ALL-SYSTEM"; //$NON-NLS-1$
284-
if (!vmArguments.contains(modAllSystem))
285-
vmArguments.add(modAllSystem);
283+
VMHelper.addNewArgument(vmArguments, "--add-modules", "ALL-SYSTEM"); //$NON-NLS-1$//$NON-NLS-2$
286284
}
287285
// if element is a test class annotated with @RunWith(JUnitPlatform.class, we add this in program arguments
288286
@SuppressWarnings("restriction")

‎ui/org.eclipse.pde.unittest.junit/src/org/eclipse/pde/unittest/junit/launcher/JUnitPluginLaunchConfigurationDelegate.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -586,9 +586,7 @@ protected void collectExecutionArguments(ILaunchConfiguration configuration, Lis
586586
IVMInstall launcher = VMHelper.createLauncher(configuration);
587587
boolean isModular = JavaRuntime.isModularJava(launcher);
588588
if (isModular) {
589-
String modAllSystem = "--add-modules=ALL-SYSTEM"; //$NON-NLS-1$
590-
if (!vmArguments.contains(modAllSystem))
591-
vmArguments.add(modAllSystem);
589+
VMHelper.addNewArgument(vmArguments, "--add-modules", "ALL-SYSTEM"); //$NON-NLS-1$//$NON-NLS-2$
592590
}
593591
// if element is a test class annotated with @RunWith(JUnitPlatform.class, we
594592
// add this in program arguments

0 commit comments

Comments
 (0)