From ba5001ebc1b65511ccb0f3e5b95144f578708e3a Mon Sep 17 00:00:00 2001 From: James Blackburn Date: Tue, 18 May 2010 16:05:28 +0000 Subject: [PATCH] Bug 312575 Updating a .cproject doesn't update settings in referencing projects. Patch 3 + JavaDoc --- ...onfigurationDescriptionExportSettings.java | 2 +- .../model/CExternalSettingsHolder.java | 4 + .../model/CExternalSettingsManager.java | 128 ++++++++++-------- .../settings/model/CRefSettingsHolder.java | 50 ++++++- .../core/settings/model/CSettingsRefInfo.java | 5 +- .../CfgExportSettingContainerFactory.java | 25 +++- .../model/ICExternalSettingsListener.java | 8 +- 7 files changed, 156 insertions(+), 66 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/CConfigurationDescriptionExportSettings.java b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/CConfigurationDescriptionExportSettings.java index c9a3e3ffd34..7515deff93c 100644 --- a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/CConfigurationDescriptionExportSettings.java +++ b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/CConfigurationDescriptionExportSettings.java @@ -299,7 +299,7 @@ public class CConfigurationDescriptionExportSettings extends BaseTestCase { * causes referencing projects to correctly pick up changes to the project exports. * https://bugs.eclipse.org/bugs/show_bug.cgi?id=312575 */ - public void _testExportedSettingsExternalUpdate() throws Exception { + public void testExportedSettingsExternalUpdate() throws Exception { final IProject libProj = ResourceHelper.createCDTProjectWithConfig("libProj312575"); final IProject mainProj = ResourceHelper.createCDTProjectWithConfig("mainProj312575"); diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsHolder.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsHolder.java index 7eaeabf44c0..eb63c0febee 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsHolder.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsHolder.java @@ -22,7 +22,11 @@ import org.eclipse.cdt.core.settings.model.ICSettingEntry; import org.eclipse.cdt.core.settings.model.ICStorageElement; import org.eclipse.cdt.internal.core.settings.model.CExternalSettinsDeltaCalculator.ExtSettingMapKey; +/** + * The raw external settings as exported by a project configuration. + */ public class CExternalSettingsHolder extends CExternalSettingsContainer { + private Map fSettingsMap; static final String ELEMENT_EXT_SETTINGS_CONTAINER = "externalSettings"; //$NON-NLS-1$ diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsManager.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsManager.java index 51c138d8205..3056b5e8796 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsManager.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsManager.java @@ -20,6 +20,7 @@ import java.util.Map; import java.util.Set; import org.eclipse.cdt.core.CCorePlugin; +import org.eclipse.cdt.core.model.CoreModel; import org.eclipse.cdt.core.settings.model.CExternalSetting; import org.eclipse.cdt.core.settings.model.CProjectDescriptionEvent; import org.eclipse.cdt.core.settings.model.ICConfigurationDescription; @@ -73,9 +74,14 @@ public class CExternalSettingsManager implements ICExternalSettingsListener, ICP return fInstance; } + /** + * A simple class representing an external settings container. + * These are uniquely identifiable by the factoryId + factory + * specific container id + */ public final static class CContainerRef { - private String fFactoryId; - private String fContainerId; + private final String fFactoryId; + private final String fContainerId; public CContainerRef(String factoryId, String containerId){ fFactoryId = factoryId; @@ -350,45 +356,49 @@ public class CExternalSettingsManager implements ICExternalSettingsListener, ICP FactoryDescriptor dr = getFactoryDescriptor(id); return dr.getFactory(); } - - public void settingsChanged(IProject project, String cfgId, - CExternalSettingChangeEvent event) { - ProjDesCfgList[] lists = null; - CExternalSettingsContainerChangeInfo[] infos = event.getChangeInfos(); - for (CExternalSettingsContainerChangeInfo info : infos) { - switch(info.getEventType()){ - case CExternalSettingsContainerChangeInfo.CHANGED: - int flags = info.getChangeFlags(); - if((flags & CExternalSettingsContainerChangeInfo.CONTAINER_CONTENTS) != 0){ - if(lists == null) - lists = createCfgListsForEvent(project, cfgId); - for (ProjDesCfgList list : lists) { - for(int i = 0; i < list.size(); i++){ - CfgListCfgContainer cr = new CfgListCfgContainer(list, i); - processContainerChange(OP_CHANGED, cr, new CfgContainerRefInfoContainer(cr), info.getContainerInfo()); + + /** + * External settings call-back from the setting container factories + * to notify that settings have changed in a container. + * + * Schedules a runnable to update any referencing projects + */ + public void settingsChanged(final IProject project, final String cfgId, final CExternalSettingChangeEvent event) { + // Modifying the project description in an asynchronous runnable is likely bad... + // Unfortunately there's nothing else we can do as it's not safe to modify the referencing configurations in place + IWorkspaceRunnable r = new IWorkspaceRunnable() { + public void run(IProgressMonitor monitor) throws CoreException { + ProjDesCfgList[] lists = null; + for (CExternalSettingsContainerChangeInfo info : event.getChangeInfos()) { + switch(info.getEventType()){ + case CExternalSettingsContainerChangeInfo.CHANGED: + int flags = info.getChangeFlags(); + if((flags & CExternalSettingsContainerChangeInfo.CONTAINER_CONTENTS) != 0){ + if(lists == null) + // Potentially all configuration in all projects need to be considered for be + lists = createCfgListsForEvent(project, cfgId); + for (ProjDesCfgList list : lists) { + for(int i = 0; i < list.size(); i++){ + CfgListCfgContainer cr = new CfgListCfgContainer(list, i); + processContainerChange(OP_CHANGED, cr, new CfgContainerRefInfoContainer(cr), info.getContainerInfo()); + } + } } + break; } } - break; - } - } - - // TODO modifying the project description in an asynchronous runnable is likely bad... - if(lists != null) { - final List list = getModifiedProjDesList(lists); - if(list.size() != 0){ - IWorkspaceRunnable r = new IWorkspaceRunnable(){ - - public void run(IProgressMonitor monitor) throws CoreException { - for(int i = 0; i < list.size(); i++){ + if (lists != null) { + final List list = getModifiedProjDesList(lists); + if(list.size() != 0) { + for(int i = 0; i < list.size(); i++) { ICProjectDescription des = list.get(i); CProjectDescriptionManager.getInstance().setProjectDescription(des.getProject(), des, false, monitor); } } - }; - CProjectDescriptionManager.runWspModification(r, new NullProgressMonitor()); + } } - } + }; + CProjectDescriptionManager.runWspModification(r, new NullProgressMonitor()); } private List getModifiedProjDesList(ProjDesCfgList[] lists){ @@ -507,6 +517,12 @@ public class CExternalSettingsManager implements ICExternalSettingsListener, ICP return null; } + /** + * Respond to Project Description events. + * - DATA_APPLIED: Data has been applied, and the description is still + * writable, store cached external settings into the configuration + * - LOADED: Check whether a reconcile is needed and update the settings atomically + */ public void handleEvent(CProjectDescriptionEvent event) { switch(event.getEventType()){ case CProjectDescriptionEvent.DATA_APPLIED: { @@ -522,33 +538,33 @@ public class CExternalSettingsManager implements ICExternalSettingsListener, ICP store(cfg, cr.fRefInfo); } } - break; } case CProjectDescriptionEvent.LOADED: - ProjDesCfgList list = new ProjDesCfgList(event.getNewCProjectDescription(), null); - boolean changed = false; - for(int i = 0; i < list.size(); i++){ - CfgListCfgContainer cfgCr = new CfgListCfgContainer(list, i); - CfgContainerRefInfoContainer ric = new CfgContainerRefInfoContainer(cfgCr); - CContainerRef[] refs = ric.getRefInfo(false).getReferences(); - for(int k = 0; k < refs.length; k++) { - if(processContainerChange(OP_CHANGED, cfgCr, new CfgContainerRefInfoContainer(cfgCr), refs[k])) - changed = true; - } - } - // TODO firing an asynchronous setProjectDescription here is likely to lead to trouble... - final ICProjectDescription prjDesc = list.fProjDes; - if(changed){ - IWorkspaceRunnable r = new IWorkspaceRunnable(){ - - public void run(IProgressMonitor monitor) throws CoreException { - CProjectDescriptionManager.getInstance().setProjectDescription(prjDesc.getProject(), prjDesc); + // Note using an asynchronous get / set here is bad. + // Unfortunately there's no other way to make this work without re-writing the project model to allow + // us to reconcile / update the cached configuration during load + final IProject project = event.getProject(); + IWorkspaceRunnable r = new IWorkspaceRunnable(){ + public void run(IProgressMonitor monitor) throws CoreException { + if (!project.isAccessible()) + return; + ProjDesCfgList list = new ProjDesCfgList(CoreModel.getDefault().getProjectDescription(project), null); + boolean changed = false; + for(int i = 0; i < list.size(); i++){ + CfgListCfgContainer cfgCr = new CfgListCfgContainer(list, i); + CfgContainerRefInfoContainer ric = new CfgContainerRefInfoContainer(cfgCr); + CContainerRef[] refs = ric.getRefInfo(false).getReferences(); + for(int k = 0; k < refs.length; k++) { + if(processContainerChange(OP_CHANGED, cfgCr, new CfgContainerRefInfoContainer(cfgCr), refs[k])) + changed = true; + } } - - }; - CProjectDescriptionManager.runWspModification(r, null); - } + if (changed) + CProjectDescriptionManager.getInstance().setProjectDescription(project, list.fProjDes); + } + }; + CProjectDescriptionManager.runWspModification(r, null); break; } } diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CRefSettingsHolder.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CRefSettingsHolder.java index f81ae299f0b..ad096da6297 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CRefSettingsHolder.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CRefSettingsHolder.java @@ -13,15 +13,61 @@ package org.eclipse.cdt.internal.core.settings.model; import org.eclipse.cdt.core.settings.model.ICStorageElement; import org.eclipse.cdt.internal.core.settings.model.CExternalSettingsManager.CContainerRef; +/** + * This class, derived from CExternalSettingsHolder, is used to cache the + * external settings exported by some container. + * + *

