Support for symlinks in mount / vfs

[pasnox]

What is the purpose of this change?

Add mount / vfs symlink support

Please, note that it has only been tested with storj backend on linux.
Especially, macOS, windows were not tested.
Please do test those platforms and any other backends !!!

Was the change discussed in an issue or in the forum before?

#2975

Checklist

• I have read the contribution guidelines.
• I have added tests for all changes in this PR if appropriate.
• I have added documentation for the changes if appropriate.
• All commit messages are in house style.
• I'm done, this Pull Request is ready for review :-)

Notes

The whole work has been (trashed mostly) refactored. They were flaws in the previous implementation, especially trying to use truncated names in the vfs dir internal nodes names which lead to inconsistency when it comes to push changes to the remotes.
For this reason, the refactoring went in 2 parts:

1. Add symlinks support in the VFS itself, in the form of always storing symlinks with the .rclonelink suffix, those being regular files.
   The VFS Readlink and Symlink additions allow support for symlinks, even if the --links command line switch is not enabled, it's just a mater of creating a .rclonelink regular file with the target inside.
   The only difference is that when --link is enabled, the VFS nodes Mode() would report the os.ModeSymlink flag hint.
   This approach limit adding regressions in the VFS itself as there is no more names tweaking.
   Some more checks were also added, to avoid to allow creating / moving files / links / directories names that would clash.
   By example, the VFS must not be able to create in the same directory something called Hello.txt and something called Hello.txt.rclonelink, this is because when using the mount operation with --links enabled, we would end up with both files claiming the same node name: Hello.txt.

2. Add symlinks support in the mount backends, they are required to translate from the full vfs names into truncated names to the end user: Test.rclonelink would be seen as Test.
   When --links command line switch is enabled, the file system will translate any *.rclonelink files into * links.
   On the other hand, if --links is disabled, the symlinks will be seen as regular files, with their .rclonelink suffix which content contains the target file path.

Those changes comes with a rather big TestSymlinks unit test, which test all possible api and combinations.
The tests are ran for a VFS and each backends, with and without --links command line switch enabled, covering all possible scenarios.

I did run the tests successfully using this bash script:

#!/bin/bash -e

# Run the VFS tests in all subdirectories
pushd vfs
go test -tags cmount -v ./...
#go test -tags cmount -v -run TestFunctional
popd

# This test uses the tests in vfs/vfstest
pushd cmd/cmount
go test -tags cmount -v
popd

# This test uses the tests in vfs/vfstest
pushd cmd/mount
go test -v
popd

# This test uses the tests in vfs/vfstest
pushd cmd/mount2
go test -v
popd

[ncw]

I gave the code a very quick review - great work :-)

It needs tests though, both in the vfs layer and in the mount layer. The mount tests are in vfs/vfstest/ and likely they will need some build constraints not to run on windows. Or maybe symlinks do work on windows - I don't know!

I don't see patches for cmd/cmount? This is the variant which runs on Windows/Mac by default. It also runs on Linux too. You'll need -tags cmount to build it.

[pasnox]

I gave the code a very quick review - great work :-)

It needs tests though, both in the vfs layer and in the mount layer. The mount tests are in vfs/vfstest/ and likely they will need some build constraints not to run on windows. Or maybe symlinks do work on windows - I don't know!

I don't see patches for cmd/cmount? This is the variant which runs on Windows/Mac by default. It also runs on Linux too. You'll need -tags cmount to build it.

cmount support added.
Now all 3 mount variants do works correctly, as per tested by me on linux/storj.
Tests needs to be performed for others platforms/backends (especially local file is to be verified).
I'm a bit lazy to add tests (as I said, I don't know much Go, I just quickly read/learned it to fix this symlinks issue), but I will try to push some in the next days.

[ncw]

Great work :-)

Let me know if you want help writing the tests.

[pasnox]

Let me know if you want help writing the tests.

Thanks, that would be greatly appreciated.

How do I build unit tests and run them ?
I saw there is plenty of places where linkSuffix was not updated correctly in tests, but I can't find a way to try to build/run them so that I can fix them all.
EDIT: I used: go test -v in the backend/local folder, seems to be good now.

[pasnox]

@ncw I just rebased atop master.
I'm trying to add tests, but I have to be honest, i'm lost.
How would I test mount (3 variants) and test Readlink / Symlink on those ?
Could you guide me or try to write skeleton ?
Af far as I can see symlinks should be supported on windows.

[ncw]

There are probably two places that tests need to be added.

