lxqt / libqtxdg

Qt implementation of freedesktop.org xdg specs
https://lxqt.github.io
GNU Lesser General Public License v2.1
72 stars 35 forks source link

Ignore the cache and do the full look-ups as before #255

Closed flychenjun closed 3 years ago

flychenjun commented 3 years ago

If the icon is not found in the cache, Do not need to clear the sub directory, just ignore the cache and do a full lookup as before.

tsujan commented 3 years ago

Please explain what problems your PR solves and a way of reproducing them without it.

flychenjun commented 3 years ago

After installing a new application, the application icon is written below the path of the Hicolor, and the information is written to the cache. The icon may be read from Qicon: FROMTHEME, but when I install another new application, qiconcachegtkreader has read and loaded the cached file, and the newly installed application information has been written to the cache, but the Qiconcachegtkreader has not re-read the cached file, so the icon information for the new application can not be found in the cached file, at this point, the lookup path is cleared and can not be found from the full path。

tsujan commented 3 years ago

That's https://github.com/lxqt/libqtxdg/issues/226. A real fix should not ignore the cache but read it when it's ready.

flychenjun commented 3 years ago

Thanks for your help, I will check whether the icon information is written to the cache file correctly after installing the application

tsujan commented 3 years ago

I think the cache is correct. I'm suspicious of our timing. Maybe QFileSystemWatcher should be used in another way...

flychenjun commented 3 years ago

Yes, the cache is correct, I received a signal that the cache file had been modified while I was tracking debugging. My requirement is that when a new application is installed on the system, my application receives a signal containing the name of the application, and then calls the QIcon:fromTheme interface to read the icon of the application, then it is possible that the update cache and installation completion signals are out of sequence and I need to check again.

tsujan commented 3 years ago

Years ago, I also circumvented the cache, exactly like you did here and because of the same problem. My patch wasn't accepted for a good reason — the same reason I mentioned above.

Since then, I didn't find time to attack the problem again but I hope we could get rid of it once for all.

flychenjun commented 3 years ago

dear tsujan: I think there's a problem with the logic of using a cache file here. The first time you install an application, when you use the QICON: fromTheme interface, you create a global object: Xdgiconloader, which read the cache file and can find the icon in the cache. but, after the second installation, when the QICON: fromTheme interface is used again, the cache is not re-read from the file. so,I think the cache file needs to be loaded again after it has been modified.

flychenjun commented 3 years ago

QIconCacheGtkReader::QIconCacheGtkReader(const QString &dirName) : m_cacheFileInfo{dirName + QLatin1String("/icon-theme.cache")} , m_data(nullptr) , m_isValid(false) { m_file.setFileName(m_cacheFileInfo.absoluteFilePath()); // Note: The cache file can be (IS) removed and newly created during the // cache update. But we hold open file descriptor for the "old" removed // file. So we need to watch the changes and reopen/remap the file. QObject::connect(gtkCachesWatcher(), &QFileSystemWatcher::fileChanged, &m_file, [this](const QString & path) { qDebug() << "Log info : QFileSystemWatcher::fileChanged, path :" << path; if (m_file.fileName() == path) { m_isValid = false; // invalidate icons to reload them ... QIconLoader::instance()->invalidateKey(); } }); reValid(false); }

In this code, I added the log information, but no information was printed during debugging

tsujan commented 3 years ago

The first time you install an application,... which read the cache file and can find the icon in the cache

As mentioned in https://github.com/lxqt/libqtxdg/issues/226, the problem appears (randomly) with the first installation; subsequent uninstallations+ installations don't trigger it.

tsujan commented 3 years ago

The problem may be related to and fixed by https://github.com/lxqt/libqtxdg/pull/261; I'm not sure though.

tsujan commented 2 years ago

@flychenjun After more than a year, no one found a way better than yours. This is the same as https://github.com/lxqt/libqtxdg/pull/283, which I don't like but, IMO, it's better than seeing application-x-executable.