Bug 295436 changes to the environment in org.eclipse.cdt.core.prefs file on disk are not noticed by existing Project Descriptions.

core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/core/envvar/IEnvironmentVariableManagerTests.java

@@ -333,6 +333,60 @@ public class IEnvironmentVariableManagerTests extends TestCase {
 		assertEquals(var, envManager.getVariable(var.getName(), prjDesc.getConfigurationById(id1), true));
 	}
 
+	/**
+	 * Tests file system change of the settings file without recreating the project description
+	 */
+	public void testSettingsOverwriteBug295436() throws Exception {
+		final IProject project = ResourceHelper.createCDTProjectWithConfig("envProject");
+
+		// Add another, derived configuration
+		ICProjectDescription prjDesc = CoreModel.getDefault().getProjectDescription(project);
+		ICConfigurationDescription desc = prjDesc.getActiveConfiguration();
+		final String id1 = desc.getId();  	      // Config 1's ID
+		final String id2 = CDataUtil.genId(id1);  // Config 2's ID
+		prjDesc.createConfiguration(id2, "config2", desc);
+		CoreModel.getDefault().setProjectDescription(project, prjDesc);
+
+		// Get all the configurations
+		prjDesc = CoreModel.getDefault().getProjectDescription(project);
+		ICConfigurationDescription[] descs = prjDesc.getConfigurations();
+		assertTrue(descs.length == 2);
+
+		IEnvironmentVariableManager envManager = CCorePlugin.getDefault().getBuildEnvironmentManager();
+		IContributedEnvironment contribEnv = envManager.getContributedEnvironment();
+
+		assertFalse(descs[0].isModified());
+		assertFalse(descs[1].isModified());
+
+		// Try setting an environment variable
+		final IEnvironmentVariable var = new EnvironmentVariable("FOO", "BAR");
+		contribEnv.addVariable(var, prjDesc.getConfigurationById(id1));
+		assertTrue(prjDesc.getConfigurationById(id1).isModified());
+		assertFalse(prjDesc.getConfigurationById(id2).isModified());
+		CoreModel.getDefault().setProjectDescription(project, prjDesc);
+
+		// Backup the settings file
+		project.getFile(".settings/org.eclipse.cdt.core.prefs.bak").create(
+				project.getFile(".settings/org.eclipse.cdt.core.prefs").getContents(), true, null);
+
+		// Change the environment variable
+		final IEnvironmentVariable var2 = new EnvironmentVariable("FOO", "BOO");
+		contribEnv.addVariable(var2, prjDesc.getConfigurationById(id1));
+		assertEquals(var2, envManager.getVariable(var2.getName(), prjDesc.getConfigurationById(id1), true));
+		CoreModel.getDefault().setProjectDescription(project, prjDesc);
+
+		// clean desc should be updated when the preference file is overwritten
+		final ICProjectDescription cleanDesc = CoreModel.getDefault().getProjectDescription(project);
+		assertEquals(contribEnv.getVariable(var.getName(), cleanDesc.getConfigurationById(id1)).getValue(), var2.getValue());
+
+		// Replace the settings with it's backup
+		project.getFile(".settings/org.eclipse.cdt.core.prefs").setContents(
+				project.getFile(".settings/org.eclipse.cdt.core.prefs.bak").getContents(), true, false, null);
+		// check that cleanDesc has been updated
+		assertEquals(contribEnv.getVariable(var.getName(), cleanDesc.getConfigurationById(id1)).getValue(), var.getValue());
+		assertEquals(var, envManager.getVariable(var.getName(), cleanDesc.getConfigurationById(id1), true));
+	}
+
 	/**
 	 * Test that on deleting and recreating the project variables haven't persisted
 	 * @throws Exception

core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/envvar/PrefsStorableEnvironment.java

@@ -10,6 +10,9 @@
  *******************************************************************************/
 package org.eclipse.cdt.utils.envvar;
 
+import java.lang.ref.Reference;
+import java.lang.ref.WeakReference;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -21,6 +24,7 @@ import org.eclipse.cdt.core.envvar.IEnvironmentVariable;
 import org.eclipse.cdt.core.settings.model.ICStorageElement;
 import org.eclipse.cdt.utils.envvar.StorableEnvironmentLoader.ISerializeInfo;
 import org.eclipse.core.runtime.preferences.IEclipsePreferences;
+import org.eclipse.core.runtime.preferences.IPreferenceNodeVisitor;
 import org.eclipse.core.runtime.preferences.IEclipsePreferences.INodeChangeListener;
 import org.eclipse.core.runtime.preferences.IEclipsePreferences.IPreferenceChangeListener;
 import org.eclipse.core.runtime.preferences.IEclipsePreferences.NodeChangeEvent;
