Skip to content

Commit 8065c3c

Browse files
Merge branch 'master' into SNOW-1524259-dynamic-library
2 parents e95c698 + 2ff27d5 commit 8065c3c

21 files changed

+4533
-104
lines changed

.github/workflows/code-quality.yml

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
name: Code quality
2+
3+
on:
4+
push:
5+
branches:
6+
- master
7+
tags:
8+
- v*
9+
pull_request:
10+
branches:
11+
- '**'
12+
13+
jobs:
14+
check-warnings:
15+
name: Extra-Warnings-Linux
16+
runs-on: ubuntu-latest
17+
strategy:
18+
fail-fast: false
19+
matrix:
20+
build_type: [ 'Release' ]
21+
steps:
22+
- uses: actions/checkout@v4
23+
- uses: actions/setup-python@v5
24+
with:
25+
python-version: '3.13'
26+
architecture: 'x64'
27+
- name: Restore cached deps
28+
id: cache-restore-deps
29+
uses: actions/cache/restore@v4
30+
with:
31+
path: dep-cache
32+
key: ${{ matrix.build_type }}-${{ github.event.pull_request.base.sha }}-Linux-dep-cache
33+
if: github.event_name == 'pull_request'
34+
- name: Build
35+
shell: bash
36+
env:
37+
USE_EXTRA_WARNINGS: "true"
38+
BUILD_TYPE: ${{ matrix.build_type }}
39+
run: ci/build_linux.sh
40+
- name: Restore cached warnings
41+
id: cache-restore-warnings
42+
uses: actions/cache/restore@v4
43+
with:
44+
path: warnings.json
45+
key: ${{ github.event.pull_request.base.sha }}-compile-warnings
46+
if: github.event_name == 'pull_request'
47+
- name: Use cached warnings as a baseline
48+
shell: bash
49+
run: cp warnings.json ./ci/scripts/warnings_baseline.json
50+
if: steps.cache-restore-warnings.outputs.cache-hit == true
51+
- name: Warning report
52+
shell: bash
53+
run: ci/scripts/warning_report.sh
54+
- name: Upload build log
55+
uses: actions/upload-artifact@v4
56+
with:
57+
name: build log
58+
path: build.log
59+
- name: Upload warning report
60+
uses: actions/upload-artifact@v4
61+
with:
62+
name: report
63+
path: report.md
64+
- name: Upload warnings
65+
uses: actions/upload-artifact@v4
66+
with:
67+
name: warnings
68+
path: warnings.json
69+
- name: Cache warnings
70+
id: cache-save-warnings
71+
uses: actions/cache/save@v4
72+
with:
73+
path: warnings.json
74+
key: ${{ github.event.pull_request.base.sha }}-compile-warnings
75+
if: github.ref_name == github.event.repository.default_branch
76+

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,4 @@ venv/
3333
wss-*-agent.config
3434
wss-unified-agent.jar
3535
whitesource/
36+
build.log

CMakeLists.txt

Lines changed: 15 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -4,93 +4,26 @@
44
#
55
cmake_minimum_required(VERSION 3.17)
66
project(snowflakeclient)
7-
8-
if (CLIENT_CODE_COVERAGE) # Only when code coverage is enabled
9-
# set(CMAKE_CXX_OUTPUT_EXTENSION_REPLACE 1)
10-
message("Code coverage is enabled CLIENT_CODE_COVERAGE=" ${CLIENT_CODE_COVERAGE})
11-
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} --coverage -fprofile-arcs -ftest-coverage -O0")
12-
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")
13-
else()
14-
message("Code coverage is disabled CLIENT_CODE_COVERAGE=" ${CLIENT_CODE_COVERAGE})
15-
endif ()
7+
include(cmake/platform.cmake)
8+
include(cmake/flags.cmake)
169

1710
# Enabling tests by Ctest. Don't use INCLUDE(Ctest) as
1811
# we don't need Dart and other tools.
1912
enable_testing()
2013

