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

Switch Python build system to use mesonpy instead of setuptools #24

Merged
merged 24 commits into from
Jan 22, 2024

Conversation

kjmeagher
Copy link
Member

@kjmeagher kjmeagher commented Nov 7, 2023

This PR solves a couple of problems at once. I got an email from a user complaining that the setuptools build system couldn't handle multiple photospline and boost being in different directories. Meson's pkg-config dependency detection is far superior to anything you can ad hoc program in setuptools. This allowed some cleanup that I had wanted to do regarding the meson build file. Also, one of the reasons that I gave up on making wheels for nuflux was that setuptools couldn't figure out how to include header files while mesonpy does this automatically. The files changed looks big but that is just because I also moved the python source directory to /src/.

The only change from the user perspective is that when compiling you now use exactly the same env vars for both meson and python. Documentation has been updated to reflect this. cvmfs target py3-v4.0.1 was dropped because the last version of meson that worked with python 3.6 doesn't support the pure option for python. And py3-v4.2.1 was dropped because mesonpy doesn't play well with the python misconfiguration present in that version of cvmfs.

also move python files to src because it was bugging me
setuptools was having a lot of problems understanding build
environments mesons superior pkg-config support fixes problems for
several people
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (07ab4b1) 56.36% compared to head (a7973b4) 56.94%.

Files Patch % Lines
src/library/SplineFlux.cpp 37.50% 2 Missing and 3 partials ⚠️
src/library/ANFlux.cpp 0.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #24      +/-   ##
==========================================
+ Coverage   56.36%   56.94%   +0.57%     
==========================================
  Files          13       13              
  Lines         715      713       -2     
  Branches      326      323       -3     
==========================================
+ Hits          403      406       +3     
+ Misses        130      128       -2     
+ Partials      182      179       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kjmeagher kjmeagher changed the title meson python Switch Python build system to use meson instead of setuptools Jan 16, 2024
@kjmeagher kjmeagher changed the title Switch Python build system to use meson instead of setuptools Switch Python build system to use mesonpy instead of setuptools Jan 16, 2024
@kjmeagher kjmeagher marked this pull request as ready for review January 16, 2024 04:14
Copy link

@mjlarson mjlarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things seem reasonable enough here, although I'm not an expert on meson. Pip works and building from source seems to generally result in a functional build. I did receive some warnings when building on cobalt with py3-v4.3.0:

(test_venv) mlarson@cobalt01 /tmp/mlarson/nuflux/nuflux $ ninja -C build                            
ninja: Entering directory `build'
[16/17] Compiling C++ object src/pybindings/_nuflux.cpython-311-x86_64-linux-gnu.so.p/module.cxx.o
In file included from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/detail/is_xxx.hpp:8,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/detail/is_auto_ptr.hpp:9,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/detail/copy_ctor_mutates_rhs.hpp:8,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/detail/value_arg.hpp:7,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/object/forward.hpp:10,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/object/pointer_holder.hpp:16,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/to_python_indirect.hpp:10,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/converter/arg_to_python.hpp:10,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/call.hpp:15,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/object_core.hpp:14,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/args.hpp:22,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python.hpp:11,
                 from ../src/pybindings/module.cxx:4:
/cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/detail/is_auto_ptr.hpp:17:40: warning: 'template<class> class std::auto_ptr' is deprecated: use 'std::unique_ptr' instead [-Wdeprecated-declarations]
   17 | BOOST_PYTHON_IS_XXX_DEF(auto_ptr, std::auto_ptr, 1)
      |                                        ^~~~~~~~
/cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/detail/is_xxx.hpp:20:4: note: in definition of macro 'BOOST_DETAIL_IS_XXX_DEF'
   20 |    qualified_name< BOOST_PP_ENUM_PARAMS_Z(1, nargs, T) >        \
      |    ^~~~~~~~~~~~~~
/cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/detail/is_auto_ptr.hpp:17:1: note: in expansion of macro 'BOOST_PYTHON_IS_XXX_DEF'
   17 | BOOST_PYTHON_IS_XXX_DEF(auto_ptr, std::auto_ptr, 1)
      | ^~~~~~~~~~~~~~~~~~~~~~~
In file included from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/spack/opt/spack/linux-centos7-x86_64_v2/gcc-4.8.5/gcc-13.1.0-5camztap56edgn7uluhb44htpco7ijzc/lib/gcc/x86_64-pc-linux-gnu/13.1.0/../../../../include/c++/13.1.0/memory:78,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/function/function_base.hpp:33,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/function/detail/prologue.hpp:18,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/function/function_template.hpp:13,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/function/detail/maybe_include.hpp:15,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/function/function0.hpp:11,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/errors.hpp:13,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/handle.hpp:11,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/args_fwd.hpp:10,
                 from /cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/include/boost/python/args.hpp:10:
/cvmfs/icecube.opensciencegrid.org/py3-v4.3.0/RHEL_7_x86_64/spack/opt/spack/linux-centos7-x86_64_v2/gcc-4.8.5/gcc-13.1.0-5camztap56edgn7uluhb44htpco7ijzc/lib/gcc/x86_64-pc-linux-gnu/13.1.0/../../../../include/c++/13.1.0/bits/unique_ptr.h:65:28: note: declared here
   65 |   template<typename> class auto_ptr;
      |                            ^~~~~~~~
[17/17] Linking target src/pybindings/_nuflux.cpython-311-x86_64-linux-gnu.so

and when running ninja,

...
Installing subdir /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_central_nue.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_central_numu.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_upper_numubar.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3a_central.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_upper_nuebar.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3a_central_nuebar.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_central_numubar.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_lower_nuebar.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_lower.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_lower_numu.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_upper_nue.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_upper.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_upper_numu.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_lower_nue.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3a_central_numu.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3a_central_nue.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_central.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_lower_numubar.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3a_central_numubar.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux/BERSS_H3p_central_nuebar.fits to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux
Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.
Installing subdir /tmp/mlarson/nuflux/nuflux/src/nuflux/data/SplineFlux2 to /tmp/mlarson/nuflux/test_venv/share/nuflux/SplineFlux2
...

It may be worth checking these to see if they can be handled better to avoid worrying users.

meson_options.txt Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@kjmeagher
Copy link
Member Author

The deprication warnings were from code in boost python. The only way to get rid of them is adding -Wno-deprecated which I did. The install warnings were coming from symlinks wich were unnescessary so I removed them. The readme and meson_options were updated to be more clear that the data directory is where the raw flux tables are stored and a issue was submitted to cmvfs about adding the needed envvars.

@kjmeagher kjmeagher merged commit daad99d into main Jan 22, 2024
10 of 11 checks passed
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.

2 participants