Bug 560211 - never save launch configs during migration

The isDirty() check to detect whether a config has been migrated has a
false-positive for temporary launch configs like 'rerun test'.

Saving them pollutes the workspace with temporary launch configs.

Because even a temporary config may need migration, we can never save
after migration. Instead, migrate always on-the-fly. The migrated config
will only be persisted by the launch configuration dialog.

Change-Id: I802460af4cefcabacb4097e766388fbfc897443b
Signed-off-by: Julian Honnen <julian.honnen@vector.com>
diff --git a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java
index 1d9e775..d3edc62 100644
--- a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java
+++ b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java
@@ -54,11 +54,12 @@
 
 	public static Map<IPluginModelBase, String> getMergedBundleMap(ILaunchConfiguration configuration, boolean osgi) throws CoreException {
 
+		ILaunchConfigurationWorkingCopy wc = getWorkingCopy(configuration);
 		if (!osgi) {
 
-			migrateLaunchConfiguration(configuration);
+			migrateLaunchConfiguration(wc);
 
-			if (configuration.getAttribute(IPDELauncherConstants.USE_DEFAULT, true)) {
+			if (wc.getAttribute(IPDELauncherConstants.USE_DEFAULT, true)) {
 				Map<IPluginModelBase, String> map = new LinkedHashMap<>();
 				IPluginModelBase[] models = PluginRegistry.getActiveModels();
 				for (IPluginModelBase model : models) {
@@ -68,16 +69,16 @@
 			}
 
 		} else {
-			migrateOsgiLaunchConfiguration(configuration);
+			migrateOsgiLaunchConfiguration(wc);
 		}
 
-		if (configuration.getAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false)) {
-			return getMergedBundleMapFeatureBased(configuration, osgi);
+		if (wc.getAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false)) {
+			return getMergedBundleMapFeatureBased(wc, osgi);
 		}
 
 		Set<String> set = new HashSet<>();
-		Map<IPluginModelBase, String> map = getWorkspaceBundleMap(configuration, set);
-		map.putAll(getTargetBundleMap(configuration, set));
+		Map<IPluginModelBase, String> map = getWorkspaceBundleMap(wc, set);
+		map.putAll(getTargetBundleMap(wc, set));
 		return map;
 	}
 
@@ -493,12 +494,11 @@
 	}
 
 	@SuppressWarnings("deprecation")