21-
add_definitions(-DLOG_USE_COLOR)
14+
add_compile_definitions(LOG_USE_COLOR)
2215

2316
option(BUILD_TESTS "True if build tests" on)
2417
option(MOCK "True if mock should be used" off)
2518
set(OPENSSL_VERSION_NUMBER 0x11100000L)
19+
2620
# Developers can uncomment this to enable mock builds on their local VMs
2721
#set(MOCK TRUE)
2822

2923
# Generates compile_commands.json file for clangd to parse.
3024
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
31-
32-
if (MOCK)
33-
set(MOCK_OBJECT_WRAPPER_FLAGS -Wl,--wrap=http_perform)
34-
add_definitions(-DMOCK_ENABLED)
35-
else()
36-
set(MOCK_OBJECT_WRAPPER_FLAGS )
37-
endif ()
38-
39-
if (UNIX AND NOT APPLE)
40-
set(LINUX TRUE)
41-
endif ()
42-
43-
if (LINUX)
44-
set(PLATFORM linux)
45-
message("Platform: Linux")
46-
endif ()
47-
if (APPLE)
48-
set(PLATFORM darwin)
49-
message("Platform: Apple OSX")
50-
endif ()
51-
if ("${CMAKE_VS_PLATFORM_NAME}" STREQUAL "x64")
52-
set(PLATFORM win64)
53-
message("Platform: Windows 64bit")
54-
endif ()
55-
if ("${CMAKE_VS_PLATFORM_NAME}" STREQUAL "Win32")
56-
set(PLATFORM win32)
57-
message("Platform: Windows 32bit")
58-
if ("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
59-
set (WIN32_DEBUG ON)
60-
message("WIN32_DEBUG: ${WIN32_DEBUG}")
61-
endif ()
62-
endif ()
63-
6425
set(CMAKE_VERBOSE_MAKEFILE ON)
65-
if (UNIX)
66-
# Linux and OSX
67-
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}")
68-
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17 -std=gnu++17 -fPIC -Werror -Wno-error=deprecated-declarations ${MOCK_OBJECT_WRAPPER_FLAGS}")
69-
else()
70-
# Windows
71-
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /ZH:SHA_256 /guard:cf /Qspectre /sdl")
72-
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /ZH:SHA_256 /guard:cf /Qspectre /sdl")
73-
if ($ENV{ARROW_FROM_SOURCE})
74-
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++17 /D_SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING")
75-
endif ()
76-
endif ()
77-
if (LINUX)
78-
# Linux. MacOS doesn't require pthread option
79-
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pthread")
80-
endif ()
81-
82-
if (LINUX)
83-
# Profiler for Linux
84-
if (NOT "$ENV{BUILD_WITH_PROFILE_OPTION}" STREQUAL "")
85-
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pg")
86-
endif ()
87-
88-
# Code coverage for Linux
89-
if (NOT "$ENV{BUILD_WITH_GCOV_OPTION}" STREQUAL "")
90-
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fprofile-arcs -ftest-coverage")
91-
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -gp -fprofile-arcs -ftest-coverage")
92-
endif ()
93-
endif ()
26+
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
9427