The first is in vfs/*_test.go

To pick an example

rclone/vfs/dir_test.go

Line 500 in 1107da7

func TestDirRemoveName(t *testing.T) {

You can see this creates a test VFS, creates files in it, then checks to see the files are as required. So you'd want to create a symlink and check it comes back in the listing.

You'll need to add tests to vfs/dir_test.go vfs/file_test.go, vfs/vfs_test.go and vfs.write_test.go to match the changes you made in each of those files.

Then you need to write a higher level test here https://github.com/rclone/rclone/tree/master/vfs/vfstest - the files in here are tested against the VFS itself and against all the different mount types. You write tests here just by creating and manipulating files - this is done either with vfs methods or os methods - the test runner abstracts that away.

That is quite a lot of tests I know, but it is the only way we keep such a complicated bit of software working. The VFS layer is by far the most complex bit of rclone and I often think its the bit of code I was only just clever enough to write and therefore it is too difficult for me to debug!

One thing I noted is that this should probably be gated on a flag - I don't think you have that at the moment?

[pasnox]

Thanks for the hints, will have a look.

One thing I noted is that this should probably be gated on a flag - I don't think you have that at the moment?

Right I do not have a flag for that. Will add it at some point in time ;)

[pasnox]

Hm, how would I trigger tests ?

I tried make quicktest but it lead to plenty of failing tests, even with bare master ?

[ncw]

You cd into the directory with the tests and run go test -v that will work for vfs cmd/mount etc

[hongkongkiwi]

When can we get this merged?

[pasnox]

Not yet, while it works fine cache less, it cause troubles with the cache system due to the names translations. Also still writing the unit tests to ensure correctness

[xmready]

@ncw v1.67 has come and gone, yet this PR is still open. Can we ever expect this amazing feature to make it into rclone?

[peter-tar]

I would love to see this merged as well.

[ncw]

I have rebased this PR. I'm going to go through it to see what needs to be done to it to get it merged.

[pasnox]

Thanks for your interest again in this area @ncw .
As far as I remember, the implementation is fully correct and working on all paltforms.
The only issue is the windows version using the winfsp or something named like for for the fuse implementation which is doing dirty hack to support symlink causing rclone issue. I cna't remember where, but I explained the problem very well.
I can't wait to have it merged, finally ! When, if, done, pelase ping me afterwards as tehre is some things we could discuss to make better performances with the symlink lookups, but each things at its time.

[ncw]

Thank you for your work on this @pasnox

I'm going to merge this into a new branch fix-2975-mount-symlinks as I'm going to work on it a bit before merging to master.

In particular I'm going to push all the symlink logic into the vfs layer so there is no duplicated code in the mount/cmount/mount2.

I'm haven't got my head around the windows support yet though.

I'll merge this, then I'll push an updated version and post a beta for people to try.

[ncw]

Here is a beta of the current build if anyone wants to give it a try

v1.69.0-beta.8381.204b90a79.fix-2975-mount-symlinks on branch fix-2975-mount-symlinks (uploaded in 15-30 mins)

[ncw]

I've merged this to master now which means it will be in the latest beta in 15-30 minutes and released in v1.69

Thank you @pasnox

Any more testing gratefully received :-)

[pasnox]

Great news @ncw !
I'm glad this finally landed in master!
I will test it, is there a rpi5 64bits build i can grab somewhere??
Were you able to fix the win FS issue I had ?

[ncw]

@pasnox wrote

Great news! I'm glad this finally landed in master! I will test it

Thanks

is there a rpi5 64bits build i can grab somewhere??

Sure - this one should work https://beta.rclone.org/rclone-beta-latest-linux-arm.zip (or .deb or .rpm if you prefer)

Were you able to fix the win FS issue I had ?

I dedicated to get this merged. I think there are likely still problems under Windows but that is a fix for another day :-)

[pasnox]

Please delete pr-6337-mount-symlink / pasnox-symlinks branches, it's no more needed then.
Yeah, let tackle windows issues as they comes, maybe winfsp is not even used by rclone users ;)
Do you have performances benchmark for symlink support ?
Improving symlinks performance was somehting i wanted to work on / fix at a later step, but maybe your refactoring fix it entirely. Basically, evaluating symlinks would trigger network request just to read its target content. It's maybe something we would prefer to store locally, maybe in the metadata so that's it's fast and does not require extra network request anytime we require evaluating symlinks target.

[ncw]

Please delete pr-6337-mount-symlink / pasnox-symlinks branches, it's no more needed then.

Done

Yeah, let tackle windows issues as they comes, maybe winfsp is not even used by rclone users ;)

Lots of people use winfsp but I think symlinks are much less used on Windows.

Do you have performances benchmark for symlink support ? Improving symlinks performance was somehting i wanted to work on / fix at a later step, but maybe your refactoring fix it entirely. Basically, evaluating symlinks would trigger network request just to read its target content. It's maybe something we would prefer to store locally, maybe in the metadata so that's it's fast and does not require extra network request anytime we require evaluating symlinks target.

The symlinks can get stored in the VFS cache so using --vfs-cache-mode full will cache them.

The OS should cache them also so they shouldn't get read too often, but I didn't measure performance, I was more worried about correctness for this pass.

[pasnox]

I tested over a sftp remote (no cache) and it worked fine.
I was able to:

• create files/dirs
• create files/dirs links pointing to the previous created entries
• move files/links in between
• delete files / links / dirs

So far for me, it seems to works great.
One exception, you did not fully refactored / ported corner case #3 from tests, as such, we are able to copy/move files with same name into dirs already containing the same name. I would really want you fix it before releasing the next version.
See f1d2f2b#diff-4b3637c29d010b5284e9bc43437f32ffb7a5a6cb85c0bea56f4bbb7df23c09a4R237

[ncw]

Thanks for testing @pasnox much appreciated

I think those corner case tests got lost in the code reshuffle, alas

Can you make an issue about the problem please so we don't forget - thank you .

Do you want to have a go at fixing?

[pasnox]

Can you make an issue about the problem please so we don't forget - thank you .

#8245

Do you want to have a go at fixing?

I will have no time for that sorry, please go ahead, most likely reading the original PR code should show how exactly it's to be checked.