vfs: add refresh flag

[beyondmeat]

What is the purpose of this change?

Add vfs/refesh recursive=true flag so it can run w/o needing rc on mount.

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

closes #6830

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 :-)

[ncw]

Looks great - thank you :-)

I'll merge this now.

[ncw]

Oops, forgot to run the CI. I'll merge this if the CI goes green, if not you'll have something to fix up!

[ncw]

I think you forgot to commit something - this definition is missing

Error: vfs/vfs.go:237:13: vfs.Opt.Refresh undefined (type vfscommon.Options has no field or method Refresh)

[ncw]

@beyondmeat can you commit the missing bit of this PR? I think you forgot to commit the vfs/vfscommon/options.go file.

[beyondmeat]

@ncw will get to this soon. I didn't finish that part yet.

[beyondmeat]

--vfs-refresh=true           Refreshes the directory listing recursively

I made the default value false. Should be ready for review @ncw

[darthShadow]

Can we add an option to do it async too? So perhaps instead of true/false, off (default)/sync/async ?

[beyondmeat]

Is there a reason why anyone would ever want the refresh to be blocking/sync?

[darthShadow]

The current behaviour is sync. I would prefer async always but not sure about others so may as well add compatibility with the rc way of doing it which is both sync and async.

[beyondmeat]

I'm unsure if non-async would ever be used. It would effectively block the mount until the read is done, which I doubt anyone would ever want because many times these mounts are ran at startup before other apps/services. I can't think of a case where I want the mount delayed until the entire dir tree is read. Sure the option is easy to have, but will it be used?

I would also need another flag to control if it's recursive or not if you just want it to match the --rc functionality, which I don't think it's neccessary b/c this only happens at mount and why wouldn't it be recursive? The whole point of this flag is to pre-warm the dir cache at mount so you don't need --rc otherwise. Having it sync and non-recursive, I fail to think of a use-case where that would be needed, if it is, they might need --rc anyway.

I'm aware my current implementation is sync, I'm exploring how to make it async, any example code I could reference to help with that. I tried code searching for async but not sure how _async=true works in the code.

[darthShadow]

It runs the async jobs in a goroutine:

rclone/fs/rc/jobs/job.go

Line 281 in 9a9ef04

go job.run(ctx, fn, in)

I guess you could do the same here by calling the readDirTree function in a goroutine.

[ncw]

This has a build failure which needs fixing

Error: vfs/vfsflags/vfsflags.go:33:59: not enough arguments in call to flags.BoolVarP
	have (*pflag.FlagSet, *bool, string, string, string)
	want (*pflag.FlagSet, *bool, string, string, bool, string)

I'd be fine if this was always async so just run the call to read tree in a go routine to do that.

[beyondmeat]

/verify

Need a slash command to reun the build/workflow. @ncw

[beyondmeat]

@ncw is this good now?

[darthShadow]

I have also approved the CI run so let's see if anything else needs to be changed.

[beyondmeat]

@darthShadow passed the go lint locally now. Hope I did it right

[beyondmeat]

@darthShadow ok fixed that lint error, please run the checks again. I am using vs code, static check, go vet, all come back fine. no problems reported in the files I modified. Hope it passes now.

[beyondmeat]

@ncw @darthShadow please check

[beyondmeat]

@ncw looks like all checks passed? Can this be merged in now?

[ncw]

@beyondmeat sorry this dropped off the radar! I rebased this - I'll wait for the CI to go green then merge :-)

[ncw]

Thank you :-)