Review Request 114812: KVersionControlPlugin2 based Dropbox version control plugin for Dolphin

Thu Feb 6 12:18:46 GMT 2014

> On Feb. 3, 2014, 11:31 a.m., Phil Schaf wrote:
> > a question: does this fix this? https://bugs.kde.org/show_bug.cgi?id=264717
> > 
> > i.e. work with symlinks to the dropbox directory?
> 
> Emmanuel Pescosta wrote:
>     > does this fix this?
>     No, this problem is (nearly) unsolvable within this plugin or within Dolphin - it can be fixed with some really (!!!) ugly hacks, but this will drastically slow down this plugin.
>     
>     This needs adjustments in the Dropbox client itself.
>     
>     It would be much better if Dropbox does smth. like git or svn for example, so that you can put every folder under version control + symlinks will also work ;)
> 
> Phil Schaf wrote:
>     > No, this problem is (nearly) unsolvable within this plugin or within Dolphin
>     
>     actually, you just have to save the real dropbox path once (say /mnt/somedisk/Dropbox), and then when querying some path, check if there’s a symlink in the path’s parents to the real dropbox path or one of its parents. (e.g. if you’re in ~/Dropbox/foobar/, you check all path components from ~/Dropbox/foobar/ up, and find that ~/Dropbox is a symlink to /mnt/somedisk/Dropbox). then you append the part of the symlink to the real dropbox path (e.g. pathjoin(/mnt/Dropbox, foobar/)) and you’re done.
>     
>     that just requires to check each path component above the directory you’re checking the status of once with .isSymLink(). neglegible. (of course you only have to check directories, not files)
>     
>     seriously, the plugin is unusable without that functionality once you have /home/ on a SSD and dropbox somewhere else because don’t want it creating heaps of IO, and the functionality isn’t IO intensive or computationally expensive at all.
> 
> Emmanuel Pescosta wrote:
>     This is the ugly hack ;)
>     
>     The idea is great in theory, but it'll introduce some other real-world problems.
>     
>     > you just have to save the real dropbox path once (say /mnt/somedisk/Dropbox)
>     Do you really think that this is a great solution? - I don't think so, because it'll be really error-prone.
>     It would be great if Dropbox has a command to request the Dropbox versioned folder path.
>     
>     > that just requires to check each path component above the directory you’re checking the status of once with .isSymLink().
>     This is ok for FileViewDropboxPlugin::itemVersion (runs in a thread), but I think this is a no-go for FileViewDropboxPlugin::actions (blocks the GUI thread).
>     
>     > seriously, the plugin is unusable without that functionality once you have /home/ on a SSD and dropbox somewhere else
>     Yes I agree with you, but this has to be fixed on the Dropbox side to provide a clean solution for this problem.
>     
>     @Frank:
>     What do you think?
>     Should we add a workaround for this problem in Dolphin?
> 
> Frank Reininghaus wrote:
>     To be honest, I'm not even sure if I understand what the actual problem is (I'm not extremely familiar with the version control code, and even less with the Dropbox plugin). Is it that the Dropbox executable does not provide the expected information if we ask it about a path that contains symbolic links?
>     
>     If that is the case, would using
>     
>     http://doc-snapshot.qt-project.org/4.8/qdir.html#canonicalPath
>     
>     to transform that path to the "canonical" one work?
> 
> Phil Schaf wrote:
>     > Do you really think that this is a great solution? - I don't think so, because it'll be really error-prone.
>     It would be great if Dropbox has a command to request the Dropbox versioned folder path.
>     
>     i just searched, and apparently there is! ~/.dropbox/host.db consists of a number of zeros (not zero bytes, but actual zeros), a linefeed, and the path in base64!
>     
>     cat ~/.dropbox/host.db | tail -n 1 | base64 -d
>     
>     > Is it that the Dropbox executable does not provide the expected information if we ask it about a path that contains symbolic links?
>     
>     if dropbox isn’t asked for a path below its literal root path, it will simply return „not managed by dropbox“, even if somewhere in the path is a symlink pointing to (or below) dropbox’ root path.
>     
>     > If that is the case, would using canonicalPath to transform that path to the "canonical" one work?
>     
>     i guess it would *in many cases*. but i think the most correct solution would be the following one. we just have to port it to C++ (i used QDir, so the only part needing serious porting is the reading of the file…)
>     
>     https://gist.github.com/flying-sheep/8832222
> 
> Frank Reininghaus wrote:
>     > cat ~/.dropbox/host.db | tail -n 1 | base64 -d
>     
>     Is it documented anywhere by Dropbox that this is a valid approach to get the path, and that it will continue to work? If not, then relying on this is not acceptable IMHO. If Dropbox decided to change the structure of the host.db file (which they can do at any time - since their executable is proprietary and we don't know anything about its internals, it could even be that the version which is installed on users' systems now will do such changes if it is notified by their central server), then everything would stop working immediately.
> 
> Phil Schaf wrote:
>     i decompiled the dropbox source code, and it does the same as me (i.e. ignoring the first line, extracting the second, and base64-decoding the path from the second line)
>     
>     of course there is no guarantee it will continue to work indefinitely, but we could still fall back to another variant if something doesn’t work (file not readable, no valid path in it, …)
>     
>     also it’s been that way since the beginning AFAIK, so low chance it’ll change.

