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

Use meson instead of configure for conda install #36489

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 14 additions & 9 deletions .github/workflows/ci-conda.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,20 @@ jobs:
conda info
conda list

- name: Configure
- name: Bootstrap
shell: bash -l {0}
continue-on-error: true
run: |
./bootstrap
echo "::add-matcher::.github/workflows/configure-systempackage-problem-matcher.json"
./configure --enable-build-as-root --with-python=$CONDA_PREFIX/bin/python --prefix=$CONDA_PREFIX --enable-system-site-packages $(for pkg in $(./sage -package list :standard: --has-file spkg-configure.m4 --has-file distros/conda.txt --exclude rpy2); do echo --with-system-$pkg=force; done)
echo "::remove-matcher owner=configure-system-package-warning::"
echo "::remove-matcher owner=configure-system-package-error::"
run: ./bootstrap

- name: Build
shell: bash -l {0}
run: |
# Use --no-deps and pip check below to verify that all necessary dependencies are installed via conda.
pip install --no-build-isolation --no-deps -v -v -e ./pkgs/sage-conf ./pkgs/sage-setup
echo "::group::sage-setup"
pip install --no-build-isolation --no-deps -v -v -e ./pkgs/sage-setup
echo "::endgroup::"
echo "::group::sage-conf"
pip install --no-build-isolation --no-deps -v -v -e ./pkgs/sage-conf_meson
echo "::endgroup::"
pip install --no-build-isolation --no-deps --config-settings editable_mode=compat -v -v -e ./src
env:
SAGE_NUM_THREADS: 2
Expand All @@ -95,6 +94,12 @@ jobs:
shell: bash -l {0}
run: ./sage -t --all -p0

# We keep this step for now to make sure that the configure-based setup still works.
- name: Test configure
shell: bash -l {0}
run: |
./configure --enable-build-as-root --with-python=$CONDA_PREFIX/bin/python --prefix=$CONDA_PREFIX --enable-system-site-packages $(for pkg in $(./sage -package list :standard: --has-file spkg-configure.m4 --has-file distros/conda.txt --exclude rpy2); do echo --with-system-$pkg=force; done) || true
tobiasdiez marked this conversation as resolved.
Show resolved Hide resolved

- name: Print logs
if: always()
run: |
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -284,3 +284,6 @@ src/.coverage/
# git worktree
worktree*
**/worktree*

# meson build directory
builddir
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,6 @@
"sagemath",
"Cython"
],
"editor.formatOnType": true
"editor.formatOnType": true,
"esbonio.sphinx.confDir": ""
Copy link
Member

Choose a reason for hiding this comment

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

