[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The patch looks quite nice, though I can't say I know much about how find_package modules are supposed to be written. I won't click accept yet to see if anyone with more cmake knowledge steps in to review. In the mean time, I have two nitty comments.



================
Comment at: cmake/modules/FindLibEdit.cmake:11-14
+#   libedit_FOUND         - true if libedit was found
+#   libedit_INCLUDE_DIRS  - include search path
+#   libedit_LIBRARIES     - libraries to link
+#   libedit_VERSION       - version number
----------------
Should we make the capitalization of these match the file name (LibEdit_FOUND) ?


================
Comment at: cmake/modules/FindLibEdit.cmake:53-54
+    set(libedit_VERSION_STRING "${libedit_major_version}.${libedit_minor_version}")
+    unset(libedit_major_version)
+    unset(libedit_minor_version)
+  endif()
----------------
Do you really need to unset these? I would hope that this file is evaluated in a fresh context, and it won't pollute the callers namespace or anything..


https://reviews.llvm.org/D46726