Support for "modern" CMake handling of installed SDK missing

[martinschwinzerl]

Would it be please possible to add support for the "modern" CMake way of working with the installed[1] SDK, i.e.

• identify components (api headers, cxx headers, icd loader)
• provide *Targets.cmake and *Config.cmake file(s) for the components and
• expose targets for working within CMake with the imported SDK

Furthermore, I would have expected that the "make install" handles the installation of all sub-modules (i.e. especially the headers, at least api and cxx) as well. Thank you for considering this!

[1]OTOH, there is no mention in the README about make install so maybe this is intentional? Just a thought.

[MathiasMagnus]

This is WIP, in fact it's done and I've had it in a PR until it fell victim to the master >> main transition. This work spans across 4 repos (Headers, CLHPP, ICD-Loader, SDK), so it requires coordinated action, but rest assured that it's 90% done and as soon as I get around to it (it also touches upon significantly improving our CI coverage, as breaking away from existing Find Module detection in backwards compatible manner is tricky and requires extensive testing). It's mostly work done, but polish is essential.

I'll update you once a PRs are in flight again.

[martinschwinzerl]

Thank you for your quick update & your efforts, they are very much appreciated! I would have otherwise tried to take a stab at this in the near future (had something cobbled together for the cxx headers locally which clashed with the changes introduces leading up to the OpenCL 3.0 release, never got around to the other components), but given the complexity you outlined and the importance, its great that you are already mostly done!

Cheers,
Martin

[Vectrobe]

Is this perchance why the headers are missing from the install directory after a cmake --install? no errors are logged during build or install, cmake simply never copies them (/lib, /bin and the exe's are however).

[MathiasMagnus]

The headers work has been merged but is not yes referenced by this project using package config syntax. You are welcome to contribute, or wait until I (or someone) gets around to merging the same feature to the ICD and the CLHPP packages.

As I said it's a big change and requires extensive testing, some of which has already been merged into the other projects. I got limited time, but am making slow and steady progress. (End of year isn't about burning through GitHub issues. Here's to forgetting a ton about 2020 🍾)

[Vectrobe]

Is it currently expected to use the source directory for the 'install'? ie just build and reference the source directory ('OpenCL-SDK') directly for headers and lib?

I haven't yet been able to use the SDK as the config.cmake files are never generated on build or install, so cmake has no idea where anything even is when you try to find_package(OpenCL or OpenCL-SDK).

I also suggest as well that the names 'OpenCL and 'OpenCL-SDK' be stripped out, and just use 'OPENCL' instead in all caps. Obviously this is a library so the '-SDK' part is pointless and only confuses the matter inside cmake...

[MathiasMagnus]

@Vectrobe The plan on how OpenCL as an API will be consumed in the "modern" way is layed out here, in the initial comment of this PR that started the work.

Referencing the source tree will not be needed. You install the SDK and point CMAKE_PREFIX_PATH to your install root, as would be expected.

I also suggest as well that the names 'OpenCL and 'OpenCL-SDK' be stripped out, and just use 'OPENCL' instead in all caps. Obviously this is a library so the '-SDK' part is pointless and only confuses the matter inside cmake...

Naming the repo is as is ,the cost of changing the name is more than the benefit of "I like it better that way". There has to be tangible benefits for such a change to occur. (I personally have no aesthetic issue with it. From a practical standpoint "OpenCL" may have been a more comfortable from a CMake perspective, but naming repos to please tools is backwards.) From a bikeshedding perspective, it is very much an SDK, not OpenCL itself. Plans are (as can be seen from the issues list) that there is an SDK library being designed aiming to take some of the tedium out of common OpenCL related tasks.

Do note (as described by the linked PR comment) that there are many things to consider, backward compatibility being a major thing. Existing

find_package(OpenCL REQUIRED)
...
target_link_libraries(${PROJECT_NAME} OpenCL::OpenCL)

scripts have to work. Breaking build scripts comes at a huge cost (manpower, losing users, etc.). I personally dislike how the SDK (all the components required to build an OpenCL app) are broken into multiple repos, but these repos have existed the way they do since the dawn of time and Linux distro maintainers have scripted to consuming them this way. The SDK has to live with the fact that the SDK needs to be installed from multiple sub-projects. The "confusion" is further aggravated by the fact that the ICD loader is called libOpenCL.so/OpenCL.dll which isn't strictly OpenCL, there are other components needed to compile an application using the C++ bindings for eg.

The targets and names as described in the (IMHO, and those of the approvers) strike a good balance between backward compat, modern and feature complete. Please keep in mind there is hidden complexity. Taking everyone from CMake modules to CMake packages in a group of repos with a decade of history without leaving anyone behind is a feat. We strive to do just that.

[Vectrobe]

Naming the repo is as is

Oh no I didn't mean the name of the repo at all, just the cmake naming.
Currently we have project(OpenCL-SDK), target_properties(OpenCL ...), OPENCL_SDK_INCLUDE_DIRS, meanwhile other projects have required OPENCL_INCLUDE_DIR, OPENCL_DIR, (nvidia), and cmake had its own FindOpenCL using OpenCL_INCLUDE_DIRS, and so on.

The main point being that most existing projects use OpenCL for the package import, and OPENCL_... for the definitions. The current cmake config uses OpenCL-SDK and OPENCL_SDK_..., which breaks that.

[MathiasMagnus]

Please, disregard the current use of OpenCL_SDK_... variables. They are hotfixes to make the SDK compile. Those parts will disappear once I get around all the repos. Rest assured, I'm making some progress today on the ICD-Loade repo. No promises. You can follow the progress here.