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

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

@@ -1,3 +1,2 @@
 Comparator Errors in 4.32 I-Build I20240304-0140
-Update to IPDELauncherConstants
-https://github.com/eclipse-pde/eclipse.pde/issues/1250
+Update to IPDELauncherConstants

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

@@ -94,7 +94,7 @@ protected void initializeBundleState(ILaunchConfigurationWorkingCopy configurati
 		}
 		configuration.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, workspaceBundles);
 		configuration.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES, targetBundles);
-		configuration.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+		configuration.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 	}
 
 	private boolean isSourceBundle(PDEState pdeState, IPluginModelBase model) {

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

@@ -154,7 +154,7 @@ public void testGetMergedBundleMap_mixedPluginsFromWorkspaceWithAutomaticAddAndT
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
 			wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0", "plugin.b"));
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 			wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*2.0.0", "plugin.c"));
 			wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES,
 					Set.of("plugin.x*2.0.0", "plugin.x*3.0.0", "plugin.y"));
@@ -371,7 +371,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_noDisabledPlug
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
 			wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0"));
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 		};
 
 		Set<BundleLocationDescriptor> expectedBundles = Set.of( //
@@ -394,7 +394,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_singleVersionP
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
 			wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0"));
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 			wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, Set.of("plugin.c"));
 		};
 
@@ -417,7 +417,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_singleVersionP
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
 			wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0"));
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 			wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, Set.of("plugin.c*1.0.0"));
 		};
 
@@ -447,7 +447,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_multiVersionPl
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
 			wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0"));
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 			wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES,
 					Set.of("plugin.a*2.0.0", "plugin.a*4.0.0"));
 		};
@@ -471,7 +471,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_multiVersionPl
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
 			wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of());
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 			wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES,
 					Set.of("plugin.a*1.0.0", "plugin.a*3.0.0"));
 		};
@@ -494,7 +494,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_multiVersionPl
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
 			wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*2.0.0"));
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 			wc.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a"));
 		};
 
@@ -516,7 +516,7 @@ public void testGetMergedBundleMap_automaticAddedWorkspacePlugins_sameMMMVersion
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
 			wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, Set.of("plugin.a*1.0.0.202111250056"));
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 		};
 
 		Set<BundleLocationDescriptor> expectedBundles = Set.of( //
@@ -777,7 +777,7 @@ public void testGetMergedBundleMap_workspacePluginAddedAutomaticallyAndTargetPlu
 				bundle("plugin.b", "1.0.1"));
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 			wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES, Set.of("plugin.a", "plugin.b*1.0.1"));
 		};
 
@@ -799,7 +799,7 @@ public void testGetMergedBundleMap_workspacePluginAddedAutomaticallyAndTargetPlu
 				bundle("plugin.a", "1.0.0.202111102345"));
 
 		Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 			wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES, Set.of("plugin.a*1.0.0.202111102345"));
 		};
 
@@ -1044,7 +1044,7 @@ private static ILaunchConfigurationWorkingCopy createPluginLaunchConfig(String n
 		ILaunchConfigurationType type = launchManager.getLaunchConfigurationType("org.eclipse.pde.ui.RuntimeWorkbench");
 		ILaunchConfigurationWorkingCopy wc = type.newInstance(null, name);
 		wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, false);
-		wc.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false);
+		wc.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
 		wc.setAttribute(IPDELauncherConstants.USE_DEFAULT, false);
 		return wc;
 	}

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

@@ -948,21 +948,36 @@ public void performApply(ILaunchConfigurationWorkingCopy config) {
 			config.setAttribute(IPDELauncherConstants.AUTOMATIC_INCLUDE_REQUIREMENTS, includeRequirements);
 			fAutoIncludeRequirementsButtonChanged = false;
 		}
-		config.setAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL, fIncludeOptionalButton.getSelection());
-		config.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, fAddWorkspaceButton.getSelection());
+		boolean includeOptional = fIncludeOptionalButton.getSelection();
+		if (!includeOptional) {
+			config.setAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL, false);
+		} else {
+			config.removeAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL);
+		}
+		boolean automaticAdd = fAddWorkspaceButton.getSelection();
+		if (!automaticAdd) {
+			config.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, false);
+		} else {
+			config.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
+		}
 		config.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, fAutoValidate.getSelection());
