Prepare for conda-forge upgrades

[mtazzari]

Some conda-forge ugprades require passing mandatory field CMAKE_INSTALL_PREFIX when doing cmake ... See https://github.com/conda-forge/galario-feedstock/pull/1 and my attempts to make the tests pass. The last test didn't pass because make install tries to install inside the CONDA_PREFIX, but actually the conda-forge automated build system wants to install it into another temporary environment. As suggested, I needed to remove the default installation in the CONDA_PREFIX from the Cmakelists.txt.

In preparation to that PR and the recent PR3 on the adoption of Python 3.7 (https://github.com/conda-forge/galario-feedstock/pull/3).

All tests pass on CPU, galario is correctly installed in the directory provided in CMAKE_INSTALL_PREFIX or, if the option is not provided, make install complains.

I need to test on GPU.

[fredRos]

I'm a bit confused looking at https://circleci.com/gh/conda-forge/galario-feedstock/30?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link Seems that the shared objects and the python code are different subdirectories so they are not found, got it. But how is cmake configured? I'd guess the problem is rather in https://github.com/mtazzari/galario/blob/master/python/CMakeLists.txt#L57

The reason I'm trying to dig this out is to avoid forcing the user to set CMake_Install_Prefix. Anyways you say in the docs this would be required but I can build fine without it. So why don't we check and fail during cmake ..? The code to do this is simple

[mtazzari]

Thanks for looking into this!

But how is cmake configured? I'd guess the problem is rather in https://github.com/mtazzari/galario/blob/master/python/CMakeLists.txt#L57

Oh, I wasn't aware of this additional variable. I thought CMAKE_INSTALL_PREFIX was it. Can you explain what is then the difference between CMAKE_INSTALL_PREFIX and DEFAULT_PYTHON_PKG_DIR?

I also see there is PY_GALARIO_DIR, but I guess it is the hack for single/double precision and does not have to do with this issue, right?

The reason I'm trying to dig this out is to avoid forcing the user to set CMake_Install_Prefix. Anyways you say in the docs this would be required but I can build fine without it. So why don't we check and fail during cmake ..? The code to do this is simple

You can build fine but not install. If you don't set CMAKE_INSTALL_PREFIX, in the current branch, you get an error when trying to make install as it tries to install it in /. So, it's fine that cmake .. doesn't fail; it can be used to check that one has all dependencies in place, etc. Then, when the user then tries to install it, realises that CMAKE_INSTALL_PREFIX has to be specified to install galario.

[fredRos]

Looking into these cmake hacks makes me scratch my head more than once. Did it have to be that complicated? Let me recount the constraint I wanted to satisfy:

When inside an activated conda environment, I wanted cmake to automatically pick this up and install galario there.

It seems the user base for this use case was essentially me and you because now people can install galario from conda-forge and wouldn't build it themselves unless they want cuda support. The reason for the hacks was that it is nontrivial to find the right directories for conda so I wanted it to be automatic. I still see value in that.

I also see there is PY_GALARIO_DIR, but I guess it is the hack for single/double precision and does not have to do with this issue, right?

Yes

Can you explain what is then the difference between CMAKE_INSTALL_PREFIX and DEFAULT_PYTHON_PKG_DIR

CMAKE_INSTALL_PREFIX is the standard cmake way of setting the prefix with the usual subdirectories lib/, bin/ etc. The twist with python and general C++ code is that the python code is in some prefix below PREFIX/lib and there was a cmake module to extract this for me portably with include(PythonInstall). On my machine the two directories are

miniconda2/envs/galario3/lib
miniconda2/envs/galario3/lib/python3.7/

[fredRos]

@mtazzari Please see the commit msg I just pushed

[fredRos]

make install tries to install inside the CONDA_PREFIX, but actually the conda-forge automated build system wants to install it into another temporary environment.

I think we can now fully accomodate that but of course the build.sh has to set CMAKE_INSTALL_PREFIX properly. But that is a different PR

[mtazzari]

Commit 81f6fe8ae2dace7355d978a9fb9c6e9c1632f3ff adds instruction to install cmake from conda-forge, following #158

[mtazzari]

@fredRos I have addressed the minor changes involved and tested the new cmake instructions on my mac. PR ready to merge.

[mtazzari]

There are no outstanding requests or issues, all tests pass. I am gonna merge this PR so that I can make a release and work on the feedstock PR.