sagemath/sage

sagelib: Declare build system dependencies using src/pyproject.toml

Closed this issue · 25 comments

Even after #30580, setup.py still has an import-time dependency on Cython (via sage_setup).

We declare this build system dependency by adding the PEP 517 metadata (pyproject.toml).

Adding pyproject.toml does not change how the Sage distribution installs sagelib because build/pkgs/sagelib/spkg-install uses setup.py install directly.


References:

CC: @tobiasdiez @jhpalmieri @dimpase

Component: build

Keywords: sd111

Branch/Commit: u/mkoeppe/pyproject_toml @ bbfc19e

Issue created by migration from https://trac.sagemath.org/ticket/30581

Author: Matthias Koeppe

Changed branch from u/mkoeppe/pyproject_toml to none

Dependencies: #30580

Commit: a05a537

comment:4

Build fails with

    File "/private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-req-build-t5v70h8w/sage/misc/package.py", line 319, in installed_packages
      installed.update(pip_installed_packages())
    File "/private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-req-build-t5v70h8w/sage/misc/package.py", line 154, in pip_installed_packages
      for package in json.loads(stdout)}
    File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/__init__.py", line 357, in loads
      return _default_decoder.decode(s)
    File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/decoder.py", line 337, in decode
      obj, end = self.raw_decode(s, idx=_w(s, 0).end())
    File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/decoder.py", line 355, in raw_decode
      raise JSONDecodeError("Expecting value", s, err.value) from None
  json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
  ************************************************************************
  Error building the Sage library
  ************************************************************************

New commits:

2c1e648src/sage_setup/command/sage_build_cython.py: Remove top-level imports from sage.env, Cython
88c4e8csrc/setup.py: Make 'setup.py sdist' work without Cython
a05a537src/pyproject.toml: New

Description changed:

--- 
+++ 
@@ -1,2 +1,7 @@
 Adding PEP 517 metadata
 
+---
+
+References:
+- https://snarky.ca/what-the-heck-is-pyproject-toml/
+

Changed dependencies from #30580 to #30580, #30712

Changed dependencies from #30580, #30712 to #30580

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-Adding PEP 517 metadata
+Adding PEP 517 metadata for sagelib
 
 ---
 

Changed commit from a05a537 to bbfc19e

Branch pushed to git repo; I updated commit sha1. New commits:

6fe4d56Merge branch 't/30779/duplicate_src_setup_py' into t/30580/sage_setup__remove_import_time_dependency___setup_requires___on__pkgconfig____numpy_
698a6easrc/sage_setup/command/sage_build_cython.py: Remove top-level imports from sage.env, Cython
bb32e80build/pkgs/sagelib/src/setup.py: Make 'setup.py sdist' work without Cython
15a6c2bMerge branch 't/30580/sage_setup__remove_import_time_dependency___setup_requires___on__pkgconfig____numpy_' into t/30581/pyproject_toml
310ebd1Merge tag '9.3.beta1' into t/30581/pyproject_toml
bbfc19ebuild/pkgs/sagelib/src: Apply PEP 517 changes

Description changed:

--- 
+++ 
@@ -1,4 +1,7 @@
-Adding PEP 517 metadata for sagelib
+Even after #30580, `setup.py` still has an import-time dependency on `Cython` (via `sage_setup`). 
+
+We declare this build system dependency by adding the PEP 517 metadata (`pyproject.toml`).
+
 
 ---
 

Reviewer: Tobias Diez, ...

comment:11

Is the following addition really necessary?

+# PEP 517 builds do not have . in sys.path
+sys.path.insert(0, os.path.dirname(__file__))

Not having . in the path is needed for the build isolation. (I'm fine with adding it for the moment and removing it in a follow-up ticket.)

Otherwise looks good to me!

comment:12

I needed it when I first worked on this ticket; I'll double check if it's still necessary now. Thanks!

comment:14

Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111

Changed keywords from none to sd111

comment:15

Replying to @tobiasdiez:

Is the following addition really necessary?

+# PEP 517 builds do not have . in sys.path
+sys.path.insert(0, os.path.dirname(__file__))

Not having . in the path is needed for the build isolation. (I'm fine with adding it for the moment and removing it in a follow-up ticket.)

Yes, this is needed for finding sage.env, for example

Changed dependencies from #30580 to none

Description changed:

--- 
+++ 
@@ -2,6 +2,7 @@
 
 We declare this build system dependency by adding the PEP 517 metadata (`pyproject.toml`).
 
+Adding `pyproject.toml` does not change how the Sage distribution installs sagelib because `build/pkgs/sagelib/spkg-install` uses `setup.py install` directly.
 
 ---
 

Changed reviewer from Tobias Diez, ... to none

Changed author from Matthias Koeppe to none

comment:17

This ticket is hard to test separately, so I have merged it into #30913. We can close this one here.