cmake(install): include vcpkg dlls #2971
Conversation
Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
|
|
||
| # 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") |
There was a problem hiding this comment.
This causes the build to fail:
CMake Error at C:/Program Files/CMake/share/cmake-3.19/Modules/CMakeDetermineSystem.cmake:110 (message):
Could not find toolchain file:
D:/a/git/git/contrib/buildsystems/../../compat/vcbuild/vcpkg/scripts/buildsystems/vcpkg.cmake
There was a problem hiding this comment.
It works locally, so something is missing in your vcpkg artifacts in GH Actions. I just checked:
The scripts folder is missing, which causes the error:
Could you please include vcpkg/scripts/buildsystems/vcpkg.cmake in your vckpg artifacts, @dscho? That should resolve the issue 👍🏼
The alternative is that we could checkout vcpkg in GH Actions and then download your artifact with the prebuilt DLLs.
There was a problem hiding this comment.
Hrm. I did that, and now the vs-build jobs take longer than even the linux-gcc build (which builds Git on Linux and then runs the test suite twice). To test the hypothesis that this is triggered by including scripts, I started https://github.com/dscho/git/actions/runs/474627271. Let's see how that fares. If its vs-build jobs also take excruciatingly long, I will have to roll back that scripts change.
There was a problem hiding this comment.
Nope, seems that https://github.com/dscho/git/runs/1674724825?check_suite_focus=true was done with the generate Visual Studio solution step within 1.5 minutes, but https://github.com/git-for-windows/git/pull/2971/checks?check_run_id=1674658393 has now been running for more than 30 minutes.
There was a problem hiding this comment.
That's weird. Looks like it just wasn't doing anything for 6 (!) hours. It's probably better if we first clone vcpkg in CI and then download your artifacts over it. I'll try that tomorrow evening if I can make it work time-wise. To be continued 👍🏼
There was a problem hiding this comment.
I don't even know what purpose this toolchain file serves.
There was a problem hiding this comment.
Doesn't that take super long because it rebuilds vcpkg.exe every single time? I'd rather have a manual copy step, using constants defined by find_package(), if that is at all possible.
There was a problem hiding this comment.
1m51, that is: https://github.com/git-for-windows/git/pull/2971/checks?check_run_id=1687684942. But I see that vcpkg.exe is also included in your artifact, so let me try to exclude the vcpkg.exe build step and see if that works
There was a problem hiding this comment.
Fixed in 09d1573. Now the new step initialize vcpkg only takes 6s 👍🏼
.github/workflows/main.yml
Outdated
| run: | | ||
| echo Fetching vcpkg in %cwd%vcpkg | ||
| git.exe clone https://github.com/Microsoft/vcpkg vcpkg | ||
| IF ERRORLEVEL 1 ( EXIT /B 1 ) |
There was a problem hiding this comment.
I think we could use actions/checkout here, simplifying the added code (and accelerating it by the implicit shallow clone).
@dennisameling what do you think?
There was a problem hiding this comment.
Done @dscho, the initialize vcpkg step now takes 12s instead of 6-7s, but I guess that's okay 😄
There was a problem hiding this comment.
I think that time is very variable, anyway.
|
Very good. Would you mind squashing the latest three commits into a single one, explaining the need for it in the commit message? No worries if you lack the time, I should have a little today. |
This commit adds a step called "initialize vcpkg" to the GitHub Actions workflow, because we need some build scripts from vcpkg that aren't present in our vcpkg artifacts from Azure Pipelines. Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
|
Done @dscho, I think you meant the last four commits instead of three, as those four were all related to GH Actions. Hope this looks good now! |
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
cmake(install): include vcpkg dlls
Fixes #2970 🎉
With this PR, the DLLs are added correctly to the CMake build output: