clinfo icon indicating copy to clipboard operation
clinfo copied to clipboard

Added Windows support via CMake.

Open pkeir opened this issue 7 years ago • 3 comments

Tested on Visual Studio 2017.

pkeir avatar Nov 12 '18 18:11 pkeir

I'd very much like this to be merged. @Oblomov I have seen your comment in #20 and I would like to give a counter-argument:

While adding an additional dependency to build this program may seem to add complexity, the opposite effect is true. Let's compare the current state of building clinfo with using CMake. Currently there are:

  • 2 Makefiles, one for Unix, one for Windows
  • 2 Batch scripts for Windows
  • 1 shell script that works as a code generator

By adopting CMake, you would replace those 5 files with exactly one (+ template for the version header) that works on every operating system CMake supports (and that's quite a lot). CMake has become so ubiquitous that even fairly niche Unix systems like Solaris have it available in their system package manager. In other words, requiring users to install it is almost the same as just installing make itself at this point.

This alone removes a lot of code maintenance burden.

The other important point is package management, both consuming OpenCL and packaging clinfo itself.

Consuming OpenCL

For Windows there is fetch-opencl-dev-win.cmd to fetch the dependency. This homebrew approach is very fragile because it makes a lot of assumptions that make development on Windows harder:

  • I used to work at a company that enforced a HTTP(S) proxy. You couldn't "just" download something. I have fond memories of fiddling with curl's environment variables and flags to ignore the MITM TLS certificate and patiently wait until the web anti-virus scanner completes. Fortunately I have switched workplaces since, but those companies do exist and developers from those companies will come here and ask for help with their broken IT if you provide such a script.
  • vcpkg is the Microsoft-sanctioned way of getting dependencies. It contains a script for the OpenCL ICD + headers already. Which brings us to the next problem:
  • There is no ergonomic way of using OpenCL if the install prefix differs. In Linux Makefile land there is PkgConfig, but it wouldn't solve the problem for Windows.
    • This point is especially important to me: I need to cross-compile software regularly. I have created my own OpenCL package for Conan so I can easily switch between compiler toolchains. With CMake, this Just Works (tm) but with custom Makefiles I would also need to write custom code to inject the path to the cross-compiled OpenCL. I would also need to do the same if I want to compile with a version of OpenCL different from the system one.
  • You as the maintainer are responsible for correctly versioning your dependencies. Again, this is unnecessary maintenance overhead. As it stands right now, you're downloading tip-of-trunk headers and an OpenCL 1.2 ICD loader on Windows. Such version mismatches can easily be avoided by using vcpkg, just like this PR does.
Packaging clinfo
  • While system-specific package managers like PKGBUILD can build clinfo relatively easy, this is not the case anymore when using cross-platform language package managers like vcpkg or conan. You'd need to special case every operating system because there are subtle incompatibilities between the Makefiles hidden everywhere. For example, the Windows Makefile lacks an install rule. And I'd argue that issues like this will occur again and again as long as multiple build systems for different operating systems are maintained. Writing build scripts is hard, writing Makefiles that cover every corner case even more so. Abstraction is key.

Last but not least: Adding a CMakeLists doesn't remove anything. It merely adds a new build system with all disadvantages of additional code, but the old stuff keeps working until removed.

This is the second PR for adding a CMake build system and the third person asking for it. If people wouldn't have issues with the current situation they wouldn't keep submitting patches. I hope you reconsider your position.

mmha avatar Nov 26 '18 18:11 mmha

First of all I would like to congratulate @Oblomov on the utility. I arrived at this repo checking the exact clinfo Canonical packages for Ubuntu. Second of all, very big yes for CMake! I understand the minimalistic approach, but do agree with @mmha that having another way of building doesn't have to interfere with the other methods. CMake has become the de facto standard of building cross-platform software (unfortunately). We might as well make the most of the situation and use what CMake has to offer.

I am the maintainer of the OpenCL dev package for Vcpkg and I would like to enhance the package with minimal utilities, such as clinfo. Whether the scripts are mainlined or not doesn't really concern me, it all boils down to having to submit the build script as a patch or not.

As far as the script goes, I would like to propose a little bikeshedding…

MathiasMagnus avatar Jan 24 '19 13:01 MathiasMagnus

Thank you, I'd very much like to see this merged as well. Building with Makefiles is a pain, and this is much more portable - VS2017 supports CMake out of the box. I just use my own fork of the repo when I need to build it.

ProGTX avatar Jan 25 '19 10:01 ProGTX