From 037b83c3094f1d275dfd699b2c62556bcec1be76 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Mon, 19 Aug 2024 16:22:55 -0500 Subject: [PATCH 01/24] Link against libz and pthread privately Fixes #235 --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 679eb6c3..06615e3a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,10 +49,10 @@ target_include_directories(correctionlib ) target_compile_features(correctionlib PUBLIC cxx_std_17) if(ZLIB_FOUND) - target_link_libraries(correctionlib ZLIB::ZLIB) + target_link_libraries(correctionlib PRIVATE ZLIB::ZLIB) endif() if(Threads_FOUND AND CMAKE_SYSTEM_NAME STREQUAL "Linux") - target_link_libraries(correctionlib Threads::Threads) + target_link_libraries(correctionlib PRIVATE Threads::Threads) endif() install(TARGETS correctionlib EXPORT correctionlib-targets From 6699018ff5b1b56e91e1d3382c9e1ea627e69f15 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 16:00:50 -0500 Subject: [PATCH 02/24] Test compilation of cpp source with CMake --- tests/test_binding.py | 61 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 6 deletions(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index 0c290709..6596de8d 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -1,14 +1,16 @@ +import os +import shutil +import subprocess +import tempfile + import pytest import correctionlib import correctionlib.schemav2 as cs -def test_pyroot_binding(): - ROOT = pytest.importorskip("ROOT") - correctionlib.register_pyroot_binding() - assert ROOT.correction.CorrectionSet - +@pytest.fixture(scope="module") +def csetstr(): ptweight = cs.Correction( name="ptweight", version=1, @@ -27,10 +29,57 @@ def test_pyroot_binding(): ), ) cset = cs.CorrectionSet(schema_version=2, corrections=[ptweight]) - csetstr = cset.model_dump_json().replace('"', r"\"") + return cset.model_dump_json().replace('"', r"\"") + + +def test_pyroot_binding(csetstr: str): + ROOT = pytest.importorskip("ROOT") + correctionlib.register_pyroot_binding() + assert ROOT.correction.CorrectionSet ROOT.gInterpreter.Declare( f'auto cset = correction::CorrectionSet::from_string("{csetstr}");' # noqa: B907 ) ROOT.gInterpreter.Declare('auto corr = cset->at("ptweight");') assert ROOT.corr.evaluate([1.2]) == 1.1 + + +CMAKELIST_SRC = """\ +cmake_minimum_required(VERSION 3.16 FATAL_ERROR) +project(test) +find_package(correctionlib) +add_executable(test test.cc) +target_link_libraries(test correctionlib) +""" + +TESTPROG_SRC = """\ +#include "correction.h" + +using correction::CorrectionSet; + +int main(int argc, char** argv) { + auto cset = CorrectionSet::from_string("%s"); + auto corr = cset->at("ptweight"); + if (corr->evaluate({1.2}) != 1.1) { + return 1; + } + return 0; +} +""" + + +@pytest.mark.skipif(shutil.which("cmake") is None, reason="cmake not found") +def test_cmake_static_compilation(csetstr: str): + with tempfile.TemporaryDirectory() as tmpdir: + cmake = os.path.join(tmpdir, "CMakeLists.txt") + with open(cmake, "w") as f: + f.write(CMAKELIST_SRC) + testprog = os.path.join(tmpdir, "test.cc") + with open(testprog, "w") as f: + f.write(TESTPROG_SRC % csetstr) + flags = subprocess.check_output(["correction", "config", "--cmake"]).split() + subprocess.run( + ["cmake", "."] + flags, check=True, capture_output=True, cwd=tmpdir + ) + subprocess.run(["make"], check=True, capture_output=True, cwd=tmpdir) + subprocess.run(["./test"], check=True, cwd=tmpdir) From 7cd41c0c9b6bbf669dc18616157ca07296bd6a4d Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 16:07:53 -0500 Subject: [PATCH 03/24] Nice error message --- tests/test_binding.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index 6596de8d..f2192241 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -78,8 +78,10 @@ def test_cmake_static_compilation(csetstr: str): with open(testprog, "w") as f: f.write(TESTPROG_SRC % csetstr) flags = subprocess.check_output(["correction", "config", "--cmake"]).split() - subprocess.run( - ["cmake", "."] + flags, check=True, capture_output=True, cwd=tmpdir - ) + ret = subprocess.run(["cmake", "."] + flags, capture_output=True, cwd=tmpdir) + if ret.returncode != 0: + print(ret.stdout) + print(ret.stderr) + raise RuntimeError("cmake failed (args: {ret.args})") subprocess.run(["make"], check=True, capture_output=True, cwd=tmpdir) subprocess.run(["./test"], check=True, cwd=tmpdir) From 065188ef7977655e4924eaa4283596be6a64eb14 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 16:25:43 -0500 Subject: [PATCH 04/24] Try private link --- tests/test_binding.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index f2192241..b34bcecc 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -49,7 +49,7 @@ def test_pyroot_binding(csetstr: str): project(test) find_package(correctionlib) add_executable(test test.cc) -target_link_libraries(test correctionlib) +target_link_libraries(test PRIVATE correctionlib) """ TESTPROG_SRC = """\ @@ -80,8 +80,8 @@ def test_cmake_static_compilation(csetstr: str): flags = subprocess.check_output(["correction", "config", "--cmake"]).split() ret = subprocess.run(["cmake", "."] + flags, capture_output=True, cwd=tmpdir) if ret.returncode != 0: - print(ret.stdout) - print(ret.stderr) + print(ret.stdout.decode()) + print(ret.stderr.decode()) raise RuntimeError("cmake failed (args: {ret.args})") subprocess.run(["make"], check=True, capture_output=True, cwd=tmpdir) subprocess.run(["./test"], check=True, cwd=tmpdir) From ac02f005a513d7261103f612aac25c073e8a9eb0 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 16:54:13 -0500 Subject: [PATCH 05/24] Try to get windows working --- CMakeLists.txt | 4 ++++ tests/test_binding.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 06615e3a..178801ec 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,8 @@ cmake_minimum_required(VERSION 3.11) +if(NOT CORRECTIONLIB_VERSION) + set(CORRECTIONLIB_VERSION "0.0.0") # overriden by setup.py +endif() string(REPLACE "." ";" VERSION_SPLIT ${CORRECTIONLIB_VERSION}) list(GET VERSION_SPLIT 0 SPLIT_VERSION_MAJOR) list(GET VERSION_SPLIT 1 SPLIT_VERSION_MINOR) @@ -57,6 +60,7 @@ endif() install(TARGETS correctionlib EXPORT correctionlib-targets LIBRARY DESTINATION ${PKG_INSTALL}/lib + ARCHIVE DESTINATION ${PKG_INSTALL}/lib RUNTIME DESTINATION ${PKG_INSTALL}/lib PUBLIC_HEADER DESTINATION ${PKG_INSTALL}/include ) diff --git a/tests/test_binding.py b/tests/test_binding.py index b34bcecc..a903d684 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -82,6 +82,6 @@ def test_cmake_static_compilation(csetstr: str): if ret.returncode != 0: print(ret.stdout.decode()) print(ret.stderr.decode()) - raise RuntimeError("cmake failed (args: {ret.args})") + raise RuntimeError(f"cmake failed (args: {ret.args})") subprocess.run(["make"], check=True, capture_output=True, cwd=tmpdir) subprocess.run(["./test"], check=True, cwd=tmpdir) From fa068b81f1b21985b35f9ab2d09f76accb835d9c Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 17:05:39 -0500 Subject: [PATCH 06/24] No make on windows :) --- tests/test_binding.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index a903d684..77ed9c0f 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -82,6 +82,10 @@ def test_cmake_static_compilation(csetstr: str): if ret.returncode != 0: print(ret.stdout.decode()) print(ret.stderr.decode()) - raise RuntimeError(f"cmake failed (args: {ret.args})") - subprocess.run(["make"], check=True, capture_output=True, cwd=tmpdir) - subprocess.run(["./test"], check=True, cwd=tmpdir) + raise RuntimeError(f"cmake configuration failed (args: {ret.args})") + ret = subprocess.run(["cmake", "--build", "."], capture_output=True, cwd=tmpdir) + if ret.returncode != 0: + print(ret.stdout.decode()) + print(ret.stderr.decode()) + raise RuntimeError(f"cmake build failed (args: {ret.args})") + subprocess.run([os.path.join(tmpdir, "test")], check=True) From 6f8ebd929b5147b88b8d3bc2ca5d69f43dee0149 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 17:25:17 -0500 Subject: [PATCH 07/24] Program name --- tests/test_binding.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index 77ed9c0f..3aae760c 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -88,4 +88,5 @@ def test_cmake_static_compilation(csetstr: str): print(ret.stdout.decode()) print(ret.stderr.decode()) raise RuntimeError(f"cmake build failed (args: {ret.args})") - subprocess.run([os.path.join(tmpdir, "test")], check=True) + prog = "test.exe" if os.name == "nt" else "test" + subprocess.run([os.path.join(tmpdir, prog)], check=True) From 1d83fff93c8495f44a1628caec45c275bc9c2a93 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 18:03:17 -0500 Subject: [PATCH 08/24] test in ci... --- tests/test_binding.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_binding.py b/tests/test_binding.py index 3aae760c..7ba0d9a4 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -1,3 +1,4 @@ +import glob import os import shutil import subprocess @@ -88,5 +89,7 @@ def test_cmake_static_compilation(csetstr: str): print(ret.stdout.decode()) print(ret.stderr.decode()) raise RuntimeError(f"cmake build failed (args: {ret.args})") + for f in glob.glob(os.path.join(tmpdir, "*")): + print(f) prog = "test.exe" if os.name == "nt" else "test" subprocess.run([os.path.join(tmpdir, prog)], check=True) From 2f36671e077023316aab69b14a8242d2aee449c7 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 18:09:40 -0500 Subject: [PATCH 09/24] test in ci... --- tests/test_binding.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index 7ba0d9a4..92d9d942 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -89,7 +89,7 @@ def test_cmake_static_compilation(csetstr: str): print(ret.stdout.decode()) print(ret.stderr.decode()) raise RuntimeError(f"cmake build failed (args: {ret.args})") - for f in glob.glob(os.path.join(tmpdir, "*")): + for f in glob.glob(os.path.join(tmpdir, r"Debug\*")): print(f) - prog = "test.exe" if os.name == "nt" else "test" + prog = r"Debug\test.exe" if os.name == "nt" else "test" subprocess.run([os.path.join(tmpdir, prog)], check=True) From 7e96f5fcad3a52ecd66e966d82a3b632805b1ff5 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 18:26:46 -0500 Subject: [PATCH 10/24] test in ci... --- tests/test_binding.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index 92d9d942..14a671af 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -1,4 +1,3 @@ -import glob import os import shutil import subprocess @@ -50,7 +49,7 @@ def test_pyroot_binding(csetstr: str): project(test) find_package(correctionlib) add_executable(test test.cc) -target_link_libraries(test PRIVATE correctionlib) +target_link_libraries(test correctionlib) """ TESTPROG_SRC = """\ @@ -78,7 +77,11 @@ def test_cmake_static_compilation(csetstr: str): testprog = os.path.join(tmpdir, "test.cc") with open(testprog, "w") as f: f.write(TESTPROG_SRC % csetstr) - flags = subprocess.check_output(["correction", "config", "--cmake"]).split() + flags = ( + subprocess.check_output(["correction", "config", "--cmake"]) + .decode() + .split() + ) ret = subprocess.run(["cmake", "."] + flags, capture_output=True, cwd=tmpdir) if ret.returncode != 0: print(ret.stdout.decode()) @@ -89,7 +92,5 @@ def test_cmake_static_compilation(csetstr: str): print(ret.stdout.decode()) print(ret.stderr.decode()) raise RuntimeError(f"cmake build failed (args: {ret.args})") - for f in glob.glob(os.path.join(tmpdir, r"Debug\*")): - print(f) prog = r"Debug\test.exe" if os.name == "nt" else "test" subprocess.run([os.path.join(tmpdir, prog)], check=True) From 41f68ea90058b5d728066c660f9e9d5ad6e315ce Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 18:52:51 -0500 Subject: [PATCH 11/24] test in ci... --- tests/test_binding.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index 14a671af..dcb2a930 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -45,11 +45,16 @@ def test_pyroot_binding(csetstr: str): CMAKELIST_SRC = """\ -cmake_minimum_required(VERSION 3.16 FATAL_ERROR) +cmake_minimum_required(VERSION 3.21 FATAL_ERROR) project(test) find_package(correctionlib) add_executable(test test.cc) -target_link_libraries(test correctionlib) +target_link_libraries(test PRIVATE correctionlib) +# Because windows has no RPATH, we need to copy the DLLs to the executable directory +add_custom_command(TARGET test POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy -t $ $ + COMMAND_EXPAND_LISTS +) """ TESTPROG_SRC = """\ From 57a63c7bea3743b232593566a6f9d08784da6fef Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 19:02:38 -0500 Subject: [PATCH 12/24] test in ci... --- tests/test_binding.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index dcb2a930..c70d2e2f 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -97,5 +97,10 @@ def test_cmake_static_compilation(csetstr: str): print(ret.stdout.decode()) print(ret.stderr.decode()) raise RuntimeError(f"cmake build failed (args: {ret.args})") + import glob + + print("\n".join(glob.glob(os.path.join(tmpdir, "*")))) + if os.name == "nt": + print("\n".join(glob.glob(os.path.join(tmpdir, "Debug", "*")))) prog = r"Debug\test.exe" if os.name == "nt" else "test" - subprocess.run([os.path.join(tmpdir, prog)], check=True) + subprocess.run([os.path.join(tmpdir, prog)], check=True, cwd=tmpdir) From 9891bd8cf2474fb8584d20fbcbf27035932a84c2 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 19:48:26 -0500 Subject: [PATCH 13/24] test in ci... --- tests/test_binding.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_binding.py b/tests/test_binding.py index c70d2e2f..ce0436e5 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -102,5 +102,6 @@ def test_cmake_static_compilation(csetstr: str): print("\n".join(glob.glob(os.path.join(tmpdir, "*")))) if os.name == "nt": print("\n".join(glob.glob(os.path.join(tmpdir, "Debug", "*")))) + subprocess.run(["dumpbin", "/dependents", "Debug\\test.exe"], cwd=tmpdir) prog = r"Debug\test.exe" if os.name == "nt" else "test" subprocess.run([os.path.join(tmpdir, prog)], check=True, cwd=tmpdir) From 6146dd2836adbbf08d7a9e561a7f632c1b3413e0 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 19:57:25 -0500 Subject: [PATCH 14/24] test in ci... --- tests/test_binding.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index ce0436e5..1c135334 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -102,6 +102,12 @@ def test_cmake_static_compilation(csetstr: str): print("\n".join(glob.glob(os.path.join(tmpdir, "*")))) if os.name == "nt": print("\n".join(glob.glob(os.path.join(tmpdir, "Debug", "*")))) - subprocess.run(["dumpbin", "/dependents", "Debug\\test.exe"], cwd=tmpdir) + ret = subprocess.run( + ["dumpbin", "/DEPENDENTS", "Debug\\test.exe"], + cwd=tmpdir, + capture_output=True, + check=True, + ) + print(ret.stdout.decode()) prog = r"Debug\test.exe" if os.name == "nt" else "test" subprocess.run([os.path.join(tmpdir, prog)], check=True, cwd=tmpdir) From 3d6e139dde2de29c80dff1ea7c2c541f369787c1 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 20:21:52 -0500 Subject: [PATCH 15/24] test in ci... --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 75fe2a4d..8e9d1ecb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,6 +45,7 @@ jobs: - uses: actions/setup-python@v3 with: python-version: ${{ matrix.python-version }} + - uses: ilammy/msvc-dev-cmd@v1 - name: Install package run: python -m pip install .[test] From b790bdf2df8c6feba1ddbe04e68adde19536345d Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 21:03:13 -0500 Subject: [PATCH 16/24] Strict compilation settings --- CMakeLists.txt | 20 ++++++++++---------- src/formula_ast.cc | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 178801ec..d3a3bc1d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,16 +22,6 @@ set(THREADS_PREFER_PTHREAD_FLAG ON) find_package(Threads) find_package(ZLIB) -include(CheckCXXCompilerFlag) - -check_cxx_compiler_flag(-Wall has_wall) -if(has_wall) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") -endif() -if(MSVC) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Zc:__cplusplus /utf-8") -endif() - configure_file(include/version.h.in ${CMAKE_CURRENT_BINARY_DIR}/include/correctionlib_version.h) install(FILES ${CMAKE_CURRENT_BINARY_DIR}/include/correctionlib_version.h DESTINATION ${PKG_INSTALL}/include) @@ -51,6 +41,11 @@ target_include_directories(correctionlib $ ) target_compile_features(correctionlib PUBLIC cxx_std_17) +if(MSVC) + target_compile_options(correctionlib PRIVATE /W4 /WX) +else() + target_compile_options(correctionlib PRIVATE -Wall -Wextra -Wpedantic -Werror) +endif() if(ZLIB_FOUND) target_link_libraries(correctionlib PRIVATE ZLIB::ZLIB) endif() @@ -67,6 +62,11 @@ install(TARGETS correctionlib pybind11_add_module(_core MODULE src/python.cc) +if(MSVC) + target_compile_options(_core PRIVATE /W4 /WX) +else() + target_compile_options(_core PRIVATE -Wall -Wextra -Wpedantic -Werror) +endif() target_link_libraries(_core PRIVATE correctionlib) set_target_properties(_core PROPERTIES BUILD_WITH_INSTALL_RPATH ON) if (APPLE) diff --git a/src/formula_ast.cc b/src/formula_ast.cc index 77187de7..9e48c11e 100644 --- a/src/formula_ast.cc +++ b/src/formula_ast.cc @@ -23,7 +23,7 @@ namespace { AstPtr peg_ast; int pos; std::string msg; - parser_.log = [&](size_t ln, size_t col, const std::string &themsg) { + parser_.log = [&](size_t, size_t col, const std::string &themsg) { pos = col; msg = themsg; }; From 16f97bd96a1fd9f76fa4c6c4d4f7af497c433684 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 21:06:03 -0500 Subject: [PATCH 17/24] test in ci... --- tests/test_binding.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_binding.py b/tests/test_binding.py index 1c135334..712447c7 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -109,5 +109,10 @@ def test_cmake_static_compilation(csetstr: str): check=True, ) print(ret.stdout.decode()) + subprocess.run( + ["devenv", "/debugexe", os.path.join(tmpdir, r"Debug\test.exe")], + check=True, + cwd=tmpdir, + ) prog = r"Debug\test.exe" if os.name == "nt" else "test" subprocess.run([os.path.join(tmpdir, prog)], check=True, cwd=tmpdir) From 0786537c00af9e1e660703d105f009c50eb72c3b Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 21:16:47 -0500 Subject: [PATCH 18/24] Fix a more pedantic warning --- CMakeLists.txt | 2 +- src/correction.cc | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d3a3bc1d..9b26c72e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,7 +42,7 @@ target_include_directories(correctionlib ) target_compile_features(correctionlib PUBLIC cxx_std_17) if(MSVC) - target_compile_options(correctionlib PRIVATE /W4 /WX) + target_compile_options(correctionlib PRIVATE /W3 /WX /Zc:__cplusplus /utf-8) else() target_compile_options(correctionlib PRIVATE -Wall -Wextra -Wpedantic -Werror) endif() diff --git a/src/correction.cc b/src/correction.cc index 7dbfb19a..5723410c 100644 --- a/src/correction.cc +++ b/src/correction.cc @@ -791,9 +791,11 @@ std::unique_ptr CorrectionSet::from_file(const std::string& fn) { if ( fp == nullptr ) { throw std::runtime_error("Failed to open file: " + fn); } - constexpr unsigned char magicref[4] = {0x1f, 0x8b}; + constexpr unsigned char magicref[2] = {0x1f, 0x8b}; unsigned char magic[2]; - fread(magic, sizeof *magic, 2, fp); + if (fread(magic, sizeof *magic, 2, fp) != 2) { + throw std::runtime_error("Failed to read file magic: " + fn); + } rewind(fp); char readBuffer[65536]; rapidjson::ParseResult ok; From cc695242da3dff8ada1ec4e8c5de2cc4abaa388c Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 21:44:06 -0500 Subject: [PATCH 19/24] Fix more pedantic warnings --- include/version.h.in | 2 +- src/formula_ast.cc | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/version.h.in b/include/version.h.in index 5248c23b..06c892fd 100644 --- a/include/version.h.in +++ b/include/version.h.in @@ -7,6 +7,6 @@ namespace correction { constexpr std::string_view correctionlib_version{"@CORRECTIONLIB_VERSION@"}; constexpr int correctionlib_major_version{@correctionlib_VERSION_MAJOR@}; constexpr int correctionlib_minor_version{@correctionlib_VERSION_MINOR@}; -}; +} #endif // CORRECTIONLIB_VERSION_H diff --git a/src/formula_ast.cc b/src/formula_ast.cc index 9e48c11e..b3d2b24d 100644 --- a/src/formula_ast.cc +++ b/src/formula_ast.cc @@ -241,11 +241,12 @@ double FormulaAst::evaluate(const std::vector& values, const std case UnaryOp::Acosh: return std::acosh(arg); case UnaryOp::Asinh: return std::asinh(arg); case UnaryOp::Atanh: return std::atanh(arg); - } + default: std::abort(); + }; } case NodeType::Binary: { - auto left = children_[0].evaluate(values, params); - auto right = children_[1].evaluate(values, params); + const auto left = children_[0].evaluate(values, params); + const auto right = children_[1].evaluate(values, params); switch (std::get(data_)) { case BinaryOp::Equal: return (left == right) ? 1. : 0.; case BinaryOp::NotEqual: return (left != right) ? 1. : 0.; @@ -261,9 +262,9 @@ double FormulaAst::evaluate(const std::vector& values, const std case BinaryOp::Atan2: return std::atan2(left, right); case BinaryOp::Max: return std::max(left, right); case BinaryOp::Min: return std::min(left, right); + default: std::abort(); }; } - default: - std::abort(); // never reached if the switch/case is exhaustive + default: std::abort(); // never reached if the switch/case is exhaustive } } From 5cb536e6921064b1f4c11713692c541a94f03b00 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 21:46:33 -0500 Subject: [PATCH 20/24] Give up on pedantic windows --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9b26c72e..d0696ea3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,7 +42,7 @@ target_include_directories(correctionlib ) target_compile_features(correctionlib PUBLIC cxx_std_17) if(MSVC) - target_compile_options(correctionlib PRIVATE /W3 /WX /Zc:__cplusplus /utf-8) + target_compile_options(correctionlib PRIVATE /Zc:__cplusplus /utf-8) else() target_compile_options(correctionlib PRIVATE -Wall -Wextra -Wpedantic -Werror) endif() From aece5ee761a00be57baa27685ac9aa2df7ffc014 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 21:56:15 -0500 Subject: [PATCH 21/24] Give up on windows --- tests/test_binding.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/tests/test_binding.py b/tests/test_binding.py index 712447c7..5f41e70d 100644 --- a/tests/test_binding.py +++ b/tests/test_binding.py @@ -74,6 +74,7 @@ def test_pyroot_binding(csetstr: str): @pytest.mark.skipif(shutil.which("cmake") is None, reason="cmake not found") +@pytest.mark.skipif(os.name == "nt", reason="there is a segfault I cannot debug") def test_cmake_static_compilation(csetstr: str): with tempfile.TemporaryDirectory() as tmpdir: cmake = os.path.join(tmpdir, "CMakeLists.txt") @@ -97,22 +98,5 @@ def test_cmake_static_compilation(csetstr: str): print(ret.stdout.decode()) print(ret.stderr.decode()) raise RuntimeError(f"cmake build failed (args: {ret.args})") - import glob - - print("\n".join(glob.glob(os.path.join(tmpdir, "*")))) - if os.name == "nt": - print("\n".join(glob.glob(os.path.join(tmpdir, "Debug", "*")))) - ret = subprocess.run( - ["dumpbin", "/DEPENDENTS", "Debug\\test.exe"], - cwd=tmpdir, - capture_output=True, - check=True, - ) - print(ret.stdout.decode()) - subprocess.run( - ["devenv", "/debugexe", os.path.join(tmpdir, r"Debug\test.exe")], - check=True, - cwd=tmpdir, - ) prog = r"Debug\test.exe" if os.name == "nt" else "test" subprocess.run([os.path.join(tmpdir, prog)], check=True, cwd=tmpdir) From 90c0ff557e5824342d11a2f2882452921e8af471 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 22:17:26 -0500 Subject: [PATCH 22/24] Use macos-13 only for py3.7 --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e9d1ecb..526336cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,12 +31,12 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.7", "3.11", "3.12"] - runs-on: [ubuntu-latest, macos-13, windows-latest] + python-version: ["3.11", "3.12"] + runs-on: [ubuntu-latest, macos-latest, windows-latest] include: - - python-version: "3.12" - runs-on: macos-latest + - python-version: "3.7" + runs-on: [ubuntu-latest, macos-13, windows-latest] steps: - uses: actions/checkout@v4 with: From f6ebae30d1eb0923d3b90e6be62be9c61fba0074 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 22:18:10 -0500 Subject: [PATCH 23/24] Remove debug stuff --- .github/workflows/ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 526336cf..e2a5e8c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,7 +45,6 @@ jobs: - uses: actions/setup-python@v3 with: python-version: ${{ matrix.python-version }} - - uses: ilammy/msvc-dev-cmd@v1 - name: Install package run: python -m pip install .[test] From 06deb652a58c46e4ba39b0c29742c7129634d0dc Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 20 Aug 2024 22:20:07 -0500 Subject: [PATCH 24/24] Use macos-13 only for py3.7 --- .github/workflows/ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e2a5e8c1..2be6c03f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,11 @@ jobs: include: - python-version: "3.7" - runs-on: [ubuntu-latest, macos-13, windows-latest] + runs-on: ubuntu-latest + - python-version: "3.7" + runs-on: macos-13 + - python-version: "3.7" + runs-on: windows-latest steps: - uses: actions/checkout@v4 with: