Detect mount points using realpath #12
Conversation
|
|
|
Thanks for the PR, I'll check it later. |
|
I'm coming from Surely, the definition for
means the mount point of the (entire) filesystem (i.e. its root) and not a bind mount to some subtree. Edit: If I am wrong here, then resolving symlinks and traversing the parents is the way to go. |
|
Ok, bind mounts are kinda wonky. If I again have a mount
Nautilus will simply offer to permanently delete the file. Edit: I found this old issue: https://gitlab.gnome.org/GNOME/glib/-/issues/251 |
kmarius
changed the title
This reverts commit 0593fe5
internal/xdg/trashdir.go
Outdated
| if m.Mountpoint == mountpoint { | ||
| break OUTER | ||
| } | ||
| if mounted, err := mountinfo_Mounted(candidate); err == nil && mounted { |
There was a problem hiding this comment.
I think an err check is needed
(If for some reason mountpoints could not be taken or something unlitely. It is better to return an error and exit early)
There was a problem hiding this comment.
That seems sensible.
internal/xdg/trashdir.go
Outdated
|
|
||
| // iterate over the parents of the real (without symlinks) path until we find a mount point | ||
|
|
||
| candidate, err := realpath_Realpath(path) |
There was a problem hiding this comment.
Can't this library be replaced by the standard library filepath.EvalSymlink?
There was a problem hiding this comment.
Yes, that works since we are passing absolute paths (I did not know about that one)
internal/xdg/trashdir.go
Outdated
| // getfsstat(2) used on Mac (BSD) | ||
| mountpoints, err := mountinfo_GetMounts(nil) | ||
|
|
||
| // iterate over the parents of the real (without symlinks) path until we find a mount point |
There was a problem hiding this comment.
Is it okay to convert to realpath if the file to be deleted is itself a link file?
We want to target the link itself for deletion, not the link destination, so we may need to skip the realpath process in the case of a link file.
So I think we need a condition to determine if path is a linked file using Lstat.
There was a problem hiding this comment.
Good point. I think we need to look up the real path of the dirname of the file since the trash directory only depends on the directory the file (or link) is in.
* use filepath.EvalSymlinks * resolve only symlinks of the parent * update trashdir_test
|
Thanks. I will release it later. |
Mountpoints are currently not detected properly, e.g. if the path contains symlinks. Consider the symlink
/home/john/Data -> /mnt/data, where/mnt/datais not the same filesystem as/(assuming/home/johnis in the root filesystem). Trashing/home/john/Data/a.txtwill find the mount point/, because only the parents (/home/john/Data,/home/john/,/home,/) are considered.This patch replaces checking the parents for mount points by comparing
st_devof the file to be trashed withst_devof all mount points.I'm not sure the removed test can be replaced usefully.