{"thread":{"id":"55401","subject":"[PATCH 1/4] cmake: support SKIP_DASHED_BUILT_INS","startedAt":"2021-03-27T23:07:05Z","lastAt":"2021-03-29T13:36:57Z","messageCount":12,"participants":["Johannes Schindelin via GitGitGadget","Dennis Ameling via GitGitGadget","Đoàn Trần Công Danh","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"420314","messageId":"ff7e8121d7a47e318954c2c115f281b56f8be21f.1616886386.git.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":"pull.887.git.1616886386.gitgitgadget@gmail.com","subject":"[PATCH 1/4] cmake: support SKIP_DASHED_BUILT_INS","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-27T23:06:22Z","receivedAt":"2021-03-27T23:07:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nJust like the Makefile-based build learned to skip hard-linking the\ndashed built-ins in 179227d6e21 (Optionally skip linking/copying the\nbuilt-ins, 2020-09-21), this patch teaches the CMake-based build the\nsame trick.\n\nNote: In contrast to the Makefile-based process, the built-ins would\nonly be linked during installation, not already when Git is built.\nTherefore, the CMake-based build that we use in our CI builds _already_\ndoes not link those built-ins (because the files are not installed\nanywhere, they are used to run the test suite in-place).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex c151dd7257f3..12c40a72bfff 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -685,13 +685,17 @@ endif()\n \n parse_makefile_for_executables(git_builtin_extra \"BUILT_INS\")\n \n+option(SKIP_DASHED_BUILT_INS \"Skip hardlinking the dashed versions of the built-ins\")\n+\n #Creating hardlinks\n+if(NOT SKIP_DASHED_BUILT_INS)\n foreach(s ${git_SOURCES} ${git_builtin_extra})\n \tstring(REPLACE \"${CMAKE_SOURCE_DIR}/builtin/\" \"\" s ${s})\n \tstring(REPLACE \".c\" \"\" s ${s})\n \tfile(APPEND ${CMAKE_BINARY_DIR}/CreateLinks.cmake \"file(CREATE_LINK git${EXE_EXTENSION} git-${s}${EXE_EXTENSION})\\n\")\n \tlist(APPEND git_links ${CMAKE_BINARY_DIR}/git-${s}${EXE_EXTENSION})\n endforeach()\n+endif()\n \n if(CURL_FOUND)\n \tset(remote_exes\n-- \ngitgitgadget\n\n"},{"id":"420315","messageId":"pull.887.git.1616886386.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":null,"subject":"[PATCH 0/4] cmake: learn to optionally skip linking dashed built-ins","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-27T23:06:21Z","receivedAt":"2021-03-27T23:07:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In Git for Windows, we would like to make use of the fact that our\nCMake-based build can also install the files into their final location. This\npatch series helps with that.\n\nDennis Ameling (2):\n  cmake(install): fix double .exe suffixes\n  cmake(install): include vcpkg dlls\n\nJohannes Schindelin (2):\n  cmake: support SKIP_DASHED_BUILT_INS\n  cmake: add a preparatory work-around to accommodate `vcpkg`\n\n .github/workflows/main.yml          |  5 +++++\n contrib/buildsystems/CMakeLists.txt | 26 +++++++++++++++++++-------\n 2 files changed, 24 insertions(+), 7 deletions(-)\n\n\nbase-commit: 773e25afc41b1b6533fa9ae2cd825d0b4a697fad\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-887%2Fdscho%2Fskip-dashed-built-ins-in-cmake-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-887/dscho/skip-dashed-built-ins-in-cmake-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/887\n-- \ngitgitgadget\n"},{"id":"420316","messageId":"69856f278645ebe1a9779aaa0a6894db33e22b3a.1616886386.git.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":"pull.887.git.1616886386.gitgitgadget@gmail.com","subject":"[PATCH 2/4] cmake(install): fix double .exe suffixes","fromName":"Dennis Ameling via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-27T23:06:23Z","receivedAt":"2021-03-27T23:07:05Z","isPatch":true,"sender":{"key":"name:Dennis Ameling","avatar":null},"body":"From: Dennis Ameling <dennis@dennisameling.com>\n\nBy mistake, the `.exe` extension is appended _twice_ when installing the\ndashed executables into `libexec/git-core/` on Windows (the extension is\nalready appended when adding items to the `git_links` list in the\n`#Creating hardlinks` section).\n\nSigned-off-by: Dennis Ameling <dennis@dennisameling.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 12c40a72bfff..da2811ae3aad 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -832,12 +832,12 @@ install(CODE \"file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git-shell${EXE_EXTENS\n \n foreach(b ${git_links})\n \tstring(REPLACE \"${CMAKE_BINARY_DIR}\" \"\" b ${b})\n-\tinstall(CODE \"file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b}${EXE_EXTENSION})\")\n+\tinstall(CODE \"file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b})\")\n endforeach()\n \n foreach(b ${git_http_links})\n \tstring(REPLACE \"${CMAKE_BINARY_DIR}\" \"\" b ${b})\n-\tinstall(CODE \"file(CREATE_LINK  ${CMAKE_INSTALL_PREFIX}/libexec/git-core/git-remote-http${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b}${EXE_EXTENSION})\")\n+\tinstall(CODE \"file(CREATE_LINK  ${CMAKE_INSTALL_PREFIX}/libexec/git-core/git-remote-http${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b})\")\n endforeach()\n \n install(PROGRAMS ${git_shell_scripts} ${git_perl_scripts} ${CMAKE_BINARY_DIR}/git-p4\n-- \ngitgitgadget\n\n"},{"id":"420317","messageId":"543fd0f5d7e5ee297364d1d28091f2004a35f2d0.1616886386.git.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":"pull.887.git.1616886386.gitgitgadget@gmail.com","subject":"[PATCH 3/4] cmake: add a preparatory work-around to accommodate `vcpkg`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-27T23:06:24Z","receivedAt":"2021-03-27T23:07:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWe are about to add support for installing the `.dll` files of Git's\ndependencies (such as libcurl) in the CMake configuration. The `vcpkg`\necosystem from which we get said dependencies makes that relatively\neasy: simply turn on `X_VCPKG_APPLOCAL_DEPS_INSTALL`.\n\nHowever, current `vcpkg` introduces a limitation if one does that:\nWhile it is totally cool with CMake to specify multiple targets within\none invocation of `install(TARGETS ...) (at least according to\nhttps://cmake.org/cmake/help/latest/command/install.html#command:install),\n`vcpkg`'s parser insists on a single target per `install(TARGETS ...)`\ninvocation.\n\nWell, that's easily accomplished: Let's feed the targets individually to\nthe `install(TARGETS ...)` function in a `foreach()` look.\n\nThis also has the advantage that we do not have to manually cull off the\ntwo entries from the `${PROGRAMS_BUILT}` array before scheduling the\nremainder to be installed into `libexec/git-core`. Instead, we iterate\nthrough the array and decide for each entry where it wants to go.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex da2811ae3aad..a166be0eb1b8 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -811,15 +811,19 @@ list(TRANSFORM git_shell_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n list(TRANSFORM git_perl_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n \n #install\n-install(TARGETS git git-shell\n+foreach(program ${PROGRAMS_BUILT})\n+if(${program} STREQUAL git OR ${program} STREQUAL git-shell)\n+install(TARGETS ${program}\n \tRUNTIME DESTINATION bin)\n+else()\n+install(TARGETS ${program}\n+\tRUNTIME DESTINATION libexec/git-core)\n+endif()\n+endforeach()\n+\n install(PROGRAMS ${CMAKE_BINARY_DIR}/git-cvsserver\n \tDESTINATION bin)\n \n-list(REMOVE_ITEM PROGRAMS_BUILT git git-shell)\n-install(TARGETS ${PROGRAMS_BUILT}\n-\tRUNTIME DESTINATION libexec/git-core)\n-\n set(bin_links\n \tgit-receive-pack git-upload-archive git-upload-pack)\n \n-- \ngitgitgadget\n\n"},{"id":"420318","messageId":"4b183c7def5839ddf679605ff74f7f3280d957d0.1616886386.git.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":"pull.887.git.1616886386.gitgitgadget@gmail.com","subject":"[PATCH 4/4] cmake(install): include vcpkg dlls","fromName":"Dennis Ameling via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-27T23:06:25Z","receivedAt":"2021-03-27T23:07:48Z","isPatch":true,"sender":{"key":"name:Dennis Ameling","avatar":null},"body":"From: Dennis Ameling <dennis@dennisameling.com>\n\nOur CMake configuration generates not only build definitions, but also\ninstall definitions: After building Git using `msbuild git.sln`, the\nbuilt artifacts can be installed via `msbuild INSTALL.vcxproj`.\n\nTo specify _where_ the files should be installed, the\n`-DCMAKE_INSTALL_PREFIX=<path>` option can be used when running CMake.\n\nHowever, this process would really only install the files that were just\nbuilt. On Windows, we need more than that: We also need the `.dll` files\nof the dependencies (such as libcurl). The `vcpkg` ecosystem, which we\nuse to obtain those dependencies, can be asked to install said `.dll`\nfiles really easily, so let's do that.\n\nThis requires more than just the built `vcpkg` artifacts in the CI build\ndefinition; We now clone the `vcpkg` repository so that the relevant\nCMake scripts are available, in particular the ones related to defining\nthe toolchain.\n\nSigned-off-by: Dennis Ameling <dennis@dennisameling.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n .github/workflows/main.yml          | 5 +++++\n contrib/buildsystems/CMakeLists.txt | 4 ++++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex f6885e88ee6b..c13afe2bf058 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -186,6 +186,11 @@ jobs:\n         ## Unzip and remove the artifact\n         unzip artifacts.zip\n         rm artifacts.zip\n+    - name: initialize vcpkg\n+      uses: actions/checkout@v2\n+      with:\n+        repository: 'microsoft/vcpkg'\n+        path: 'compat/vcbuild/vcpkg'\n     - name: download vcpkg artifacts\n       shell: powershell\n       run: |\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex a166be0eb1b8..98b2507f222e 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -58,6 +58,10 @@ if(WIN32)\n \n \t# In the vcpkg edition, we need this to be able to link to libcurl\n \tset(CURL_NO_CURL_CMAKE ON)\n+\n+\t# Copy the necessary vcpkg DLLs (like iconv) to the install dir\n+\tset(X_VCPKG_APPLOCAL_DEPS_INSTALL ON)\n+\tset(CMAKE_TOOLCHAIN_FILE ${VCPKG_DIR}/scripts/buildsystems/vcpkg.cmake CACHE STRING \"Vcpkg toolchain file\")\n endif()\n \n find_program(SH_EXE sh PATHS \"C:/Program Files/Git/bin\")\n-- \ngitgitgadget\n"},{"id":"420357","messageId":"YF/117C6LcNGQ7sm@danh.dev","threadId":"55401","inReplyTo":"543fd0f5d7e5ee297364d1d28091f2004a35f2d0.1616886386.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/4] cmake: add a preparatory work-around to accommodate `vcpkg`","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-03-28T03:19:51Z","receivedAt":"2021-03-28T03:26:06Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-03-27 23:06:24+0000, Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> \n> We are about to add support for installing the `.dll` files of Git's\n> dependencies (such as libcurl) in the CMake configuration. The `vcpkg`\n> ecosystem from which we get said dependencies makes that relatively\n> easy: simply turn on `X_VCPKG_APPLOCAL_DEPS_INSTALL`.\n> \n> However, current `vcpkg` introduces a limitation if one does that:\n> While it is totally cool with CMake to specify multiple targets within\n> one invocation of `install(TARGETS ...) (at least according to\n> https://cmake.org/cmake/help/latest/command/install.html#command:install),\n> `vcpkg`'s parser insists on a single target per `install(TARGETS ...)`\n> invocation.\n> \n> Well, that's easily accomplished: Let's feed the targets individually to\n> the `install(TARGETS ...)` function in a `foreach()` look.\n> \n> This also has the advantage that we do not have to manually cull off the\n> two entries from the `${PROGRAMS_BUILT}` array before scheduling the\n> remainder to be installed into `libexec/git-core`. Instead, we iterate\n> through the array and decide for each entry where it wants to go.\n> \n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  contrib/buildsystems/CMakeLists.txt | 14 +++++++++-----\n>  1 file changed, 9 insertions(+), 5 deletions(-)\n> \n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index da2811ae3aad..a166be0eb1b8 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -811,15 +811,19 @@ list(TRANSFORM git_shell_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n>  list(TRANSFORM git_perl_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n>  \n>  #install\n> -install(TARGETS git git-shell\n> +foreach(program ${PROGRAMS_BUILT})\n> +if(${program} STREQUAL git OR ${program} STREQUAL git-shell)\n\nPlease don't use `${}` around variable inside `if()`, and quote the\nstring. CMake has a quirk with the `${}` inside if (expanded variable\nwill be treated as a variable if it is defined, or string otherwise).\nUnquoted string will be seen as a variable if it's defined, string\notherwise. IOW, suggested command:\n\n\tif (program STREQUAL \"git\" OR program STREQUAL \"git-shell\")\n\nWe also have another problem with quoted arguments could be interpreted\nas variable or keyword if CMP0054 policy not enabled, too.\nI think it's better to have it enabled, but it's not in the scope of\nthis patch.\n\n https://cmake.org/cmake/help/latest/policy/CMP0054.html\n\n\n-- \nDanh\n"},{"id":"420456","messageId":"ff7e8121d7a47e318954c2c115f281b56f8be21f.1617021705.git.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":"pull.887.v2.git.1617021705.gitgitgadget@gmail.com","subject":"[PATCH v2 1/4] cmake: support SKIP_DASHED_BUILT_INS","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-29T12:41:42Z","receivedAt":"2021-03-29T12:42:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nJust like the Makefile-based build learned to skip hard-linking the\ndashed built-ins in 179227d6e21 (Optionally skip linking/copying the\nbuilt-ins, 2020-09-21), this patch teaches the CMake-based build the\nsame trick.\n\nNote: In contrast to the Makefile-based process, the built-ins would\nonly be linked during installation, not already when Git is built.\nTherefore, the CMake-based build that we use in our CI builds _already_\ndoes not link those built-ins (because the files are not installed\nanywhere, they are used to run the test suite in-place).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex c151dd7257f3..12c40a72bfff 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -685,13 +685,17 @@ endif()\n \n parse_makefile_for_executables(git_builtin_extra \"BUILT_INS\")\n \n+option(SKIP_DASHED_BUILT_INS \"Skip hardlinking the dashed versions of the built-ins\")\n+\n #Creating hardlinks\n+if(NOT SKIP_DASHED_BUILT_INS)\n foreach(s ${git_SOURCES} ${git_builtin_extra})\n \tstring(REPLACE \"${CMAKE_SOURCE_DIR}/builtin/\" \"\" s ${s})\n \tstring(REPLACE \".c\" \"\" s ${s})\n \tfile(APPEND ${CMAKE_BINARY_DIR}/CreateLinks.cmake \"file(CREATE_LINK git${EXE_EXTENSION} git-${s}${EXE_EXTENSION})\\n\")\n \tlist(APPEND git_links ${CMAKE_BINARY_DIR}/git-${s}${EXE_EXTENSION})\n endforeach()\n+endif()\n \n if(CURL_FOUND)\n \tset(remote_exes\n-- \ngitgitgadget\n\n"},{"id":"420457","messageId":"69856f278645ebe1a9779aaa0a6894db33e22b3a.1617021705.git.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":"pull.887.v2.git.1617021705.gitgitgadget@gmail.com","subject":"[PATCH v2 2/4] cmake(install): fix double .exe suffixes","fromName":"Dennis Ameling via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-29T12:41:43Z","receivedAt":"2021-03-29T12:42:26Z","isPatch":true,"sender":{"key":"name:Dennis Ameling","avatar":null},"body":"From: Dennis Ameling <dennis@dennisameling.com>\n\nBy mistake, the `.exe` extension is appended _twice_ when installing the\ndashed executables into `libexec/git-core/` on Windows (the extension is\nalready appended when adding items to the `git_links` list in the\n`#Creating hardlinks` section).\n\nSigned-off-by: Dennis Ameling <dennis@dennisameling.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 12c40a72bfff..da2811ae3aad 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -832,12 +832,12 @@ install(CODE \"file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git-shell${EXE_EXTENS\n \n foreach(b ${git_links})\n \tstring(REPLACE \"${CMAKE_BINARY_DIR}\" \"\" b ${b})\n-\tinstall(CODE \"file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b}${EXE_EXTENSION})\")\n+\tinstall(CODE \"file(CREATE_LINK ${CMAKE_INSTALL_PREFIX}/bin/git${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b})\")\n endforeach()\n \n foreach(b ${git_http_links})\n \tstring(REPLACE \"${CMAKE_BINARY_DIR}\" \"\" b ${b})\n-\tinstall(CODE \"file(CREATE_LINK  ${CMAKE_INSTALL_PREFIX}/libexec/git-core/git-remote-http${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b}${EXE_EXTENSION})\")\n+\tinstall(CODE \"file(CREATE_LINK  ${CMAKE_INSTALL_PREFIX}/libexec/git-core/git-remote-http${EXE_EXTENSION} ${CMAKE_INSTALL_PREFIX}/libexec/git-core/${b})\")\n endforeach()\n \n install(PROGRAMS ${git_shell_scripts} ${git_perl_scripts} ${CMAKE_BINARY_DIR}/git-p4\n-- \ngitgitgadget\n\n"},{"id":"420458","messageId":"pull.887.v2.git.1617021705.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":"pull.887.git.1616886386.gitgitgadget@gmail.com","subject":"[PATCH v2 0/4] cmake: learn to optionally skip linking dashed built-ins","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-29T12:41:41Z","receivedAt":"2021-03-29T12:42:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In Git for Windows, we would like to make use of the fact that our\nCMake-based build can also install the files into their final location. This\npatch series helps with that.\n\nChanges since v1:\n\n * Use proper string/variable CMake syntax, as pointed out by Danh\n\nDennis Ameling (2):\n  cmake(install): fix double .exe suffixes\n  cmake(install): include vcpkg dlls\n\nJohannes Schindelin (2):\n  cmake: support SKIP_DASHED_BUILT_INS\n  cmake: add a preparatory work-around to accommodate `vcpkg`\n\n .github/workflows/main.yml          |  5 +++++\n contrib/buildsystems/CMakeLists.txt | 26 +++++++++++++++++++-------\n 2 files changed, 24 insertions(+), 7 deletions(-)\n\n\nbase-commit: 773e25afc41b1b6533fa9ae2cd825d0b4a697fad\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-887%2Fdscho%2Fskip-dashed-built-ins-in-cmake-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-887/dscho/skip-dashed-built-ins-in-cmake-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/887\n\nRange-diff vs v1:\n\n 1:  ff7e8121d7a4 = 1:  ff7e8121d7a4 cmake: support SKIP_DASHED_BUILT_INS\n 2:  69856f278645 = 2:  69856f278645 cmake(install): fix double .exe suffixes\n 3:  543fd0f5d7e5 ! 3:  5d953a21e9bd cmake: add a preparatory work-around to accommodate `vcpkg`\n     @@ contrib/buildsystems/CMakeLists.txt: list(TRANSFORM git_shell_scripts PREPEND \"$\n       #install\n      -install(TARGETS git git-shell\n      +foreach(program ${PROGRAMS_BUILT})\n     -+if(${program} STREQUAL git OR ${program} STREQUAL git-shell)\n     ++if(program STREQUAL \"git\" OR program STREQUAL \"git-shell\")\n      +install(TARGETS ${program}\n       \tRUNTIME DESTINATION bin)\n      +else()\n 4:  4b183c7def58 = 4:  f020cb517dfc cmake(install): include vcpkg dlls\n\n-- \ngitgitgadget\n"},{"id":"420459","messageId":"f020cb517dfc48e2619815e09110d4044eed1ebe.1617021705.git.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":"pull.887.v2.git.1617021705.gitgitgadget@gmail.com","subject":"[PATCH v2 4/4] cmake(install): include vcpkg dlls","fromName":"Dennis Ameling via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-29T12:41:45Z","receivedAt":"2021-03-29T12:42:27Z","isPatch":true,"sender":{"key":"name:Dennis Ameling","avatar":null},"body":"From: Dennis Ameling <dennis@dennisameling.com>\n\nOur CMake configuration generates not only build definitions, but also\ninstall definitions: After building Git using `msbuild git.sln`, the\nbuilt artifacts can be installed via `msbuild INSTALL.vcxproj`.\n\nTo specify _where_ the files should be installed, the\n`-DCMAKE_INSTALL_PREFIX=<path>` option can be used when running CMake.\n\nHowever, this process would really only install the files that were just\nbuilt. On Windows, we need more than that: We also need the `.dll` files\nof the dependencies (such as libcurl). The `vcpkg` ecosystem, which we\nuse to obtain those dependencies, can be asked to install said `.dll`\nfiles really easily, so let's do that.\n\nThis requires more than just the built `vcpkg` artifacts in the CI build\ndefinition; We now clone the `vcpkg` repository so that the relevant\nCMake scripts are available, in particular the ones related to defining\nthe toolchain.\n\nSigned-off-by: Dennis Ameling <dennis@dennisameling.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n .github/workflows/main.yml          | 5 +++++\n contrib/buildsystems/CMakeLists.txt | 4 ++++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex f6885e88ee6b..c13afe2bf058 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -186,6 +186,11 @@ jobs:\n         ## Unzip and remove the artifact\n         unzip artifacts.zip\n         rm artifacts.zip\n+    - name: initialize vcpkg\n+      uses: actions/checkout@v2\n+      with:\n+        repository: 'microsoft/vcpkg'\n+        path: 'compat/vcbuild/vcpkg'\n     - name: download vcpkg artifacts\n       shell: powershell\n       run: |\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 3b94b5f62109..485c7662dc58 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -58,6 +58,10 @@ if(WIN32)\n \n \t# In the vcpkg edition, we need this to be able to link to libcurl\n \tset(CURL_NO_CURL_CMAKE ON)\n+\n+\t# Copy the necessary vcpkg DLLs (like iconv) to the install dir\n+\tset(X_VCPKG_APPLOCAL_DEPS_INSTALL ON)\n+\tset(CMAKE_TOOLCHAIN_FILE ${VCPKG_DIR}/scripts/buildsystems/vcpkg.cmake CACHE STRING \"Vcpkg toolchain file\")\n endif()\n \n find_program(SH_EXE sh PATHS \"C:/Program Files/Git/bin\")\n-- \ngitgitgadget\n"},{"id":"420460","messageId":"5d953a21e9bd6b5da562c9d285159bda2289b4fa.1617021705.git.gitgitgadget@gmail.com","threadId":"55401","inReplyTo":"pull.887.v2.git.1617021705.gitgitgadget@gmail.com","subject":"[PATCH v2 3/4] cmake: add a preparatory work-around to accommodate `vcpkg`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-29T12:41:44Z","receivedAt":"2021-03-29T12:42:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWe are about to add support for installing the `.dll` files of Git's\ndependencies (such as libcurl) in the CMake configuration. The `vcpkg`\necosystem from which we get said dependencies makes that relatively\neasy: simply turn on `X_VCPKG_APPLOCAL_DEPS_INSTALL`.\n\nHowever, current `vcpkg` introduces a limitation if one does that:\nWhile it is totally cool with CMake to specify multiple targets within\none invocation of `install(TARGETS ...) (at least according to\nhttps://cmake.org/cmake/help/latest/command/install.html#command:install),\n`vcpkg`'s parser insists on a single target per `install(TARGETS ...)`\ninvocation.\n\nWell, that's easily accomplished: Let's feed the targets individually to\nthe `install(TARGETS ...)` function in a `foreach()` look.\n\nThis also has the advantage that we do not have to manually cull off the\ntwo entries from the `${PROGRAMS_BUILT}` array before scheduling the\nremainder to be installed into `libexec/git-core`. Instead, we iterate\nthrough the array and decide for each entry where it wants to go.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex da2811ae3aad..3b94b5f62109 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -811,15 +811,19 @@ list(TRANSFORM git_shell_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n list(TRANSFORM git_perl_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n \n #install\n-install(TARGETS git git-shell\n+foreach(program ${PROGRAMS_BUILT})\n+if(program STREQUAL \"git\" OR program STREQUAL \"git-shell\")\n+install(TARGETS ${program}\n \tRUNTIME DESTINATION bin)\n+else()\n+install(TARGETS ${program}\n+\tRUNTIME DESTINATION libexec/git-core)\n+endif()\n+endforeach()\n+\n install(PROGRAMS ${CMAKE_BINARY_DIR}/git-cvsserver\n \tDESTINATION bin)\n \n-list(REMOVE_ITEM PROGRAMS_BUILT git git-shell)\n-install(TARGETS ${PROGRAMS_BUILT}\n-\tRUNTIME DESTINATION libexec/git-core)\n-\n set(bin_links\n \tgit-receive-pack git-upload-archive git-upload-pack)\n \n-- \ngitgitgadget\n\n"},{"id":"420468","messageId":"nycvar.QRO.7.76.6.2103291535220.53@tvgsbejvaqbjf.bet","threadId":"55401","inReplyTo":"YF/117C6LcNGQ7sm@danh.dev","subject":"Re: [PATCH 3/4] cmake: add a preparatory work-around to accommodate `vcpkg`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-03-29T13:36:01Z","receivedAt":"2021-03-29T13:36:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Danh,\n\nOn Sun, 28 Mar 2021, Đoàn Trần Công Danh wrote:\n\n> On 2021-03-27 23:06:24+0000, Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote:\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > We are about to add support for installing the `.dll` files of Git's\n> > dependencies (such as libcurl) in the CMake configuration. The `vcpkg`\n> > ecosystem from which we get said dependencies makes that relatively\n> > easy: simply turn on `X_VCPKG_APPLOCAL_DEPS_INSTALL`.\n> >\n> > However, current `vcpkg` introduces a limitation if one does that:\n> > While it is totally cool with CMake to specify multiple targets within\n> > one invocation of `install(TARGETS ...) (at least according to\n> > https://cmake.org/cmake/help/latest/command/install.html#command:install),\n> > `vcpkg`'s parser insists on a single target per `install(TARGETS ...)`\n> > invocation.\n> >\n> > Well, that's easily accomplished: Let's feed the targets individually to\n> > the `install(TARGETS ...)` function in a `foreach()` look.\n> >\n> > This also has the advantage that we do not have to manually cull off the\n> > two entries from the `${PROGRAMS_BUILT}` array before scheduling the\n> > remainder to be installed into `libexec/git-core`. Instead, we iterate\n> > through the array and decide for each entry where it wants to go.\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >  contrib/buildsystems/CMakeLists.txt | 14 +++++++++-----\n> >  1 file changed, 9 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> > index da2811ae3aad..a166be0eb1b8 100644\n> > --- a/contrib/buildsystems/CMakeLists.txt\n> > +++ b/contrib/buildsystems/CMakeLists.txt\n> > @@ -811,15 +811,19 @@ list(TRANSFORM git_shell_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n> >  list(TRANSFORM git_perl_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n> >\n> >  #install\n> > -install(TARGETS git git-shell\n> > +foreach(program ${PROGRAMS_BUILT})\n> > +if(${program} STREQUAL git OR ${program} STREQUAL git-shell)\n>\n> Please don't use `${}` around variable inside `if()`, and quote the\n> string. CMake has a quirk with the `${}` inside if (expanded variable\n> will be treated as a variable if it is defined, or string otherwise).\n> Unquoted string will be seen as a variable if it's defined, string\n> otherwise. IOW, suggested command:\n>\n> \tif (program STREQUAL \"git\" OR program STREQUAL \"git-shell\")\n>\n> We also have another problem with quoted arguments could be interpreted\n> as variable or keyword if CMP0054 policy not enabled, too.\n> I think it's better to have it enabled, but it's not in the scope of\n> this patch.\n>\n>  https://cmake.org/cmake/help/latest/policy/CMP0054.html\n\nThank you for this information! I've sent out v2 based on your suggestion.\n\nThanks,\nDscho\n"}]}