SNOW-1812871: Setup pipeline with warnings

.github/workflows/code-quality.yml

@@ -0,0 +1,76 @@
+name: Code quality
+
+on:
+  push:
+    branches:
+      - master
+    tags:
+      - v*
+  pull_request:
+    branches:
+      - '**'
+
+jobs:
+  check-warnings:
+    name: Extra-Warnings-Linux
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        build_type: [ 'Release' ]
+    steps:
+      - uses: actions/checkout@v1
+      - uses: actions/setup-python@v1
+        with:
+          python-version: '3.7'
+          architecture: 'x64'
+      - name: Restore cached deps
+        id: cache-restore-deps
+        uses: actions/cache/restore@v4
+        with:
+          path: dep-cache
+          key: ${{ matrix.build_type }}-${{ github.event.pull_request.base.sha }}-Linux-dep-cache
+        if: github.event_name == 'pull_request'
+      - name: Build
+        shell: bash
+        env:
+          USE_EXTRA_WARNINGS: "true"
+          BUILD_TYPE: ${{ matrix.build_type }}
+        run: ci/build_linux.sh
+      - name: Restore cached warnings
+        id: cache-restore-warnings
+        uses: actions/cache/restore@v4
+        with:
+          path: warnings.json
+          key: ${{ github.event.pull_request.base.sha }}-compile-warnings
+        if: github.event_name == 'pull_request'
+      - name: Use cached warnings as a baseline
+        shell: bash
+        run: cp warnings.json ./ci/scripts/warnings_baseline.json
+        if: steps.cache-restore-warnings.outputs.cache-hit == true
+      - name: Warning report
+        shell: bash
+        run: ci/scripts/warning_report.sh
+      - name: Upload build log
+        uses: actions/upload-artifact@v4
+        with:
+          name: build log
+          path: build.log
+      - name: Upload warning report
+        uses: actions/upload-artifact@v4
+        with:
+          name: report
+          path: report.md
+      - name: Upload warnings
+        uses: actions/upload-artifact@v4
+        with:
+          name: warnings
+          path: warnings.json
+      - name: Cache warnings
+        id: cache-save-warnings
+        uses: actions/cache/save@v4
+        with:
+          path: warnings.json
+          key: ${{ github.event.pull_request.base.sha }}-compile-warnings
+        if: github.ref_name == github.event.repository.default_branch
+

CMakeLists.txt

@@ -4,93 +4,26 @@
 #
 cmake_minimum_required(VERSION 3.17)
 project(snowflakeclient)
-
-if (CLIENT_CODE_COVERAGE)    # Only when code coverage is enabled
-    #  set(CMAKE_CXX_OUTPUT_EXTENSION_REPLACE 1)
-    message("Code coverage is enabled CLIENT_CODE_COVERAGE=" ${CLIENT_CODE_COVERAGE})
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} --coverage -fprofile-arcs -ftest-coverage -O0")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} --coverage -fprofile-arcs -ftest-coverage -O0 -fno-elide-constructors -fno-inline  -fno-inline-small-functions  -fno-default-inline")
-else()
-    message("Code coverage is disabled CLIENT_CODE_COVERAGE=" ${CLIENT_CODE_COVERAGE})
-endif ()
+include(cmake/platform.cmake)
+include(cmake/flags.cmake)
 
 # Enabling tests by Ctest. Don't use INCLUDE(Ctest) as
 # we don't need Dart and other tools.
 enable_testing()
 
-add_definitions(-DLOG_USE_COLOR)
+add_compile_definitions(LOG_USE_COLOR)
 
 option(BUILD_TESTS "True if build tests" on)
 option(MOCK "True if mock should be used" off)
 set(OPENSSL_VERSION_NUMBER  0x11100000L)