-	public static void migrateLaunchConfiguration(ILaunchConfiguration configuration) throws CoreException {
-		ILaunchConfigurationWorkingCopy wc = getWorkingCopy(configuration);
+	public static void migrateLaunchConfiguration(ILaunchConfigurationWorkingCopy configuration) throws CoreException {
 
 		String value = configuration.getAttribute("wsproject", (String) null); //$NON-NLS-1$
 		if (value != null) {
-			wc.setAttribute("wsproject", (String) null); //$NON-NLS-1$
+			configuration.setAttribute("wsproject", (String) null); //$NON-NLS-1$
 			if (value.indexOf(';') != -1) {
 				value = value.replace(';', ',');
 			} else if (value.indexOf(':') != -1) {
@@ -510,24 +510,24 @@
 
 			boolean automatic = configuration.getAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
 			String attr = automatic ? IPDELauncherConstants.DESELECTED_WORKSPACE_PLUGINS : IPDELauncherConstants.SELECTED_WORKSPACE_PLUGINS;
-			wc.setAttribute(attr, value);
+			configuration.setAttribute(attr, value);
 		}
 
 		String value2 = configuration.getAttribute("extplugins", (String) null); //$NON-NLS-1$
 		if (value2 != null) {
-			wc.setAttribute("extplugins", (String) null); //$NON-NLS-1$
+			configuration.setAttribute("extplugins", (String) null); //$NON-NLS-1$
 			if (value2.indexOf(';') != -1) {
 				value2 = value2.replace(';', ',');
 			} else if (value2.indexOf(':') != -1) {
 				value2 = value2.replace(':', ',');
 			}
 			value2 = (value2.length() == 0 || value2.equals(",")) ? null : value2.substring(0, value2.length() - 1); //$NON-NLS-1$
-			wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_PLUGINS, value2);
+			configuration.setAttribute(IPDELauncherConstants.SELECTED_TARGET_PLUGINS, value2);
 		}
 
-		convertToSet(wc, IPDELauncherConstants.SELECTED_TARGET_PLUGINS, IPDELauncherConstants.SELECTED_TARGET_BUNDLES);
-		convertToSet(wc, IPDELauncherConstants.SELECTED_WORKSPACE_PLUGINS, IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES);
-		convertToSet(wc, IPDELauncherConstants.DESELECTED_WORKSPACE_PLUGINS, IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES);
+		convertToSet(configuration, IPDELauncherConstants.SELECTED_TARGET_PLUGINS, IPDELauncherConstants.SELECTED_TARGET_BUNDLES);
+		convertToSet(configuration, IPDELauncherConstants.SELECTED_WORKSPACE_PLUGINS, IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES);
+		convertToSet(configuration, IPDELauncherConstants.DESELECTED_WORKSPACE_PLUGINS, IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES);
 
 		String version = configuration.getAttribute(IPDEConstants.LAUNCHER_PDE_VERSION, (String) null);
 		boolean newApp = TargetPlatformHelper.usesNewApplicationModel();
@@ -535,7 +535,7 @@
 		if (!upgrade)
 			upgrade = TargetPlatformHelper.getTargetVersion() >= 3.2 && version == null;
 		if (upgrade) {
-			wc.setAttribute(IPDEConstants.LAUNCHER_PDE_VERSION, newApp ? "3.3" : "3.2a"); //$NON-NLS-1$ //$NON-NLS-2$
+			configuration.setAttribute(IPDEConstants.LAUNCHER_PDE_VERSION, newApp ? "3.3" : "3.2a"); //$NON-NLS-1$ //$NON-NLS-2$
 			boolean usedefault = configuration.getAttribute(IPDELauncherConstants.USE_DEFAULT, true);
 			boolean automaticAdd = configuration.getAttribute(IPDELauncherConstants.AUTOMATIC_ADD, true);
 			if (!usedefault) {
@@ -564,15 +564,11 @@
 					}
 				}
 				if (!extensions.isEmpty())
-					wc.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, extensions);
+					configuration.setAttribute(IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES, extensions);
 				if (!target.isEmpty())
-					wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES, target);
+					configuration.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES, target);
 			}
 		}
-
-		if (wc.isDirty()) {
-			wc.doSave();
-		}
 	}
 
 	private static ILaunchConfigurationWorkingCopy getWorkingCopy(ILaunchConfiguration configuration) throws CoreException {
@@ -583,16 +579,10 @@
 	}
 
 	@SuppressWarnings("deprecation")
