Opened 16 months ago

Closed 6 months ago

#30581 closed enhancement (duplicate)

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

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords: sd111
Cc: gh-tobiasdiez, jhpalmieri, dimpase 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 mkoeppe)

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 16 months ago by mkoeppe

  • Branch set to u/mkoeppe/pyproject_toml

comment:2 Changed 16 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Branch u/mkoeppe/pyproject_toml deleted
  • Dependencies set to #30580

comment:3 Changed 16 months ago by mkoeppe

  • Branch set to u/mkoeppe/pyproject_toml

comment:4 Changed 16 months ago by mkoeppe

  • Commit set to 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 16 months ago by mkoeppe

  • Cc gh-tobiasdiez added

comment:6 Changed 16 months ago by mkoeppe

  • Description modified (diff)

comment:7 Changed 15 months ago by mkoeppe

  • Dependencies changed from #30580 to #30580, #30712

comment:8 Changed 14 months ago by mkoeppe

  • Dependencies changed from #30580, #30712 to #30580
  • Description modified (diff)
  • Summary changed from src/pyproject.toml to Declare build system dependencies using src/pyproject.toml

comment:9 Changed 14 months ago by git

  • Commit changed from a05a537dff521aed6560694f26d3ec19a6f5a1dc to bbfc19eb5ed799c2f0302ecaecddd904a9cf3bf1

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 14 months ago by mkoeppe

  • Cc jhpalmieri dimpase added
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from Declare build system dependencies using src/pyproject.toml to sagelib: Declare build system dependencies using src/pyproject.toml

comment:11 follow-up: Changed 14 months ago by gh-tobiasdiez

  • Reviewers set to 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 14 months ago by mkoeppe

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

comment:13 Changed 14 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:14 Changed 14 months ago by mkoeppe

  • 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 12 months ago by mkoeppe

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 12 months ago by mkoeppe

  • Dependencies #30580 deleted
  • Description modified (diff)

comment:17 Changed 12 months ago by mkoeppe

  • Authors Matthias Koeppe deleted
  • Milestone changed from sage-9.3 to sage-duplicate/invalid/wontfix
  • Reviewers Tobias Diez, ... deleted
  • Status changed from needs_work to needs_review

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

comment:18 Changed 6 months ago by chapoton

  • Resolution set to duplicate
  • Status changed from needs_review to closed
Note: See TracTickets for help on using tickets.