D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs
meven updated this revision to Diff 73261. meven added a comment. remove a commo in comment, fix @since version REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26407?vs=73260=73261 BRANCH arcpatch-D26407 REVISION DETAIL https://phabricator.kde.org/D26407 AFFECTED FILES src/core/kfileitem.cpp src/core/kmountpoint.cpp src/core/kmountpoint.h To: meven, #frameworks, ngraham, broulik, dfaure Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs
meven updated this revision to Diff 73260. meven marked an inline comment as done. meven added a comment. And KMountPoint::List::findByPathDirectly to check mount point without resolving symlinks REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26407?vs=72738=73260 BRANCH arcpatch-D26407 REVISION DETAIL https://phabricator.kde.org/D26407 AFFECTED FILES src/core/kfileitem.cpp src/core/kmountpoint.cpp src/core/kmountpoint.h To: meven, #frameworks, ngraham, broulik, dfaure Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs
meven added inline comments. INLINE COMMENTS > broulik wrote in kfileitem.cpp:783 > > when a network mount is unresponsive > > If I do a `QFileInfo` on an unresponsive mount it will still block. You > already freeze before you even know it's a network mount. Good point, indeed the current solution does not work. We need an alternative to KMountPoint::findByPath to check the path without calling QFileInfo. > dfaure wrote in kfileitem.cpp:787 > You're right, the issue on the FreeBSD jail is fstab, not mnttab. Ignore what > I said. > > I still don't really like the assumption "no mountpoint found for a given > path => we're on android" in a comment. I bet there are other corner cases > where this can happen. If you really want to find out you're on android, > surely there's Q_OS_ANDROID. Or until we find out what those other corner > cases are, the code can stay, but the comment should say "for instance" or > "maybe", not "can only mean". I did not mean to avoid android, just surfaced the comment in KMountPoint::currentMountPoints regarding its limitation to make it clear why we need an else block here in the first place. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26407 To: meven, #frameworks, ngraham, broulik, dfaure Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > anthonyfieroni wrote in kfileitem.cpp:49-50 > Put them in `getMountPoints` as local static variables, global static are > no-go. I agree. > meven wrote in kfileitem.cpp:787 > Here I am using KMountPoint::currentMountPoints which uses mnttab which > should be supported on *nix. > The part about android came from KMountPoint::currentMountPoints comment. > > I was thing about merging the two KMountPoint::currentMountPoints and > KMountPoint::possibleMountPoints for better accuracy. You're right, the issue on the FreeBSD jail is fstab, not mnttab. Ignore what I said. I still don't really like the assumption "no mountpoint found for a given path => we're on android" in a comment. I bet there are other corner cases where this can happen. If you really want to find out you're on android, surely there's Q_OS_ANDROID. Or until we find out what those other corner cases are, the code can stay, but the comment should say "for instance" or "maybe", not "can only mean". REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26407 To: meven, #frameworks, ngraham, broulik, dfaure Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs
broulik added inline comments. INLINE COMMENTS > meven wrote in kfileitem.cpp:783 > Good point I have edited the commit message > when a network mount is unresponsive If I do a `QFileInfo` on an unresponsive mount it will still block. You already freeze before you even know it's a network mount. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26407 To: meven, #frameworks, ngraham, broulik, dfaure Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs
meven marked an inline comment as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26407 To: meven, #frameworks, ngraham, broulik, dfaure Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs
meven added inline comments. INLINE COMMENTS > broulik wrote in kfileitem.cpp:783 > Yes, but the "never blocks" claim in the commit message is misleading. Good point I have edited the commit message REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26407 To: meven, #frameworks, ngraham, broulik, dfaure Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs
meven updated this revision to Diff 72738. meven marked 3 inline comments as done. meven added a comment. amend commit title REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26407?vs=72732=72738 BRANCH arcpatch-D26407 REVISION DETAIL https://phabricator.kde.org/D26407 AFFECTED FILES src/core/kfileitem.cpp To: meven, #frameworks, ngraham, broulik, dfaure Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs
meven retitled this revision from "KFileItem: improve isSlow to never block, make SkipMimeTypeFromContent skip only network fs" to "KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on network fs". REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26407 To: meven, #frameworks, ngraham, broulik, dfaure Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
