(fix) Computer feature: update removable devices on Linux#12893
Conversation
13cebcc to
6349bb7
Compare
| const QStringList linuxRemovableDrivePaths() { | ||
| QStringList paths; | ||
| paths.append("/media"); | ||
| paths.append(QStringLiteral("/run/media/") + QString::fromLocal8Bit(qgetenv("USER"))); |
There was a problem hiding this comment.
It should be the same for /media on Ubuntu/Pop-Os - /media/$USER
There was a problem hiding this comment.
I know, it's like that on Ubuntu (which Pop!OS is based on).
Though TLDP https://tldp.org/LDP/Linux-Filesystem-Hierarchy/html/media.html says (can't tell how up-to-date that is, and which distros have their own rules, but /media seems to be the common base):
The following directories, or symbolic links to directories, must be in /media, if the corresponding subsystem is installed:
floppy Floppy drive (optional)
cdrom CD-ROM drive (optional)
cdrecorder CD writer (optional)
zip Zip drive (optional)On systems where more than one device exists for mounting a certain type of
media, mount directories can be created by appending a digit to the name of
those available above starting with '0', but the unqualified name must also
exist.A compliant implementation with two CDROM drives might have /media/cdrom0
and /media/cdrom1 with /media/cdrom a symlink to either of these.
This comment was marked as outdated.
This comment was marked as outdated.
409f382 to
0a25887
Compare
0a25887 to
e6e66e1
Compare
… ... when Devices is expanded, not the cached state.
e6e66e1 to
f73c8b1
Compare
|
Oh, one issue with moving [$USER] subdirs to the top level: I'll check if that can be fixed without hacks, else I'll revert the respective commit. |
Okay, this is not a regression: I tested 2.4 after removing /media/USER and it's the same. This is ready for merge. |
|
Tested and working as expected! I have some custom mount points configured with udisk which aren't shown in Mixxx but this is expected due to the static path lists in (Not for this PR but for a next iteration) - what do you think of using |
|
Sure, might work. |
|
Thank you @ronso0 for the PR and @acolombier for testing! |
|
@acolombier Wanna file an issue re: |
Re #12891
Issue 1
I didn't see my SD card in /media/user after I re-mounted it.
I have been focused only on
/media/[user]because this is where my USB/SD drives are mounted. Devices changes (or removed and re-mounted devices) where not reflected in the tree view because these are subdirs of the device lookup path /media for which the respective 'hasChildren()` state has been cached.This is not the case for 1st-level children of the lookup paths, e.g. /run/media/[user]. In those we show devices correctly (newly added or removed) when (re-)expanding the 'Removable Devices' node.
Fix:
add /media/[user] to lookup paths, but don't show it when adding children of /media ✔️
= all devices are now leaves of the Devices branch
Issue 2
We won't see changes in the dir tree after initial expand due to
hasChildren()caching.While for static mounts this may be an okayish tradeoff for (assumed) faster repaint for static mounts, it is much more likely that the dir tree of a removable device has changed.
Ideas and simple benchmarking:
a) when expanding Devices, tell the FolderTreeitemModel to remove all child paths from the cache:
with 10.000 cached paths this takes about 2.5ms, and is only done once when [devices] is expanded
b) in
directoryHasChildren, check ifpathstarts with one of the device root paths, if yes: don't cachehasChildrenwith 10.000 cached paths this takes about 7ms per dir, and causes some notable delay under some circumstances
Fix:
So the winner is a) ✔️
To refresh, just collapse & expand 'Removable Devices' (click on branch icon or hit Return twice).
Addon / Proposal
We could add a new Browse item action 'Refresh directory tree', mapped to the new method
FolderTreeModel::removeChildDirsFromCache(QStringList).This would allow to force-refresh the dir tree of that item, e.g. when you copied dirs to a previously empty QuickLink dir.
#12908
TODO
make Refresh more discoverable (tooltip), mention it in the manual
Related meta discussion: https://mixxx.zulipchat.com/#narrow/stream/109122-general/topic/linux.3A.20mounting.20drive.20after.20starting.20mixxx