Related diff D19784
BUG: 401579
No Linters Available |
No Unit Test Coverage |
Buildable 15174 | |
Build 15192: arc lint + arc unit |
fileSystemType calls statvfs which might also block, so I don't think this helps.
applets/kicker/plugin/recentusagemodel.cpp | ||
---|---|---|
252 | toLocalFile() instead of path() |
True, but then that implies that KFileItem::isSlow() which has the same code is flawed in the same way.
Well I had the same reflection in https://phabricator.kde.org/D19784#503088 but after reflection it is ok :
A file whose filesystem is not mounted, is a non-existing file which statvfs report as its error ENOENT http://man7.org/linux/man-pages/man3/statvfs.3.html
I still have to test it at home where I have a nfs setup.
True, but then that implies that KFileItem::isSlow() which has the same code is flawed in the same way.
It is. We have ironic lockups there.
My last change is more a Proof of Concept : using Solid to avoid mime check.
I would be in favor of adding a static function to Solid encompassing this use case.
For instance :
bool Solid::isMountedOnNetworkShare(const QString &filePath) static;
But the good part is that this solution actually works, we get back icons for local files and don't block plasma with a stale mount.
I am open for suggestion.
Why not do this in KFileItem::isSlow() itself?
(and enable SkipMimeTypeFromContent when isSlow is true)
First because KFileItem::isSlow is a member function of KFileItem and this code is used to select the correct constructor of KFileItem in the first place...
Second I already tried using KFileItem::isSlow method i.e KFileSystemType::fileSystemType and statvfs syscall in a previous attempt in this diff and it was not usable for this context :
statvfs block the thread in case of a stale network mount until the network is back up (case of a nfs mounted when network is cut).
Meaning it would freeze plasmashell when the case occurred, and shows that KFileItem::isSlow/KFileSystemType::fileSystemType cause freezes whenever they are used for a file on a network stale mount.
This new method is actually implementing what I suggested in https://phabricator.kde.org/D19784#502503
What I mean is:
However maybe we don't want that in dolphin? Not sure. (In that case a new enum value can be added, to disable some features "if slow".)
In any case I see no reason for this code to be in recentusagemodel, it's just one out of many KFileItem users, with similar needs as most others.
(And all this is the reason I hate network mounts, and the much-requested fuse-kio integration... what looks like a local path ends up being unusable as a local path and needs special treatment anyway...)
Sorry I misread you.
However maybe we don't want that in dolphin? Not sure. (In that case a new enum value can be added, to disable some features "if slow".)
That could be a good idea, allowing users to prevent freezes.
In any case I see no reason for this code to be in recentusagemodel, it's just one out of many KFileItem users, with similar needs as most others.
Yes I thought so, https://phabricator.kde.org/D23198#516506
I really don't like the "isSlow" naming, it hides its meaning, network filesystem aren't necessarly slow, "isNetworkMounted" or similar would be more descriptive IMHO.
No because isSlow is a member function but the purpose here is to choose a constructor for KFileItem in the first place.
Hence my idea to have a function encompassing this in a library, because this ought to be reused.
My suggestion is (still) "improve KFileItem to enable SkipMimeTypeFromContent automatically if isSlow is true"
[possibly with another flag if you think there are actually use cases for mime-magic over network mounts]
Then all this code isn't needed.