What does this change do? Does it have a relation to the goal of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vscode is adding this every time I'm opening the project. I was bound to commit it at some point ;-). I doesn't hurt...

}
1 change: 1 addition & 0 deletions bootstrap-conda
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ echo >&2 $0:$LINENO: generate conda environment files
) > environment-template.yml
(
sed 's/name: sage-build/name: sage/' environment-template.yml
echo " - meson"
echo " # Additional packages providing all dependencies for the Sage library"
for pkg in $SAGELIB_SYSTEM_PACKAGES; do
echo " - $pkg"
Expand Down
10 changes: 10 additions & 0 deletions meson.build
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
project('sage', 'c')
Copy link
Member

Choose a reason for hiding this comment

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

I'd be more comfortable with this file being local to sage-conf_meson, so as not to create expectations that Sage-the-distribution will switch to meson

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea in the middle to long term is to support running meson compile from the root directory to compile sagelib with meson (using conda). For this the meson build file needs to be there (and its pretty much convention to have such a build file in the root). Sage-the-distro supporting meson would mean that there is a meson file in the build directory, or not?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not aware of such a plan, and I'd suggest that if you have a plan to use a new Issue in which you explain it.

From what you wrote above, it seems unlikely that it would be the same meson.build file that would be suitable for both purposes, so nothing is gained by putting it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's part of #34630. In the long-term you want to use of course meson-python (configured via pyproject.toml). But in the short to midterm its easier to just call meson directly. That's at least how scipy/numpy proceeded: https://github.com/scipy/scipy/pull/14847/files#diff-e42998d51257e6f84aa51ffa5f4ed1544cb12f97a94978c44f3bc281b5324d79R390-R500

And yes, the meson file added in this PR can be used for this purpose. In fact, I believe we can get ride of the conf and setup packages, but of course there still needs to be some way to provide the runtime-config (probably by just putting the generated conf file in the correct place in the builddir).

Copy link
Member

Choose a reason for hiding this comment

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

Well, #34630 has nothing to with stuff that belongs in SAGE_ROOT.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it's for compiling the Sage library, for the single use case of the all-conda install?

There is nothing conda-specific about the meson files, it's only that conda provides a nice fixed environment and thus is a good starting point. You can try to run meson setup outside of a conda environment and it should work (if you have all the necessary packages installed in the system).

Copy link
Member

Choose a reason for hiding this comment

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

I'll explain and summarize once more:

  • What you just described ("it should work (if you have all the necessary packages installed in the system).") means that you are configuring / building the Sage library.
  • This is different from what our configure.ac and Makefile do. They configure / build the Sage distribution.
  • There is no plan to switch the Sage distribution to using meson.
  • When people see a file meson.build next to configure.ac and Makefile, they are bound to think that using meson is an alternative for building the same thing. So this has the potential to mislead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • When people see a file meson.build next to configure.ac and Makefile, they are bound to think that using meson is an alternative for building the same thing. So this has the potential to mislead.

So if you see a hammer and a saw next to each other you are bound to see them as alternatives for building the same thing?

Conceptually, meson setup does the same thing as configure: go through the declared dependencies and write out a config file. Similarly, meson compile will do the same thing as make sage-all (or whatever the correct target is for building sagelib).

It's also not technically possible to move the meson file in the root to src since meson's subdir only walks down the tree.

Anyway, I'm tired of this stupid discussion and will stop participating in it. If you think that the harm created by the meson file in the root outweighs the positives of this PR, then please go ahead and continue blocking it. Please leave the "ready for review" tag on, as there are no work items that referee and PR creator agree on (it just that you don't like a small design choice in the PR).

Copy link
Member

Choose a reason for hiding this comment

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

Similarly, meson compile will do the same thing as make sage-all (or whatever the correct target is for building sagelib).

The target make sage-all does not exist. make all, the default target, builds the Sage distribution.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, but there are a lot of files in the root folder that are not related to the distribution.
[...] tox ini [...]

Actually SAGE_ROOT/tox.ini defines the portability tests of the Sage distribution.

cc = meson.get_compiler('c')

maxima = find_program('maxima', required: true)
ecl_config = find_program('ecl-config', required: true)
openmp = dependency('openmp', required : false)
#ntl = dependency('ntl', required: true) doesn't work, so ask the compiler directly:
ntl = cc.find_library('ntl', required: true)

subdir('pkgs/sage-conf_meson')
7 changes: 7 additions & 0 deletions pkgs/sage-conf/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
[build-system]
requires = ["setuptools", "wheel"]
build-backend = "setuptools.build_meta"

[project]
name = "sage-conf"
dynamic = ["version"]

