Skip to content

Commit

Permalink
Use FetchContent to get vcpkg. (#4484)
Browse files Browse the repository at this point in the history
[SC-36205](https://app.shortcut.com/tiledb-inc/story/36205/use-fetchcontent-to-acquire-vcpkg)

Currently, building TileDB from a GitHub archive fails by default[^1],
because we use vcpkg from a submodule, and if we are not on a cloned
repository that submodules does not exist.

With this PR we change our approach to using CMake's
[`FetchContent`](https://cmake.org/cmake/help/latest/module/FetchContent.html)
module, and remove the `externals/vcpkg` submodule. [This is what the
Azure SDK for C++
does.](https://github.com/Azure/azure-sdk-for-cpp/blob/12407e8bfcb9bc1aa43b253c1d0ec93bf795ae3b/cmake-modules/AzureVcpkg.cmake#L14-L40)

This has the disadvantage that by default[^1] the vcpkg repository will
be downloaded for every clean configure. It can by mitigated by
specifying a `VCPKG_ROOT` environment variable to a local path of the
vcpkg repository.

[^1]: Unless the vcpkg toolchain file is manually specified.

---
TYPE: BUILD
DESC: Remove the vcpkg submodule and download it automatically when
configuring
  • Loading branch information
teo-tsirpanis authored Nov 3, 2023
1 parent a688248 commit 1f4ed0a
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 50 deletions.
12 changes: 0 additions & 12 deletions .github/workflows/ci-linux_mac.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,6 @@ jobs:
core.exportVariable('ACTIONS_CACHE_URL', process.env.ACTIONS_CACHE_URL || '');
core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || '');
- name: Setup vcpkg
uses: lukka/run-vcpkg@v11
id: runvcpkg
with:
vcpkgJsonGlob: 'vcpkg.json'
vcpkgDirectory: '${{ github.workspace }}/external/vcpkg'

- name: Prints output of run-vcpkg's action.
run: |
echo "run-vcpkg outputs:"
echo "${{ toJSON(steps.runvcpkg.outputs) }}"
- name: Prevent vpckg from building debug variants
run: python ./scripts/ci/patch_vcpkg_triplets.py

Expand Down
3 changes: 0 additions & 3 deletions .gitmodules

This file was deleted.

19 changes: 14 additions & 5 deletions cmake/Options/TileDBToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,20 @@ if (NOT DEFINED CMAKE_TOOLCHAIN_FILE)
set(CMAKE_TOOLCHAIN_FILE
"$ENV{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake"
CACHE STRING "Vcpkg toolchain file")
else()
include(init-submodule)
set(CMAKE_TOOLCHAIN_FILE
"${CMAKE_CURRENT_SOURCE_DIR}/external/vcpkg/scripts/buildsystems/vcpkg.cmake"
CACHE STRING "Vcpkg toolchain file")
elseif(NOT DEFINED ENV{TILEDB_DISABLE_AUTO_VCPKG})
# Inspired from https://github.com/Azure/azure-sdk-for-cpp/blob/azure-core_1.10.3/cmake-modules/AzureVcpkg.cmake
message("TILEDB_DISABLE_AUTO_VCPKG is not defined. Fetch a local copy of vcpkg.")
# To help with resolving conflicts, when you update the commit, also update its date.
set(VCPKG_COMMIT_STRING 1b4d69f3028d74401a001aa316986a670ca6289a) # 2023-09-27
message("Vcpkg commit string used: ${VCPKG_COMMIT_STRING}")
include(FetchContent)
FetchContent_Declare(
vcpkg
GIT_REPOSITORY https://github.com/microsoft/vcpkg.git
GIT_TAG ${VCPKG_COMMIT_STRING}
)
FetchContent_MakeAvailable(vcpkg)
set(CMAKE_TOOLCHAIN_FILE "${vcpkg_SOURCE_DIR}/scripts/buildsystems/vcpkg.cmake" CACHE STRING "Vcpkg toolchain file")
endif()
endif()

Expand Down
27 changes: 0 additions & 27 deletions cmake/init-submodule.cmake

This file was deleted.

1 change: 0 additions & 1 deletion external/vcpkg
Submodule vcpkg deleted from 1b4d69
3 changes: 1 addition & 2 deletions scripts/ci/patch_vcpkg_triplets.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import os

workspace = os.curdir
triplet_paths = [os.path.join(workspace, "external", "vcpkg", "triplets"),
os.path.join(workspace, "ports", "triplets")]
triplet_paths = [os.path.join(workspace, "ports", "triplets")]

for triplet_path in triplet_paths:
for path, dnames, fnames in os.walk(triplet_path):
Expand Down

0 comments on commit 1f4ed0a

Please sign in to comment.