Bug 361675 - Order mismatch when saving/restoring workspace treesv20120117-2305

3 files changed, 96 insertions, 17 deletions

diff --git a/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/SaveManager.java b/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/SaveManager.java
index 54a0b49a9..aa0ed35d8 100644
--- a/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/SaveManager.java
+++ b/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/SaveManager.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2011 IBM Corporation and others.
+ * Copyright (c) 2000, 2012 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,9 +7,10 @@
  * 
  * Contributors:
  *     IBM Corporation - initial API and implementation
- * Francis Lynch (Wind River) - [301563] Save and load tree snapshots
- * Francis Lynch (Wind River) - [305718] Allow reading snapshot into renamed project
- * Broadcom Corporation - ongoing development
+ *     Francis Lynch (Wind River) - [301563] Save and load tree snapshots
+ *     Francis Lynch (Wind River) - [305718] Allow reading snapshot into renamed project
+ *     Baltasar Belyavsky (Texas Instruments) - [361675] Order mismatch when saving/restoring workspace trees
+ *     Broadcom Corporation - ongoing development
  *******************************************************************************/
 package org.eclipse.core.internal.resources;
 
@@ -1822,11 +1823,12 @@ public class SaveManager implements IElementInfoFlattener, IManager, IStringPool
 	 * @param trees list of trees to be persisted
 	 * @param builderInfos list of builder infos; one per builder 
 	 * @param configNames configuration names persisted for builder infos above
-	 * @param additionalBuilderInfos remaining trees to be persisted for other configurations
+	 * @param additionalTrees remaining trees to be persisted for other configurations
+	 * @param additionalBuilderInfos remaining builder infos for other configurations
 	 * @param additionalConfigNames configuration names of the remaining per-configuration trees
 	 * @throws CoreException
 	 */
-	private void getTreesToSave(IProject project, List<ElementTree> trees, List<BuilderPersistentInfo> builderInfos, List<String> configNames, List<BuilderPersistentInfo> additionalBuilderInfos, List<String> additionalConfigNames) throws CoreException {
+	private void getTreesToSave(IProject project, List<ElementTree> trees, List<BuilderPersistentInfo> builderInfos, List<String> configNames, List<ElementTree> additionalTrees, List<BuilderPersistentInfo> additionalBuilderInfos, List<String> additionalConfigNames) throws CoreException {
 		if (project.isOpen()) {
 			String activeConfigName = project.getActiveBuildConfig().getName();
 			List<BuilderPersistentInfo> infos = workspace.getBuildManager().createBuildersPersistentInfo(project);
@@ -1846,13 +1848,12 @@ public class SaveManager implements IElementInfoFlattener, IManager, IStringPool
 						// TODO could probably do better by serializing the 'oldest' tree
 						builderInfos.add(info);
 						configNames.add(configName);
+						trees.add(info.getLastBuiltTree());
 					} else {
 						additionalBuilderInfos.add(info);
 						additionalConfigNames.add(configName);
+						additionalTrees.add(info.getLastBuiltTree());
 					}
-					// Add the builder's tree
-					ElementTree tree = info.getLastBuiltTree();
-					trees.add(tree);
 				}
 			}
 		}
@@ -1906,14 +1907,19 @@ public class SaveManager implements IElementInfoFlattener, IManager, IStringPool
 				IProject[] projects = workspace.getRoot().getProjects(IContainer.INCLUDE_HIDDEN);
 				List<BuilderPersistentInfo> builderInfos = new ArrayList<BuilderPersistentInfo>(projects.length * 2);
 				List<String> configNames = new ArrayList<String>(projects.length);
+				List<ElementTree> additionalTrees = new ArrayList<ElementTree>(projects.length * 2);
 				List<BuilderPersistentInfo> additionalBuilderInfos = new ArrayList<BuilderPersistentInfo>(projects.length * 2);
 				List<String> additionalConfigNames = new ArrayList<String>(projects.length);
 				for (int i = 0; i < projects.length; i++)
-					getTreesToSave(projects[i], trees, builderInfos, configNames, additionalBuilderInfos, additionalConfigNames);
+					getTreesToSave(projects[i], trees, builderInfos, configNames, additionalTrees, additionalBuilderInfos, additionalConfigNames);
 
 				// Save the version 2 builders info
 				writeBuilderPersistentInfo(output, builderInfos, Policy.subMonitorFor(monitor, Policy.totalWork * 10 / 100));
 
+				// Builder infos of non-active configurations are persisted after the active 
+				// configuration's builder infos. So, their trees have to follow the same order.
+				trees.addAll(additionalTrees);
+				
 				// add the current tree in the list as the last tree in the chain
 				trees.add(current);
 
@@ -1976,11 +1982,16 @@ public class SaveManager implements IElementInfoFlattener, IManager, IStringPool
 				List<BuilderPersistentInfo> builderInfos = new ArrayList<BuilderPersistentInfo>(5);
 				List<String> additionalConfigNames = new ArrayList<String>(5);
 				List<BuilderPersistentInfo> additionalBuilderInfos = new ArrayList<BuilderPersistentInfo>(5);
