THRIFT-4732:refine cmake scripts (#1688)

* add cmake support for clib and remove unused variables
* add targets for code analysis
* add wall to all compilers and add utf-8 options for msvc

8 files changed, 125 insertions, 9 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index f0165f8f4..c77a6010c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -47,6 +47,9 @@ include(DefineInstallationPaths)
 # Based on the options set some platform specifics
 include(DefinePlatformSpecifc)
 
+# Add CMake targets for static code analysis
+include(StaticCodeAnalysis)
+
 # Generate the config.h file
 include(ConfigureChecks)
 
@@ -98,6 +101,9 @@ endif()
 
 if(BUILD_C_GLIB)
     add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/lib/c_glib)
+    if(BUILD_TESTING)
+        add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/test/c_glib)
+    endif()
 endif()
 
 if(BUILD_JAVA)
diff --git a/build/cmake/DefinePlatformSpecifc.cmake b/build/cmake/DefinePlatformSpecifc.cmake
index b87fd6524..c0bb529c3 100644
--- a/build/cmake/DefinePlatformSpecifc.cmake
+++ b/build/cmake/DefinePlatformSpecifc.cmake
@@ -23,6 +23,31 @@
 # For Debug build types, append a "d" to the library names.
 set(CMAKE_DEBUG_POSTFIX "d" CACHE STRING "Set debug library postfix" FORCE)
 
+# basic options
+foreach(lang IN ITEMS C CXX)
+  if(CMAKE_${lang}_COMPILER_ID STREQUAL "Clang")
+    set(CMAKE_${lang}_FLAGS "${CMAKE_${lang}_FLAGS} -Wall")
+    set(CMAKE_${lang}_FLAGS "${CMAKE_${lang}_FLAGS} -ferror-limit=1")
+  elseif(CMAKE_${lang}_COMPILER_ID STREQUAL "GNU")
+    set(CMAKE_${lang}_FLAGS "${CMAKE_${lang}_FLAGS} -Wall -Wextra")
+    set(CMAKE_${lang}_FLAGS "${CMAKE_${lang}_FLAGS} -fmax-errors=1")
+  elseif(CMAKE_${lang}_COMPILER_ID STREQUAL "MSVC")
+    set(CMAKE_${lang}_FLAGS "${CMAKE_${lang}_FLAGS} /MP") # parallel build
+    set(CMAKE_${lang}_FLAGS "${CMAKE_${lang}_FLAGS} /W3") # warning level 3
+    include(CheckCXXCompilerFlag)
+    set(CMAKE_REQUIRED_QUIET ON)
+    check_cxx_compiler_flag("/source-charset:utf-8" res_var)
+    if (res_var)
+      set(CMAKE_${lang}_FLAGS "${CMAKE_${lang}_FLAGS} /source-charset:utf-8")
+    endif()
+    check_cxx_compiler_flag("/execution-charset:utf-8" res_var)
+    if (res_var)
+      set(CMAKE_${lang}_FLAGS "${CMAKE_${lang}_FLAGS} /execution-charset:utf-8")
+    endif()
+    add_definitions("-DUNICODE -D_UNICODE")
+  endif()
+endforeach()
+
 # Visual Studio specific options
 if(MSVC)
     # Allow for shared library builds
@@ -66,11 +91,6 @@ if(MSVC)
 
     # Disable boost auto linking pragmas - cmake includes the right files
     add_definitions("-DBOOST_ALL_NO_LIB")
-
-    add_definitions("/MP") # parallel build
-    add_definitions("/W3") # warning level 3
-
-    add_definitions("-DUNICODE -D_UNICODE")
 elseif(UNIX)
   find_program( MEMORYCHECK_COMMAND valgrind )
   set( MEMORYCHECK_COMMAND_OPTIONS "--gen-suppressions=all --leak-check=full" )
