arrowd added inline comments.

INLINE COMMENTS

> dfaure wrote in ConfigureChecks.cmake:12
> Ah, I see, OK. No extra lib needed makes it simple.
> 
> The FindACL.cmake stuff is a bit of a mess now, with the need for extended 
> attributes outside the ACL related code.
> Maybe this could be split up into "find extended attribute stuff" and "find 
> ACL stuff", the latter relying on the former.
> But this merge request has been pending for long enough, let's do buildsystem 
> refactoring as part of it.
> 
> Let's leave this part as is for now.
> 
> 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.

> 1. enable the ACL stuff on systems with extattr, see the little bit of code 
> in kpropertiesdialog.cpp

By that you mean that I should edit the CMake module to define `HAVE_POSIX_ACL` 
when extattr headers are found?

Or should I change checks in kpropertiesdialog.cpp from `HAVE_POSIX_ACL` to 
`HAVE_*ATTR_H`?

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

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

Reply via email to