Add Fuse mounting support.#33
Conversation
|
|
overheadhunter
left a comment
overheadhunter
left a comment
There was a problem hiding this comment.
Neat PR. Just two remarks from my side.
| private static WebDav initWebDavServer(Args args) { | ||
| WebDav server = null; | ||
| if (args.hasValidWebDavConf()) { | ||
| server = new WebDav(args.getBindAddr(), args.getPort()); | ||
| } | ||
| return server; | ||
| } |
There was a problem hiding this comment.
Better return an Optional<WebDav> instead of null
| try { | ||
| while (true) { | ||
| System.in.read(); | ||
| } | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| } |
There was a problem hiding this comment.
This is to retain the previous functionality for FUSE mounts (if no WebDAV server is not running): Starting the app mounts the vault and ctrl-c quits the app + umounts. If there is no loop for stdin it just quits the app immediately.
Please let me know if you have suggestions how to achieve this more cleanly!
There was a problem hiding this comment.
Please let me know if you have suggestions how to achieve this more cleanly!
You can run FUSE in "blocking mode" (second argument of the mount method), but you'd need to also overload the Mounter's mount method, as blocking is currently always false:
I'd prefer this approach but leave it open to you.
There was a problem hiding this comment.
Thanks for the suggestion. I have a couple of concerns, though. Please let me know if they are valid:
- If I mount in blocking mode it implies that I can only have a single FUSE mount when running the CLI-app, right? Or surely I could add some threading, but that would IMO complicate this app unnecessarily. With the current approach there can be multiple simultaneous FUSE and/or WebDAV mounts running with a single CLI-app instance and the logic is fairly straight forward.
- What is the release cadence of fuse-nio-adapter? I mean if I make a pull request, when will it be available in Maven?
- I probably should also make similar changes to MacMounter.java. I currently do not have means to test that it works.
There was a problem hiding this comment.
If I mount in blocking mode it implies that I can only have a single FUSE mount when running the CLI-app, right? Or surely I could add some threading, but that would IMO complicate this app unnecessarily. With the current approach there can be multiple simultaneous FUSE and/or WebDAV mounts running with a single CLI-app instance and the logic is fairly straight forward.
You're right. You can only have a single mount in this case. Ctrl+C will unmount it and end the process.
What is the release cadence of fuse-nio-adapter? I mean if I make a pull request, when will it be available in Maven?
It can be released any time, there is no schedule we need to follow. It is just a matter of tagging and letting CI do its job.
I probably should also make similar changes to MacMounter.java. I currently do not have means to test that it works.
MacMounter has the same boolean flag, which works the same way. I have already tested it locally.
There was a problem hiding this comment.
Ok. I think I would then keep this functionality as it is currently implemented as the previous WebDAV implementation also supports multiple mounts. Now you can basically add any number of WebDAV and/or FUSE mounts with a single instance.
From my side this PR is ready to be merged now in case it's fine for you too.
There was a problem hiding this comment.
Now you can basically add any number of WebDAV and/or FUSE mounts with a single instance.
I still think this is a debatable feature, since you can as well spawn multiple processes and thus have better control over each mounted vault. However, this is out of scope of this PR and should be discussed in a separate issue.
|
Thanks for the feedback @overheadhunter ! Please let me know if you have other improvement ideas. |
Well you have a lot of places where you simply |
True. Added some better handling/logging. I prefer keeping the org.cryptomator.frontend.fuse.mount namespaced Exceptions (CommandException) contained in FuseMount-class. |
…ug, EnvironmentVariables envVars)` to Mounter (might be required by cryptomator/cli#33)
3 participants
Add support for mounting vaults to filesystem with fuse-nio-adapter.
Tested on Linux.
Fixes #25.