External settings have two sides. The external settings exporter (represented + * by a pure CExternalSettingsHolder) and the settings referencer referenced by this class. + * The CRefSettingsHolder holds a cache of the settings exports by the settings holder + * + *

Concretely, in the .cproject you might have: + * + *

In the exporting config: + * + *
<cconfiguration ... + *
 <storageModule buildSystemId="org.eclipse.cdt.managedbuilder.core.configurationDataProvider" id="..." moduleId="org.eclipse.cdt.core.settings" name="Debug"> + *
 <externalSettings> + *
  <externalSetting> + *
   <entry flags="" kind="includePath" name="libProj"/> + *
  </externalSetting> + *
 </externalSettings> + * + * + * + *

In the referencing project: + * + * + *
<configuration ... > + *
<storageModule moduleId="org.eclipse.cdt.core.externalSettings"> + *
 <externalSettings containerId="libProj;" factoryId="org.eclipse.cdt.core.cfg.export.settings.sipplier"> + *
  <externalSetting> + *
   <entry flags="" kind="includePath" name="libProj"/> + *
  </externalSetting> + *
 </externalSettings> + *
</storageModule> + * + */ public class CRefSettingsHolder extends CExternalSettingsHolder { + + /** + * The factory responsible for the setting. + * One of + *

    + *
  • {@link CfgExportSettingContainerFactory#FACTORY_ID}
    • + *
  • {@link ExtensionContainerFactory#FACTORY_ID} + */ private static final String ATTR_FACTORY_ID = "factoryId"; //$NON-NLS-1$ + /** Factory specific containerId used to resolve the settings container */ private static final String ATTR_CONTAINER_ID = "containerId"; //$NON-NLS-1$ - private CContainerRef fContainerRef; + + /** The container we get settings from */ + private final CContainerRef fContainerRef; private boolean fIsReconsiled; public CRefSettingsHolder(CContainerRef ref) { super(); - fContainerRef = ref; } diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CSettingsRefInfo.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CSettingsRefInfo.java index 99a0df77ef7..4ad6b180bc0 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CSettingsRefInfo.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CSettingsRefInfo.java @@ -26,8 +26,9 @@ import org.eclipse.cdt.core.settings.model.util.EntryNameKey; import org.eclipse.cdt.internal.core.settings.model.CExternalSettingsManager.CContainerRef; class CSettingsRefInfo { - final static String ELEMENT_REFERENCE_INFO = "referenceInfo"; //$NON-NLS-1$ - /** External Settings Holder Map */ + + /** External Settings Holder Map + * From references container -> to concrete held settings */ private HashMap fESHolderMap = new LinkedHashMap(); CSettingsRefInfo(){ diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CfgExportSettingContainerFactory.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CfgExportSettingContainerFactory.java index e5996944b1f..a33a2b3e849 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CfgExportSettingContainerFactory.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CfgExportSettingContainerFactory.java @@ -212,12 +212,31 @@ public class CfgExportSettingContainerFactory extends } /** - * Notify the ExternalSettingManager that there's been a change in the configurations mapped by this external settings provider - * (as a result of a proejct configuration change) + * Notify the ExternalSettingManager that there's been a change in the configuration which may require referencing configs to update + * their cache of the external settings */ public void handleEvent(CProjectDescriptionEvent event) { switch(event.getEventType()){ - case CProjectDescriptionEvent.LOADED: + case CProjectDescriptionEvent.LOADED: { + // Bug 312575 on Project load, event.getProjectDelta() == null => report all configs as potentially changed + // Referencing projects should be reconciled and potentially updated. + String projName = event.getProject().getName(); + ICConfigurationDescription[] descs = event.getNewCProjectDescription().getConfigurations(); + CExternalSettingsContainerChangeInfo[] changeInfos = new CExternalSettingsContainerChangeInfo[descs.length + 1]; + int i = 0; + for (ICConfigurationDescription desc : event.getNewCProjectDescription().getConfigurations()) + changeInfos[i++] = new CExternalSettingsContainerChangeInfo( + CExternalSettingsContainerChangeInfo.CONTAINER_CONTENTS, + new CContainerRef(FACTORY_ID, createId(projName, desc.getId())), + null); + // Active configuration too + changeInfos[i] = new CExternalSettingsContainerChangeInfo( + CExternalSettingsContainerChangeInfo.CONTAINER_CONTENTS, + new CContainerRef(FACTORY_ID, createId(projName, ACTIVE_CONFIG_ID)), + null); + notifySettingsChange(null, null, changeInfos); + break; + } case CProjectDescriptionEvent.APPLIED: String[] ids = getContainerIds(event.getProjectDelta()); if(ids.length != 0){ diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/ICExternalSettingsListener.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/ICExternalSettingsListener.java index 010762f812f..971c0e57dde 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/ICExternalSettingsListener.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/ICExternalSettingsListener.java @@ -18,8 +18,12 @@ import org.eclipse.core.resources.IProject; public interface ICExternalSettingsListener { /** - * Notifies the listener that the configuration with id cfgId has changed in the project - * project. + * Notifies the listener that external settings in a particular container have changed. + * The CExternalSettingsManager will try to reconcile changes into the project + config + * specified by the call-back. If these are null (which they currently always are) + * the external settings manager will check all projects and configurations to see + * if there are any referencing configs which need reconciling. + * * @param project or null indicating all projects should be considered * @param cfgId or null indicating all configurations should be considered * @param event CExternalSettingsChangeEvent