Skip to content

Commit

Permalink
SNOW-1812871: Setup pipeline with warnings (#777)
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-jszczerbinski authored Dec 5, 2024
1 parent bee0e58 commit 2ff27d5
Show file tree
Hide file tree
Showing 13 changed files with 4,412 additions and 84 deletions.
76 changes: 76 additions & 0 deletions .github/workflows/code-quality.yml
Original file line number Diff line number Diff line change
@@ -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@v4
- uses: actions/setup-python@v5
with:
python-version: '3.13'
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

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@ venv/
wss-*-agent.config
wss-unified-agent.jar
whitesource/
build.log
90 changes: 14 additions & 76 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions ci/build_linux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
152 changes: 152 additions & 0 deletions ci/scripts/generate_warning_report.py
Original file line number Diff line number Diff line change
@@ -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))

13 changes: 13 additions & 0 deletions ci/scripts/warning_report.sh
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 2ff27d5

Please sign in to comment.