-		config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, fFilterButton.getSelection());
+		boolean showSelectedOnly = fFilterButton.getSelection();
+		if (showSelectedOnly) {
+			config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, true);
+		} else {
+			config.removeAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY);
+		}
 		savePluginState(config);
 		updateCounter();
 	}
 
 	protected abstract void savePluginState(ILaunchConfigurationWorkingCopy config);
 
 	public void setDefaults(ILaunchConfigurationWorkingCopy config) {
-		config.setAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL, true);
-		config.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+		config.removeAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL);
+		config.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 		config.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, true);
-		config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, false);
+		config.removeAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY);
 	}
 
 	public void enableViewer(boolean enable) {

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

@@ -1203,7 +1203,12 @@ public void performApply(ILaunchConfigurationWorkingCopy config) {
 			config.setAttribute(IPDELauncherConstants.AUTOMATIC_INCLUDE_REQUIREMENTS, includeRequirements);
 			fAutoIncludeRequirementsButtonChanged = false;
 		}
-		config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, fFilterButton.getSelection());
+		boolean showSelectedOnly = fFilterButton.getSelection();
+		if (showSelectedOnly) {
+			config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, true);
+		} else {
+			config.removeAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY);
+		}
 		config.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, fFeatureWorkspaceButton.getSelection() ? IPDELauncherConstants.LOCATION_WORKSPACE : IPDELauncherConstants.LOCATION_EXTERNAL);
 		config.setAttribute(IPDELauncherConstants.FEATURE_PLUGIN_RESOLUTION, fWorkspacePluginButton.getSelection() ? IPDELauncherConstants.LOCATION_WORKSPACE : IPDELauncherConstants.LOCATION_EXTERNAL);
 		config.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, fAutoValidate.getSelection());
@@ -1256,7 +1261,7 @@ private void saveSortOrder() {
 	}
 
 	public void setDefaults(ILaunchConfigurationWorkingCopy config) {
-		config.setAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY, false);
+		config.removeAttribute(IPDELauncherConstants.SHOW_SELECTED_ONLY);
 		config.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_WORKSPACE);
 		config.setAttribute(IPDELauncherConstants.FEATURE_PLUGIN_RESOLUTION, IPDELauncherConstants.LOCATION_WORKSPACE);
 		config.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, true);

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

@@ -61,7 +61,11 @@ protected void savePluginState(ILaunchConfigurationWorkingCopy config) {
 				}
 			}
 		}
-		config.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, buffer.getNameSet());
+		if (!buffer.getNameSet().isEmpty()) {
+			config.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, buffer.getNameSet());
+		} else {
+			config.removeAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES);
+		}
 	}
 
 	public void initializeFrom(ILaunchConfiguration configuration) throws CoreException {

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

@@ -160,7 +160,12 @@ private void initializeFramework(ILaunchConfiguration config) throws CoreExcepti
 
 	public void performApply(ILaunchConfigurationWorkingCopy config) {
 		config.setAttribute(IPDELauncherConstants.USE_DEFAULT, fLaunchWithCombo.getSelectionIndex() == 0);
-		config.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, fLaunchWithCombo.getSelectionIndex() == 1);
+		boolean useCustomFeatures = fLaunchWithCombo.getSelectionIndex() == 1;
+		if (useCustomFeatures) {
+			config.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, true);
+		} else {
+			config.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
+		}
 		config.setAttribute(IPDELauncherConstants.DEFAULT_AUTO_START, Boolean.toString(true).equals(fDefaultAutoStart.getText()));
 		config.setAttribute(IPDELauncherConstants.DEFAULT_START_LEVEL, fDefaultStartLevel.getSelection());
 

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

@@ -156,7 +156,12 @@ protected void savePluginState(ILaunchConfigurationWorkingCopy config) {
 					}
 				}
 			}
-			config.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES, buffer.getNameSet());
+			if (!buffer.getNameSet().isEmpty()) {
+				config.setAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES,
+						buffer.getNameSet());
+			} else {
+				config.removeAttribute(IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES);
+			}
 		}
 	}
 

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