-	public static void migrateOsgiLaunchConfiguration(ILaunchConfiguration configuration) throws CoreException {
-		ILaunchConfigurationWorkingCopy wc = getWorkingCopy(configuration);
-
-		convertToSet(wc, IPDELauncherConstants.WORKSPACE_BUNDLES, IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES);
-		convertToSet(wc, IPDELauncherConstants.TARGET_BUNDLES, IPDELauncherConstants.SELECTED_TARGET_BUNDLES);
-		convertToSet(wc, IPDELauncherConstants.DESELECTED_WORKSPACE_PLUGINS, IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES);
-
-		if (wc.isDirty()) {
-			wc.doSave();
-		}
+	public static void migrateOsgiLaunchConfiguration(ILaunchConfigurationWorkingCopy configuration) throws CoreException {
+		convertToSet(configuration, IPDELauncherConstants.WORKSPACE_BUNDLES, IPDELauncherConstants.SELECTED_WORKSPACE_BUNDLES);
+		convertToSet(configuration, IPDELauncherConstants.TARGET_BUNDLES, IPDELauncherConstants.SELECTED_TARGET_BUNDLES);
+		convertToSet(configuration, IPDELauncherConstants.DESELECTED_WORKSPACE_PLUGINS, IPDELauncherConstants.DESELECTED_WORKSPACE_BUNDLES);
 	}
 
 	private static void convertToSet(ILaunchConfigurationWorkingCopy wc, String stringAttribute, String listAttribute) throws CoreException {
diff --git a/ui/org.eclipse.pde.ui.tests/META-INF/MANIFEST.MF b/ui/org.eclipse.pde.ui.tests/META-INF/MANIFEST.MF
index 9c41832..665c924 100644
--- a/ui/org.eclipse.pde.ui.tests/META-INF/MANIFEST.MF
+++ b/ui/org.eclipse.pde.ui.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@
 Bundle-ManifestVersion: 2
 Bundle-Name: PDE JUnit Tests
 Bundle-SymbolicName: org.eclipse.pde.ui.tests; singleton:=true
-Bundle-Version: 3.10.800.qualifier
+Bundle-Version: 3.10.900.qualifier
 Bundle-ClassPath: tests.jar
 Bundle-Activator: org.eclipse.pde.ui.tests.PDETestsPlugin
 Bundle-Vendor: Eclipse.org
diff --git a/ui/org.eclipse.pde.ui.tests/pom.xml b/ui/org.eclipse.pde.ui.tests/pom.xml
index db1b1d6..0e82776 100644
--- a/ui/org.eclipse.pde.ui.tests/pom.xml
+++ b/ui/org.eclipse.pde.ui.tests/pom.xml
@@ -19,7 +19,7 @@
   </parent>
   <groupId>org.eclipse.pde</groupId>
   <artifactId>org.eclipse.pde.ui.tests</artifactId>
-  <version>3.10.800-SNAPSHOT</version>
+  <version>3.10.900-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
 
   <properties>
diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/LaunchConfigurationMigrationTest.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/LaunchConfigurationMigrationTest.java
index 3a35bdc..8120df9 100644
--- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/LaunchConfigurationMigrationTest.java
+++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/LaunchConfigurationMigrationTest.java
@@ -57,6 +57,7 @@
 
 		ILaunchConfigurationWorkingCopy wc = configuration.getWorkingCopy();
 		BundleLauncherHelper.migrateLaunchConfiguration(wc);
+		assertTrue(wc.isDirty());
 
 		assertOldPropertiesRemoved(wc);
 
@@ -75,6 +76,7 @@
 
 		ILaunchConfigurationWorkingCopy wc = configuration.getWorkingCopy();
 		BundleLauncherHelper.migrateLaunchConfiguration(wc);
+		assertTrue(wc.isDirty());
 
 		assertOldPropertiesRemoved(wc);
 
@@ -92,6 +94,7 @@
 
 		ILaunchConfigurationWorkingCopy wc = configuration.getWorkingCopy();
 		BundleLauncherHelper.migrateOsgiLaunchConfiguration(wc);
+		assertTrue(wc.isDirty());
 
 		assertOldOsgiPropertiesRemoved(wc);
 
@@ -113,7 +116,7 @@
 	}
 
 	@SuppressWarnings("deprecation")
-	private void assertOldOsgiPropertiesRemoved(ILaunchConfigurationWorkingCopy wc) throws CoreException {
+	private void assertOldOsgiPropertiesRemoved(ILaunchConfiguration wc) throws CoreException {
 		assertFalse("workspace_bundles should not be present",
 				wc.hasAttribute(IPDELauncherConstants.WORKSPACE_BUNDLES));
 		assertFalse("target_bundles should not be present", wc.hasAttribute(IPDELauncherConstants.TARGET_BUNDLES));
diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/OSGiLauncherTabGroup.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/OSGiLauncherTabGroup.java
index b8992c5..adc84ab 100644
--- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/OSGiLauncherTabGroup.java
+++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/OSGiLauncherTabGroup.java
@@ -48,7 +48,7 @@
 		BusyIndicator.showWhile(Display.getCurrent(), () -> {
 			try {
 				if (config instanceof ILaunchConfigurationWorkingCopy) {
-					BundleLauncherHelper.migrateOsgiLaunchConfiguration(config);
+					BundleLauncherHelper.migrateOsgiLaunchConfiguration((ILaunchConfigurationWorkingCopy) config);
 				}
 			} catch (CoreException e) {
 			}