Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update casacore to 3.1.1 and make less version needy #19

Merged
merged 13 commits into from Jun 12, 2019

Conversation

gijzelaerr
Copy link
Member

unfortunately this still can't find the new boost python.

a minimal CMakeLists.txt like:

cmake_minimum_required(VERSION 3.13)
find_package(Boost REQUIRED COMPONENTS python)

still gives:

$  cmake ..
CMake Error at /usr/local/Cellar/cmake/3.13.1/share/cmake/Modules/FindBoost.cmake:2100 (message):
  Unable to find the requested Boost libraries.

  Boost version: 1.68.0

  Boost include path: /usr/local/include

  Could not find the following Boost libraries:

          boost_python

  No Boost libraries were found.  You may need to set BOOST_LIBRARYDIR to the
  directory containing Boost libraries or BOOST_ROOT to the location of
  Boost.
Call Stack (most recent call first):
  CMakeLists.txt:2 (find_package)


-- Configuring incomplete, errors occurred!
See also "/Users/gijs/bla/build/CMakeFiles/CMakeOutput.log".

with cmake 3.13.1 and boost 1.68.0 while it looks like this bug has been fixed since cmake 3.11.0

https://gitlab.kitware.com/cmake/cmake/commit/1673923c303c6a4184904c4c5849911feddb87e7

@gijzelaerr
Copy link
Member Author

gijzelaerr commented Dec 13, 2018

bingo!

https://cmake.org/cmake/help/v3.13/module/FindBoost.html

Note that Boost Python components require a Python version suffix (Boost 1.67 and later), e.g. python36 or python27 for the versions built against Python 3.6 and 2.7, respectively. This also applies to additional components using Python including mpi_python and numpy. Earlier Boost releases may use distribution-specific suffixes such as 2, 3 or 2.7. These may also be used as suffixes, but note that they are not portable.

@gijzelaerr
Copy link
Member Author

i've issued a PR with a fix upstream casacore/casacore#846

Boost 1.67 Python components require a Python version suffix.
The logic for this was added to the CMake machinery of the upstream
library in casacore/casacore#846, but this won't make it into a release
for a while. Add a patch in the meantime, to be removed on the next
release.
@gijzelaerr
Copy link
Member Author

we also need to make sure PYTHON2_LIBRARIES is correctly set, since by default something goes wrong:

-- PYTHON2_EXECUTABLE ......... = /usr/local/bin/python2
-- PYTHON2_LIBRARIES........... = /usr/lib/libpython2.7.dylib
-- PYTHON2_NUMPY_INCLUDE_DIRS . = /usr/local/lib/python2.7/site-packages/numpy/core/include
-- PYTHON2_Boost_LIBRARIES .... = /usr/local/lib/libboost_python27-mt.dylib
-- PYTHON2_Boost_INCLUDE_DIRS . = /usr/local/include
-- PYTHON3_EXECUTABLE ......... = /usr/local/bin/python3.7
-- PYTHON3_LIBRARIES .......... = /usr/local/Frameworks/Python.framework/Versions/3.7/lib/libpython3.7m.dylib
-- PYTHON3_NUMPY_INCLUDE_DIRS . = /usr/local/lib/python3.7/site-packages/numpy/core/include
-- PYTHON3_Boost_LIBRARIES .... = /usr/local/lib/libboost_python37-mt.dylib
-- PYTHON3_Boost_INCLUDE_DIRS . = /usr/local/include

@gijzelaerr
Copy link
Member Author

ah you already do that. @ludwigschwardt you are my hero.

@ludwigschwardt
Copy link
Contributor

I aim to please!

@ludwigschwardt
Copy link
Contributor

Anything more here? What about PYTHON2_LIBRARIES?

I guess it's picking up the system Python (associated with the default command python) instead of the brew-provided python2.

@gijzelaerr
Copy link
Member Author

yes, we need to make sure that is overwritten in the recipe

@ludwigschwardt
Copy link
Contributor

I'm busy extending this branch to catch up with the latest changes...

The wcslib formula moved back to homebrew/core in July 2018 after
brewsci grew stale. The boost-python library for Python 3 has a
separate formula since 1.66.0 (Feb 2018). Restore gcc dependency
since casacore looks for a Fortran compiler during cmake configure.

Since casacore 3.0 it is now mandatory to use C++11 features (see
casacore/casacore#580). Also ensure that undefined symbols are
ignored until runtime in the Python shared library.
Only apply the patch to current releases (3.0 and 3.1). The HEAD and
future 3.2 release already has it (due to casacore/casacore#846).
These provide more robust detection of Python versions on macOS and
replace the functionality of casacore's FindPython.cmake and
FindNUMPY.cmake. Previously the Homebrew Python 3 include dirs were
confused with those of Homebrew Python 2. There is also no need to
hardcode the Python executable and library (always a bad idea).

While these modules are available since cmake 3.12, they could be
copied to casacore and still run on cmake 3.7, in case the bigger
project has need of them.
The stable patch turns v3.0.0 into c349f27, effectively applying #846.
The original patch only included the first commit of this PR (d'oh).
The cmake patch now applies on top of #846 and also works on the latest
HEAD.
The boost-python3 formula has no --with-python option (that sounds
tautological anyway).
Use latest stable version from GitHub.
Follow the "Python for Formula Authors" document, which states that
'Python 2 libraries do not need a depends_on "python@2" declaration;
they will be built with the system Python, but should still be usable
with any other Python 2.7.' Thereby make the optional python@2 support
mandatory, since system Python 2.7 is always available.
@ludwigschwardt
Copy link
Contributor

This currently does v3.1.0 fine but I suspect HEAD is broken. Should we merge this so long and open another PR once 3.1.1 comes out? That would probably allow me to get rid of the patches in that PR.

@ludwigschwardt ludwigschwardt changed the title Update casacore to 3.0 and make less version needy Update casacore to 3.1 and make less version needy May 22, 2019
The HEAD no longer needs a patch (although it is currently running off
a feature branch to be merged as casacore/casacore#922). Now use a
single patch that turns v3.1.0 into HEAD (at least as far as the
Python build scripts are concerned).
@ludwigschwardt
Copy link
Contributor

The HEAD support now works again (based off casacore/casacore#922).

@ludwigschwardt
Copy link
Contributor

@gijzelaerr, if we can get casacore/casacore#922 merged in time for the 3.1.1 release, I can remove all the patches in this formula... 😉

Thanks to casacore/casacore#922 we can also drop all the patches.
@ludwigschwardt ludwigschwardt changed the title Update casacore to 3.1 and make less version needy Update casacore to 3.1.1 and make less version needy Jun 5, 2019
@ludwigschwardt
Copy link
Contributor

This works on my laptop with no patches now... Happy to merge if you are.

@ludwigschwardt ludwigschwardt merged commit ca26a14 into master Jun 12, 2019
@ludwigschwardt ludwigschwardt deleted the casacore30 branch June 12, 2019 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants