Skip to content

Commit 1cdde48

Browse files
fedejeannefedejeanne
committed
Skip the method PluginsTab::performApply until after activation
If this method (or part of it) is run before the tab has been activated then the default configuration passed as parameter will be modified and incorrect values will be introduced. Remove attributes in PDE launch configurations instead of explicitly setting them to their default values The attributes in question are: - DESELECTED_WORKSPACE_BUNDLES --> [] - USE_CUSTOM_FEATURES --> false - SHOW_SELECTED_ONLY --> false - INCLUDE_OPTIONAL --> true - AUTOMATIC_ADD --> true The reason to do this is that 2 launch configurations are considered different if one of them explicitly sets an attribute to its default value (because it then has 1 more attribute) This commit fixes a regression introduced in 98a5865 Fixes #1250
1 parent 33efe8c commit 1cdde48

File tree

11 files changed

+86
-29
lines changed

11 files changed

+86
-29
lines changed

‎org.eclipse.pde.doc.user/forceQualifierUpdate.txt

+1
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@ Modernize pde.project description interfaces: https://github.com/eclipse-pde/ecl
44
Rework PluginRegistry API and introduce VersionMatchRule enum : https://github.com/eclipse-pde/eclipse.pde/pull/1163
55
Comparator errors in I20240904-0240
66
Add missing reference/api content
7+
https://github.com/eclipse-pde/eclipse.pde/issues/1250

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ protected void initializeBundleState(ILaunchConfigurationWorkingCopy configurati
9494
}
9595
configuration.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, workspaceBundles);
9696
configuration.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES, targetBundles);
97-
configuration.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
97+
configuration.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
9898
}
9999

100100
private boolean isSourceBundle(PDEState pdeState, IPluginModelBase model) {

‎ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/PluginBasedLaunchTest.java

+11-11
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public void testGetMergedBundleMap_mixedPluginsFromWorkspaceWithAutomaticAddAndT
154154

155155
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
156156
wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0", "plugin.b"));
157-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
157+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
158158
wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*2.0.0", "plugin.c"));
159159
wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES,
160160
Set.of("plugin.x*2.0.0", "plugin.x*3.0.0", "plugin.y"));
@@ -371,7 +371,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_noDisabledPlug
371371

372372
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
373373
wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0"));
374-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
374+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
375375
};
376376

377377
Set<BundleLocationDescriptor> expectedBundles = Set.of( //
@@ -394,7 +394,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_singleVersionP
394394

395395
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
396396
wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0"));
397-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
397+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
398398
wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, Set.of("plugin.c"));
399399
};
400400

@@ -417,7 +417,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_singleVersionP
417417

418418
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
419419
wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0"));
420-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
420+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
421421
wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, Set.of("plugin.c*1.0.0"));
422422
};
423423

@@ -447,7 +447,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_multiVersionPl
447447

448448
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
449449
wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0"));
450-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
450+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
451451
wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES,
452452
Set.of("plugin.a*2.0.0", "plugin.a*4.0.0"));
453453
};
@@ -471,7 +471,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_multiVersionPl
471471

472472
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
473473
wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of());
474-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
474+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
475475
wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES,
476476
Set.of("plugin.a*1.0.0", "plugin.a*3.0.0"));
477477
};
@@ -494,7 +494,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_multiVersionPl
494494

495495
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
496496
wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*2.0.0"));
497-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
497+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
498498
wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a"));
499499
};
500500

@@ -516,7 +516,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_sameMMMVersion
516516

517517
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
518518
wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0.202111250056"));
519-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
519+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
520520
};
521521

522522
Set<BundleLocationDescriptor> expectedBundles = Set.of( //
@@ -777,7 +777,7 @@ public void testGetMergedBundleMap_workspacePluginAddedAutomaticallyAndTargetPlu
777777
bundle("plugin.b", "1.0.1"));
778778

779779
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
780-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
780+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
781781
wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES, Set.of("plugin.a", "plugin.b*1.0.1"));
782782
};
783783

@@ -799,7 +799,7 @@ public void testGetMergedBundleMap_workspacePluginAddedAutomaticallyAndTargetPlu
799799
bundle("plugin.a", "1.0.0.202111102345"));
800800

801801
Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
802-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
802+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
803803
wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES, Set.of("plugin.a*1.0.0.202111102345"));
804804
};
805805

@@ -1044,7 +1044,7 @@ private static ILaunchConfigurationWorkingCopy createPluginLaunchConfig(String n
10441044
ILaunchConfigurationType type = launchManager.getLaunchConfigurationType("org.eclipse.pde.ui.RuntimeWorkbench");
10451045
ILaunchConfigurationWorkingCopy wc = type.newInstance(null, name);
10461046
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, false);
1047-
wc.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false);
1047+
wc.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
10481048
wc.setAttribute(IPDELauncherConstants.USE_DEFAULT, false);
10491049
return wc;
10501050
}

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/AbstractPluginBlock.java

