clMathLibraries / clSPARSE

a software library containing Sparse functions written in OpenCL
Apache License 2.0
173 stars 61 forks source link

FTBFS due to missing cl.hpp header file with OpenCL 2.0 #120

Open ghisvail opened 9 years ago

ghisvail commented 9 years ago

Debian now ships the Khronos OpenCL 2.0 headers, which no longer includes the C++ headers.

The build fails with the following error:

[  5%] Building CXX object library/CMakeFiles/clSPARSE.dir/clsparse-init.cpp.o
cd /home/ghislain/debian/packages/build-area/clsparse-0.6.1.0/obj-x86_64-linux-gnu/library && /usr/bin/c++   -DBUILD_CLVERSION=120 -DCL_USE_DEPRECATED_OPENCL_2_0_APIS -DclSPARSE_EXPORTS -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2  -fvisibility=hidden -fvisibility-inlines-hidden -O2 -g -DNDEBUG -fPIC -I/home/ghislain/debian/packages/build-area/clsparse-0.6.1.0/src/library -I/home/ghislain/debian/packages/build-area/clsparse-0.6.1.0/obj-x86_64-linux-gnu/library -I/home/ghislain/debian/packages/build-area/clsparse-0.6.1.0/src/include -I/home/ghislain/debian/packages/build-area/clsparse-0.6.1.0/obj-x86_64-linux-gnu/include -I/home/ghislain/debian/packages/build-area/clsparse-0.6.1.0/obj-x86_64-linux-gnu/clsparseTimer    -std=c++11 -pedantic -o CMakeFiles/clSPARSE.dir/clsparse-init.cpp.o -c /home/ghislain/debian/packages/build-area/clsparse-0.6.1.0/src/library/clsparse-init.cpp
In file included from /home/ghislain/debian/packages/build-area/clsparse-0.6.1.0/src/library/clsparse-init.cpp:21:0:
/home/ghislain/debian/packages/build-area/clsparse-0.6.1.0/src/library/include/clSPARSE-private.hpp:31:21: fatal error: CL/cl.hpp: No such file or directory
 #include <CL/cl.hpp>
                     ^
compilation terminated.
library/CMakeFiles/clSPARSE.dir/build.make:62: recipe for target 'library/CMakeFiles/clSPARSE.dir/clsparse-init.cpp.o' failed

A solution may be to build with a vendorized version of cl.hpp from OpenCL 1.2.

kknox commented 9 years ago

I'm working on getting clspare to build on travis ci, and I'm running into the same problem on macosx. For now, I'm wget/copying cl.hpp from the khronos website into the OpenCL installation. You can take a peak at my .travis.yml file. Still working on it; macosx works but linux is failing for other reasons.

ghisvail commented 9 years ago

FYI, I can't download anything during the package build process (same comment as in our gtest discussion). So the solution you applied to travis won't work for Debian.

Best would be to ship a vendorized cl.hpp from Khronos OpenCL 1.2 as part of the project and conditionally use it during the build process.

kknox commented 9 years ago

Maybe I could make it a conditional step in our cmake configure process?

ghisvail commented 9 years ago

As long as it does not require us to fetch any additional resource online during the configure / build / install process, any solution is possible.

kknox commented 9 years ago

I am thinking that we could introduce a step in the superbuild that downloads the khronos header files. I assume that is satisfactory. Are you able to run the build as sudo? Is it possible to write to /usr/local or /opt? Strictly keep the header files in the build tree?

ghisvail commented 9 years ago

a step in the superbuild that downloads the khronos header files

Did I mention that we cannot fetch remote resources during the package build process?

Are you able to run the build as sudo?

The build is performed in a chroot. Not sure why sudo is relevant to the discussion here.

Is it possible to write to /usr/local or /opt?

Packages shipped as part of the Debian archive are expected to install their corresponding binaries / files under the /usr prefix.

Strictly keep the header files in the build tree?

Not sure what you mean here.

kknox commented 9 years ago

Did I mention that we cannot fetch remote resources during the package build process?

It was not clear to me if you implied you can't fetch external dependencies yourself, or if the cmake script can't do it internally; how did you solve googletest dependency, or maybe you haven't? I suppose you could always turn off building the tests BUILD_TESTS & benchmarks BUILD_BENCHMARKS and remove the dependencies of boost and googletest.

The build is performed in a chroot. Not sure why sudo is relevant to the discussion here.

