Bug 312575 Updating a .cproject doesn't update settings in referencing projects. Patch 3 + JavaDoc

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");
 

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<ExtSettingMapKey, CExternalSetting> fSettingsMap;
 	static final String ELEMENT_EXT_SETTINGS_CONTAINER = "externalSettings"; //$NON-NLS-1$
 	

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 final String fFactoryId;
-		private String fContainerId;
+		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) {
+	 * External settings call-back from the setting container factories
-		ProjDesCfgList[] lists = null;
+	 * to notify that settings have changed in a container.
-		CExternalSettingsContainerChangeInfo[] infos = event.getChangeInfos();
+	 * 
-		for (CExternalSettingsContainerChangeInfo info : infos) {
+	 * Schedules a runnable to update any referencing projects
-			switch(info.getEventType()){
+	 */
-			case CExternalSettingsContainerChangeInfo.CHANGED:
+	public void settingsChanged(final IProject project, final String cfgId,	final CExternalSettingChangeEvent event) {
-				int flags = info.getChangeFlags();
+		// Modifying the project description in an asynchronous runnable is likely bad...
-				if((flags & CExternalSettingsContainerChangeInfo.CONTAINER_CONTENTS) != 0){
+		// Unfortunately there's nothing else we can do as it's not safe to modify the referencing configurations in place
-					if(lists == null)
+		IWorkspaceRunnable r = new IWorkspaceRunnable() {
-						lists = createCfgListsForEvent(project, cfgId);
+			public void run(IProgressMonitor monitor) throws CoreException {
-					for (ProjDesCfgList list : lists) {
+				ProjDesCfgList[] lists = null;
-						for(int i = 0; i < list.size(); i++){
+				for (CExternalSettingsContainerChangeInfo info : event.getChangeInfos()) {
-							CfgListCfgContainer cr = new CfgListCfgContainer(list, i);
+					switch(info.getEventType()){
-							processContainerChange(OP_CHANGED, cr, new CfgContainerRefInfoContainer(cr), info.getContainerInfo());
+					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;
+				if (lists != null) {
-			}
+					final List<ICProjectDescription> list = getModifiedProjDesList(lists);
-		}
+					if(list.size() != 0) {
-		
+						for(int i = 0; i < list.size(); i++) {
-		// TODO modifying the project description in an asynchronous runnable is likely bad...
-		if(lists != null) {
-			final List<ICProjectDescription> 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++){
 							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<ICProjectDescription> 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);
+			// Note using an asynchronous get / set here is bad.
-			boolean changed = false;
+			// Unfortunately there's no other way to make this work without re-writing the project model to allow
-			for(int i = 0; i < list.size(); i++){
+			// us to reconcile / update the cached configuration during load
-				CfgListCfgContainer cfgCr = new CfgListCfgContainer(list, i);
+			final IProject project = event.getProject();
-				CfgContainerRefInfoContainer ric = new CfgContainerRefInfoContainer(cfgCr);
+			IWorkspaceRunnable r = new IWorkspaceRunnable(){
-				CContainerRef[] refs = ric.getRefInfo(false).getReferences();
+				public void run(IProgressMonitor monitor) throws CoreException {
-				for(int k = 0; k < refs.length; k++) {
+					if (!project.isAccessible())
-					if(processContainerChange(OP_CHANGED, cfgCr, new CfgContainerRefInfoContainer(cfgCr), refs[k]))
+						return;
-						changed = true;
+					ProjDesCfgList list = new ProjDesCfgList(CoreModel.getDefault().getProjectDescription(project), null);
-				}
+					boolean changed = false;
-			}
+					for(int i = 0; i < list.size(); i++){
-			// TODO firing an asynchronous setProjectDescription here is likely to lead to trouble...
+						CfgListCfgContainer cfgCr = new CfgListCfgContainer(list, i);
-			final ICProjectDescription prjDesc = list.fProjDes;
+						CfgContainerRefInfoContainer ric = new CfgContainerRefInfoContainer(cfgCr);
-			if(changed){
+						CContainerRef[] refs = ric.getRefInfo(false).getReferences();
-				IWorkspaceRunnable r = new IWorkspaceRunnable(){
+						for(int k = 0; k < refs.length; k++) {
-
+							if(processContainerChange(OP_CHANGED, cfgCr, new CfgContainerRefInfoContainer(cfgCr), refs[k]))
-					public void run(IProgressMonitor monitor) throws CoreException {
+								changed = true;
-						CProjectDescriptionManager.getInstance().setProjectDescription(prjDesc.getProject(), prjDesc);
+						}
 					}
-					
+					if (changed)
-				};
+						CProjectDescriptionManager.getInstance().setProjectDescription(project, list.fProjDes);
-				CProjectDescriptionManager.runWspModification(r, null);
+				}
-			}
+			};
+			CProjectDescriptionManager.runWspModification(r, null);
 			break;
 		}
 	}

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.
+ * 
+ * <p> 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
+ * 
+ * <p>Concretely, in the .cproject you might have:
+ * 
+ * <p> In the exporting config:
+ * <code>
+ * <br/>&lt;cconfiguration ... 
+ * <br/>&nbsp;&lt;storageModule buildSystemId="org.eclipse.cdt.managedbuilder.core.configurationDataProvider" id="..." moduleId="org.eclipse.cdt.core.settings" name="Debug"&gt;
+ * <br/>&nbsp;&lt;externalSettings&gt;
+ * 		<br/>&nbsp;&nbsp;&lt;externalSetting&gt;
+ *			<br/>&nbsp;&nbsp;&nbsp;&lt;entry flags="" kind="includePath" name="libProj"/&gt;
+ *		<br/>&nbsp;&nbsp;&lt;/externalSetting&gt;
+ * 	<br/>&nbsp;&lt;/externalSettings&gt;
+ * 
+ * </code>
+ * 
+ * <p>In the referencing project:
+ * 
+ * <code>
+ * <br/>&lt;configuration ... &gt;
+ * <br/>&lt;storageModule moduleId="org.eclipse.cdt.core.externalSettings"&gt;
+ * 	<br/>&nbsp;&lt;externalSettings containerId="libProj;" factoryId="org.eclipse.cdt.core.cfg.export.settings.sipplier"&gt;
+ * 		<br/>&nbsp;&nbsp;&lt;externalSetting&gt;
+ *			<br/>&nbsp;&nbsp;&nbsp;&lt;entry flags="" kind="includePath" name="libProj"/&gt;
+ *		<br/>&nbsp;&nbsp;&lt;/externalSetting&gt;
+ * 	<br/>&nbsp;&lt;/externalSettings&gt;
+ * <br/>&lt;/storageModule&gt;
+ * </code>
+ */
 public class CRefSettingsHolder extends CExternalSettingsHolder {
+
+	/**
+	 * The factory responsible for the setting.
+	 * One of 
+	 * <ul>
+	 * <li> {@link CfgExportSettingContainerFactory#FACTORY_ID} </li>
+	 * <li> {@link ExtensionContainerFactory#FACTORY_ID} </lid
+	 * </ul>
+	 */
 	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;
 	}
 

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<CContainerRef, CRefSettingsHolder> fESHolderMap = new LinkedHashMap<CContainerRef, CRefSettingsHolder>();
 
 	CSettingsRefInfo(){

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
+	 * Notify the ExternalSettingManager that there's been a change in the configuration which may require referencing configs to update
-	 * (as a result of a proejct configuration change)
+	 * 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){

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
+	 * Notifies the listener that external settings in a particular container have changed.
-	 * project.
+	 * 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