All LXQt::Settings and QSettings file change monitoring stop working.

[PCMan]

I really think this is a critical issue since this breaks lxqt-qtplugin and liblxqt.
All code that relying on file change monitoring of the config files are broken since Qt 5.4.
I'm not sure about how many components are affected by this.
There is one thing for sure, it's one of the causes of broken theme update happened recently.
So I hope you guys can take a look at it and don't regard it as a minor issue that can be ignored.
(ping @paulolieuthier , @jleclanche , @luis-pereira )
The problem is caused by this commit in Qt 5.4.
qt/qtbase@8d15068#diff-e691c0405f02f3478f4f50a27bdaecde
By using QSaveFile instead of the original file saving mechanism, the config file is written to a temporary file, and replace the old file later.
The original config file is never written. It's deleted and replaced.
This unfortunately breaks the QFileSystemWatcher API so we can no longer receive file change notifications after the first change.
I already found a way to workaround this issue and is now working on it.

[jleclanche]

damn it, qt bugs :(

Let's have a workaround in 0.9 and make sure it's filed and fixed for 5.5.

[luis-pereira]

On Sun, Jan 18, 2015 at 12:20 PM, PCMan notifications@github.com wrote:

I really think this is a critical issue since this breaks lxqt-qtplugin
and liblxqt.
All code that relying on file change monitoring of the config files are
broken since Qt 5.4.
I'm not sure about how many components are affected by this.

Widget style, Icons Theme, LXQt Theme, Font

There is one thing for sure, it's one of the causes of broken theme update
happened recently.
So I hope you guys can take a look at it and don't regard it as a minor
issue that can be ignored.

This is a major issue. It completely and utterly breaks global settings.

(ping @paulolieuthier https://github.com/paulolieuthier , @jleclanche
https://github.com/jleclanche , @luis-pereira
https://github.com/luis-pereira )
The problem is caused by this commit in Qt 5.4.
qt/qtbase@8d15068#diff-e691c0405f02f3478f4f50a27bdaecde
qt/qtbase@8d15068#diff-e691c0405f02f3478f4f50a27bdaecde
By using QSaveFile instead of the original file saving mechanism, the
config file is written to a temporary file, and replace the old file later.
The original config file is never written. It's deleted and replaced.
This unfortunately breaks the QFileSystemWatcher API so we can no longer
receive file change notifications after the first change.

I already found a way to workaround this issue and is now working on it.

Good :)

[paulolieuthier]

@PCMan you are the man of the workarounds! Looking forward to seeing the fix.

[PCMan]

Well, strictly speaking, it's not a Qt bug. It's a feature.
Using QSaveFile ensures atomic change of config files, which is actuallty better.
This can prevent someone from reading an incomplete config file that is still being written by others.
Gtk/Glib did the same with g_file_set_contents() API for so many years. We have the problem because QFileSystemWatcher is quite useless and limited. This kind of problem can be easily handled by glib/gio's GFileMonitor.

Anyway, here is my simple workaround:
lxqt/lxqt-qtplugin@f1a0712

Just check if the file is deleted. If it is, re-add the path to the watcher.
(Though it's also ridiculous that QFileSystemWatcher does not tell you wether a file is deleted or not, which makes the API even more useless.)

[jleclanche]

@PCMan Looks good. I understand the issue better now. It's worth asking on the Qt ML how we're actually supposed to do this.

But, in the end, I suspect it comes down to using gsettings. Kind of annoying they would pull this without actually supporting even dconf, isn't it?

[PCMan]

I can fix it. Just give me 1 or 2 days.
In addition to gsettings, another option is KConfig + KCoreAddons.
However, gsettings might be lighter since everyone has glib so we don't
need any additional libs.

On Mon, Jan 19, 2015 at 6:19 PM, Jerome Leclanche notifications@github.com
wrote:

@PCMan https://github.com/PCMan Looks good. I understand the issue
better now. It's worth asking on the Qt ML how we're actually supposed
to do this.

But, in the end, I suspect it comes down to using gsettings. Kind of
annoying they would pull this without actually supporting it, isn't it?

—
Reply to this email directly or view it on GitHub
#441 (comment).

[PCMan]

Here comes the pull request.
lxqt/liblxqt#33

[pmattern]

PR lxqt/liblxqt#33 is a workaround only, an eventual adaption of LXQt to the changes in Qt covered by this issue is still pending.
This was stated in the said PR as well and it was proposed to go on discussing the problem in a new issue.
Reopening for now as the latter doesn't seem to have happened yet so as to prevent this issue from getting forgotten. IMHO it even wouldn't hurt to go on discussing here - this issue isn't too complex yet after all but does cover all necessary details already.

[palinek]

IMHO lxqt/liblxqt#33 is not a workaround, but a proper solution.

[palinek]

@pmattern do you still want this to be open?

[paulolieuthier]

Yes, I think we can close this. We are doing it in a correct way (which doesn't mean there are not better ways).

[pmattern]

@palinek
Reopening was just meant as a reminder as I was under the impression that the current solution was considered rather a workaround and not something that should be kept in the long run after reading both this issue and the comments on lxqt/liblxqt#33, see in particular this one.
Unfortunately I cannot judge all technical details. So if the solution is all right, this issue can very well be closed again.
Maybe @PCMan can drop a note, too.

Removing the milestone as the solution sure works for now.

[agaida]

does it really work - see #1560

[palinek]

does it really work - see #1560

This was something completely different...

[agaida]

ok - so we can close this right now?

[palinek]

It shouldn't have been reopened in the first place :-D.... so definitely yes