From cd0a852981162fb77f9fac68f231e3d41a99c993 Mon Sep 17 00:00:00 2001 From: Matthew Rogers Date: Sun, 6 Jun 2021 12:02:52 +0000 Subject: [PATCH 1/3] cmake: add knob to disable vcpkg When building on windows users have the option to use vcpkg to provide the dependencies needed to compile. Previously, this was used only when using the Visual Studio generator which was not ideal because: - Not all users who want to use vcpkg use the Visual Studio generators. - Some versions of Visual Studio 2019 moved away from using the VS 2019 generator by default, making it impossible for Visual Studio to configure the project in the likely event that it couldn't find the dependencies. - Inexperienced users of CMake are very likely to get tripped up by the errors caused by a lack of vcpkg, making the above bullet point both annoying and hard to debug. As such, let's make using vcpkg the default on windows. Users who want to avoid using vcpkg can disable it by passing -DNO_VCPKG=TRUE. Signed-off-by: Matthew Rogers Acked-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/buildsystems/CMakeLists.txt | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index ac3dbc079a..083a3bf31f 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -43,14 +43,23 @@ NOTE: By default CMake uses Makefile as the build tool on Linux and Visual Studi to use another tool say `ninja` add this to the command line when configuring. `-G Ninja` +NOTE: By default CMake will install vcpkg locally to your source tree on configuration, +to avoid this, add `-DNO_VCPKG=TRUE` to the command line when configuring. + ]] cmake_minimum_required(VERSION 3.14) #set the source directory to root of git set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..) -if(WIN32) + +option(USE_VCPKG "Whether or not to use vcpkg for obtaining dependencies. Only applicable to Windows platforms" ON) +if(NOT WIN32) + set(USE_VCPKG OFF CACHE BOOL FORCE) +endif() + +if(USE_VCPKG) set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg") - if(MSVC AND NOT EXISTS ${VCPKG_DIR}) + if(NOT EXISTS ${VCPKG_DIR}) message("Initializing vcpkg and building the Git's dependencies (this will take a while...)") execute_process(COMMAND ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg_install.bat) endif() @@ -174,7 +183,9 @@ endif() find_program(MSGFMT_EXE msgfmt) if(NOT MSGFMT_EXE) - set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe) + if (USE_VCPKG) + set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe) + endif() if(NOT EXISTS ${MSGFMT_EXE}) message(WARNING "Text Translations won't be built") unset(MSGFMT_EXE) @@ -956,7 +967,7 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "X='${EXE_EXTENSION}'\n") file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n") file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "RUNTIME_PREFIX='${RUNTIME_PREFIX}'\n") file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PYTHON='${NO_PYTHON}'\n") -if(WIN32) +if(USE_VCPKG) file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PATH=\"$PATH:$TEST_DIRECTORY/../compat/vcbuild/vcpkg/installed/x64-windows/bin\"\n") endif() From 409047a2b3fabb6a5f3fdbb28d93a5db3a7de28c Mon Sep 17 00:00:00 2001 From: Matthew Rogers Date: Sun, 6 Jun 2021 12:02:53 +0000 Subject: [PATCH 2/3] cmake: create compile_commands.json by default Some users have expressed interest in a more "batteries included" way of building via CMake[1], and a big part of that is providing easier access to tooling external tools. A straightforward way to accomplish this is to make it as simple as possible is to enable the generation of the compile_commands.json file, which is supported by many tools such as: clang-tidy, clang-format, sourcetrail, etc. This does come with a small run-time overhead during the configuration step (~6 seconds on my machine): Time to configure with CMAKE_EXPORT_COMPILE_COMMANDS=TRUE real 1m9.840s user 0m0.031s sys 0m0.031s Time to configure with CMAKE_EXPORT_COMPILE_COMMANDS=FALSE real 1m3.195s user 0m0.015s sys 0m0.015s This seems like a small enough price to pay to make the project more accessible to newer users. Additionally there are other large projects like llvm [2] which has had this enabled by default for >6 years at the time of this writing, and no real negative consequences that I can find with my search-skills. NOTE: That the compile_commands.json is currently produced only when using the Ninja and Makefile generators. See The CMake documentation[3] for more info. 1: https://lore.kernel.org/git/CAOjrSZusMSvs7AS-ZDsV8aQUgsF2ZA754vSDjgFKMRgi_oZAWw@mail.gmail.com/ 2: https://github.com/llvm/llvm-project/commit/2c5712051b31b316a9fc972f692579bd8efa6e67 3: https://cmake.org/cmake/help/latest/variable/CMAKE_EXPORT_COMPILE_COMMANDS.html Signed-off-by: Matthew Rogers Acked-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/buildsystems/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 083a3bf31f..67b722ffd4 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -57,6 +57,10 @@ if(NOT WIN32) set(USE_VCPKG OFF CACHE BOOL FORCE) endif() +if(NOT DEFINED CMAKE_EXPORT_COMPILE_COMMANDS) + set(CMAKE_EXPORT_COMPILE_COMMANDS TRUE) +endif() + if(USE_VCPKG) set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg") if(NOT EXISTS ${VCPKG_DIR}) From ce24797d388ab0fc13605d25239ae08bcb9c6738 Mon Sep 17 00:00:00 2001 From: Matthew Rogers Date: Sun, 6 Jun 2021 12:02:54 +0000 Subject: [PATCH 3/3] cmake: add warning for ignored MSGFMT_EXE It does not make sense to attempt to set MSGFMT_EXE when NO_GETTEXT is configured, as such add a check for NO_GETTEXT before attempting to set it. Suggested-by: Johannes Schindelin Signed-off-by: Matthew Rogers Acked-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/buildsystems/CMakeLists.txt | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 67b722ffd4..675e2ae6fb 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -185,14 +185,18 @@ if(WIN32 AND NOT MSVC)#not required for visual studio builds endif() endif() -find_program(MSGFMT_EXE msgfmt) -if(NOT MSGFMT_EXE) - if (USE_VCPKG) - set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe) - endif() - if(NOT EXISTS ${MSGFMT_EXE}) - message(WARNING "Text Translations won't be built") - unset(MSGFMT_EXE) +if(NO_GETTEXT) + message(STATUS "msgfmt not used under NO_GETTEXT") +else() + find_program(MSGFMT_EXE msgfmt) + if(NOT MSGFMT_EXE) + if(USE_VCPKG) + set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe) + endif() + if(NOT EXISTS ${MSGFMT_EXE}) + message(WARNING "Text Translations won't be built") + unset(MSGFMT_EXE) + endif() endif() endif()