+21-6
Original file line numberDiff line numberDiff line change
@@ -948,21 +948,36 @@ public void performApply(ILaunchConfigurationWorkingCopy config) {
948948
config.setAttribute(IPDELauncherConstants.AUTOMATIC_INCLUDE_REQUIREMENTS, includeRequirements);
949949
fAutoIncludeRequirementsButtonChanged = false;
950950
}
951-
config.setAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL, fIncludeOptionalButton.getSelection());
952-
config.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, fAddWorkspaceButton.getSelection());
951+
boolean includeOptional = fIncludeOptionalButton.getSelection();
952+
if (!includeOptional) {
953+
config.setAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL, false);
954+
} else {
955+
config.removeAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL);
956+
}
957+
boolean automaticAdd = fAddWorkspaceButton.getSelection();
958+
if (!automaticAdd) {
959+
config.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, false);
960+
} else {
961+
config.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
962+
}
953963
config.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, fAutoValidate.getSelection());
954-
config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, fFilterButton.getSelection());
964+
boolean showSelectedOnly = fFilterButton.getSelection();
965+
if (showSelectedOnly) {
966+
config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, true);
967+
} else {
968+
config.removeAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY);
969+
}
955970
savePluginState(config);
956971
updateCounter();
957972
}
958973

959974
protected abstract void savePluginState(ILaunchConfigurationWorkingCopy config);
960975

961976
public void setDefaults(ILaunchConfigurationWorkingCopy config) {
962-
config.setAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL, true);
963-
config.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
977+
config.removeAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL);
978+
config.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
964979
config.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, true);
965-
config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, false);
980+
config.removeAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY);
966981
}
967982

968983
public void enableViewer(boolean enable) {

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/FeatureBlock.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -1203,7 +1203,12 @@ public void performApply(ILaunchConfigurationWorkingCopy config) {
12031203
config.setAttribute(IPDELauncherConstants.AUTOMATIC_INCLUDE_REQUIREMENTS, includeRequirements);
12041204
fAutoIncludeRequirementsButtonChanged = false;
12051205
}
1206-
config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, fFilterButton.getSelection());
1206+
boolean showSelectedOnly = fFilterButton.getSelection();
1207+
if (showSelectedOnly) {
1208+
config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, true);
1209+
} else {
1210+
config.removeAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY);
1211+
}
12071212
config.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, fFeatureWorkspaceButton.getSelection() ? IPDELauncherConstants.LOCATION_WORKSPACE : IPDELauncherConstants.LOCATION_EXTERNAL);
12081213
config.setAttribute(IPDELauncherConstants.FEATURE_PLUGIN_RESOLUTION, fWorkspacePluginButton.getSelection() ? IPDELauncherConstants.LOCATION_WORKSPACE : IPDELauncherConstants.LOCATION_EXTERNAL);
12091214
config.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, fAutoValidate.getSelection());
@@ -1256,7 +1261,7 @@ private void saveSortOrder() {
12561261
}
12571262

12581263
public void setDefaults(ILaunchConfigurationWorkingCopy config) {
1259-
config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, false);
1264+
config.removeAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY);
12601265
config.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_WORKSPACE);
12611266
config.setAttribute(IPDELauncherConstants.FEATURE_PLUGIN_RESOLUTION, IPDELauncherConstants.LOCATION_WORKSPACE);
12621267
config.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, true);

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/OSGiBundleBlock.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ protected void savePluginState(ILaunchConfigurationWorkingCopy config) {
6161
}
6262
}
6363
}
64-
config.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, buffer.getNameSet());
64+
if (!buffer.getNameSet().isEmpty()) {
65+
config.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, buffer.getNameSet());
66+
} else {
67+
config.removeAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES);
68+
}
6569
}
6670