@@ -74,10 +78,15 @@ public class PrefsStorableEnvironment extends StorableEnvironment {
 		/** boolean indicating whether preferences have changed */
 		private volatile boolean prefsChanged = true;
 
-		/** The node we're registered on */
+		private Set<IEclipsePreferences> registeredOn = Collections.synchronizedSet(new HashSet<IEclipsePreferences>());
-		volatile IEclipsePreferences prefs;
 
-		public PrefListener(ISerializeInfo info) {
+		/** The node we're registered on */
+		volatile IEclipsePreferences root;
+
+		Reference<PrefsStorableEnvironment> parentRef;
+
+		public PrefListener(PrefsStorableEnvironment parent, ISerializeInfo info) {
+			this.parentRef = new WeakReference<PrefsStorableEnvironment>(parent);
 			register (info);
 		}
 
@@ -85,15 +94,15 @@ public class PrefsStorableEnvironment extends StorableEnvironment {
 		 * Remove the listener
 		 */
 		public void remove() {
-			if (prefs != null) {
+			if (root != null) {
 				try {
-					prefs.removePreferenceChangeListener(this);
+					removeListener();
-					prefs.removeNodeChangeListener(this);
 				} catch (Exception e) {
+					CCorePlugin.log(e);
 					// Catch all exceptions, this is called during parent finalization which we don't want to prevent...
 					//   e.g. IllegalStateException may occur during de-register
 				}
-				prefs = null;
+				root = null;
 			}
 		}
 
@@ -101,36 +110,70 @@ public class PrefsStorableEnvironment extends StorableEnvironment {
 		 * Register the Prefs change listener
 		 */
 		private void register(ISerializeInfo info) {
-			if (prefs != null)
+			if (root != null)
 				return;
-			prefs = (IEclipsePreferences)info.getNode();
+			root = (IEclipsePreferences)info.getNode();
-			if (prefs != null) {
+			if (root != null)
-				prefs.addPreferenceChangeListener(this);
+				addListener(root);
-				prefs.addNodeChangeListener(this);
-			}
 			prefsChanged = true;
 		}
 
+		private void addListener(IEclipsePreferences node) {
+			try {
+				node.accept(new IPreferenceNodeVisitor() {
+					public boolean visit(IEclipsePreferences node) throws BackingStoreException {
+						// {environment/{project|workspace/}config_name/variable/...
+						node.addPreferenceChangeListener(PrefListener.this);
+						node.addNodeChangeListener(PrefListener.this);
+						registeredOn.add(node);
+						return true;
+					}
+				});
+			} catch (BackingStoreException e) {
+				CCorePlugin.log(e);
+			}
+		}
+		private void removeListener() {
+			synchronized(registeredOn) {
+				for (IEclipsePreferences pref : registeredOn) {
+					try {
+						// {environment/{project|workspace/}config_name/variable/...
+						pref.removePreferenceChangeListener(PrefListener.this);
+						pref.removeNodeChangeListener(PrefListener.this);
+					} catch (IllegalStateException e) {
+						// Catch all exceptions, this is called during parent finalization which we don't want to prevent...
+						//   e.g. IllegalStateException may occur during de-register
+					}
+				}
+			}
+		}
+
 		/**
 		 * Return & unset flag indicating if there has been a change in the backing store
 		 * @return whether there's been a change in the backing store
 		 */
 		public boolean preferencesChanged(ISerializeInfo info) {
-			if (prefs == null)
+			if (root == null)
 				register(info);
 
 			boolean retVal = prefsChanged;
 			// If we're registered for change, then unset
-			if (prefs != null)
+			if (root != null)
 				prefsChanged = false;
 			return retVal;
 		}
 
 		public void preferenceChange(PreferenceChangeEvent event) {
 			prefsChanged = true;
+			if (parentRef.get() == null)
+				removeListener();
 		}
 		public void added(NodeChangeEvent event) {
 			prefsChanged = true;
+			if (parentRef.get() == null)
+				removeListener();
+			else
+				addListener((IEclipsePreferences)event.getChild());
 		}
 		public void removed(NodeChangeEvent event) {
 			prefsChanged = true;
@@ -219,7 +262,7 @@ public class PrefsStorableEnvironment extends StorableEnvironment {
 		if (fPrefsChangedListener != null)
 			fPrefsChangedListener.remove();
  		fSerialEnv = serializeInfo;
- 		fPrefsChangedListener = new PrefListener(fSerialEnv);
+ 		fPrefsChangedListener = new PrefListener(this, fSerialEnv);
 
 		// Update the cached state
 		checkBackingSerializeInfo();