Review Request 128618: Fix creating symlink in Folder View

[David Faure]

> On Aug. 6, 2016, 12:13 p.m., David Faure wrote:
> > src/core/copyjob.cpp, line 1531
> > <https://git.reviewboard.kde.org/r/128618/diff/1/?file=473180#file473180line1531>
> >
> >     CopyJob stats the destination already, no need to do it again.
> >     
> >     See line 410, the local path for the dest is even stored already:
> >     m_dest = QUrl::fromLocalFile(sLocalPath);
> >     
> >     So you should be able to just use if (m_dest.isLocalFile()) { ... } here.
> 
> Chinmoy Ranjan Pradhan wrote:
>     In case of symlinks KIO::stat will fail because the destination doesn't exist at the time of stat.
> 
> David Faure wrote:
>     Wrong, it's stating the destination *directory*.
> 
> Chinmoy Ranjan Pradhan wrote:
>     CopyJob will stat the destination directory only if it has been passed as a parameter. But in KNewFileMenu the url of final link that is going to be created is passed to KIO::link(this is also the case with KIO::copyAs as well). That's why the stating fails.
>     
>     (I used bunch of debug statements to confirm this failure)
> 
> David Faure wrote:
>     Then KNewFileMenu should use KIO::linkAs rather than KIO::link.
>     
>     (Not sure this will change what is being stat'ed though, please check)
> 
> Chinmoy Ranjan Pradhan wrote:
>     I dont think using KIO::linkAs will make any difference. In its implementation *asMethod* is set to false. So its as good as KIO::link.
> 
> David Faure wrote:
>     Indeed, that was a copy/paste error. I now added unittests for KIO::link and KIO::linkAs, and fixed KIO::linkAs to work as advertised. So now I can say it for sure: KNewFileMenu should use KIO::linkAs (so that it fails when the user types the name of an existing directory, rather than create the link under that directory).
>     
>     However this indeed doesn't help with the issue at hand, since CopyJob will stat the given dest URL, which doesn't exist yet (that's the whole point, checking if we can create it).
>     
>     OK then for the additional KIO::stat, but it has to be done asynchronously, no exec() inside a job. I.e. move the code under the exec to a separate slot.
> 
> Chinmoy Ranjan Pradhan wrote:
>     Just one quick question, why is it necessary to stat asyncronously and not using a simple function call?

exec() is not "a simple function call", it runs a nested event loop, waiting for the job to terminate, but it means anything else could happen while in that event loop (timers, sockets...), so this creates lots of unexpected problems in complex programs.


- David


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


On Aug. 6, 2016, 11:53 a.m., Chinmoy Ranjan Pradhan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128618/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2016, 11:53 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> KIO::link creates symlink when either protocol+host+port+username+password of the source and the link are same or the link is going to be created locally. In case of plasma's folder view none of the above cases are true therefore creating a symlink in folder view plasmoid gives an error.  
> This patch aims to fix this issue.
> 
> 
> Diffs
> -----
> 
>   src/core/copyjob.cpp c4c6b2c 
> 
> Diff: https://git.reviewboard.kde.org/r/128618/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> error message
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/08/06/d4da6ff3-53d8-49d1-a826-0c8cf12d7aa0__symlink_folderview.png
> 
> 
> Thanks,
> 
> Chinmoy Ranjan Pradhan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160807/b4640947/attachment-0001.html>