tiledb: cleanup and migrate from legacy dependency management mode #21081
Conversation
The name `TILEDB_VCPKG` is misleading because it does not force the use of vcpkg. Auto-downloading of vcpkg is disabled by the `TILEDB_DISABLE_AUTO_VCPKG` environment variable.
|
The real solution is to keep a CMake option to disable VCPKG, This is the first time I see someone using an environment variable to enable/disable something. |
|
The vcpkg auto-download mechanism was based on the Azure SDK for C++, which uses an environment variable for reasons I don't know. I opened TileDB-Inc/TileDB#5048 to add a CMake option in addition to the environment variable. I would suggest you to merge this PR either way. |
|
Isn't it weird to see the upstream patching the downstream for their own issue. |
|
I don't understand. The MinGW package builds fine right now without any patches. I opened the upstream PR because I found the feature request reasonable and useful by itself, but that upstream PR should not block this PR. |
You are working on TileDB and you want to patch Findlz4 for MinGW-packages. Why don't you fix it in TileDB first. |
|
Ah you are talking about the lz4 patch. That patch is a temporary workaround that disables finding lz4 from a CMake package (which is provided only by vcpkg at the moment) and not suitable for upstreaming. The situation with lz4 is admittedly a bit messy and will be resolved in TileDB-Inc/TileDB#5021, which is a big PR that will be merged in the 2.26 timeframe. I could extract the lz4 fix from that PR but that too is non-trivial. |
|
This PR doesn't fix anything. |
|
Indeed it does not fix anything now, but will make updating easier after a couple releases. If you prefer it, you can defer these changes and make them when 2.26 gets released. |
|
Let's wait for the next release and see if they come with a friendly way to package tiledb (like any other library Instead of forcing vcpkg on all platforms) |
[SC-48929](https://app.shortcut.com/tiledb-inc/story/48929/add-cmake-option-to-disable-auto-downloading-vcpkg) This PR adds a CMake option to disable auto-downloading of vcpkg, as requested in msys2/MINGW-packages#21081 (comment). There is already an environment variable that serves the same purpose, but the CMake option has the advantage of being cached across configures. I also updated the error message when `TILEDB_VCPKG` is disabled, making the migration path more clear. Validated by configuring locally with this option specified, and observing that vcpkg was not downloaded, and `find_package` calls failed. --- TYPE: BUILD DESC: Automatic downloading of vcpkg can be disabled by enabling the `TILEDB_DISABLE_AUTO_VCPKG` CMake option, in addition to setting the environment variable with trhe same name.
This PR updates the
tiledbpackage to obviate the need for001-do-not-use-vcpkg.patch, and updates the build command to build only the necessary targets.See each commit message for more details, and also macports/macports-ports#24239 and TileDB-Inc/TileDB#4960 (comment) for more background.
Validated locally by running
makepkg-mingw --noconfirm --syncdeps.