KConfig::addConfigSources is broken

[Thomas Braxton]

On 11/18/07, Aaron J. Seigo <aseigo at kde.org> wrote:
> On Sunday 18 November 2007, Thomas Braxton wrote:
> > On 11/18/07, Andreas Pakulat <apaku at gmx.de> wrote:
> > > On 18.11.07 12:08:59, Thomas Braxton wrote:
> > > > On 11/18/07, Andreas Pakulat <apaku at gmx.de> wrote:
> > > > > On 18.11.07 11:30:33, Thomas Braxton wrote:
> > > > > > On 11/18/07, Andreas Pakulat <apaku at gmx.de> wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > seems like KConfig::addConfigSources is broken, wrt. writing to
> > > > > > > the configuration.
> > > > > > >
> > > > > > > When creating a KSharedConfig instance with:
> > > > > > >
> > > > > > > d->m_cfg = KSharedConfig::openConfig( d->projectTempFile );
> > > > > > > d->m_cfg->addConfigSources( QStringList() << d->developerTempFile
> > > > > > > );
> > > > > >
> > > > > > try changing it to
> > > > > > d->m_cfg = KSharedConfig::openConfig( d->developerTempFile );
> > > > > > d->m_cfg = addConfigSources( QStringList() << d->projectTempFile );
> > > > > > since the developer file is imho the most specific file, the file
> > > > > > name passed to KConfig is considered the most specific file.
> > > > >
> > > > > Thats
> > > > >
> > > > > a) the opposite of what the API docs say
> > > > > b) a behavioural change thats undocumented
> > > > >
> > > > > So either fix the api docs and add mentioning of this to
> > > > > KDE4PORTING.html or revert to the old behaviour of "last in, first
> > > > > out".
> > > >
> > > > so the name passed to KConfig doesn't matter, the last file passed in
> > > > is considered to be the most specific file?
> > >
> > > I guess you mean filename, if so the answer is no. The code I posted
> > > creates a cascading KConfig object, i.e. entries that are in
> > > projectTempFile are visible when reading them only if they're not in
> > > developerTempFile. So for instance for:
> > >
> > > projectTempFile:
> > > [Project]
> > > Name=Foo
> > > VcsPlugin=Bar
> > >
> > > developerTempFile:
> > > [Project]
> > > VcsPlugin=Foobar
> > >
> > > I get "Foo" when reading "Name" from the resulting KConfigGroup object
> > > and "Foobar" when reading "VcsPlugin". And when writing
> > > "ManagerPlugin=Baz" it writes to developerTempFile and not
> > > projectTempFile. Thats the behaviour KConfig had until the first merge
> > > of the kconfig branch in October (IIR).
> >
> > Huh? I'm not sure but I think you're talking about two different
> > things here.
>
> yes, cascading and writing. they are, however, related.
>
> > The way I understand you, when you read "Name" from
> > current KConfig you get "Foo"? That's the way it's supposed to be,
>
> which is what Andreas is saying, yes...
>
> > because "projectTempFile" was passed to KConfig in the code you
> > posted. It's the most specific file, because you opened it, why would
> > you want to open one of the defaults instead of the config you plan to
> > write to?
>
> no, it's because projectTempFile is the *less* specific file. that is why it
> is used only for values not in developerTempFile; developerTempFile is
> further down the cascade and therefore overrides projectTempFile values when
> they exist in developerTempFile.
>
> this is how config cascading has always worked in kconfig to my knowledge.
>
> > The other thing I think you're talking about is that pre-merge KConfig
> > writes to "developerTempFile". If so, then the filename passed to
> > KConfig doesn't matter, because the last filename in the list of files
> > passed to addConfigSources becomes the most specific file?
>
> and here's the problem: reading is done as if projectTempFile is prior in the
> cascade (which is correct) but writing is done as if projectTempFile is the
> the bottom of the cascade (or, in your terms, the "most specific" file).
>
> so reading and writing are now assymetrical in this case, and that is
> obviously wrong. as you note, the reading is proper; now the writing needs to
> be fixed to follow that.
>
That's what I was saying, in the code he posted he expects writing to
be to a file he added with addConfigSources not to the file he opened.
imho you should be opening the file you expect to write to, not one of
it's defaults which is what he did.