You need root privileges to write to /usr/local; this is a question to see if sudo cmake ... is viable. Our default policy is to not check in files from outside clSPARSE into the repo, especially if it's related to build. I don't want the hassle of pushing external code through our legal process and I don't want to set the precedent of maintaining external code. Our build scripts do their best to automate the gathering and setting up of dependencies for the user, but we don't check in those dependencies.

Outside of checking cl.hpp into clSPARSE, how can I help you move forward? At a minimum, if I can't script it for you, you could make a 'debian-fork' that you could check in your own cl.hpp and maintain yourself.

ghisvail commented 9 years ago

how did you solve googletest dependency?

Use the version that is packaged in Debian.

You need root privileges to write to /usr/local

Sure, that's why you also need sudo to install packages.

I don't want the hassle of pushing external code through our legal process and I don't want to set the precedent of maintaining external code. Our build scripts do their best to automate the gathering and setting up of dependencies for the user, but we don't check in those dependencies.

I don't follow you here. Which external code / dependencies are we talking about here? The missing cl.hpp which is MIT licensed?

Outside of checking cl.hpp into clSPARSE, how can I help you move forward?

I sent an email to our OpenCL maintainers' mailing list requesting their opinion on how to handle the missing cl.hpp.

FYI, you are not the first project facing this problem and my suggestion of shipping your own copy of cl.hpp was based on what has been applied successfully elsewhere.

At a minimum, if I can't script it for you, you could make a 'debian-fork' that you could check in your own cl.hpp and maintain yourself.

This solution is out of the picture AFAIC.

ghisvail commented 9 years ago

Just a quick update. It seems that OpenCL 2.0 now uses a new header: cl2.hpp for the C++ bindings.

kknox commented 9 years ago

We'll have to play with the new header; must have been posted in the last week. I didn't see it when I was working on travis integration. Do you know if the debian opencl maintainers will include the new header in a package you can use?

ghisvail commented 9 years ago

I opened a bug to the Debian BTS to get the OpenCL header package updated to the latest version, which include the new C++ bindings header.

ghisvail commented 9 years ago

I have not heard from the OpenCL Debian maintainers yet.

Meanwhile, it seems that other projects which rely on the OpenCL C++ bindings (ArrayFire, ASL...) do use a vendorized copy of cl.hpp instead of using the system one. API breakage were also reported from one version of cl.hpp to another.

Considering the volatility of these C++ bindings, it may be wiser to rely on your own copy. I am confident that the OpenCL Debian maintainers will second this statement.

kknox commented 9 years ago

Would you kindly send me a link to the 'rules/guidelines' for how to create packages in Debian? I would like to better understand the restrictions place on builds, and what I can do about them as a package maintainer.
For instance, if my automated travis build produced .deb's (with cpack), would that be acceptable? I just want to read it because I have so many questions.

ghisvail commented 9 years ago

Would you kindly send me a link to the 'rules/guidelines' for how to create packages in Debian?

The Debian New Maintainer's Guide should answer most of your questions.

For instance, if my automated travis build produced .deb's (with cpack), would that be acceptable?

Nope, they don't qualify as a source package.

kknox commented 9 years ago

Hi @ghisvail I've browsed the resources that you linked above, but I could not find the restrictions placed on builds. For instance, the restrictions on downloading files during build. Could you help me find the relevant reading material; i'd really like to understand the do's/don'ts.

ghisvail commented 9 years ago

There is no available Internet access on Debian package autobuilders. Period.

It is assumed that binary packages can be produced from source without requirement to fetch additional data from the Internet.

pavanky commented 9 years ago

@ghisvail Could Debian potentially ship multiple versions of the OpenCL headers ? It is not uncommon for Debian to support multiple versions of the same software.

This is important because many open source and NVIDIA's implementations do not have 2.0 support yet.

While we have the cl.hpp as a file in the ArrayFire repo, I'd rather not have it if there was a choice.

@kknox

Having said the above, cl.hpp is very finicky. Tying it down to a version local to the repo will result in the least amount of headache for anyone wanting to build the package.

Alternatively work could be done to abstract out cl.hpp and cl2.hpp header files.

ghisvail commented 9 years ago

Could Debian potentially ship multiple versions of the OpenCL headers ?

What would be the rationales for it ? OpenCL 2.0 seems to be the future as far as the C++ API is concerned (not talking about the C-API here), and some documented backward compatibility exists with 1.x apparently. I fail to see how to pitch the extra amount of work to the package maintainers.

While we have the cl.hpp as a file in the ArrayFire repo, I'd rather not have it if there was a choice.

Assuming these C++ bindings were not so volatile, yes this would be ideal.

Tying it down to a version local to the repo will result in the least amount of headache

