turbov added inline comments. INLINE COMMENTS
> cgiboudeaux wrote in FindPythonModuleGeneration.cmake:212 > NAMES clang clang-3.8 clang-3.9 > > and remove the lines 234-239 What distribution is this? Why only 3.8 and 3.9?? Why not 4.0 or 5.0??? I've just checked Ubuntu 14.04 (Clang 3.3 to 3.9 available) and 17.10 (up to 5.0 available)... installing `libclang-X.Y` (or `libclang-X.Y-devel` obviously) always bring `libclang.so` library (which is of cause a symlink to a versioned name), but it always here... Also, I've checked CentOS 7 w/ DTS7 <https://www.softwarecollections.org/en/scls/rhscl/devtoolset-7/> (Clang 4.0) -- same here... so the question is "why to search a versioned name?" -- I don't know any distro where `libclang.so` is not available... do you? > cgiboudeaux wrote in FindPythonModuleGeneration.cmake:213 > Replace PATH with HINTS. Why `HINTS` is "better"?? As for me this is not a "hint" -- at this point we know exactly the correct libdir location... it is why I use `PATH`... > FindPythonModuleGeneration.cmake:217 > + else() > + # Ok, try w/o `llvm-config` then... > + set(_LIBCLANG_MAX_MAJOR_VERSION 7) As for me, I would remove whole this part of "brute forcing" available clang version. I can't see how it could work w/o additional hints due a location of `libclang` is not a standard directory (typically `/usr/lib/llvm-X.Y/`) and no `HINTS` given to `find_library`... Also this code is really lack of flexibility: what if I have few versions of `clang`, but want to use only particular one... (which is not a newest one)?? -- Nowadays, this scenario not handled at all... Instead, I propose to be not so aggressive: - If a user has `clang` installed, let's ask the version of it (`clang --version`), then "guess" a library dir - if nothing found, then OK... just skip the generation of SIP files... - If a user has some private installation of Clang (possibly built somewhere in `/home/john/work/tools/clang/...`) and really want to build Python bindings, let's give him an option to provide a libdir via CMake CLI (like `-DGPB_LIBCLANG_PATH=...`) - If a user wants a particular version, he always can set a hint variable to the desired path... REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D8780 To: turbov Cc: cgiboudeaux, shaheed, #frameworks, #build_system
