diff --git a/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/core/envvar/IEnvironmentVariableManagerTests.java b/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/core/envvar/IEnvironmentVariableManagerTests.java index a659f351f55..892e5f02728 100644 --- a/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/core/envvar/IEnvironmentVariableManagerTests.java +++ b/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 diff --git a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/envvar/PrefsStorableEnvironment.java b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/envvar/PrefsStorableEnvironment.java index 21b98e57ec2..11090263968 100644 --- a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/envvar/PrefsStorableEnvironment.java +++ b/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 */ - volatile IEclipsePreferences prefs; + private Set registeredOn = Collections.synchronizedSet(new HashSet()); - public PrefListener(ISerializeInfo info) { + /** The node we're registered on */ + volatile IEclipsePreferences root; + + Reference parentRef; + + public PrefListener(PrefsStorableEnvironment parent, ISerializeInfo info) { + this.parentRef = new WeakReference(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); - prefs.removeNodeChangeListener(this); + removeListener(); } 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(); - if (prefs != null) { - prefs.addPreferenceChangeListener(this); - prefs.addNodeChangeListener(this); - } + root = (IEclipsePreferences)info.getNode(); + if (root != null) + addListener(root); 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();