@@ -251,7 +251,7 @@ else if (TargetPlatformHelper.getTargetVersion() >= 3.2)
 				wc.setAttribute(IPDELauncherConstants.USE_PRODUCT, true);
 				wc.setAttribute(IPDELauncherConstants.PRODUCT, product);
 			}
-			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
+			wc.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
 		} else {
 			String defaultProduct = TargetPlatform.getDefaultProduct();
 			if (defaultProduct != null) {

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

@@ -72,9 +72,9 @@ protected ILaunchConfigurationWorkingCopy createLaunchConfiguration(IJavaElement
 		// Plug-ins to launch
 		configuration.setAttribute(IPDELauncherConstants.USE_DEFAULT, true);
 		configuration.setAttribute(IPDELauncherConstants.AUTOMATIC_VALIDATE, false);
-		configuration.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false); // ignored
-		configuration.setAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true); // ignored
-		configuration.setAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL, true); // ignored
+		configuration.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
+		configuration.removeAttribute(IPDELauncherConstants.AUTOMATIC_ADD);
+		configuration.removeAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL);
 
 		// Program arguments
 		String programArgs = LaunchArgumentsHelper.getInitialProgramArguments();

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

@@ -59,6 +59,7 @@ public class PluginsTab extends AbstractLauncherTab {
 	private Combo fDefaultAutoStart;
 	private Spinner fDefaultStartLevel;
 	private final Listener fListener;
+	private boolean fActivated;
 
 	private static final int DEFAULT_SELECTION = 0;
 	private static final int PLUGIN_SELECTION = 1;
@@ -156,6 +157,17 @@ public void createControl(Composite parent) {
 
 	@Override
 	public void initializeFrom(ILaunchConfiguration configuration) {
+		// Long-running initialization happens on first activation of this tab
+		System.out.println("PluginsTab.initializeFrom()");
+	}
+
+	@Override
+	public void activated(ILaunchConfigurationWorkingCopy configuration) {
+		if (fActivated) {
+			// Since this method can be expensive, only activate this tab once.
+			return;
+		}
+
 		try {
 			int index = DEFAULT_SELECTION;
 			if (configuration.getAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false)) {
@@ -171,6 +183,9 @@ public void initializeFrom(ILaunchConfiguration configuration) {
 			fDefaultAutoStart.setText(Boolean.toString(auto));
 			int level = configuration.getAttribute(IPDELauncherConstants.DEFAULT_START_LEVEL, 4);
 			fDefaultStartLevel.setSelection(level);
+
+			// If everything ran smoothly, this tab is activated
+			fActivated = true;
 		} catch (CoreException e) {
 			PDEPlugin.log(e);
 		}
@@ -179,15 +194,23 @@ public void initializeFrom(ILaunchConfiguration configuration) {
 	@Override
 	public void setDefaults(ILaunchConfigurationWorkingCopy configuration) {
 		configuration.setAttribute(IPDELauncherConstants.USE_DEFAULT, true);
-		configuration.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false);
+		configuration.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
 		fBlock.setDefaults(configuration);
 	}
 
 	@Override
 	public void performApply(ILaunchConfigurationWorkingCopy configuration) {
+		if (!fActivated) {
+			return;
+		}
 		int index = fSelectionCombo.getSelectionIndex();
 		configuration.setAttribute(IPDELauncherConstants.USE_DEFAULT, index == DEFAULT_SELECTION);
-		configuration.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, index == FEATURE_SELECTION);
+		boolean useCustomFeatures = index == FEATURE_SELECTION;
+		if (useCustomFeatures) {
+			configuration.setAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, true);
+		} else {
+			configuration.removeAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES);
+		}
 		fBlock.performApply(configuration);
 		// clear default values for auto-start and start-level if default
 		String autoText = fDefaultAutoStart.getText();
@@ -217,16 +240,9 @@ public Image getImage() {
 		return fImage;
 	}
 
-	/**
-	 * Validates the tab.  If the feature option is chosen, and the workspace is not correctly set up,
-	 * the error message is set.
-	 *
-	 * @see org.eclipse.pde.ui.launcher.AbstractLauncherTab#validateTab()
-	 */
 	@Override
 	public void validateTab() {
-		String errorMessage = null;
-		setErrorMessage(errorMessage);
+		setErrorMessage(null);
 	}
 
 	@Override