Thanks for your work! 
But I have found an easier solution (see diff r5).

Frank's idea with canonical path does the job.

For example:
Dropbox-Directory: /home/emmanuel/Dropbox
Symbolic-Link: /home/emmanuel/test/Dropbox1234 -> /home/emmanuel/Dropbox

We open the directory: /home/emmanuel/test/Dropbox1234/Images

Dolphin calls FileViewDropboxPlugin::beginRetrieval, which checks if this directory (canonical path) is served by Dropbox.

Canonical Path: /home/emmanuel/test/Dropbox1234/Images -> /home/emmanuel/Dropbox/Images

Can you please test, if it also works for you (different storage devices)? - Thanks!

- Emmanuel

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114812/#review48802
-----------------------------------------------------------

On Feb. 4, 2014, 9:12 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114812/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2014, 9:12 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Bugs: 264717 and 298199
>     http://bugs.kde.org/show_bug.cgi?id=264717
>     http://bugs.kde.org/show_bug.cgi?id=298199
> 
> 
> Repository: dolphin-plugins
> 
> 
> Description
> -------
> 
> Added a Dropbox version control plugin for Dolphin.
> 
> This Dropbox plugin is based on the work of:
> Sergei Stolyarovs - https://bitbucket.org/cancel/dolphin-dropbox-plugin
> Thomas Richards - http://trichard-kde.blogspot.co.at/2010/12/introducing-dropbox-integration-for.html
> 
> What I have done:
> - Ported the old source code to the newer KVersionControlPlugin2 interface
> - Use the Dropbox client to form the context menu more dynamically
>   (If the Dropbox guys add a new feature to their client, Dolphin can make use of it automatically)
> - Fixed a crash (Dolphin-4.8.2 segfaults when a file with special characters is present)
> - Replaced the item version changed timer with a file system watcher -> No useless updates every 10 seconds and immediate update on real changes
> - A lot of code/coding style related changes
> 
> I think that this plugin is small enough to include it into the official Dolphin-plugins collection. ;)
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 4d87420 
>   dropbox/CMakeLists.txt PRE-CREATION 
>   dropbox/fileviewdropboxplugin.cpp PRE-CREATION 
>   dropbox/fileviewdropboxplugin.desktop PRE-CREATION 
>   dropbox/fileviewdropboxplugin.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/114812/diff/
> 
> 
> Testing
> -------
> 
> Works fine for me.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140206/d0b064db/attachment.htm>