diff --git a/build/cmake/FindClangTools.cmake b/build/cmake/FindClangTools.cmake
new file mode 100644
index 000000000..b72bea765
--- /dev/null
+++ b/build/cmake/FindClangTools.cmake
@@ -0,0 +1,28 @@
+# - Try to find Clang tools
+#
+# The following are set after configuration is done:
+#  clang-tidy_FOUND
+#  ClangTools::clang-tidy
+#  clang-apply-replacements_FOUND
+#  ClangTools::clang-apply-replacements
+#  run-clang-tidy_FOUND
+#  ClangTools::run-clang-tidy
+
+include_guard()
+include(FindPackageHandleStandardArgs)
+
+foreach(program_name IN ITEMS clang-tidy clang-apply-replacements)
+  find_program(${program_name}_BINARY NAMES ${program_name}-devel ${program_name}-8 ${program_name} PATH_SUFFIXES "LLVM/bin")
+  find_package_handle_standard_args(${program_name} DEFAULT_MSG ${program_name}_BINARY)
+  if(${program_name}_FOUND AND NOT TARGET ClangTools::${program_name})
+    add_executable(ClangTools::${program_name} IMPORTED)
+    set_property(TARGET ClangTools::${program_name} PROPERTY IMPORTED_LOCATION "${${program_name}_BINARY}")
+  endif()
+endforeach()
+
+find_program(run-clang-tidy_BINARY NAMES run-clang-tidy run-clang-tidy.py PATH_SUFFIXES "LLVM/bin" "llvm-devel/share/clang")
+find_package_handle_standard_args(run-clang-tidy DEFAULT_MSG run-clang-tidy_BINARY)
+if(run-clang-tidy_FOUND AND NOT TARGET ClangTools::run-clang-tidy)
+  add_executable(ClangTools::run-clang-tidy IMPORTED)
+  set_property(TARGET ClangTools::run-clang-tidy PROPERTY IMPORTED_LOCATION "${run-clang-tidy_BINARY}")
+endif()
diff --git a/build/cmake/StaticCodeAnalysis.cmake b/build/cmake/StaticCodeAnalysis.cmake
new file mode 100644
index 000000000..3356c761d
--- /dev/null
+++ b/build/cmake/StaticCodeAnalysis.cmake
@@ -0,0 +1,9 @@
+find_package(ClangTools QUIET)
+if(clang-tidy_FOUND AND run-clang-tidy_FOUND AND NOT TARGET do_run_clang_tidy)
+    add_custom_target(
+      do_run_clang_tidy
+      COMMAND ClangTools::run-clang-tidy -clang-tidy-binary "$<TARGET_FILE:ClangTools::clang-tidy>" -p ${CMAKE_BINARY_DIR} "-quiet" > ./run-clang-tidy.txt
+      DEPENDS ${CMAKE_BINARY_DIR}/compile_commands.json
+      WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
+      )
+endif()
diff --git a/lib/c_glib/CMakeLists.txt b/lib/c_glib/CMakeLists.txt
index 3743a68dd..3e4a154b8 100644
--- a/lib/c_glib/CMakeLists.txt
+++ b/lib/c_glib/CMakeLists.txt
@@ -39,6 +39,7 @@ set(thrift_c_glib_SOURCES
     src/thrift/c_glib/protocol/thrift_protocol_decorator.c
     src/thrift/c_glib/protocol/thrift_binary_protocol.c
     src/thrift/c_glib/protocol/thrift_stored_message_protocol.c
+    src/thrift/c_glib/protocol/thrift_multiplexed_protocol.c
     src/thrift/c_glib/protocol/thrift_binary_protocol_factory.c
     src/thrift/c_glib/protocol/thrift_compact_protocol.c
     src/thrift/c_glib/protocol/thrift_compact_protocol_factory.c
@@ -60,7 +61,7 @@ set(thrift_c_glib_SOURCES
 # If OpenSSL is not found just ignore the OpenSSL stuff
 find_package(OpenSSL)
 if(OPENSSL_FOUND AND WITH_OPENSSL)
-    list( APPEND thriftcpp_SOURCES
+    list( APPEND thrift_c_glib_SOURCES
 	    src/thrift/c_glib/transport/thrift_ssl_socket.c
     )
     include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}")
diff --git a/lib/c_glib/src/thrift/c_glib/protocol/thrift_multiplexed_protocol.c b/lib/c_glib/src/thrift/c_glib/protocol/thrift_multiplexed_protocol.c
index 727f4a87c..352e3016c 100644
--- a/lib/c_glib/src/thrift/c_glib/protocol/thrift_multiplexed_protocol.c
+++ b/lib/c_glib/src/thrift/c_glib/protocol/thrift_multiplexed_protocol.c
@@ -49,7 +49,6 @@ thrift_multiplexed_protocol_write_message_begin (ThriftMultiplexedProtocol *prot
   g_return_val_if_fail (THRIFT_IS_MULTIPLEXED_PROTOCOL (protocol), -1);
 
   ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL (protocol);
-  ThriftMultiplexedProtocolClass *multiplexClass = THRIFT_MULTIPLEXED_PROTOCOL_GET_CLASS(self);
 
   if( (message_type == T_CALL || message_type == T_ONEWAY) && self->service_name != NULL) {
     service_name = g_strdup_printf("%s%s%s", self->service_name, THRIFT_MULTIPLEXED_PROTOCOL_DEFAULT_SEPARATOR, name);
diff --git a/lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c b/lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c
index ee5540685..df17fa6ee 100644
--- a/lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c
+++ b/lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c
@@ -369,8 +369,6 @@ gboolean
 thrift_ssl_socket_flush (ThriftTransport *transport, GError **error)
 {
   ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
-  gint ret = 0;
-  guint sent = 0;
 
   ThriftSocket *socket = THRIFT_SOCKET (transport);
   g_return_val_if_fail (socket->sd != THRIFT_INVALID_SOCKET && ssl_socket->ssl!=NULL, FALSE);
diff --git a/test/c_glib/CMakeLists.txt b/test/c_glib/CMakeLists.txt
new file mode 100644
index 000000000..98173155b
--- /dev/null
+++ b/test/c_glib/CMakeLists.txt
@@ -0,0 +1,55 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# Contains the thrift specific LINK_AGAINST_THRIFT_LIBRARY
+include(ThriftMacros)
+
+find_package(GLIB REQUIRED COMPONENTS gobject)
+include_directories(SYSTEM "${GLIB_INCLUDE_DIR}")
+include_directories(SYSTEM "${GLIBCONFIG_INCLUDE_DIR}")
+
+#Make sure gen-c_glib files can be included
+include_directories("${CMAKE_CURRENT_BINARY_DIR}")
+include_directories("${CMAKE_CURRENT_BINARY_DIR}/gen-c_glib")
+include_directories("${PROJECT_SOURCE_DIR}/lib/c_glib/src")
+
+set(crosstestgencglib_SOURCES
+	gen-c_glib/t_test_second_service.c
+	gen-c_glib/t_test_second_service.h
+	gen-c_glib/t_test_thrift_test.c
+	gen-c_glib/t_test_thrift_test.h
+	gen-c_glib/t_test_thrift_test_types.c
+	gen-c_glib/t_test_thrift_test_types.h
+)
+add_library(crosstestgencglib STATIC ${crosstestgencglib_SOURCES})
+LINK_AGAINST_THRIFT_LIBRARY(crosstestgencglib thrift_c_glib)
+
+add_executable(test_server src/test_server.c src/thrift_test_handler.c src/thrift_second_service_handler.c)
+target_link_libraries(test_server crosstestgencglib)
+
+add_executable(test_client src/test_client.c)
+target_link_libraries(test_client crosstestgencglib)
+
+#
+# Common thrift code generation rules
+#
+
+add_custom_command(OUTPUT gen-c_glib/t_test_second_service.c  gen-c_glib/t_test_second_service.h  gen-c_glib/t_test_thrift_test.c  gen-c_glib/t_test_thrift_test.h  gen-c_glib/t_test_thrift_test_types.c  gen-c_glib/t_test_thrift_test_types.h
+    COMMAND ${THRIFT_COMPILER} --gen c_glib -r ${PROJECT_SOURCE_DIR}/test/ThriftTest.thrift
+)