cmake: learn to optionally skip linking dashed built-ins
cover cmake: learn to optionally skip linking dashed built-ins
In Git for Windows, we would like to make use of the fact that our CMake-based build can also install the files into their final location. This patch series helps with that.
Dennis Ameling (2):
cmake(install): fix double .exe suffixes
cmake(install): include vcpkg dlls
Johannes Schindelin (2):
cmake: support SKIP_DASHED_BUILT_INS
cmake: add a preparatory work-around to accommodate vcpkg
.github/workflows/main.yml | 5 +++++ contrib/buildsystems/CMakeLists.txt | 26 +++++++++++++++++++------- 2 files changed, 24 insertions(+), 7 deletions(-)
base-commit: 773e25afc41b1b6533fa9ae2cd825d0b4a697fad
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-887%2Fdscho%2Fskip-dashed-built-ins-in-cmake-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-887/dscho/skip-dashed-built-ins-in-cmake-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/887
Signature · 1 line
gitgitgadget
PATCH 1/4 cmake: support SKIP_DASHED_BUILT_INS
From: Johannes Schindelin <[email protected]>
Just like the Makefile-based build learned to skip hard-linking the dashed built-ins in 179227d6e21 (Optionally skip linking/copying the built-ins, 2020-09-21), this patch teaches the CMake-based build the same trick.
Note: In contrast to the Makefile-based process, the built-ins would only be linked during installation, not already when Git is built. Therefore, the CMake-based build that we use in our CI builds _already_ does not link those built-ins (because the files are not installed anywhere, they are used to run the test suite in-place).
--- contrib/buildsystems/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+)
contrib/buildsystems/CMakeLists.txt +4 -0
@@ -685,13 +685,17 @@ endif() parse_makefile_for_executables(git_builtin_extra "BUILT_INS") +option(SKIP_DASHED_BUILT_INS "Skip hardlinking the dashed versions of the built-ins") + #Creating hardlinks +if(NOT SKIP_DASHED_BUILT_INS) foreach(s ${git_SOURCES} ${git_builtin_extra}) string(REPLACE "${CMAKE_SOURCE_DIR}/builtin/" "" s ${s}) string(REPLACE ".c" "" s ${s}) file(APPEND ${CMAKE_BINARY_DIR}/CreateLinks.cmake "file(CREATE_LINK git${EXE_EXTENSION} git-${s}${EXE_EXTENSION})\n") list(APPEND git_links ${CMAKE_BINARY_DIR}/git-${s}${EXE_EXTENSION}) endforeach() +endif() if(CURL_FOUND) set(remote_exes
Signature · 1 line
gitgitgadget
PATCH 2/4 cmake(install): fix double .exe suffixes
In reply to Johannes Schindelin via GitGitGadgetFrom: Dennis Ameling <[email protected]>
By mistake, the .exe extension is appended _twice_ when installing the dashed executables into libexec/git-core/ on Windows (the extension is already appended when adding items to the git_links list in the #Creating hardlinks section).
--- contrib/buildsystems/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
contrib/buildsystems/CMakeLists.txt +2 -2
@@ -832,12 +832,12 @@ install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git-shell${EXE_EXTENS foreach(b ${git_links}) string(REPLACE "${CMAKE_BINARY_DIR}" "" b ${b}) - install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b}${EXE_EXTENSION})") + install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b})") endforeach() foreach(b ${git_http_links}) string(REPLACE "${CMAKE_BINARY_DIR}" "" b ${b}) - install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/libexec/git-core/git-remote-http${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b}${EXE_EXTENSION})") + install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/libexec/git-core/git-remote-http${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b})") endforeach() install(PROGRAMS ${git_shell_scripts} ${git_perl_scripts} ${CMAKE_BINARY_DIR}/git-p4
Signature · 1 line
gitgitgadget
PATCH 3/4 cmake: add a preparatory work-around to accommodate `vcpkg`
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
We are about to add support for installing the .dll files of Git's dependencies (such as libcurl) in the CMake configuration. The vcpkg ecosystem from which we get said dependencies makes that relatively
easy: simply turn on X_VCPKG_APPLOCAL_DEPS_INSTALL.
However, current vcpkg introduces a limitation if one does that:
While it is totally cool with CMake to specify multiple targets within one invocation of `install(TARGETS ...) (at least according to
https://cmake.org/cmake/help/latest/command/install.html#command:install),vcpkg's parser insists on a single target per install(TARGETS ...) invocation.
Well, that's easily accomplished: Let's feed the targets individually to the install(TARGETS ...) function in a foreach() look.
This also has the advantage that we do not have to manually cull off the two entries from the ${PROGRAMS_BUILT} array before scheduling the remainder to be installed into libexec/git-core. Instead, we iterate through the array and decide for each entry where it wants to go.
--- contrib/buildsystems/CMakeLists.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
contrib/buildsystems/CMakeLists.txt +9 -5
@@ -811,15 +811,19 @@ list(TRANSFORM git_shell_scripts PREPEND "${CMAKE_BINARY_DIR}/") list(TRANSFORM git_perl_scripts PREPEND "${CMAKE_BINARY_DIR}/") #install -install(TARGETS git git-shell +foreach(program ${PROGRAMS_BUILT}) +if(${program} STREQUAL git OR ${program} STREQUAL git-shell) +install(TARGETS ${program} RUNTIME DESTINATION bin) +else() +install(TARGETS ${program} + RUNTIME DESTINATION libexec/git-core) +endif() +endforeach() + install(PROGRAMS ${CMAKE_BINARY_DIR}/git-cvsserver DESTINATION bin) -list(REMOVE_ITEM PROGRAMS_BUILT git git-shell) -install(TARGETS ${PROGRAMS_BUILT} - RUNTIME DESTINATION libexec/git-core) - set(bin_links git-receive-pack git-upload-archive git-upload-pack)
Signature · 1 line
gitgitgadget
Re: [PATCH 3/4] cmake: add a preparatory work-around to accommodate `vcpkg`
Quoted 32 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
We are about to add support for installing the
.dllfiles of Git's dependencies (such as libcurl) in the CMake configuration. Thevcpkgecosystem from which we get said dependencies makes that relatively
easy: simply turn onX_VCPKG_APPLOCAL_DEPS_INSTALL.However, current
vcpkgintroduces a limitation if one does that:
While it is totally cool with CMake to specify multiple targets within one invocation of `install(TARGETS ...) (at least according to
https://cmake.org/cmake/help/latest/command/install.html#command:install),vcpkg's parser insists on a single target perinstall(TARGETS ...)invocation.Well, that's easily accomplished: Let's feed the targets individually to the
install(TARGETS ...)function in aforeach()look.This also has the advantage that we do not have to manually cull off the two entries from the
${PROGRAMS_BUILT}array before scheduling the remainder to be installed intolibexec/git-core. Instead, we iterate through the array and decide for each entry where it wants to go.Signed-off-by Johannes Schindelin <[email protected]>--- contrib/buildsystems/CMakeLists.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)contrib/buildsystems/CMakeLists.txt +0
-0
contrib/buildsystems/CMakeLists.txt list(TRANSFORM git_shell_scripts PREPEND "${CMAKE_BINARY_DIR}/") · line 815
Quoted 6 lines
contrib/buildsystems/CMakeLists.txt +2
-1@@ -811,15 +811,19 @@ list(TRANSFORM git_shell_scripts PREPEND "${CMAKE_BINARY_DIR}/") list(TRANSFORM git_perl_scripts PREPEND "${CMAKE_BINARY_DIR}/") #install -install(TARGETS git git-shell +foreach(program ${PROGRAMS_BUILT}) +if(${program} STREQUAL git OR ${program} STREQUAL git-shell)
Please don't use ${} around variable inside if(), and quote the string. CMake has a quirk with the ${} inside if (expanded variable will be treated as a variable if it is defined, or string otherwise). Unquoted string will be seen as a variable if it's defined, string otherwise. IOW, suggested command:
if (program STREQUAL "git" OR program STREQUAL "git-shell")
We also have another problem with quoted arguments could be interpreted as variable or keyword if CMP0054 policy not enabled, too.
I think it's better to have it enabled, but it's not in the scope of this patch.
https://cmake.org/cmake/help/latest/policy/CMP0054.html
Signature · 1 line
Danh
Re: [PATCH 3/4] cmake: add a preparatory work-around to accommodate `vcpkg`
Hi Danh,
Quoted 46 lines · Đoàn Trần Công Danh
Quoted 32 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
We are about to add support for installing the
.dllfiles of Git's dependencies (such as libcurl) in the CMake configuration. Thevcpkgecosystem from which we get said dependencies makes that relatively
easy: simply turn onX_VCPKG_APPLOCAL_DEPS_INSTALL.However, current
vcpkgintroduces a limitation if one does that:
While it is totally cool with CMake to specify multiple targets within one invocation of `install(TARGETS ...) (at least according to
https://cmake.org/cmake/help/latest/command/install.html#command:install),vcpkg's parser insists on a single target perinstall(TARGETS ...)invocation.Well, that's easily accomplished: Let's feed the targets individually to the
install(TARGETS ...)function in aforeach()look.This also has the advantage that we do not have to manually cull off the two entries from the
${PROGRAMS_BUILT}array before scheduling the remainder to be installed intolibexec/git-core. Instead, we iterate through the array and decide for each entry where it wants to go.Signed-off-by Johannes Schindelin <[email protected]>--- contrib/buildsystems/CMakeLists.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)contrib/buildsystems/CMakeLists.txt +0
-0contrib/buildsystems/CMakeLists.txt list(TRANSFORM git_shell_scripts PREPEND "${CMAKE_BINARY_DIR}/") · line 815
Quoted 6 lines
contrib/buildsystems/CMakeLists.txt +2
-1@@ -811,15 +811,19 @@ list(TRANSFORM git_shell_scripts PREPEND "${CMAKE_BINARY_DIR}/") list(TRANSFORM git_perl_scripts PREPEND "${CMAKE_BINARY_DIR}/") #install -install(TARGETS git git-shell +foreach(program ${PROGRAMS_BUILT}) +if(${program} STREQUAL git OR ${program} STREQUAL git-shell)Please don't use
${}around variable insideif(), and quote the string. CMake has a quirk with the${}inside if (expanded variable will be treated as a variable if it is defined, or string otherwise). Unquoted string will be seen as a variable if it's defined, string otherwise. IOW, suggested command:
Quoted 8 lines
if (program STREQUAL "git" OR program STREQUAL "git-shell")
We also have another problem with quoted arguments could be interpreted as variable or keyword if CMP0054 policy not enabled, too.
I think it's better to have it enabled, but it's not in the scope of this patch.
Thank you for this information! I've sent out v2 based on your suggestion.
Thanks,
Dscho
PATCH 4/4 cmake(install): include vcpkg dlls
In reply to Johannes Schindelin via GitGitGadgetFrom: Dennis Ameling <[email protected]>
Our CMake configuration generates not only build definitions, but also install definitions: After building Git using msbuild git.sln, the built artifacts can be installed via msbuild INSTALL.vcxproj.
To specify _where_ the files should be installed, the -DCMAKE_INSTALL_PREFIX=<path> option can be used when running CMake.
However, this process would really only install the files that were just built. On Windows, we need more than that: We also need the .dll files of the dependencies (such as libcurl). The vcpkg ecosystem, which we use to obtain those dependencies, can be asked to install said .dll files really easily, so let's do that.
This requires more than just the built vcpkg artifacts in the CI build definition; We now clone the vcpkg repository so that the relevant CMake scripts are available, in particular the ones related to defining the toolchain.
--- .github/workflows/main.yml | 5 +++++ contrib/buildsystems/CMakeLists.txt | 4 ++++ 2 files changed, 9 insertions(+)
.github/workflows/main.yml +5 -0
@@ -186,6 +186,11 @@ jobs: ## Unzip and remove the artifact unzip artifacts.zip rm artifacts.zip + - name: initialize vcpkg + uses: actions/checkout@v2 + with: + repository: 'microsoft/vcpkg' + path: 'compat/vcbuild/vcpkg' - name: download vcpkg artifacts shell: powershell run: |
contrib/buildsystems/CMakeLists.txt +4 -0
@@ -58,6 +58,10 @@ if(WIN32) # In the vcpkg edition, we need this to be able to link to libcurl set(CURL_NO_CURL_CMAKE ON) + + # Copy the necessary vcpkg DLLs (like iconv) to the install dir + set(X_VCPKG_APPLOCAL_DEPS_INSTALL ON) + set(CMAKE_TOOLCHAIN_FILE ${VCPKG_DIR}/scripts/buildsystems/vcpkg.cmake CACHE STRING "Vcpkg toolchain file") endif() find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin")
Signature · 1 line
gitgitgadget
v2 cover cmake: learn to optionally skip linking dashed built-ins
In reply to Johannes Schindelin via GitGitGadgetIn Git for Windows, we would like to make use of the fact that our CMake-based build can also install the files into their final location. This patch series helps with that.
Changes since v1:
* Use proper string/variable CMake syntax, as pointed out by Danh
Dennis Ameling (2):
cmake(install): fix double .exe suffixes
cmake(install): include vcpkg dlls
Johannes Schindelin (2):
cmake: support SKIP_DASHED_BUILT_INS
cmake: add a preparatory work-around to accommodate vcpkg
.github/workflows/main.yml | 5 +++++ contrib/buildsystems/CMakeLists.txt | 26 +++++++++++++++++++------- 2 files changed, 24 insertions(+), 7 deletions(-)
base-commit: 773e25afc41b1b6533fa9ae2cd825d0b4a697fad
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-887%2Fdscho%2Fskip-dashed-built-ins-in-cmake-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-887/dscho/skip-dashed-built-ins-in-cmake-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/887
Range-diff vs v1:
1: ff7e8121d7a4 = 1: ff7e8121d7a4 cmake: support SKIP_DASHED_BUILT_INS 2: 69856f278645 = 2: 69856f278645 cmake(install): fix double .exe suffixes 3: 543fd0f5d7e5 ! 3: 5d953a21e9bd cmake: add a preparatory work-around to accommodate `vcpkg`
@@ contrib/buildsystems/CMakeLists.txt: list(TRANSFORM git_shell_scripts PREPEND "$
#install
-install(TARGETS git git-shell
+foreach(program ${PROGRAMS_BUILT})
-+if(${program} STREQUAL git OR ${program} STREQUAL git-shell) ++if(program STREQUAL "git" OR program STREQUAL "git-shell")
+install(TARGETS ${program}
RUNTIME DESTINATION bin)
+else()
4: 4b183c7def58 = 4: f020cb517dfc cmake(install): include vcpkg dlls
Signature · 1 line
gitgitgadget
PATCH v2 1/4 cmake: support SKIP_DASHED_BUILT_INS
From: Johannes Schindelin <[email protected]>
Just like the Makefile-based build learned to skip hard-linking the dashed built-ins in 179227d6e21 (Optionally skip linking/copying the built-ins, 2020-09-21), this patch teaches the CMake-based build the same trick.
Note: In contrast to the Makefile-based process, the built-ins would only be linked during installation, not already when Git is built. Therefore, the CMake-based build that we use in our CI builds _already_ does not link those built-ins (because the files are not installed anywhere, they are used to run the test suite in-place).
--- contrib/buildsystems/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+)
contrib/buildsystems/CMakeLists.txt +4 -0
@@ -685,13 +685,17 @@ endif() parse_makefile_for_executables(git_builtin_extra "BUILT_INS") +option(SKIP_DASHED_BUILT_INS "Skip hardlinking the dashed versions of the built-ins") + #Creating hardlinks +if(NOT SKIP_DASHED_BUILT_INS) foreach(s ${git_SOURCES} ${git_builtin_extra}) string(REPLACE "${CMAKE_SOURCE_DIR}/builtin/" "" s ${s}) string(REPLACE ".c" "" s ${s}) file(APPEND ${CMAKE_BINARY_DIR}/CreateLinks.cmake "file(CREATE_LINK git${EXE_EXTENSION} git-${s}${EXE_EXTENSION})\n") list(APPEND git_links ${CMAKE_BINARY_DIR}/git-${s}${EXE_EXTENSION}) endforeach() +endif() if(CURL_FOUND) set(remote_exes
Signature · 1 line
gitgitgadget
PATCH v2 2/4 cmake(install): fix double .exe suffixes
In reply to Johannes Schindelin via GitGitGadgetFrom: Dennis Ameling <[email protected]>
By mistake, the .exe extension is appended _twice_ when installing the dashed executables into libexec/git-core/ on Windows (the extension is already appended when adding items to the git_links list in the #Creating hardlinks section).
--- contrib/buildsystems/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
contrib/buildsystems/CMakeLists.txt +2 -2
@@ -832,12 +832,12 @@ install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git-shell${EXE_EXTENS foreach(b ${git_links}) string(REPLACE "${CMAKE_BINARY_DIR}" "" b ${b}) - install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b}${EXE_EXTENSION})") + install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b})") endforeach() foreach(b ${git_http_links}) string(REPLACE "${CMAKE_BINARY_DIR}" "" b ${b}) - install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/libexec/git-core/git-remote-http${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b}${EXE_EXTENSION})") + install(CODE "file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/libexec/git-core/git-remote-http${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b})") endforeach() install(PROGRAMS ${git_shell_scripts} ${git_perl_scripts} ${CMAKE_BINARY_DIR}/git-p4
Signature · 1 line
gitgitgadget
PATCH v2 3/4 cmake: add a preparatory work-around to accommodate `vcpkg`
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
We are about to add support for installing the .dll files of Git's dependencies (such as libcurl) in the CMake configuration. The vcpkg ecosystem from which we get said dependencies makes that relatively
easy: simply turn on X_VCPKG_APPLOCAL_DEPS_INSTALL.
However, current vcpkg introduces a limitation if one does that:
While it is totally cool with CMake to specify multiple targets within one invocation of `install(TARGETS ...) (at least according to
https://cmake.org/cmake/help/latest/command/install.html#command:install),vcpkg's parser insists on a single target per install(TARGETS ...) invocation.
Well, that's easily accomplished: Let's feed the targets individually to the install(TARGETS ...) function in a foreach() look.
This also has the advantage that we do not have to manually cull off the two entries from the ${PROGRAMS_BUILT} array before scheduling the remainder to be installed into libexec/git-core. Instead, we iterate through the array and decide for each entry where it wants to go.
--- contrib/buildsystems/CMakeLists.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
contrib/buildsystems/CMakeLists.txt +9 -5
@@ -811,15 +811,19 @@ list(TRANSFORM git_shell_scripts PREPEND "${CMAKE_BINARY_DIR}/") list(TRANSFORM git_perl_scripts PREPEND "${CMAKE_BINARY_DIR}/") #install -install(TARGETS git git-shell +foreach(program ${PROGRAMS_BUILT}) +if(program STREQUAL "git" OR program STREQUAL "git-shell") +install(TARGETS ${program} RUNTIME DESTINATION bin) +else() +install(TARGETS ${program} + RUNTIME DESTINATION libexec/git-core) +endif() +endforeach() + install(PROGRAMS ${CMAKE_BINARY_DIR}/git-cvsserver DESTINATION bin) -list(REMOVE_ITEM PROGRAMS_BUILT git git-shell) -install(TARGETS ${PROGRAMS_BUILT} - RUNTIME DESTINATION libexec/git-core) - set(bin_links git-receive-pack git-upload-archive git-upload-pack)
Signature · 1 line
gitgitgadget
PATCH v2 4/4 cmake(install): include vcpkg dlls
In reply to Johannes Schindelin via GitGitGadgetFrom: Dennis Ameling <[email protected]>
Our CMake configuration generates not only build definitions, but also install definitions: After building Git using msbuild git.sln, the built artifacts can be installed via msbuild INSTALL.vcxproj.
To specify _where_ the files should be installed, the -DCMAKE_INSTALL_PREFIX=<path> option can be used when running CMake.
However, this process would really only install the files that were just built. On Windows, we need more than that: We also need the .dll files of the dependencies (such as libcurl). The vcpkg ecosystem, which we use to obtain those dependencies, can be asked to install said .dll files really easily, so let's do that.
This requires more than just the built vcpkg artifacts in the CI build definition; We now clone the vcpkg repository so that the relevant CMake scripts are available, in particular the ones related to defining the toolchain.
--- .github/workflows/main.yml | 5 +++++ contrib/buildsystems/CMakeLists.txt | 4 ++++ 2 files changed, 9 insertions(+)
.github/workflows/main.yml +5 -0
@@ -186,6 +186,11 @@ jobs: ## Unzip and remove the artifact unzip artifacts.zip rm artifacts.zip + - name: initialize vcpkg + uses: actions/checkout@v2 + with: + repository: 'microsoft/vcpkg' + path: 'compat/vcbuild/vcpkg' - name: download vcpkg artifacts shell: powershell run: |
contrib/buildsystems/CMakeLists.txt +4 -0
@@ -58,6 +58,10 @@ if(WIN32) # In the vcpkg edition, we need this to be able to link to libcurl set(CURL_NO_CURL_CMAKE ON) + + # Copy the necessary vcpkg DLLs (like iconv) to the install dir + set(X_VCPKG_APPLOCAL_DEPS_INSTALL ON) + set(CMAKE_TOOLCHAIN_FILE ${VCPKG_DIR}/scripts/buildsystems/vcpkg.cmake CACHE STRING "Vcpkg toolchain file") endif() find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin")
Signature · 1 line
gitgitgadget