tmarshall added inline comments.

INLINE COMMENTS

> dfaure wrote in copyjob.cpp:1119
> That sounds racy; the subjob might not be finished by the time the main job 
> is finished, if you just "start and forget".
> 
> I cannot accept this commit with an exec() in here. The easy and modern way 
> to make this async is just a connect and a lambda (which contains the rest of 
> the code here).
> 
> I have to wonder though: do we have a fast way to determine whether we 
> actually need the additional subjob? (to avoid making this slower on systems 
> -- or files -- without xattr)
> 
> Hmm, or a much bigger architectural question: why doesn't kio_file copy the 
> xattr as part of FileProtocol::copy(), as it already does with e.g. 
> permissions and ACLs, instead of doing this in a separate job?

Thanks to @ahmad78 for chatting with me about this in irc.

Everything works for me if I replace the `exec` call with a `start` call. 
Currently the result of the exec call is thrown away as the job itself does the 
error handling and there isn't much to do if the xattrs can't be copied for 
whatever reason.

As for why this is a different job, that was a design decision by the original 
author. It seems like a decent enough way forward to me as it leans on the side 
of modularity. The applications of xattrs are so broad that it's hard to say 
what they will be used for in the future or what the desired functionality 
might be. Having a separate job will allow for more flexibility regarding 
xattrs going forward.

In short, I don't really think it hurts to have a separate job and there is 
certainly the potential that it comes in handy.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17816

To: tmarshall, dfaure, chinmoyr, bruns, #frameworks, cochise
Cc: tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, funkybomber, abika, 
pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh

Reply via email to