<table><tr><td style="">tmarshall added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D17816">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D17816#inline-147544">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:1119</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">That sounds racy; the subjob might not be finished by the time the main job is finished, if you just "start and forget".</p>

<p style="padding: 0; margin: 8px;">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).</p>

<p style="padding: 0; margin: 8px;">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)</p>

<p style="padding: 0; margin: 8px;">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?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Thanks to <span class="phabricator-remarkup-mention-unknown">@ahmad78</span> for chatting with me about this in irc.</p>

<p style="padding: 0; margin: 8px;">Everything works for me if I replace the <tt style="background: #ebebeb; font-size: 13px;">exec</tt> call with a <tt style="background: #ebebeb; font-size: 13px;">start</tt> 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.</p>

<p style="padding: 0; margin: 8px;">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.</p>

<p style="padding: 0; margin: 8px;">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.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D17816">https://phabricator.kde.org/D17816</a></div></div><br /><div><strong>To: </strong>tmarshall, dfaure, chinmoyr, bruns, Frameworks, cochise<br /><strong>Cc: </strong>tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, GB_2, michaelh<br /></div>