<table><tr><td style="">dfaure accepted this revision.<br />dfaure 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-172325">View Inline</a><span style="color: #4b4d51; font-weight: bold;">arrowd</span> wrote in <span style="color: #4b4d51; font-weight: bold;">ConfigureChecks.cmake:12</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">Has this been tested on a system with sys/extattr.h?</p></blockquote>

<p style="padding: 0; margin: 8px;">I was working on this revision on such system all the time. FreeBSD contains extattr bits in its <tt style="background: #ebebeb; font-size: 13px;">libc</tt>, so no extra libraries are required.</p>

<p style="padding: 0; margin: 8px;">So, what should be done here, then? Just move <tt style="background: #ebebeb; font-size: 13px;">HAVE_SYS_EXTATTR_H</tt> check to <tt style="background: #ebebeb; font-size: 13px;">FindACL.cmake</tt> module?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Ah, I see, OK. No extra lib needed makes it simple.</p>

<p style="padding: 0; margin: 8px;">The FindACL.cmake stuff is a bit of a mess now, with the need for extended attributes outside the ACL related code.<br />
Maybe this could be split up into "find extended attribute stuff" and "find ACL stuff", the latter relying on the former.<br />
But this merge request has been pending for long enough, let's do buildsystem refactoring as part of it.</p>

<p style="padding: 0; margin: 8px;">Let's leave this part as is for now.</p>

<p style="padding: 0; margin: 8px;">If you feel like it, I suggest followup commits to 1) enable the ACL stuff on systems with extattr, see the little bit of code in kpropertiesdialog.cpp, and 2) separate the cmake stuff for ACLs from the cmake stuff for extended attributes.</p></div></div></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>arrowd, dfaure, chinmoyr, bruns, Frameworks, tmarshall, usta, cochise<br /><strong>Cc: </strong>usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh<br /></div>