Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'symlinks' test fails when /tmp is on a different filesystem to /home #87

Open
sourcejedi opened this issue Aug 4, 2019 · 1 comment

Comments

@sourcejedi
Copy link

  1 test failed

  symlinks


  Rejected promise returned by test. Reason:

  Error {
    code: 'ENOENT',
    dest: '/home/alan-sysop/.local/share/Trash/files/689b96e0-ef36-4532-9f56-30c86009afec',
    errno: -2,
    path: '/tmp/10fa99e2-c8ea-4643-ab3e-8b0f10156755/ccc',
    syscall: 'copyfile',
    message: 'ENOENT: no such file or directory, copyfile \'/tmp/10fa99e2-c8ea-4643-ab3e-8b0f10156755/ccc\' -> \'/home/alan-sysop/.local/share/Trash/files/689b96e0-ef36-4532-9f56-30c86009afec\'',
  }

Analysis

  1. fs.copyFile() fails because it tries to copy the target of the symbolic link being trashed. And in this test, ccc is a symlink to a non-existent file (ddd).

  2. We never want copyFile to be copying the target of a symbolic link. This is a bug in move-file. move-file should use something other than copyFile. (copyFile does not have a NOFOLLOW option that we could use).

  3. It should be surprising that move-file needed to fall back to copyFile at all. Linux does add a number of EXDEV cases - overlayfs copy-up, crossing btrfs volumes, or on bcachefs if the new parent directory has different storage options. However none of these applies in my case. The surprise is a deficiency in xdg-trashdir, partly caused by mount-point. (Here is a test case which shows the inconsistency in xdg-trashdir).

  4. mount-point does not do what we want for symlinks - it is dereferencing them. mount-point relies on @sindresorhus/df, which literally runs the df command, which always dereferences symbolic links. mount-point tries to follow this dangling symbolic link, so it fails. The failure causes xdg-trashdir to fall back to /home/alan-sysop/.local/share/Trash.

Side notes

  1. You might still hit EXDEV e.g. from the cross-btrfs-subvolume case, and need to fall back to copying. Currently, move-file is not suitable for this. It does not support copying symbolic links or directories.

  2. Trashing a regular file in /tmp will use /tmp/.Trash-1000, avoiding any problems with cross-filesystem renames. This does not seem very desirable. trash currently relies entirely on other implementations to provide the critical "un-trash" feature :-). The Gnome file browser does not support trashing files in /tmp, at least if it is on a different filesystem, and nor does it not any trashed files from /tmp/.Trash-1000.

Possible solutions

  1. To solve side note 1, maybe use move-concurrently. On the plus side, it's fairly widely used, well written, and small. There are several alternatives, which vary in how suitable they are. fs-extra should work fine as well, but it's bigger, and the fact that it's coupled to graceful-fs is weirding me out: https://softwarerecs.stackexchange.com/questions/65653/node-js-library-package-to-move-a-file-directory-or-symlink/

  2. Addressing side note 1 would make the test pass. However, on its own, this would ignore side note 2 and the inconsistency in xdg-trashdir. This problem is pretty tricky! But IMO we've already been doing weird things when crossing filesystems. The simplest thing that could work would be to drop support for creating .Trash-$UID. It would be possible to followup later, with a closer emulation of what the Gnome file manager does (this might involve querying udisks using dbus method calls).

@sindresorhus
Copy link
Owner

Thanks for your thorough analysis. 🙌

(I must have missed the notification for this)


fs.copyFile() fails because it tries to copy the target of the symbolic link being trashed. And in this test, ccc is a symlink to a non-existent file (ddd).

That is surprising behavior to me. The Node.js docs says nothing about how symlinks are treated. https://nodejs.org/api/fs.html#fs_fs_copyfile_src_dest_flags_callback

I think you should open an issue on Node.js to clarify how symlinks are handled and also add an option to not follow symlinks (should IMHO be on by default).

mount-point does not do what we want for symlinks - it is dereferencing them. mount-point relies on @sindresorhus/df, which literally runs the df command, which always dereferences symbolic links. mount-point tries to follow this dangling symbolic link, so it fails. The failure causes xdg-trashdir to fall back to /home/alan-sysop/.local/share/Trash.

I'm happy to consider solutions. I don't use Linux and the Linux parts of the trash codebase was not written by me. So what I'm saying is that PRs are welcome and I will review, but I won't be working on any of this.

To solve side note 1, maybe use move-concurrently. On the plus side, it's fairly widely used, well written, and small.

I'm vary of using packages under the npm GitHub org. They are very often buggy and badly maintained. I've been burned by that many times in the past. For example, https://github.com/npm/write-file-atomic/issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants