From bd98762cb708fadc14ae484526bba4c9e6d6d52f Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 13:27:27 +0000 Subject: [PATCH 01/19] Add more gated models that work (cf. https://github.com/ggerganov/llama.cpp/pull/10572 ) --- tests/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1277813..f997f71 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -58,6 +58,8 @@ set(MODEL_IDS # "google/gemma-2-2b-it" # "mistralai/Mistral-7B-Instruct-v0.2" # "mistralai/Mixtral-8x7B-Instruct-v0.1" + # "mistralai/Mistral-Large-Instruct-2407" + # "mistralai/Mistral-Large-Instruct-2411" # "mistralai/Mistral-Nemo-Instruct-2407", # "CohereForAI/c4ai-command-r-plus" ) From 0b9ae2a83b81bce43b870fd49515dd95d2174327 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 13:27:43 +0000 Subject: [PATCH 02/19] Set tighter timeout on github workflow (default = 6h) --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5093635..a686f55 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -19,6 +19,7 @@ jobs: matrix: os: [macos-12, macos-13, macos-14, macos-latest, ubuntu-22.04, ubuntu-latest, windows-2019, windows-latest] runs-on: ${{ matrix.os }} + timeout-minutes: 30 steps: - name: Clone From f8f322b77ba2b52ead1364cc4ee4e615fc490741 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 13:54:32 +0000 Subject: [PATCH 03/19] build: disable fuzztest w/ msvc (not supported) --- CMakeLists.txt | 21 ++++++++++++++++++--- tests/CMakeLists.txt | 20 +++++++++++--------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9d18166..ccaff35 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,14 @@ project(minja VERSION 1.0.0 LANGUAGES CXX) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +if (MSVC) + set(MINJA_FUZZTEST_ENABLED_DEFAULT OFF) +else() + set(MINJA_FUZZTEST_ENABLED_DEFAULT ON) +endif() +option(MINJA_FUZZTEST_ENABLED "minja: fuzztests enabled" MINJA_FUZZTEST_ENABLED_DEFAULT) +option(MINJA_FUZZTEST_FUZZING_MODE "minja: run fuzztests (if enabled) in fuzzing mode" OFF) + # Note: tests require C++14 because google/fuzztest depends on a version of gtest that requires it # (and we don't want to use an older version of fuzztest) # Examples are built w/ C++11 to check the compatibility of the library. @@ -22,9 +30,15 @@ include(FetchContent) FetchContent_Declare(json URL https://github.com/nlohmann/json/archive/refs/heads/develop.zip) FetchContent_MakeAvailable(json) -# Fetch google/fuzztest (and indirectly, gtest) -FetchContent_Declare(fuzztest URL https://github.com/google/fuzztest/archive/refs/heads/main.zip) -FetchContent_MakeAvailable(fuzztest) +if (MINJA_FUZZTEST_ENABLED) + # Fetch google/fuzztest (and indirectly, gtest) + FetchContent_Declare(fuzztest URL https://github.com/google/fuzztest/archive/refs/heads/main.zip) + FetchContent_MakeAvailable(fuzztest) +else() + # Fetch gtest + FetchContent_Declare(googletest URL https://github.com/google/googletest/archive/refs/heads/main.zip) + FetchContent_MakeAvailable(googletest) +endif() # Use ccache if installed find_program(CCACHE_PATH ccache) @@ -92,4 +106,5 @@ include_directories( add_subdirectory(examples) enable_testing() +include(GoogleTest) add_subdirectory(tests) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f997f71..413ab55 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -83,13 +83,15 @@ foreach(test_case ${CHAT_TEMPLATE_TEST_CASES}) add_test(NAME ${test_name} COMMAND $ ${test_args}) endforeach() -if (FUZZTEST_FUZZING_MODE) - message(STATUS "Fuzzing mode enabled") - fuzztest_setup_fuzzing_flags() +if (MINJA_FUZZTEST_ENABLED) + if (MINJA_FUZZTEST_FUZZING_MODE) + message(STATUS "Fuzzing mode enabled") + fuzztest_setup_fuzzing_flags() + endif() + add_executable(test-fuzz test-fuzz.cpp) + set_target_properties(test-fuzz PROPERTIES CXX_STANDARD 17) + target_include_directories(test-fuzz PRIVATE ${fuzztest_BINARY_DIR}) + target_link_libraries(test-fuzz PRIVATE nlohmann_json::nlohmann_json) + link_fuzztest(test-fuzz) + gtest_discover_tests(test-fuzz) endif() -add_executable(test-fuzz test-fuzz.cpp) -set_target_properties(test-fuzz PROPERTIES CXX_STANDARD 17) -target_include_directories(test-fuzz PRIVATE ${fuzztest_BINARY_DIR}) -target_link_libraries(test-fuzz PRIVATE nlohmann_json::nlohmann_json) -link_fuzztest(test-fuzz) -gtest_discover_tests(test-fuzz) From c0a92332c30c9c4c4af035b4e000477ac4d8e5a6 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 13:58:21 +0000 Subject: [PATCH 04/19] Add -DMINJA_TEST_GATED_MODELS=1 build option --- CMakeLists.txt | 1 + README.md | 2 +- tests/CMakeLists.txt | 28 ++++++++++++++++------------ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ccaff35..9e8dc4b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,7 @@ project(minja VERSION 1.0.0 LANGUAGES CXX) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +option(MINJA_TEST_GATED_MODELS "minja: test gated models" OFF) if (MSVC) set(MINJA_FUZZTEST_ENABLED_DEFAULT OFF) else() diff --git a/README.md b/README.md index 3f2ceed..1b0c979 100644 --- a/README.md +++ b/README.md @@ -199,7 +199,7 @@ Main limitations (non-exhaustive list): - Which version of GCC / clang did you compile the tests with? On which OS version? - If you intend to contribute a fix: - Please read [CONTRIBUTING](./CONTRIBUTING.md) first. You'd have to sign a CLA, which your employer may need to accept. - - Please test as many gated models as possible + - Please test as many gated models as possible (use `cmake -B build -DMINJA_TEST_GATED_MODELS=1 ...` and edit `MODEL_IDS` in [tests/CMakeLists.txt](./tests/CMakeLists.txt) appropriately if you don't have access to some models) - For bonus points, check the style of your edits with: diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 413ab55..eaae929 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -50,20 +50,24 @@ set(MODEL_IDS "Qwen/Qwen2.5-Math-7B-Instruct" "teknium/OpenHermes-2.5-Mistral-7B" "TheBloke/FusionNet_34Bx2_MoE-AWQ" - - # Gated models: you will need to run `huggingface-cli login` (and be granted access) to download these - # "meta-llama/Llama-3.2-3B-Instruct" - # "meta-llama/Meta-Llama-3.1-8B-Instruct" - # "google/gemma-7b-it" - # "google/gemma-2-2b-it" - # "mistralai/Mistral-7B-Instruct-v0.2" - # "mistralai/Mixtral-8x7B-Instruct-v0.1" - # "mistralai/Mistral-Large-Instruct-2407" - # "mistralai/Mistral-Large-Instruct-2411" - # "mistralai/Mistral-Nemo-Instruct-2407", - # "CohereForAI/c4ai-command-r-plus" ) +# Gated models: you will need to run `huggingface-cli login` (and be granted access) to download these +if (MINJA_TEST_GATED_MODELS) + list(APPEND MODEL_IDS + "meta-llama/Llama-3.2-3B-Instruct" + "meta-llama/Meta-Llama-3.1-8B-Instruct" + "google/gemma-7b-it" + "google/gemma-2-2b-it" + "mistralai/Mistral-7B-Instruct-v0.2" + "mistralai/Mixtral-8x7B-Instruct-v0.1" + "mistralai/Mistral-Large-Instruct-2407" + "mistralai/Mistral-Large-Instruct-2411" + "mistralai/Mistral-Nemo-Instruct-2407" + "CohereForAI/c4ai-command-r-plus" + ) +endif() + # Create one test case for each {template, context} combination file(GLOB CONTEXT_FILES "${CMAKE_SOURCE_DIR}/tests/contexts/*.json") execute_process( From f9b366d7a91ff7a8c24e8a3d061836ce18a4d4e6 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:00:13 +0000 Subject: [PATCH 05/19] build: use ccache to speed up CI (and test debug to slow it down) --- .github/workflows/build.yml | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a686f55..c3ff044 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -17,7 +17,17 @@ jobs: build_and_test: strategy: matrix: - os: [macos-12, macos-13, macos-14, macos-latest, ubuntu-22.04, ubuntu-latest, windows-2019, windows-latest] + os: [ + macos-12, + macos-13, + macos-14, + macos-latest, + ubuntu-22.04, + ubuntu-latest, + windows-2019, + windows-latest, + ] + type: [Release, Debug] runs-on: ${{ matrix.os }} timeout-minutes: 30 @@ -27,11 +37,16 @@ jobs: with: fetch-depth: 0 + - name: ccache + uses: hendrikmuhs/ccache-action@v1.2.11 + with: + key: ${{ matrix.os }}-${{ matrix.type }} + - name: Set up CMake uses: lukka/get-cmake@latest - name: Configure CMake - run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=Release + run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{ matrix.type }} -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache - name: Build run: cmake --build ${{github.workspace}}/build --config Release --parallel From 2141ba172d40e910e47d9ccb66cb209ac4149a99 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:00:39 +0000 Subject: [PATCH 06/19] build: disable parallel make --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c3ff044..e5fc232 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -49,7 +49,7 @@ jobs: run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{ matrix.type }} -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache - name: Build - run: cmake --build ${{github.workspace}}/build --config Release --parallel + run: cmake --build ${{github.workspace}}/build --config Release - name: Test run: ctest --test-dir build/tests --verbose From e04c48890bd8a9a60977bf704a650d8d5d05b9d1 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:30:21 +0000 Subject: [PATCH 07/19] build: fix python path on Windows --- CMakeLists.txt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9e8dc4b..aa1685b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,14 +59,20 @@ find_package(Python COMPONENTS Interpreter REQUIRED) function(setup_python_environment) set(VENV_DIR "${CMAKE_BINARY_DIR}/venv") + if(WIN32) + set(VENV_PYTHON "${VENV_DIR}/Scripts/python.exe") + else() + set(VENV_PYTHON "${VENV_DIR}/bin/python") + endif() + # Create a virtual environment if it doesnt exist yet if(EXISTS ${VENV_DIR}) - set(Python_EXECUTABLE ${VENV_DIR}/bin/python PARENT_SCOPE) + set(Python_EXECUTABLE ${VENV_PYTHON} PARENT_SCOPE) return() endif() execute_process( - COMMAND ${Python_EXECUTABLE} -m venv ${VENV_DIR} + COMMAND ${Python_EXECUTABLE} -m venv ${VENV_DIR} --upgrade-deps RESULT_VARIABLE VENV_RESULT ) if(NOT VENV_RESULT EQUAL 0) From 14af11e191e694f57c397c2bd0134eac88d493c3 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:30:53 +0000 Subject: [PATCH 08/19] build: Enable new cmake behaviour w/ fetched archives timestamps --- CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index aa1685b..ae3814d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,6 +7,8 @@ # SPDX-License-Identifier: MIT cmake_minimum_required(VERSION 3.14) +cmake_policy(SET CMP0135 NEW) # https://cmake.org/cmake/help/latest/policy/CMP0135.html + project(minja VERSION 1.0.0 LANGUAGES CXX) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) From 0fea7b5acb4c35e492921e065e61169c19ea43d8 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:31:14 +0000 Subject: [PATCH 09/19] scripts: pass cmake parameters to scripts/run_tests.sh --- scripts/run_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index eb4354d..36bee3e 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -13,6 +13,6 @@ # set -euo pipefail -cmake -B build && \ +cmake -B build "$@" && \ cmake --build build -j && \ ctest --test-dir build -j --output-on-failure From 20cebcaceb826d44f80924b477daecdb81d6774a Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:32:14 +0000 Subject: [PATCH 10/19] ci: remove macos-12 --- .github/workflows/build.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e5fc232..164d955 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -18,7 +18,6 @@ jobs: strategy: matrix: os: [ - macos-12, macos-13, macos-14, macos-latest, From c3009b126331a6d63944467bc26135199d840bf2 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:41:44 +0000 Subject: [PATCH 11/19] build: rm redundant venv path --- CMakeLists.txt | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ae3814d..f0cfad4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -81,11 +81,6 @@ function(setup_python_environment) message(FATAL_ERROR "Failed to create virtual environment") endif() - if(WIN32) - set(VENV_PYTHON "${VENV_DIR}/Scripts/python.exe") - else() - set(VENV_PYTHON "${VENV_DIR}/bin/python") - endif() execute_process( COMMAND ${VENV_PYTHON} -m pip install -r "${CMAKE_SOURCE_DIR}/requirements.txt" From 3324de3743d514ffdd22a01bc82f9e5a7ba22a3c Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:46:22 +0000 Subject: [PATCH 12/19] ci: smaller matrix for now --- .github/workflows/build.yml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 164d955..c188633 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -18,15 +18,18 @@ jobs: strategy: matrix: os: [ - macos-13, - macos-14, + # macos-13, + # macos-14, macos-latest, - ubuntu-22.04, + # ubuntu-22.04, ubuntu-latest, - windows-2019, + # windows-2019, windows-latest, ] - type: [Release, Debug] + type: [ + Release, + # Debug, + ] runs-on: ${{ matrix.os }} timeout-minutes: 30 From 49e948a1d57ae38c22a3a0d3cd1ced3fc96e73a5 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:53:04 +0000 Subject: [PATCH 13/19] ci: more granular concurrency groups + don't cancel all jobs if one fails --- .github/workflows/build.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c188633..785b0a0 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,12 +10,13 @@ on: paths: ['.github/workflows/build.yml', '**/CMakeLists.txt', '**/*.hpp', '**/*.cpp'] concurrency: - group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }} + group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }}-${{ matrix.os }}-${{ matrix.type }} cancel-in-progress: true jobs: build_and_test: strategy: + fail-fast: false matrix: os: [ # macos-13, From fbd4cba4c6a74eca5c82057c8180d1def3f05a9b Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:53:23 +0000 Subject: [PATCH 14/19] ci: reenable parallelism + add verbose error output --- .github/workflows/build.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 785b0a0..bc06e3d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -52,7 +52,8 @@ jobs: run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{ matrix.type }} -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache - name: Build - run: cmake --build ${{github.workspace}}/build --config Release + run: cmake --build ${{github.workspace}}/build --config Release --parallel - name: Test - run: ctest --test-dir build/tests --verbose + working-directory: ${{github.workspace}}/build + run: ctest --test-dir tests --output-on-failure --verbose From 00dff7fecc2f0440a186c1e70917a4d8138d1b69 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 14:57:57 +0000 Subject: [PATCH 15/19] ci: revert change --- .github/workflows/build.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bc06e3d..005d6a9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -19,17 +19,17 @@ jobs: fail-fast: false matrix: os: [ - # macos-13, - # macos-14, + macos-13, + macos-14, macos-latest, - # ubuntu-22.04, + ubuntu-22.04, ubuntu-latest, - # windows-2019, + windows-2019, windows-latest, ] type: [ Release, - # Debug, + Debug, ] runs-on: ${{ matrix.os }} timeout-minutes: 30 From a95ff02813e59fc7b882ab20f837b637f00364f1 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 15:03:34 +0000 Subject: [PATCH 16/19] ci: revert more --- .github/workflows/build.yml | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 005d6a9..0b45bd8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,8 +10,9 @@ on: paths: ['.github/workflows/build.yml', '**/CMakeLists.txt', '**/*.hpp', '**/*.cpp'] concurrency: - group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }}-${{ matrix.os }}-${{ matrix.type }} - cancel-in-progress: true + group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }} + # group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }}-${{ matrix.os }}-${{ matrix.type }} + # cancel-in-progress: true jobs: build_and_test: @@ -19,17 +20,17 @@ jobs: fail-fast: false matrix: os: [ - macos-13, - macos-14, + # macos-13, + # macos-14, macos-latest, - ubuntu-22.04, + # ubuntu-22.04, ubuntu-latest, - windows-2019, + # windows-2019, windows-latest, ] type: [ Release, - Debug, + # Debug, ] runs-on: ${{ matrix.os }} timeout-minutes: 30 From ad9c6dd6ded07cef185146d19ec7d7d6ff32f91a Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 15:14:16 +0000 Subject: [PATCH 17/19] build: fix python venv order --- CMakeLists.txt | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f0cfad4..f6ec3e2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -68,26 +68,22 @@ function(setup_python_environment) endif() # Create a virtual environment if it doesnt exist yet - if(EXISTS ${VENV_DIR}) - set(Python_EXECUTABLE ${VENV_PYTHON} PARENT_SCOPE) - return() - endif() - - execute_process( - COMMAND ${Python_EXECUTABLE} -m venv ${VENV_DIR} --upgrade-deps - RESULT_VARIABLE VENV_RESULT - ) - if(NOT VENV_RESULT EQUAL 0) - message(FATAL_ERROR "Failed to create virtual environment") - endif() - - - execute_process( - COMMAND ${VENV_PYTHON} -m pip install -r "${CMAKE_SOURCE_DIR}/requirements.txt" - RESULT_VARIABLE PIP_RESULT - ) - if(NOT PIP_RESULT EQUAL 0) - message(FATAL_ERROR "Failed to install Python dependencies") + if(NOT EXISTS ${VENV_DIR}) + execute_process( + COMMAND ${Python_EXECUTABLE} -m venv ${VENV_DIR} --upgrade-deps + RESULT_VARIABLE VENV_RESULT + ) + if(NOT VENV_RESULT EQUAL 0) + message(FATAL_ERROR "Failed to create virtual environment") + endif() + + execute_process( + COMMAND ${VENV_PYTHON} -m pip install -r "${CMAKE_SOURCE_DIR}/requirements.txt" + RESULT_VARIABLE PIP_RESULT + ) + if(NOT PIP_RESULT EQUAL 0) + message(FATAL_ERROR "Failed to install Python dependencies") + endif() endif() set(Python_EXECUTABLE ${VENV_PYTHON} PARENT_SCOPE) From e9096fe1abcdf08ae3ba5624fe393f63f2a68278 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 15:33:32 +0000 Subject: [PATCH 18/19] build: simplify python venv setup --- CMakeLists.txt | 48 ++++++++++++++++-------------------------------- 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f6ec3e2..0b4501f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -57,38 +57,22 @@ if (NOT XCODE AND NOT MSVC AND NOT CMAKE_BUILD_TYPE) endif() # Create a python venv w/ the required dependencies -find_package(Python COMPONENTS Interpreter REQUIRED) -function(setup_python_environment) - set(VENV_DIR "${CMAKE_BINARY_DIR}/venv") - - if(WIN32) - set(VENV_PYTHON "${VENV_DIR}/Scripts/python.exe") - else() - set(VENV_PYTHON "${VENV_DIR}/bin/python") - endif() - - # Create a virtual environment if it doesnt exist yet - if(NOT EXISTS ${VENV_DIR}) - execute_process( - COMMAND ${Python_EXECUTABLE} -m venv ${VENV_DIR} --upgrade-deps - RESULT_VARIABLE VENV_RESULT - ) - if(NOT VENV_RESULT EQUAL 0) - message(FATAL_ERROR "Failed to create virtual environment") - endif() - - execute_process( - COMMAND ${VENV_PYTHON} -m pip install -r "${CMAKE_SOURCE_DIR}/requirements.txt" - RESULT_VARIABLE PIP_RESULT - ) - if(NOT PIP_RESULT EQUAL 0) - message(FATAL_ERROR "Failed to install Python dependencies") - endif() - endif() - - set(Python_EXECUTABLE ${VENV_PYTHON} PARENT_SCOPE) -endfunction() -setup_python_environment() +find_package(Python3 COMPONENTS Interpreter REQUIRED) +set(VENV_DIR "${CMAKE_BINARY_DIR}/venv") +if(WIN32) + set(VENV_PYTHON "${VENV_DIR}/Scripts/python.exe") +else() + set(VENV_PYTHON "${VENV_DIR}/bin/python") +endif() +execute_process( + COMMAND ${Python3_EXECUTABLE} -m venv "${VENV_DIR}" + COMMAND_ERROR_IS_FATAL ANY) +execute_process( + COMMAND ${VENV_PYTHON} -m pip install --upgrade pip + COMMAND ${VENV_PYTHON} -m pip install -r "${CMAKE_SOURCE_DIR}/requirements.txt" + COMMAND_ERROR_IS_FATAL ANY) +set(Python_EXECUTABLE "${VENV_PYTHON}" CACHE FILEPATH "Path to Python executable in venv" FORCE) +message(STATUS "Python executable: ${Python_EXECUTABLE}") find_program(CPPCHECK cppcheck) if(CPPCHECK) From 7eb7f55f2aec42531f73b27c0b4dea20939af815 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 5 Dec 2024 15:37:35 +0000 Subject: [PATCH 19/19] build: skip pip upgrade --- CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0b4501f..9b19d4b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -68,7 +68,6 @@ execute_process( COMMAND ${Python3_EXECUTABLE} -m venv "${VENV_DIR}" COMMAND_ERROR_IS_FATAL ANY) execute_process( - COMMAND ${VENV_PYTHON} -m pip install --upgrade pip COMMAND ${VENV_PYTHON} -m pip install -r "${CMAKE_SOURCE_DIR}/requirements.txt" COMMAND_ERROR_IS_FATAL ANY) set(Python_EXECUTABLE "${VENV_PYTHON}" CACHE FILEPATH "Path to Python executable in venv" FORCE)