Github user iyerr3 commented on a diff in the pull request:
https://github.com/apache/madlib/pull/306#discussion_r207707569
--- Diff: CMakeLists.txt ---
@@ -124,10 +124,29 @@ else()
set(M4_ARGUMENTS "--prefix-builtins")
endif()
+# -- Local includes
------------------------------------------------------------
+
+list(APPEND CMAKE_MODULE_PATH
+ "${MAD_BUILD_TOOLS}")
+
+# -- Include all parts
---------------------------------------------------------
+
+include(Utils)
+include(LinuxUtils)
+include(OSXUtils)
+include(Options)
+
+# -- Get madlib version info
----------------------------------------------------
# Read and parse Version.yml file
file(READ "${MADLIB_VERSION_YML}" _MADLIB_VERSION_CONTENTS)
string(REGEX REPLACE "^.*version:[ \t]*([^\n]*)\n.*" "\\1"
MADLIB_VERSION_STRING "${_MADLIB_VERSION_CONTENTS}")
-string(REPLACE "-" "_" MADLIB_VERSION_STRING_NO_HYPHEN
"${MADLIB_VERSION_STRING}")
+rh_version(RH_VERSION)
+debian_version(DEB_VERSION)
+if(APPLE OR (NOT (RH_VERSION STREQUAL "RH_VERSION-NOTFOUND")))
--- End diff --
Couple of concerns:
- Can we use the `IS_REDHAT` flag instead of making the string comparison
here?
- This switch makes the `MADLIB_VERSION_STRING_NO_HYPHEN` a misnomer since
it will have hyphen in the case of Debian. Instead do you think it's better to
move this switch to the place where `MADLIB_VERSION_STRING_NO_HYPHEN` is used
and use it only for RPM (which was the intended reason, IIRC)?
---
