Review Request: When a source file for kconf_update does not exist, do not create it

[Lubos Lunak]

> On 2010-06-07 12:48:15, Oswald Buddenhagen wrote:
> > > The QFileInfo checks if the file is actually present in the user's config directory,
> > > the other check is based on merged content.
> > >
> > hmm, but that's just right. if there is no user config, but the system wide config contains info which needs updating, an updated user config needs to be created. of course that means that if the system config is a pristine one as installed by us, it needs to be fixed. and the installed files should probably contain version tags, so no user-specific files would have to be created.
> >
> 
> Lubos Lunak wrote:
>     The system config can come from distribution, can differ from KDE's defaults (of course, that's the point) and can over time become obsolete in their format.
>     
>     So the check is indeed unsufficient and the patch is broken in this form.
>
> 
> Kevin Krammer wrote:
>     So somthing like:
>     - check if the file is locally available
>     - if not, remember that but continue anyways
>     - at the end check whether the config is still equal to the initial state and if yes and it didn't exist locally, remove it again
>     
>     no file + no change -> no file?
> 
> Oswald Buddenhagen wrote:
>     that sounds about right
> 
> Lubos Lunak wrote:
>     Sounds needlessly complex to me. I think using KStandardDirs::calcResourceHash() to check if there's any config file at any level should do.
>
> 
> Oswald Buddenhagen wrote:
>     but that means that if any system config is installed at all, an empty user config would be created as well, no? and we do have quite some default system configs, after all ...

So? An empty user config file is no problem, and for the reason that triggered this patch, the PIM migration, it is also correct (=some settings already do exist, they just happen to be global).

Thinking of that, the PIM approach of just checking existence of the user config file is broken in general. Kconf_update is _the_ migrator tool, so this is just a case of reinventing the wheel and now wondering what to do with it when it's not completely round.


- Lubos


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4243/#review6010
-----------------------------------------------------------


On 2010-06-07 11:38:56, Kevin Krammer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4243/
> -----------------------------------------------------------
> 
> (Updated 2010-06-07 11:38:56)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Currently, when kconf_update is being run by kdeinit4 on a new user account, it creates all config files that it has updates for, regardless whether these files exist.
> Which mean .kde/share/config gets populated with a lot of files with only kconf_update's entry in them.
> 
> Unfortunately that means that programs checking for a specific file's existance, e.g. PIM apps which are checking whether to run a migrator tool, will be fooled into assuming that there are actually valid settings.
> 
> The patch uses the already set m_skipFile flag to decide when not to sync the KConfig object, thus not creating the file if it didn't exist before
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kconf_update/kconf_update.cpp 1135468 
>   /trunk/KDE/kdelibs/kconf_update/tests/test_kconf_update.h 1135468 
>   /trunk/KDE/kdelibs/kconf_update/tests/test_kconf_update.cpp 1135468 
> 
> Diff: http://reviewboard.kde.org/r/4243/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin
> 
>