Bug 369836 - Restore defaults in "C/C++ Build - Settings" throws away external settings
Summary: Restore defaults in "C/C++ Build - Settings" throws away external settings
Status: NEW
Alias: None
Product: CDT
Classification: Tools
Component: cdt-build-managed (show other bugs)
Version: 8.1.0   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact: Jonah Graham CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-26 11:52 EST by Raphael Zulliger CLA
Modified: 2020-09-04 15:26 EDT (History)
2 users (show)

See Also:


Attachments
Patch (early prototype) (5.73 KB, patch)
2012-01-26 11:55 EST, Raphael Zulliger CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Raphael Zulliger CLA 2012-01-26 11:52:53 EST
Build Identifier: 20110615-0604

I'm using an external settings provider that provides, among others, macros. When I open the "C/C++ Build - Settings" page in the properties dialog, the macro appears as expected. But after pressing the "Restore Defaults" button that macro disappears. This is not the behavior I would have expected because "by default" the macro was there.

Depending on the duty of an external settings provider, this issue can well be a show stopper because bringing back that macro from external settings provider seems to be impossible by only using the UI (at least I was unable to find a solution/workaround doable in the UI). 

IMHO the expected behavior should be to restore all settings provided by all external settings provider as well (not just the settings of the toolchain)

Technically, the issue is somewhat related to https://bugs.eclipse.org/bugs/show_bug.cgi?id=331979. IMHO, while the expected behavior for issue 331979 is not necessarily obvious it is much more obvious for this one (at least that's what I personally think). Therefore, I decided to create a separate bug report for this issue.

Reproducible: Always

Steps to Reproduce:
1. Take https://bugs.eclipse.org/bugs/attachment.cgi?id=184771 (from https://bugs.eclipse.org/bugs/show_bug.cgi?id=331979)
2. Execute the provider on a C/C++ project (Right-click onto the project folder - "Index" - "Call setting provider")
3. Open project properties - "C/C++ Build - Settings", have a look at the C or C++ "Preprocessor" symbols: The macro "name=value" is present
4. Press "Restore defaults"
5. Check for the macro again: it's gone
Comment 1 Raphael Zulliger CLA 2012-01-26 11:53:59 EST
I tried to understand the code and mechanisms related to external settings and what happens when executing "Restore Defaults"... I guess that I partially succeeded. (:-)). According to my understanding, there are two problems causing this bug:
	- ManagedBuildManager.resetConfiguration(), which is called by pressing the "Restore Defaults" button, does reset the "Configuration". This basically causes the removal of all IOption objects from every ITool and from the IToolChain object. Unfortunately the options added by external settings will also be removed by this approach. Moreover, the external settings manager is not asked to bring the external settings back to the configuration... That's problem #1. (Note that the enablements of the options of the toolchain will be executed in the context of ArtifactTab.performDefaults(). That's why thing like "Debug level" and "Optimization level" will be set as expected).
	- Even after fixing problem #1 by calling the external settings manager, it still doesn't work because of the external settings cache mechanism that avoids adding options because it knows that the settings have already been added to the configuration before. This is Problem #2 which is technically related to the issue mentioned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=331979: we need a way to invalidate the cache.
Comment 2 Raphael Zulliger CLA 2012-01-26 11:55:30 EST
Created attachment 210132 [details]
Patch (early prototype)

The attached patch tries to address both mentioned problems. This patch is by no mean ready to be committed. It just tries to show what needs to be done to fix the issue and may therefore be useful for further discussions.

I'm really willing to do further work on this issue in order to end up with a committable patch! But in order to do so, I definitely need to know what you require the patch to look like.

One question would be how to best get rid of the cast to CConfigurationDescription in ManagedBuildManager.java. We probably need to add the "reApplyExternalSettings" to a new interface which needs to be implemented by CConfigurationDescription. Because we cannot change ICConfigurationDescription because it it's a public interface, right?

Next, we could decide to not introduce new functions at all because there exists a workaround (or solution?) to "clear" the cache by temporarily removing the external settings (ICConfigurationDescription.removeExternalSetting and ICConfigurationDescription.setExternalSettingsProviderIds). I tried that approach and it worked fine for the tested use case. I fear that this could introduce unexpected side effects.