Web lists-archives.com

Re: Review Request 128554: Check for xattr during config step, otherwise the build might fail (if xattr.h is missing). Missing xattr should now trigger an error message to prompt the user into installing libattr + development packages.




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

On agosto 2nd, 2016, 3:03 p.m. CEST, Vishesh Handa wrote:

Ship It!

On agosto 2nd, 2016, 10:41 p.m. CEST, Johan Ouwerkerk wrote:

Should I push to master, or to another branch?

On agosto 3rd, 2016, 5:01 p.m. CEST, Johan Ouwerkerk wrote:

Nevermind, there's only the one branch. :)

Late to the party, but why didn't you follow the same pattern as in the other review that you linked (ConfigureChecks), introducing an external check with no license information?


- Luigi


On agosto 3rd, 2016, 11:03 p.m. CEST, Johan Ouwerkerk wrote:

Review request for Baloo.
By Johan Ouwerkerk.

Updated Ago. 3, 2016, 11:03 p.m.

Repository: kfilemetadata

Description

Check for xattr during config step, otherwise the build might fail (if xattr.h is missing). Missing xattr should now trigger an error message to prompt the user into installing libattr + development packages.

CMake logic is based on: https://github.com/rpm-software-management/librepo/blob/master/cmake/Modules/FindXattr.cmake Taking some cues from an older KDE review request that Googling turned up: https://git.reviewboard.kde.org/r/115877/

Note: the rationale for this change is purely to 'document'/warn about the previously hidden dependency on xattr when building from source. Currently this is a hard dependency, compilation simply errors out if xattr headers aren't available.

Testing

Without xattr development headers cmake now complains when building with kdesrc-build. With xattr development headers installed, cmake & compilation steps pass with kdesrc-build.

Diffs

  • CMakeLists.txt (4ec8eebe54fa8220c30930efffd8e76fd5eb0695)
  • cmake/FindXattr.cmake (PRE-CREATION)

View Diff