6771
public void initializeFrom(ILaunchConfiguration configuration) throws CoreException {

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/OSGiFrameworkBlock.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,12 @@ private void initializeFramework(ILaunchConfiguration config) throws CoreExcepti
160160

161161
public void performApply(ILaunchConfigurationWorkingCopy config) {
162162
config.setAttribute(IPDELauncherConstants.USE_DEFAULT, fLaunchWithCombo.getSelectionIndex() == 0);
163-
config.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, fLaunchWithCombo.getSelectionIndex() == 1);
163+
boolean useCustomFeatures = fLaunchWithCombo.getSelectionIndex() == 1;
164+
if (useCustomFeatures) {
165+
config.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, true);
166+
} else {
167+
config.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
168+
}
164169
config.setAttribute(IPDELauncherConstants.DEFAULT_AUTO_START, Boolean.toString(true).equals(fDefaultAutoStart.getText()));
165170
config.setAttribute(IPDELauncherConstants.DEFAULT_START_LEVEL, fDefaultStartLevel.getSelection());
166171

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/PluginBlock.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,12 @@ protected void savePluginState(ILaunchConfigurationWorkingCopy config) {
156156
}
157157
}
158158
}
159-
config.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, buffer.getNameSet());
159+
if (!buffer.getNameSet().isEmpty()) {
160+
config.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES,
161+
buffer.getNameSet());
162+
} else {
163+
config.removeAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES);
164+
}
160165
}
161166
}
162167

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/EclipseLaunchShortcut.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ else if (TargetPlatformHelper.getTargetVersion() >= 3.2)
251251
wc.setAttribute(IPDELauncherConstants.USE_PRODUCT, true);
252252
wc.setAttribute(IPDELauncherConstants.PRODUCT, product);
253253
}
254-
wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
254+
wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
255255
} else {
256256
String defaultProduct = TargetPlatform.getDefaultProduct();
257257
if (defaultProduct != null) {

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/JUnitWorkbenchLaunchShortcut.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ protected ILaunchConfigurationWorkingCopy createLaunchConfiguration(IJavaElement
7272
// Plug-ins to launch
7373
configuration.setAttribute(IPDELauncherConstants.USE_DEFAULT, true);
7474
configuration.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, false);
75-
configuration.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false); // ignored
76-
configuration.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true); // ignored
77-
configuration.setAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL, true); // ignored
75+
configuration.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
76+
configuration.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
77+
configuration.removeAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL);
7878

7979
// Program arguments
8080
String programArgs = LaunchArgumentsHelper.getInitialProgramArguments();

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/PluginsTab.java

+24-2
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ public class PluginsTab extends AbstractLauncherTab {
5959
private Combo fDefaultAutoStart;
6060
private Spinner fDefaultStartLevel;
6161
private final Listener fListener;
62+
private boolean fActivated;
6263

6364
private static final int DEFAULT_SELECTION = 0;
6465
private static final int PLUGIN_SELECTION = 1;
@@ -156,6 +157,16 @@ public void createControl(Composite parent) {
156157

157158
@Override
158159
public void initializeFrom(ILaunchConfiguration configuration) {
160+
// Long-running initialization happens on first activation of this tab
161+
}
162+
163+
@Override
164+
public void activated(ILaunchConfigurationWorkingCopy configuration) {
165+
if (fActivated) {
166+
// Since this method can be expensive, only activate this tab once.
167+
return;
168+
}
169+
159170
try {
160171
int index = DEFAULT_SELECTION;
161172
if (configuration.getAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false)) {
@@ -171,6 +182,9 @@ public void initializeFrom(ILaunchConfiguration configuration) {
171182
fDefaultAutoStart.setText(Boolean.toString(auto));
172183
int level = configuration.getAttribute(IPDELauncherConstants.DEFAULT_START_LEVEL, 4);
173184
fDefaultStartLevel.setSelection(level);
185+
186+
// If everything ran smoothly, this tab is activated
187+
fActivated = true;
174188
} catch (CoreException e) {
175189
PDEPlugin.log(e);
176190
}
@@ -179,15 +193,23 @@ public void initializeFrom(ILaunchConfiguration configuration) {
179193
@Override
180194
public void setDefaults(ILaunchConfigurationWorkingCopy configuration) {
181195
configuration.setAttribute(IPDELauncherConstants.USE_DEFAULT, true);
182-
configuration.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false);
196+
configuration.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
183197
fBlock.setDefaults(configuration);
184198
}
185199

186200
@Override
187201
public void performApply(ILaunchConfigurationWorkingCopy configuration) {
202+
if (!fActivated) {
203+
return;
204+
}
188205
int index = fSelectionCombo.getSelectionIndex();
189206
configuration.setAttribute(IPDELauncherConstants.USE_DEFAULT, index == DEFAULT_SELECTION);
190-
configuration.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, index == FEATURE_SELECTION);
207+
boolean useCustomFeatures = index == FEATURE_SELECTION;
208+
if (useCustomFeatures) {
209+
configuration.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, true);
210+
} else {
211+
configuration.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
212+
}
191213
fBlock.performApply(configuration);
192214
// clear default values for auto-start and start-level if default
193215
String autoText = fDefaultAutoStart.getText();

0 commit comments

Comments
 (0)