-				getTreesToSave(project, trees, builderInfos, configNames, additionalBuilderInfos, additionalConfigNames);
+				List<ElementTree> additionalTrees = new ArrayList<ElementTree>(5);
+				getTreesToSave(project, trees, builderInfos, configNames, additionalTrees, additionalBuilderInfos, additionalConfigNames);
 
 				// Save the version 2 builders info
 				writeBuilderPersistentInfo(output, builderInfos, Policy.subMonitorFor(monitor, Policy.totalWork * 20 / 100));
 
+				// Builder infos of non-active configurations are persisted after the active 
+				// configuration's builder infos. So, their trees have to follow the same order.
+				trees.addAll(additionalTrees);
+
 				// Add the current tree in the list as the last tree in the chain
 				trees.add(current);
 
diff --git a/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/WorkspaceTreeReader_2.java b/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/WorkspaceTreeReader_2.java
index 2e8b4a5ea..41194b2a6 100644
--- a/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/WorkspaceTreeReader_2.java
+++ b/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/WorkspaceTreeReader_2.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2011 IBM Corporation and others.
+ * Copyright (c) 2000, 2012 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  * 
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Baltasar Belyavsky (Texas Instruments) - [361675] Order mismatch when saving/restoring workspace trees
  *     Broadcom Corporation - ongoing development
  *******************************************************************************/
 package org.eclipse.core.internal.resources;