Which is the solution I am in favour with too.

Alternatively work could be done to abstract out cl.hpp and cl2.hpp header files.

Isn't it what Boost.Compute does ?

pavanky commented 9 years ago

What would be the rationales for it ?

It is giving the false impression that Debian has the ability to support OpenCL 2.0 where it does not support it at all with the default packages. It is going to cause linker issues all over the place when the user uses functions from OpenCL 2.0 where as none of the libOpenCL.so files shipped by debian have those symbols. It literally makes no sense to add OpenCL 2.0 headers when none of the libraries in the Debian repos support them.

OpenCL 2.0 seems to be the future

It is. The same can be said of gcc-5.2. Yet Debian unstable has gcc-4.4, gcc-4.6, gcc-4.7, gcc-4.8, and gcc-4.9. The same is true for eigen2 and eigen3, opencv2 and opencv3 and many other packages. I am not sure why OpenCL headers are different.

Assuming these C++ bindings were not so volatile, yes this would be ideal.

I am assuming you based this on the changes we had to make to ArrayFire. The changes we did were backwards compatible. Asking to support the latest version of cl.hpp is a very different request than asking to put a 12,000 line file.

Isn't it what Boost.Compute does ?

I am not sure if Boost.Compute uses cl.hpp. From what I can tell they re-implemented their own C++ API for OpenCL.

kknox commented 9 years ago

@pavanky @ghisvail, thank you for the discussion.

I am warming to the idea of checking in a Khronos copy of cl.hpp, but this is something that won't happen until our next major beta push. This is just as well, because we are thinking of making breaking API changes (non-backwards compatible) and it's probably better to to keep v0.6.x.x out of the distros for now. clSPARSE API will be slightly volatile until we hit our v1.x.x.x release.

@ghisvail, please keep me posted of further efforts needed to support your work. Also, please keep me in the loop on what happens with Debian's OpenCL headers packages.

pavanky commented 9 years ago

@kknox Looks like some OpenCL implementations do not include cl.hpp. It sounds like pushing the header into the source repo is the better option at the moment: #139

ghisvail commented 9 years ago

@pavanky

It is giving the false impression that Debian has the ability to support OpenCL 2.0 where it does not support it at all with the default packages.

Depends what you mean by support. Our default is to use the Khronos OpenCL headers and the ocl-icd-opencl-dev package to compile all OpenCL related packages. Then, the ICD loader will dispatch to your implementation of choice, be it Nvidia, AMD, pocl, beignet, mesa. These may or may not partially / fully support the 1.1 / 1.2 / 2.0 specifications. However, this is nothing package maintainers can do much about though, can they ?

The same can be said of gcc-5.2

...which is the default now.

Yet Debian unstable has gcc-4.4, gcc-4.6, gcc-4.7, gcc-4.8, and gcc-4.9. The same is true for eigen2 and eigen3, opencv2 and opencv3 and many other packages.

Due to legacy packages which have strict versioned dependencies on those. Package maintainers don't keep old versions of these packages alive by choice.

AFAIK, none of the packages currently in the archive do require a strict dependency on a certain version of OpenCL. So again, there is no incentive from a package maintenance point-of-view to keep legacy versions of the opencl-headers alive.

I can't comment on other distributions, but a quick search revealed that Arch is at 2.0, whilst Fedora is at 1.2 with plans to go 2.0 too.

Asking to support the latest version of cl.hpp is a very different request than asking to put a 12,000 line file.

I completely understand. Hence myself highlighting that the points listed above are from a package maintenance perspective, which can be different from upstream development considerations.

@kknox I acknowledge your recommendation not to push anything to the archive before 1.0 is reached. I will keep you posted with what the OpenCL maintainers have to say about this.

pavanky commented 9 years ago

@ghisvail My point is that many libraries do compile time checks for the OpenCL version. None of the free OpenCL implementations in Debian supports 2.0. IMO this is the incorrect way to package the headers.

pavanky commented 9 years ago

Anyway, I do not want to pollute the issue further when there seems to be consensus on what needs to be done.

@ghisvail Ping me on gitter if you think I have very wrong misconceptions :)

ghisvail commented 9 years ago

Just a heads up on the OpenCL situation in Debian. An updated snapshot of the khronos OpenCL headers has been uploaded, witch contains the new cl2.hpp.

kknox commented 9 years ago

Thank you for the heads up. Given the instability of cl?.hpp distribution, my current hope is to eliminate our use of it with boost.compute.

ghisvail commented 9 years ago

Sounds like a good plan. Good luck with that.