Community
Participate
Working Groups
Exported settings are currently cached in referencing projects. The cache is updated on certain change events in the model - for example when changing the set of references. They don't however seem to be updated when the .cproject is updated - e.g. after an updated to HEAD from version control. If a team checks-in all the .cprojects when exports / references change, and update all projects from HEAD simultaneously, this works correctly. However if a user just updates one project in the project set, then the project metadata for referencing projects is left out of date w.r.t. the exported settings.
Created attachment 168173 [details] test 1 A couple more tests CConfigDescExportSettings. I discovered a couple things don't work properly: 1) If you reference the '[Active]' configuration, then updating the exported settings in the referenced project's active configuration doesn't correctly propagate. If you reference a specific configuration (e.g. activeCfg.getId(), it works as expected). 2) The issue at hand: updating the .cproject without doing setProjectDescription doesn't correctly propagate the external settings.
Created attachment 168378 [details] patch 1 I keep forgetting how frustrating this code is to work with :(. So many layers of indirection with no documentation its impossible to work out who's responsible for what... That said I've figure a few things out and JavaDoc'd it along the way. For External Settings the main actors are: CExternalSettingContainerFactory: This is the base class for the external settings container factories (note this makes 2 layers of indirection here. The factories creates containers which themselves contain settings). There are two such containers hard-coded into CDT (non-extensible): - CfgExportSettingContainerFactory: Responsible for noticing and propagating changes to exported settings referenced from other configurations - ExtensionContainerFactory: Responsible for creating the externally contributed containers from the externalSettings extension point - (Abstract class CExternalSettingContainerFactoryWithListener should probably be collapsed into CExternalSettingContainerFactory) CExternalSettingsManager : This is the root of everything external setting related. It contains the references to the 2 concrete ExternalSettingContainerFactories. It also contains 850 lines of spaghetti with 16(!) helpfully named nested classes: for example the difference between: CfgContainer, CfgListCfgContainer & CfgContainerRefInfoContainer is obvious to someone, right? How do these things communicate? Even though these internal classes are non-extendable, they don't communicate directly. CExternalSettingsManager implements ICExternalSettingsListener (it is the only implementer of this interface). The 2 container factories each have a listener list of length 1, and notify the manager of external settings via the listener call back. To plug-into the model, both the CExternalSettingsManager, and the CfgExportSettingContainerFactory are ICProjectDescriptionListeners. When model changes occur the CfgExportSettingContainerFactory notifies the CExternalSettingsManager that a change has occurred. The CExternalSettingsManager, also listening to the same event, checks whether a reconciling is needed and updates the ProjectDescription in place as its passing through. There's clearly and ordering issue here as both respond to the same event, with one prodding the other causing changes which will in turn generate more events... This is all sparked off a CProjectDescriptionManager generated delta. The delta containing the external settings changed, which is acted upon by the listeners, and fed through to a change in the configuration & a change in the project description. The attached patch adds JavaDoc, removes some dead-code, and fixes the two issues reported in the CfgExportSettingContainerFactory. It contains 4 tests, the last of which contains a reference cycle which doesn't work correctly. I don't understand how this code could be so successfully over-engineered (from the first commit!) to be entirely unmaintainable. It seems very wrong that internals of this project description model update the model in the notification event especially as the delta event contains the changes the listeners will use to modify the model!
Created attachment 168389 [details] patch for commit Path with tests for the active configuration issue. The only semantic change is the fix in CfgExportSettingContainerFactory#collectrCfgIds. Other changes are JavaDoc + tidy.
Created attachment 168528 [details] tidy patch No functional changes. Refactor / tidy CExternalSettingsManager: - Inline single line private methods - Inline some private methods with only one caller - Remove unused nested classes - Remove nested interface only implemented by one internal nested class
Created attachment 168952 [details] patch 3 This patch 'fixes' the original reported bug, and enables another test. Note that as this stands, the way the external settings hooks in is pretty broken. In the two CExternalSettingsManager call-backs (one from the project model, one from the settings container) the manager checks if the settings cache is out of date, and if so reconciles the changes into the configuraiton description and then _asynchronously_ sets the new description. This happens to work most the time, but fails when: - .cprojects are being changed by some other mechanism (e.g. source control update) - project description has been set by some other thread concurrently. When you have dozens of inter-dependent projects in your workspace, and start to perform source control actions, external settings reconciling goes wrong. This patch changes two things: - CExternalSettingsManager does the entire getProjDesc/setProjDesc in a single CProjDescManager#WSP runnable. This prevents it from setting a stale description if a source control update is happening concurrently - CfgExportSettingContainerFactory notifies the manager, on load, that all configurations need reconciling. The existing code is broken as proj desc delta is NULL on a LOADED event. Ideally there should be no asynchronous reads / writes internal to the model. It should reconcile changes directly into the project description. Unfortunately this is impossible as the project description is loaded in its read-only state which makes it impossible to reconcile [in this state the data strcuture (suffixed with Cached) really can't be written...]. Added some more JavaDoc.
Behavioural change is confined to: CExternalSettingsManager#settingsChanged(...) CExternalSettingsManager#handleEvent(...) - In both cases perform the reconcile in the same proj desc runnable as the set CfgExportSettingContainerFactory#handleEvent(...) - Handle the LOADED event, and notify the ext settings manager that the configurations might have changed. All existing tests look good.
Created attachment 168972 [details] preserve order tweak And another tweak. When doing this reconciling we need to ensure that the order of the reconciled settings is correct (the order of referenced settings is driven by the references tab in CDT 7).
Created attachment 169082 [details] performance Don't impact performance of loading projects without references / external settings: - #handleEvent LOADED , no need to reconcile project if it doesn't reference any projects nor has external setting provider ids - #settingsChanged Only have one workspace reconcile runnable scheduled at any time.
*** cdt cvs genie on behalf of jblackburn *** Bug 312575 Changes aren't propagated when the referencing project references the active configuration. JavaDoc + tests. [*] ICConfigurationDescription.java 1.14 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/core/settings/model/ICConfigurationDescription.java?root=Tools_Project&r1=1.13&r2=1.14 [*] CExternalSettingContainerFactory.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingContainerFactory.java?root=Tools_Project&r1=1.2&r2=1.3 [*] CfgExportSettingContainerFactory.java 1.15 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CfgExportSettingContainerFactory.java?root=Tools_Project&r1=1.14&r2=1.15 [*] CExternalSettingsHolder.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsHolder.java?root=Tools_Project&r1=1.4&r2=1.5 [*] ICExternalSettingsListener.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/ICExternalSettingsListener.java?root=Tools_Project&r1=1.1&r2=1.2 [*] CExternalSettingsManager.java 1.15 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsManager.java?root=Tools_Project&r1=1.14&r2=1.15 [*] CExternalSettingsContainer.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsContainer.java?root=Tools_Project&r1=1.1&r2=1.2 [*] CConfigurationDescriptionExportSettings.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/CConfigurationDescriptionExportSettings.java?root=Tools_Project&r1=1.2&r2=1.3
*** cdt cvs genie on behalf of jblackburn *** Bug 312575 tidy patch [*] CExternalSettingsManager.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsManager.java?root=Tools_Project&r1=1.15&r2=1.16 [*] CSettingsRefInfo.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CSettingsRefInfo.java?root=Tools_Project&r1=1.4&r2=1.5 [*] CExternalSettingsHolder.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsHolder.java?root=Tools_Project&r1=1.5&r2=1.6 [*] CExternalSettingChangeEvent.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingChangeEvent.java?root=Tools_Project&r1=1.2&r2=1.3 [*] ICExternalSettingsListener.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/ICExternalSettingsListener.java?root=Tools_Project&r1=1.2&r2=1.3
*** cdt cvs genie on behalf of jblackburn *** Bug 312575 Updating a .cproject doesn't update settings in referencing projects. Patch 3 + JavaDoc [*] CConfigurationDescriptionExportSettings.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/CConfigurationDescriptionExportSettings.java?root=Tools_Project&r1=1.3&r2=1.4 [*] CfgExportSettingContainerFactory.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CfgExportSettingContainerFactory.java?root=Tools_Project&r1=1.15&r2=1.16 [*] CSettingsRefInfo.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CSettingsRefInfo.java?root=Tools_Project&r1=1.5&r2=1.6 [*] CRefSettingsHolder.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CRefSettingsHolder.java?root=Tools_Project&r1=1.3&r2=1.4 [*] CExternalSettingsHolder.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsHolder.java?root=Tools_Project&r1=1.6&r2=1.7 [*] CExternalSettingsManager.java 1.17 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsManager.java?root=Tools_Project&r1=1.16&r2=1.17 [*] ICExternalSettingsListener.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/ICExternalSettingsListener.java?root=Tools_Project&r1=1.3&r2=1.4
*** cdt cvs genie on behalf of jblackburn *** Bug 312575 Reconciling external settings should ensure config referenced settings are in the correct order. [*] CExternalSettingsManager.java 1.18 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsManager.java?root=Tools_Project&r1=1.17&r2=1.18
*** cdt cvs genie on behalf of jblackburn *** Bug 312575 Performance patch, don't reconcile if not needed [*] CExternalSettingsManager.java 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsManager.java?root=Tools_Project&r1=1.18&r2=1.19