@@ -95,18 +96,22 @@ public class WorkspaceTreeReader_2 extends WorkspaceTreeReader_1 {
 			readPluginsSavedStates(input, savedStates, pluginsToBeLinked, Policy.subMonitorFor(monitor, Policy.opWork * 10 / 100));
 			workspace.getSaveManager().setPluginsSavedState(savedStates);
 
+			int treeIndex = pluginsToBeLinked.size();
+
 			List<BuilderPersistentInfo> buildersToBeLinked = new ArrayList<BuilderPersistentInfo>(20);
 			readBuildersPersistentInfo(null, input, buildersToBeLinked, Policy.subMonitorFor(monitor, Policy.opWork * 10 / 100));
 
 			final ElementTree[] trees = readTrees(Path.ROOT, input, Policy.subMonitorFor(monitor, Policy.opWork * 40 / 100));
 			linkPluginsSavedStateToTrees(pluginsToBeLinked, trees, Policy.subMonitorFor(monitor, Policy.opWork * 10 / 100));
-			linkBuildersToTrees(buildersToBeLinked, trees, pluginsToBeLinked.size(), Policy.subMonitorFor(monitor, Policy.opWork * 10 / 100));
+			linkBuildersToTrees(buildersToBeLinked, trees, treeIndex, Policy.subMonitorFor(monitor, Policy.opWork * 10 / 100));
 
 			// Since 3.7: Read the per-configuration trees if available
 			if (input.available() > 0) {
+				treeIndex += buildersToBeLinked.size();
+
 				buildersToBeLinked.clear();
 				readBuildersPersistentInfo(null, input, buildersToBeLinked, Policy.subMonitorFor(monitor, Policy.opWork * 10 / 100));
-				linkBuildersToTrees(buildersToBeLinked, trees, 0, Policy.subMonitorFor(monitor, Policy.opWork * 10 / 100));
+				linkBuildersToTrees(buildersToBeLinked, trees, treeIndex, Policy.subMonitorFor(monitor, Policy.opWork * 10 / 100));
 
 				for (Iterator<BuilderPersistentInfo> it = builderInfos.iterator(); it.hasNext();)
 					it.next().setConfigName(input.readUTF());
@@ -139,18 +144,21 @@ public class WorkspaceTreeReader_2 extends WorkspaceTreeReader_1 {
 			// Read the version 2 part of the file, but don't set the builder info in
 			// the projects. It is stored in builderInfos instead.
 
+			int treeIndex = 0;
+			
 			List<BuilderPersistentInfo> buildersToBeLinked = new ArrayList<BuilderPersistentInfo>(20);
 			readBuildersPersistentInfo(project, input, buildersToBeLinked, Policy.subMonitorFor(monitor, 1));
 
 			ElementTree[] trees = readTrees(project.getFullPath(), input, Policy.subMonitorFor(monitor, 8));
-			linkBuildersToTrees(buildersToBeLinked, trees, 0, Policy.subMonitorFor(monitor, 1));
+			linkBuildersToTrees(buildersToBeLinked, trees, treeIndex, Policy.subMonitorFor(monitor, 1));
 
 			// Since 3.7: Read the additional builder information
 			if (input.available() > 0) {
+				treeIndex += buildersToBeLinked.size();
 
 				List<BuilderPersistentInfo> infos = new ArrayList<BuilderPersistentInfo>(5);
 				readBuildersPersistentInfo(project, input, infos, Policy.subMonitorFor(monitor, 1));
-				linkBuildersToTrees(infos, trees, 0, Policy.subMonitorFor(monitor, 1));
+				linkBuildersToTrees(infos, trees, treeIndex, Policy.subMonitorFor(monitor, 1));
 
 				for (Iterator<BuilderPersistentInfo> it = builderInfos.iterator(); it.hasNext();)
 					it.next().setConfigName(input.readUTF());
diff --git a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/BuildConfigurationsTest.java b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/BuildConfigurationsTest.java
index 52a25064f..a4939356b 100644
--- a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/BuildConfigurationsTest.java
+++ b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/BuildConfigurationsTest.java
@@ -1,11 +1,12 @@
 /*******************************************************************************
- * Copyright (c) 2010 Broadcom Corporation and others. All rights reserved.
+ * Copyright (c) 2010, 2012 Broadcom Corporation and others. All rights reserved.
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
  * and is available at http://www.eclipse.org/legal/epl-v10.html
  *
  * Contributors:
  * Broadcom Corporation - initial API and implementation
+ * Baltasar Belyavsky (Texas Instruments) - [361675] Order mismatch when saving/restoring workspace trees
  ******************************************************************************/
 package org.eclipse.core.tests.internal.builders;
 
@@ -13,6 +14,7 @@ import junit.framework.Test;
 import junit.framework.TestSuite;
 import org.eclipse.core.resources.*;
 import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.tests.resources.ResourceDeltaVerifier;
 
 /**
  * These tests exercise the project buildConfigs functionality which allows a different
@@ -117,6 +119,64 @@ public class BuildConfigurationsTest extends AbstractBuilderTest {
 	}
 
 	/**
+	 * Tests that deltas are restored in the correct order per variant when a project is closed then opened.
+	 */
+	public void testCloseAndOpenProject_Bug361675() throws CoreException {
+		IWorkspaceRoot root = getWorkspace().getRoot();
+		IProject tempProject = root.getProject("BuildVariantTest_pTemp");
+		IFile tempFile0 = tempProject.getFile("File0");
+		IFile tempFile1 = tempProject.getFile("File1");
+		IResource[] resources = {tempProject, tempFile0, tempFile1};
+		ensureExistsInWorkspace(resources, true);
+		setupProject(tempProject);
+
+		try {
+			ConfigurationBuilder.clearStats();
+
+			tempFile0.setContents(getRandomContents(), true, true, getMonitor());
+			tempFile1.setContents(getRandomContents(), true, true, getMonitor());
+			incrementalBuild(1, tempProject, variant0, true, 1, IncrementalProjectBuilder.FULL_BUILD);
+			incrementalBuild(2, tempProject, variant1, true, 1, IncrementalProjectBuilder.FULL_BUILD);
+			incrementalBuild(3, tempProject, variant2, true, 1, IncrementalProjectBuilder.FULL_BUILD);
+
+			tempFile0.setContents(getRandomContents(), true, true, getMonitor());
+			incrementalBuild(4, tempProject, variant1, true, 2, IncrementalProjectBuilder.INCREMENTAL_BUILD);
+
+			tempFile1.setContents(getRandomContents(), true, true, getMonitor());
+			incrementalBuild(5, tempProject, variant2, true, 2, IncrementalProjectBuilder.INCREMENTAL_BUILD);
+
+			tempProject.close(getMonitor());
+			ConfigurationBuilder.clearStats();
+			tempProject.open(getMonitor());
+
+			// verify variant0 - both File0 and File1 are expected to have changed since it was last built
+			incrementalBuild(6, tempProject, variant0, true, 1, IncrementalProjectBuilder.INCREMENTAL_BUILD);
+			ConfigurationBuilder builder0 = ConfigurationBuilder.getBuilder(tempProject.getBuildConfig(variant0));
+			assertNotNull("6.10", builder0);
+			ResourceDeltaVerifier verifier0 = new ResourceDeltaVerifier();
+			verifier0.addExpectedChange(tempFile0, tempProject, IResourceDelta.CHANGED, IResourceDelta.CONTENT);
+			verifier0.addExpectedChange(tempFile1, tempProject, IResourceDelta.CHANGED, IResourceDelta.CONTENT);
+			verifier0.verifyDelta(builder0.deltaForLastBuild);
+			assertTrue("6.11: " + verifier0.getMessage(), verifier0.isDeltaValid());
+
+			// verify variant1 - only File1 is expected to have changed since it was last built
+			incrementalBuild(7, tempProject, variant1, true, 1, IncrementalProjectBuilder.INCREMENTAL_BUILD);
+			ConfigurationBuilder builder1 = ConfigurationBuilder.getBuilder(tempProject.getBuildConfig(variant1));
+			assertNotNull("7.10", builder1);
+			ResourceDeltaVerifier verifier1 = new ResourceDeltaVerifier();
+			verifier1.addExpectedChange(tempFile1, tempProject, IResourceDelta.CHANGED, IResourceDelta.CONTENT);
+			verifier1.verifyDelta(builder1.deltaForLastBuild);
+			assertTrue("7.11: " + verifier1.getMessage(), verifier1.isDeltaValid());
+
+			// verify variant2 - no changes are expected since it was last built
+			incrementalBuild(8, tempProject, variant2, false, 0, 0);
+
+		} finally {
+			tempProject.delete(true, getMonitor());
+		}
+	}
+
+	/**
 	 * Run a workspace build with project references
 	 *
 	 * References are: