Opened 2 years ago

Closed 16 months ago

#30581 closed enhancement (duplicate)

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

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords: sd111
Cc: Tobias Diez, John Palmieri, Dima Pasechnik Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: u/mkoeppe/pyproject_toml (Commits, GitHub, GitLab) Commit: bbfc19eb5ed799c2f0302ecaecddd904a9cf3bf1
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

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:

Change History (18)

comment:1 Changed 2 years ago by Matthias Köppe

Branch: u/mkoeppe/pyproject_toml

comment:2 Changed 2 years ago by Matthias Köppe

Authors: Matthias Koeppe
Branch: u/mkoeppe/pyproject_toml
Dependencies: #30580

comment:3 Changed 2 years ago by Matthias Köppe

Branch: u/mkoeppe/pyproject_toml

comment:4 Changed 2 years ago by Matthias Köppe

Commit: a05a537dff521aed6560694f26d3ec19a6f5a1dc

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

comment:5 Changed 2 years ago by Matthias Köppe

Cc: Tobias Diez added

comment:6 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:7 Changed 2 years ago by Matthias Köppe

Dependencies: #30580#30580, #30712

comment:8 Changed 2 years ago by Matthias Köppe

Dependencies: #30580, #30712#30580
Description: modified (diff)
Summary: src/pyproject.tomlDeclare build system dependencies using src/pyproject.toml

comment:9 Changed 2 years ago by git

Commit: a05a537dff521aed6560694f26d3ec19a6f5a1dcbbfc19eb5ed799c2f0302ecaecddd904a9cf3bf1

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

comment:10 Changed 2 years ago by Matthias Köppe

Cc: John Palmieri Dima Pasechnik added
Description: modified (diff)
Status: newneeds_review
Summary: Declare build system dependencies using src/pyproject.tomlsagelib: Declare build system dependencies using src/pyproject.toml

comment:11 Changed 2 years ago by Tobias Diez

Reviewers: Tobias Diez, ...

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 Changed 2 years ago by Matthias Köppe

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

comment:13 Changed 2 years ago by Matthias Köppe

Status: needs_reviewneeds_work

comment:14 Changed 2 years ago by Matthias Köppe

Keywords: sd111 added

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

comment:15 in reply to:  11 Changed 23 months ago by Matthias Köppe

Replying to gh-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

comment:16 Changed 23 months ago by Matthias Köppe

Dependencies: #30580
Description: modified (diff)

comment:17 Changed 23 months ago by Matthias Köppe

Authors: Matthias Koeppe
Milestone: sage-9.3sage-duplicate/invalid/wontfix
Reviewers: Tobias Diez, ...
Status: needs_workneeds_review

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

comment:18 Changed 16 months ago by Frédéric Chapoton

Resolution: duplicate
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.