Use official Clang/GCC containers #3703
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ execute_process(COMMAND ${ASTYLE_TOOL} --version OUTPUT_VARIABLE ASTYLE_TOOL_VER | |
| string(REGEX MATCH "[0-9]+(\\.[0-9]+)+" ASTYLE_TOOL_VERSION "${ASTYLE_TOOL_VERSION}") | ||
| message(STATUS "🔖 Artistic Style ${ASTYLE_TOOL_VERSION} (${ASTYLE_TOOL})") | ||
|
|
||
| find_program(CLANG_TOOL NAMES clang++-HEAD clang++-16 clang++-15 clang++-14 clang++-13 clang++-12 clang++-11 clang++) | ||
| execute_process(COMMAND ${CLANG_TOOL} --version OUTPUT_VARIABLE CLANG_TOOL_VERSION ERROR_VARIABLE CLANG_TOOL_VERSION) | ||
| string(REGEX MATCH "[0-9]+(\\.[0-9]+)+" CLANG_TOOL_VERSION "${CLANG_TOOL_VERSION}") | ||
| message(STATUS "🔖 Clang ${CLANG_TOOL_VERSION} (${CLANG_TOOL})") | ||
|
|
@@ -30,7 +30,7 @@ execute_process(COMMAND ${CPPCHECK_TOOL} --version OUTPUT_VARIABLE CPPCHECK_TOOL | |
| string(REGEX MATCH "[0-9]+(\\.[0-9]+)+" CPPCHECK_TOOL_VERSION "${CPPCHECK_TOOL_VERSION}") | ||
| message(STATUS "🔖 Cppcheck ${CPPCHECK_TOOL_VERSION} (${CPPCHECK_TOOL})") | ||
|
|
||
| find_program(GCC_TOOL NAMES g++-latest g++-HEAD g++-12 g++-11 g++-10) | ||
| execute_process(COMMAND ${GCC_TOOL} --version OUTPUT_VARIABLE GCC_TOOL_VERSION ERROR_VARIABLE GCC_TOOL_VERSION) | ||
| string(REGEX MATCH "[0-9]+(\\.[0-9]+)+" GCC_TOOL_VERSION "${GCC_TOOL_VERSION}") | ||
| message(STATUS "🔖 GCC ${GCC_TOOL_VERSION} (${GCC_TOOL})") | ||
|
|
@@ -667,8 +667,11 @@ add_custom_target(ci_cppcheck | |
| ############################################################################### | ||
|
|
||
| add_custom_target(ci_cpplint | ||
| COMMAND ${Python3_EXECUTABLE} -mvenv venv_cpplint | ||
|
There was a problem hiding this comment. I'm not super happy about more venvs being hard-coded. In general, I'd like to be able to specify a) don't use a venv at all, or b) create/use a venv at a specific location. (I'm experimenting with this approach here: https://github.com/falbrechtskirchinger/json-ci/blob/split3/Makefile) One more thing for the ever-growing to-do list, I guess. ;-) There was a problem hiding this comment. I don't understand what's bad about this venv. Like this, the target is independent and just needs a Python interpreter - and allows to execute this target independently of any Docker image. There was a problem hiding this comment. I'd just like the option to use or not use a venv, and if I choose to use a venv, specify its location. There's no reason to have a separate venv for every working tree and in the case of cpplint, there's no reason to use any venv on my system at all. venvs are a (small) reason why my json directory (with currently 31 working trees) is over 20GB in size. There was a problem hiding this comment. We could set up the venv in the GitHub Actions job and assume in this target that There was a problem hiding this comment. Most importantly, things should work consistently across Makefiles and CMake. I like the idea of having two environment variables per venv: This works in the Makefile linked above and I'd want to repeat it for Eventually, I envision adding the following to my JSON_MKDOCS_USE_VENV=true
JSON_MKDOCS_VENV="$HOME/projects/json/.share/venvs/mkdocs"
JSON_REUSE_USE_VENV=true
JSON_REUSE_VENV="$HOME/projects/json/.share/venvs/reuse"
JSON_CPPLINT_USE_VENV=false
JSON_TEST_DATA_DIRECTORY="$HOME/projects/json/.share/json_test_data"There was a problem hiding this comment. Why not just hard-code There was a problem hiding this comment.
The whole point is for the venv to be able to sit outside of the git checkout and to be shared by multiple working trees. There was a problem hiding this comment. Oh, sorry - I mistook the directory. |
||
| COMMAND venv_cpplint/bin/pip3 --quiet install cpplint | ||
| COMMAND venv_cpplint/bin/cpplint --filter=-whitespace,-legal,-runtime/references,-runtime/explicit,-runtime/indentation_namespace,-readability/casting,-readability/nolint --quiet --recursive ${SRC_FILES} | ||
| COMMENT "Check code with cpplint" | ||
| WORKING_DIRECTORY ${PROJECT_BINARY_DIR} | ||
| ) | ||
|
|
||
| ############################################################################### | ||
|
|
@@ -929,6 +932,17 @@ foreach(COMPILER g++-4.8 g++-4.9 g++-5 g++-6 g++-7 g++-8 g++-9 g++-10 g++-11 cla | |
| unset(COMPILER_TOOL CACHE) | ||
| endforeach() | ||
|
|
||
| add_custom_target(ci_test_compiler_default | ||
| COMMAND ${CMAKE_COMMAND} | ||
| -DCMAKE_BUILD_TYPE=Debug -GNinja | ||
| -DJSON_BuildTests=ON -DJSON_FastTests=ON | ||
| -S${PROJECT_SOURCE_DIR} -B${PROJECT_BINARY_DIR}/build_compiler_default | ||
| ${ADDITIONAL_FLAGS} | ||
| COMMAND ${CMAKE_COMMAND} --build ${PROJECT_BINARY_DIR}/build_compiler_default --parallel ${N} | ||
| COMMAND cd ${PROJECT_BINARY_DIR}/build_compiler_default && ${CMAKE_CTEST_COMMAND} --parallel ${N} --exclude-regex "test-unicode" -LE git_required --output-on-failure | ||
| COMMENT "Compile and test with default C++ compiler" | ||
| ) | ||
|
|
||
| ############################################################################### | ||
| # CUDA example | ||
| ############################################################################### | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GCC compiler detection needs some love. This doesn't work on Gentoo and I always have to remember to specify
GCC_TOOLmanually.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. I would be able to remove this altogether once we clean up the CI image. I could not use gcc:latest btw. due to incompatible compiler flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compiler flags are also a challenge for the CMake preset file. I was really hoping for a convenient way to use the CI warning flags in local builds. But that might resolve your issue, as I have to sort all flags by compiler version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, the CI targets are meant to work in the context of this project's CI. If they are useful on a different machine, then this is fine, but only a coincidence.