[tool.setuptools.dynamic]
version = {file = ["VERSION.txt"]}
tobiasdiez marked this conversation as resolved.
Show resolved Hide resolved
1 change: 0 additions & 1 deletion pkgs/sage-conf/setup.cfg
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
[metadata]
name = sage-conf
version = file: VERSION.txt
description = Sage: Open Source Mathematics Software: Configuration module for the SageMath library
long_description = file: README.rst
license = GNU General Public License (GPL) v3 or later
Expand Down
6 changes: 6 additions & 0 deletions pkgs/sage-conf_meson/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/sage_conf/builddir
/build
/dist
/*.egg-info
/.tox
/bin/sage-env-config
1 change: 1 addition & 0 deletions pkgs/sage-conf_meson/README.rst
1 change: 1 addition & 0 deletions pkgs/sage-conf_meson/VERSION.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
10.2.beta8
32 changes: 32 additions & 0 deletions pkgs/sage-conf_meson/meson.build
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
conf_data = configuration_data()

conf_data.set('PACKAGE_VERSION', '1.2.3')

conf_data.set('SAGE_MAXIMA', maxima.path())
# Conda's ecl does not have any problems with Maxima, so nothing needs to be set here:
conf_data.set('SAGE_MAXIMA_FAS', '')

# Kenzo cannot yet be provided by the system, so we always use the SAGE_LOCAL path for now.
conf_data.set('SAGE_KENZO_FAS', '\'${prefix}\'/lib/ecl/kenzo.fas')

conf_data.set('SAGE_ARB_LIBRARY', 'arb')

# It can be found, so we don't have to set anything here:
conf_data.set('NTL_INCDIR', '')
conf_data.set('NTL_LIBDIR', '')

conf_data.set('SAGE_ECL_CONFIG', ecl_config.path())

conf_data.set('SAGE_ARCHFLAGS', 'unset')

# not needed when using conda, as we then don't build any pc files
conf_data.set('SAGE_PKG_CONFIG_PATH', '')

if openmp.found()
conf_data.set('OPENMP_CFLAGS', '-fopenmp')
conf_data.set('OPENMP_CXXFLAGS', '-fopenmp')
endif

configure_file(input : '../sage-conf_conda/_sage_conf/_conf.py.in',
output : '_conf.py',
configuration : conf_data)
1 change: 1 addition & 0 deletions pkgs/sage-conf_meson/pyproject.toml
9 changes: 9 additions & 0 deletions pkgs/sage-conf_meson/sage_conf/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import importlib

from .builddir.build_info import CONDA_PREFIX

conf = importlib.import_module('.builddir.pkgs.sage-conf_meson._conf', 'sage_conf')
conf_options = [x for x in conf.__dict__ if not x.startswith("_")]
globals().update({k: getattr(conf, k) for k in conf_options})

SAGE_LOCAL = CONDA_PREFIX
53 changes: 53 additions & 0 deletions pkgs/sage-conf_meson/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import os
import sys
from pathlib import Path

from setuptools import setup
from setuptools.command.build_py import build_py as setuptools_build_py
from setuptools.command.editable_wheel import (
editable_wheel as setuptools_editable_wheel,
)
from setuptools.errors import SetupError


class build_py(setuptools_build_py):
def run(self):
here = Path(__file__).parent
if self.editable_mode:
root = here.parent.parent
else:
raise SetupError('Not supported')

conda_prefix = os.environ.get('CONDA_PREFIX', '')
if not conda_prefix:
raise SetupError(
'No conda environment is active. '
'See https://doc.sagemath.org/html/en/installation/conda.html on how to get started.'
)

builddir = here / "sage_conf" / "builddir"
cmd = f"cd {root} && meson setup {builddir} --wipe"
print(f"Running {cmd}")
sys.stdout.flush()
if os.system(cmd) != 0:
raise SetupError("configure failed")

# Write build info
with open(builddir / 'build_info.py', 'w', encoding="utf-8") as build_info:
build_info.write(f'SAGE_ROOT = "{root}"\n')
build_info.write(f'CONDA_PREFIX = "{conda_prefix}"\n')


class editable_wheel(setuptools_editable_wheel):
r"""
Customized so that exceptions raised by our build_py
do not lead to the "Customization incompatible with editable install" message
"""
_safely_run = setuptools_editable_wheel.run_command


setup(
cmdclass=dict(
build_py=build_py, editable_wheel=editable_wheel
)
)
5 changes: 4 additions & 1 deletion src/doc/en/installation/conda.rst
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,12 @@ Here we assume that you are using a git checkout.
- Bootstrap the source tree and install the build prerequisites and the Sage library::

$ ./bootstrap
$ pip install --no-build-isolation -v -v --editable ./pkgs/sage-conf_conda ./pkgs/sage-setup
$ pip install --no-build-isolation -v -v --editable ./pkgs/sage-conf_meson ./pkgs/sage-setup
$ pip install --no-build-isolation --config-settings editable_mode=compat -v -v --editable ./src

In case of errors, try to use ``sage-conf_conda`` instead of ``sage-conf_meson``,
and please report the problem by opening an issue on GitHub.

- Verify that Sage has been installed::

$ sage -c 'print(version())'
Expand Down