+
 # Developers can uncomment this to enable mock builds on their local VMs
 #set(MOCK TRUE)
 
 # Generates compile_commands.json file for clangd to parse.
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
-
-if (MOCK)
-    set(MOCK_OBJECT_WRAPPER_FLAGS -Wl,--wrap=http_perform)
-    add_definitions(-DMOCK_ENABLED)
-else()
-    set(MOCK_OBJECT_WRAPPER_FLAGS )
-endif ()
-
-if (UNIX AND NOT APPLE)
-    set(LINUX TRUE)
-endif ()
-
-if (LINUX)
-    set(PLATFORM linux)
-    message("Platform: Linux")
-endif ()
-if (APPLE)
-    set(PLATFORM darwin)
-    message("Platform: Apple OSX")
-endif ()
-if ("${CMAKE_VS_PLATFORM_NAME}" STREQUAL "x64")
-    set(PLATFORM win64)
-    message("Platform: Windows 64bit")
-endif ()
-if ("${CMAKE_VS_PLATFORM_NAME}" STREQUAL "Win32")
-    set(PLATFORM win32)
-    message("Platform: Windows 32bit")
-    if ("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
-        set (WIN32_DEBUG ON)
-        message("WIN32_DEBUG: ${WIN32_DEBUG}")
-    endif ()
-endif ()
-
 set(CMAKE_VERBOSE_MAKEFILE ON)
-if (UNIX)
-    # Linux and OSX
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c99 -std=gnu99 -g -fPIC -Werror -Wno-error=deprecated-declarations -D_LARGEFILE64_SOURCE ${MOCK_OBJECT_WRAPPER_FLAGS}")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17 -std=gnu++17 -fPIC -Werror -Wno-error=deprecated-declarations ${MOCK_OBJECT_WRAPPER_FLAGS}")
-else()
-    # Windows
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /ZH:SHA_256 /guard:cf /Qspectre /sdl")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /ZH:SHA_256 /guard:cf /Qspectre /sdl")
-    if ($ENV{ARROW_FROM_SOURCE})
-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++17 /D_SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING")
-    endif ()
-endif ()
-if (LINUX)
-    # Linux. MacOS doesn't require pthread option
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pthread")
-endif ()
-
-if (LINUX)
-    # Profiler for Linux
-    if (NOT "$ENV{BUILD_WITH_PROFILE_OPTION}" STREQUAL "")
-        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pg")
-    endif ()
-
-    # Code coverage for Linux
-    if (NOT "$ENV{BUILD_WITH_GCOV_OPTION}" STREQUAL "")
-        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fprofile-arcs -ftest-coverage")
-        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -gp -fprofile-arcs -ftest-coverage")
-    endif ()
-endif ()
+set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 
 set(SOURCE_FILES
         include/snowflake/basic_types.h
@@ -340,8 +273,6 @@ if (WIN32)
     find_library(AWS_C_SDKUTILS_LIB aws-c-sdkutils.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/aws/lib/ REQUIRED NO_DEFAULT_PATH)
     find_library(AZURE_STORAGE_LITE_LIB azure-storage-lite.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/azure/lib/ REQUIRED NO_DEFAULT_PATH)
 	if ($ENV{ARROW_FROM_SOURCE})
-        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DBOOST_ALL_NO_LIB")
-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DBOOST_ALL_NO_LIB")
         find_library(BOOST_FILESYSTEM_LIB libboost_filesystem.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/boost/lib/ REQUIRED NO_DEFAULT_PATH)
         find_library(BOOST_REGEX_LIB libboost_regex.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/boost/lib/ REQUIRED NO_DEFAULT_PATH)
         find_library(BOOST_SYSTEM_LIB libboost_system.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/boost/lib/ REQUIRED NO_DEFAULT_PATH)
@@ -459,9 +390,16 @@ if (MOCK)
 endif ()
 
 add_library(snowflakeclient STATIC ${SOURCE_FILES} ${SOURCE_FILES_PUT_GET} ${SOURCE_FILES_CPP_WRAPPER})
+target_compile_features(snowflakeclient PUBLIC cxx_std_17)
+target_compile_features(snowflakeclient PUBLIC c_std_99)
+if (UNIX)
+    target_compile_definitions(snowflakeclient PUBLIC _LARGEFILE64_SOURCE)
+endif ()
 
-set_target_properties(snowflakeclient PROPERTIES LINKER_LANGUAGE CXX)
-set_property(TARGET snowflakeclient PROPERTY C_STANDARD 99)
+if (LINUX)
+    target_compile_options(snowflakeclient PUBLIC -pthread)
+    target_link_options(snowflakeclient PUBLIC -pthread)
+endif ()
 #set (CMAKE_CXX_STANDARD 11)
 
 if(LINUX)

ci/build_linux.sh

@@ -44,6 +44,7 @@ docker run \
         -e GITHUB_EVENT_NAME \
         -e GITHUB_REF \
         -e CLIENT_CODE_COVERAGE \
+        -e USE_EXTRA_WARNINGS \
         -w /mnt/host \
         "${BUILD_IMAGE_NAME}" \
         "/mnt/host/ci/build/build.sh"

ci/scripts/generate_warning_report.py

@@ -0,0 +1,152 @@
+import re
+import enum
+from dataclasses import dataclass
+from sys import stderr
+from typing import Optional, List
+import dataclasses
+import json
+import argparse
+
+"""
+Regexes for matching warnings:
+/tmp/libsnowflakeclient/include/snowflake/SF_CRTFunctionSafe.h:223:16: warning: unused parameter 'in_sizeOfBuffer' [-Wunused-parameter]     <- message
+  223 |         size_t in_sizeOfBuffer,                                                                                                     <- snippet
+"""
+message_re = re.compile(r"""(?P<file_path>[^:]*):(?P<line>\d+):(?:(?P<col>\d+):)?\s+warning:(?P<message>.*)\[(?P<flag>.*)\]""")
+
+class ParserState(enum.Enum):
+    MESSAGE = "MESSAGE"
+    SNIPPET = "SNIPPET"
+
+@dataclass
+class CompilerWarning:
+    file_path: str
+    line: int
+    column: Optional[int]
+    message: str
+    flag: str
+    snippet: Optional[str]
+    source: str
+    def key(self):
+        return self.file_path, self.message, self.flag
+
+@dataclass
+class WarningDiff:
+    new: List[CompilerWarning]
+    old: List[CompilerWarning]
+
+"""
+Parses warnings from compiler output
+"""
+def parse_warnings(path: str) -> List[CompilerWarning]:
+    warnings = []
+    state = ParserState.MESSAGE
+    with open(path, "r") as f:
+        lines = f.readlines()
+        for line in lines:
+            if state == ParserState.MESSAGE:
+                m_match = message_re.match(line)
+                if not m_match:
+                    continue
+
+                col = m_match.group("col")
+                if col:
+                    col = int(col)
+
+                warning = CompilerWarning(
+                    file_path=m_match.group("file_path"),
+                    line=int(m_match.group("line")),
+                    column=col,
+                    message=m_match.group("message"),
+                    flag=m_match.group("flag"),
+                    snippet=None,
+                    source=line
+                )
+
+                warnings.append(warning)
+                state = ParserState.SNIPPET
+                continue
+
+            if state == ParserState.SNIPPET:
+                warning.snippet = line
+                warning.source += line
+
+                state = ParserState.MESSAGE
+                continue
+
+    result = []
+    for w in warnings:
+        if w not in result:
+            result.append(w)
+    return result
+
+def dump_warnings(warnings: List[CompilerWarning]) -> str:
+    warnings_as_dict = [dataclasses.asdict(w) for w in warnings]
+    return json.dumps(warnings_as_dict, indent=2)
+
+def load_warnings(warnings_json: str) -> List[CompilerWarning]:
+    warnings_as_dict = json.loads(warnings_json)
+    return [CompilerWarning(**w) for w in warnings_as_dict]
+
+def write(path: str, data: str):
+    with open(path, "w") as f:
+        f.write(data)
+
+def read(path: str) -> str:
+    with open(path, "r") as f:
+        return f.read()
+
+def generate_report(path: str, new_warnings: List[CompilerWarning], old_warnings: List[CompilerWarning]):
+    with open(path, "w") as f:
+        diff = {}
+        for nw in new_warnings:
+            if nw.key() not in diff:
+                diff[nw.key()] = WarningDiff(new=[], old=[])
+
+            diff[nw.key()].new.append(nw)
+
+        for ow in old_warnings:
+            if ow.key() not in diff:
+                diff[ow.key()] = WarningDiff(new=[], old=[])
+
+            diff[ow.key()].old.append(ow)
+
+        failed = False
+        for d in diff.values():
+            if len(d.new) > len(d.old):
+                failed = True
+
+        if failed:
+            f.write("### Failed\n\n")
+        else:
+            f.write("### Succeeded\n\n")
+
+        for d in diff.values():
+            balance = len(d.new) - len(d.old)
+            if balance < 0:
+                f.write("- Removed {} compiler warnings from {} [{}].\n".format(-balance, d.old[0].file_path, d.old[0].flag))
+
+            if balance > 0:
+                f.write("- Added {} compiler warnings to {} [{}]. Please remove following warnings:\n".format(balance, d.new[0].file_path, d.new[0].flag))
+                for w in d.new:
+                    f.write("```\n")
+                    f.write(w.source)
+                    f.write("```\n")
+
+parser = argparse.ArgumentParser(
+    prog='generate_warning_report',
+    description='Generate compiler warning report',
+    epilog='Text at the bottom of help'
+)
+
+parser.add_argument('--build-log', required=True)      # option that takes a value
+parser.add_argument('--load-warnings', required=True)
+parser.add_argument('--dump-warnings', required=True)
+parser.add_argument('--report', required=True)
+args = parser.parse_args()
+
+new_warnings = parse_warnings(args.build_log)
+old_warnings = load_warnings(read(args.load_warnings))
+generate_report(args.report, new_warnings, old_warnings)
+write(args.dump_warnings, dump_warnings(new_warnings))
+

ci/scripts/report.txt

@@ -0,0 +1,3 @@
+Added 1 compiler warnings to /tmp/libsnowflakeclient/lib/client.c [-Wunused-variable]. Please remove following warnings:
+/tmp/libsnowflakeclient/lib/client.c:1896:12: warning: unused variable 'raw_row_result' [-Wunused-variable]
+ 1896 |     cJSON *raw_row_result;

ci/scripts/warning_report.sh

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+python3 ci/scripts/generate_warning_report.py --build-log build.log --load-warnings ci/scripts/warnings_baseline.json --dump-warnings warnings.json --report report.md
+
+if [[ -n "${GITHUB_STEP_SUMMARY}" ]];
+then
+  cat report.md >> "$GITHUB_STEP_SUMMARY"
+fi
+
+if [[ "$(head -n 1 report.md)" == "### Failed" ]];
+then
+  exit 1
+fi