9528
set(SOURCE_FILES
9629
include/snowflake/basic_types.h
@@ -340,8 +273,6 @@ if (WIN32)
340273
find_library(AWS_C_SDKUTILS_LIB aws-c-sdkutils.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/aws/lib/ REQUIRED NO_DEFAULT_PATH)
341274
find_library(AZURE_STORAGE_LITE_LIB azure-storage-lite.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/azure/lib/ REQUIRED NO_DEFAULT_PATH)
342275
if ($ENV{ARROW_FROM_SOURCE})
343-
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DBOOST_ALL_NO_LIB")
344-
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DBOOST_ALL_NO_LIB")
345276
find_library(BOOST_FILESYSTEM_LIB libboost_filesystem.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/boost/lib/ REQUIRED NO_DEFAULT_PATH)
346277
find_library(BOOST_REGEX_LIB libboost_regex.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/boost/lib/ REQUIRED NO_DEFAULT_PATH)
347278
find_library(BOOST_SYSTEM_LIB libboost_system.lib PATHS deps-build/${PLATFORM}/${VSDIR}/${CMAKE_BUILD_TYPE}/boost/lib/ REQUIRED NO_DEFAULT_PATH)
@@ -459,8 +390,16 @@ if (MOCK)
459390
endif ()
460391

461392
add_library(snowflakeclient STATIC ${SOURCE_FILES} ${SOURCE_FILES_PUT_GET} ${SOURCE_FILES_CPP_WRAPPER})
462-
set_target_properties(snowflakeclient PROPERTIES LINKER_LANGUAGE CXX)
463-
set_property(TARGET snowflakeclient PROPERTY C_STANDARD 99)
393+
target_compile_features(snowflakeclient PUBLIC cxx_std_17)
394+
target_compile_features(snowflakeclient PUBLIC c_std_99)
395+
if (UNIX)
396+
target_compile_definitions(snowflakeclient PUBLIC _LARGEFILE64_SOURCE)
397+
endif ()
398+
399+
if (LINUX)
400+
target_compile_options(snowflakeclient PUBLIC -pthread)
401+
target_link_options(snowflakeclient PUBLIC -pthread)
402+
endif ()
464403
#set (CMAKE_CXX_STANDARD 11)
465404

466405
if (BUILD_SHARED_LIBS)

ci/build_linux.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ docker run \
4444
-e GITHUB_EVENT_NAME \
4545
-e GITHUB_REF \
4646
-e CLIENT_CODE_COVERAGE \
47+
-e USE_EXTRA_WARNINGS \
4748
-w /mnt/host \
4849
"${BUILD_IMAGE_NAME}" \
4950
"/mnt/host/ci/build/build.sh"
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
import re
2+
import enum
3+
from dataclasses import dataclass
4+
from sys import stderr
5+
from typing import Optional, List
6+
import dataclasses
7+
import json
8+
import argparse
9+
10+
"""
11+
Regexes for matching warnings:
12+
/tmp/libsnowflakeclient/include/snowflake/SF_CRTFunctionSafe.h:223:16: warning: unused parameter 'in_sizeOfBuffer' [-Wunused-parameter] <- message
13+
223 | size_t in_sizeOfBuffer, <- snippet
14+
"""
15+
message_re = re.compile(r"""(?P<file_path>[^:]*):(?P<line>\d+):(?:(?P<col>\d+):)?\s+warning:(?P<message>.*)\[(?P<flag>.*)\]""")
16+
17+
class ParserState(enum.Enum):
18+
MESSAGE = "MESSAGE"
19+
SNIPPET = "SNIPPET"
20+
21+
@dataclass
22+
class CompilerWarning:
23+
file_path: str
24+
line: int
25+
column: Optional[int]
26+
message: str
27+
flag: str
28+
snippet: Optional[str]
29+
source: str
30+
def key(self):
31+
return self.file_path, self.message, self.flag
32+
33+
@dataclass
34+
class WarningDiff:
35+
new: List[CompilerWarning]
36+
old: List[CompilerWarning]
37+
38+
"""
39+
Parses warnings from compiler output
40+
"""
41+
def parse_warnings(path: str) -> List[CompilerWarning]:
42+
warnings = []
43+
state = ParserState.MESSAGE
44+
with open(path, "r") as f:
45+
lines = f.readlines()
46+
for line in lines:
47+
if state == ParserState.MESSAGE:
48+
m_match = message_re.match(line)
49+
if not m_match:
50+
continue
51+
52+
col = m_match.group("col")
53+
if col:
54+
col = int(col)
55+
56+
warning = CompilerWarning(
57+
file_path=m_match.group("file_path"),
58+
line=int(m_match.group("line")),
59+
column=col,
60+
message=m_match.group("message"),
61+
flag=m_match.group("flag"),
62+
snippet=None,
63+
source=line
64+
)
65+
66+
warnings.append(warning)
67+
state = ParserState.SNIPPET
68+
continue
69+
70+
if state == ParserState.SNIPPET:
71+
warning.snippet = line
72+
warning.source += line
73+
74+
state = ParserState.MESSAGE
75+
continue
76+
77+
result = []
78+
for w in warnings:
79+
if w not in result:
80+
result.append(w)
81+
return result
82+
83+
def dump_warnings(warnings: List[CompilerWarning]) -> str:
84+
warnings_as_dict = [dataclasses.asdict(w) for w in warnings]
85+
return json.dumps(warnings_as_dict, indent=2)
86+
87+
def load_warnings(warnings_json: str) -> List[CompilerWarning]:
88+
warnings_as_dict = json.loads(warnings_json)
89+
return [CompilerWarning(**w) for w in warnings_as_dict]
90+
91+
def write(path: str, data: str):
92+
with open(path, "w") as f:
93+
f.write(data)
94+
95+
def read(path: str) -> str:
96+
with open(path, "r") as f:
97+
return f.read()
98+
99+
def generate_report(path: str, new_warnings: List[CompilerWarning], old_warnings: List[CompilerWarning]):
100+
with open(path, "w") as f:
101+
diff = {}
102+
for nw in new_warnings:
103+
if nw.key() not in diff:
104+
diff[nw.key()] = WarningDiff(new=[], old=[])
105+
106+
diff[nw.key()].new.append(nw)
107+
108+
for ow in old_warnings:
109+
if ow.key() not in diff:
110+
diff[ow.key()] = WarningDiff(new=[], old=[])
111+
112+
diff[ow.key()].old.append(ow)
113+
114+
failed = False
115+
for d in diff.values():
116+
if len(d.new) > len(d.old):
117+
failed = True
118+
119+
if failed:
120+
f.write("### Failed\n\n")
121+
else:
122+
f.write("### Succeeded\n\n")
123+
124+
for d in diff.values():
125+
balance = len(d.new) - len(d.old)
126+
if balance < 0:
127+
f.write("- Removed {} compiler warnings from {} [{}].\n".format(-balance, d.old[0].file_path, d.old[0].flag))
128+
129+
if balance > 0:
130+
f.write("- Added {} compiler warnings to {} [{}]. Please remove following warnings:\n".format(balance, d.new[0].file_path, d.new[0].flag))
131+
for w in d.new:
132+
f.write("```\n")
133+
f.write(w.source)
134+
f.write("```\n")
135+
136+
parser = argparse.ArgumentParser(
137+
prog='generate_warning_report',
138+
description='Generate compiler warning report',
139+
epilog='Text at the bottom of help'
140+
)
141+
142+
parser.add_argument('--build-log', required=True) # option that takes a value
143+
parser.add_argument('--load-warnings', required=True)
144+
parser.add_argument('--dump-warnings', required=True)
145+
parser.add_argument('--report', required=True)
146+
args = parser.parse_args()
147+
148+
new_warnings = parse_warnings(args.build_log)
149+
old_warnings = load_warnings(read(args.load_warnings))
150+
generate_report(args.report, new_warnings, old_warnings)
151+
write(args.dump_warnings, dump_warnings(new_warnings))
152+

ci/scripts/warning_report.sh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#!/bin/bash
2+
3+
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
4+
5+
if [[ -n "${GITHUB_STEP_SUMMARY}" ]];
6+
then
7+
cat report.md >> "$GITHUB_STEP_SUMMARY"
8+
fi
9+
10+
if [[ "$(head -n 1 report.md)" == "### Failed" ]];
11+
then
12+
exit 1
13+
fi

0 commit comments

Comments
 (0)