Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-dev] Project Preference Store

Thanks everyone for your comments.

Doug: I completely agree with you: current core.settings.model is a
mess.  (Or maybe it's not a mess it's just misunderstood.  JavaDoc or
comments noting intention even at the class level would have been
great...

Without documentation the current practise of many actors tweaking
bits of state all over the place is fatal:

For example:
The SetCProjectDescription ModelOperation is responsible for making a
given ProjectDescription the default readonly version (the operation
is indirectly called via
CProjectDescriptionManager.setProjectDescription(...))

It does this by creating a 'fNewDescriptionCache' from the description
being set (fSetDescription), the ProjectDescription constructor it
uses converts fSetDescription's CConfigurationDescriptions ->
CConfigurationDescriptionCaches in fNewDescriptionCache.  It then
fires a bunch of deltas to CProjectDescriptionEventListeners.
Amazingly the event listeners can make changes to the
fNewDescriptionCache project description. There's no javadoc that says
this is possible.
Having notified the event listeners, SetCProjectDescription updates
the cached data of the original fSetDescription to the contents of
fNewDescriptionCache see:
CProjectDescription.switchToCachedAppliedData(...).

CProjectDescription, CConfigurationDescription(Cache),
CConfigBasedDescriptor(Manager), SetCProjectDescription... all seem to
be tightly coupled.  It seems that many of the classes in
internal.core.settings.model change each other's state in mysterious
ways even if to the outside world some of the data is 'readonly'.
There's a good smattering of
CProjectDescriptionManager.copyElement(root) to ensure that the new
configurations and project descriptions are backed by different trees,
but given that all the writable descriptions are now thread local this
should no longer be necessary.  Looking at the code there seems to be
have been a transition from a state where writable descriptions where
shared between threads to the current thread local versions.  The lack
of JavaDoc makes the whole thing deeply impenetrable:

Why, for example, is CConfigurationDescriptionCache needed?
  - SetCProjectDescriptionOperation create a new project description
based on the old with CConfigurationDescriptionCache's for the
original CConfigurationDescriptions
    However this new project description is still mutable during the
DATA_APPLIED event.
  - CProjectDescription.loadDatas/applyData/doneInitializing happly
downcasts iCConfigurationDescription -> CConfigurationDescriptionCache
(fCfgMap at various times
    contains CConfigurationDescription / CConfigurationDescriptionCache)...

If MikhailS or Oleg or anyone else in the know could chip in on why
things are the way they are, in particular:
- How do {ClassName}Cache classes relate to {ClassName} classes.  Why
are they needed?
- What contract do you expect CProjectDescriptionEvent handlers to
have -- are they only allowed to modify fNewDescription, are they
always allowed to modify it?
- What's the difference between CProjectDescriptionEvent.APPLIED &&
CProjectDescriptionEvent.DATA_APPLIED?
- How does CConfigBasedDescriptor and the ICDescriptor interface fit
into ICProjectDescription model. To me it seems that ICDescriptor is
just for org.eclipse.cdt.settings based settings and this, with
managed build, have been retrofitted into a single project model
document?
- Everything from the base InternalXmlStorageElements all the way up
to the ProjectDescription have readonly and modfiied flags.  It
doens't make sense that you can have a single XmlElement readonly
while the project description is writable.  Why is this state
replicated all over model?

I keep tripping over cases where subtle changes to the project
description are causing other bits in internal.core.setttings.model to
break due to some implicit assumption changing.    To me it would seem
good to reduce the complexity and coupling here.  I don't really
understand why there should be caches as well as writable parts of the
descriptions -- especially when they both have lifetimes that overlap.
 In my mind the project model should be a very lightweight layer on
top of the Tree-based setting store.

For the moment I'm trying to modify internal.core.settings.model as
little as possible (except for the serialization backend).  If there
really is no one who knows how this code works, and there is no
documentation then has this code really died?  If so, Doug, I'm right
behind you with the rewrite!

Andrew:
> Another problem is how to compare two project descriptions (or their
> components). The problem is in sheer number of parts involved. It breaks to
> 50 or more classes, most of them not being serialized but probably should be
> checked anyway. I've been doing some of that this way, it is doable:

Indeed, so this is currently not possible.  The interface in my
previous email could be extended to include a serialize to string
method, but fundamentally not all backends might serialize to strings
(and even if they do, they won't be comparable).  A better solution
might be to help Doug sort out the model, standardise the contract
between project descriptions and users, description event listeners
and extenders, and provide a ProjectDescription.equals(...) which
could be reasonably heavyweight but suitable for unit testing the
setProjectDesction(pdesc); pdesc1 = getProjectDesctiption(...);
assertTrue(pdesc.equals(pdesc1))

Cheers,

James

On Tue, Oct 21, 2008 at 3:37 PM, Andrew Gvozdev <angvoz.dev@xxxxxxxxx> wrote:
> I've been looking recently at creating unit test cases related to
> serialization. Something pretty basic like serialize a project description
> to a storage (preferable to a local string), then load to another project
> description and compare. Have similar unit test cases for subcomponents of
> ICProjectDescription. Turns out to be not a trivial task. For one thing,
> serialize API does not look suited to be used for unit testing. Can anybody
> tell how to serialize to a string or to at least to a different file and
> read from there? Serialize methods in subcomponents of ICProjectDescription
> are private and not easy to get hold on. Can we standardize serialize API
> for every component of project description being serialized and expose it as
> public methods? I suppose this way is correct:
> public void serialize(ICStorageElement element);
> public static CSerializedClass load(ICStorageElement element);
>

> public class AssertIConfiguration {
> static public void assertEquals(String message, IConfiguration expected,
> IConfiguration actual) {
> if (expected == actual) return;
>
> if (expected != null && actual != null) {
> AssertIBuildObject.assertEquals(message, expected, actual);
> AssertIBuildObjectPropertiesContainer.assertEquals(message, expected,
> actual);
> Assert.assertEquals(message, expected.getArtifactExtension(),
> actual.getArtifactExtension());
> Assert.assertEquals(message, expected.getArtifactName(),
> actual.getArtifactName());
>                         ....
>                         AssertIBuilder.assertEquals(message,
> expected.getBuilder(), actual.getBuilder());
>                         ...
> return;
> }
> junit.framework.Assert.failNotEquals(message, expected, actual);
>          }
> }
>
> I think that kind of unit testing would be helpful to ensure that
> serialization is not being broken while being cleaning up.
> Andrew
> On Tue, Oct 21, 2008 at 8:27 AM, Schaefer, Doug
> <Doug.Schaefer@xxxxxxxxxxxxx> wrote:
>>
>> Yes, there are many ways of doing trees in a property set. You could use
>> qualifiers or paths in keys as well.
>>
>> I guess my hope was that it would be good enough to get out of the
>> setting store business. The less time we spend on figuring out how to
>> store build settings, the more we can spend on cleaning up some of the
>> more serious issues we have, like redoing scanner discovery.
>>
>> Doug.
>>
>> > -----Original Message-----
>> > From: cdt-dev-bounces@xxxxxxxxxxx
>> > [mailto:cdt-dev-bounces@xxxxxxxxxxx] On Behalf Of James Blackburn
>> > Sent: Tuesday, October 21, 2008 4:17 AM
>> > To: CDT General developers list.
>> > Subject: Re: [cdt-dev] Project Preference Store
>> >
>> > > There are a couple of issues I can forsee with this:
>> > > - How do you handle ICSettingsStorage ICStoargeElement trees ?
>> > >  Currently everything is stored in a tree. Key / Value preferences
>> > > aren't really a suitable replacement (and I think storing
>> > xml trees in
>> > > values will make version control even harder...)
>> >
>> > Taking a look at IEclipsePreferences more closely, they can
>> > represent a tree like store.. So that objection goes out the
>> > window :)...
>> >
>> > My aim with my current work is to allow any tree based
>> > storage to be used for persisting the model -- so something
>> > like this would plug-in nicely with (hopefully) little effort.
>> >
>> > James
>> > _______________________________________________
>> > cdt-dev mailing list
>> > cdt-dev@xxxxxxxxxxx
>> > https://dev.eclipse.org/mailman/listinfo/cdt-dev
>> >
>> _______________________________________________
>> cdt-dev mailing list
>> cdt-dev@xxxxxxxxxxx
>> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>
>
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>
>


Back to the top