Properly handle home trash behind complex symlinks/mounts#143
Properly handle home trash behind complex symlinks/mounts#143Byron merged 4 commits intoByron:masterfrom
Conversation
only recurse in canonicalize_path when error is NotFound
| } else if topdir.to_str() == Some("/var/home") && home_topdir.to_str() == Some("/") { | ||
| debug!("The topdir is '/var/home' but the home_topdir is '/', moving to the home trash anyway."); | ||
| move_to_trash(path, &home_trash, topdir).map_err(|(p, e)| fs_error(p, e))?; | ||
| } else { |
There was a problem hiding this comment.
The changes in this PR allow removing of this hardcoded logic to support Fedora Atomic desktops. This logic was originally needed because on Fedora Atomic desktops:
HOMEis set to/home/<user>- So the home trash is calculated as:
$HOME/.local/share/Trash->/home/<user>/.local/share/Trash /homeis a symlink to/var/home/var/homeis a mount/is a mount
The previous logic would incorrectly determine the topdir of the home trash /home/<user>/.local/share/Trash to be / because it didn't follow the symlink at /home.
There was a problem hiding this comment.
Pull request overview
This PR updates the freedesktop trash implementation to correctly resolve the actual home trash location in the presence of symlinks and nested mount points, so files are routed to the appropriate trash folder/topdir per the freedesktop spec behavior.
Changes:
- Canonicalize the computed home trash path (following symlinks), even when the final path component doesn’t exist yet.
- Determine the “home” topdir based on the resolved home trash folder itself (instead of
$XDG_DATA_HOME/$HOME). - Remove the old
home_topdirhelper and addcanonicalize_path_or_parentsto support the above behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@null-dev Thanks a lot! It think this can be merged once the auto-review comments were resolved. |
vp-oryn
force-pushed
the
nd/canonicalize-trash-path
branch
from
4859b39 to
8997e9a
Compare
Sweet, I've addressed all the comments. |
Byron
left a comment
Byron
left a comment
There was a problem hiding this comment.
Alright, thanks, let's just merge this.
Admittedly, I didn't see to what extend the canonicalisation test makes sense, but at least could see that it uses the respective function under test.
Overall, I'd think this is an improvement, or so I hope as errors around trashing can be fatal.
Consider the following system configuration:
XDG_DATA_HOME=/home/john/.local/shareso the home trash folder is located at:/home/john/.local/share/Trash/homeis a mount/home/john/.localis another mount/home/john/.local/share/Trashis a symlink to/home/trash/home/john/foo.This was not handled properly by the previous logic because the previous logic:
$XDG_DATA_HOMEor$HOME, and not$XDG_DATA_HOME/Trashor$HOME/.local/share/Trash.This PR fixes these issues.