Skip to content

Comments

Remove legacy direct installation of sagelib (configure --disable-editable --disable-wheels)#37973

Merged
vbraun merged 3 commits intosagemath:developfrom
mkoeppe:sagelib_remove_direct_installation
May 25, 2024
Merged

Remove legacy direct installation of sagelib (configure --disable-editable --disable-wheels)#37973
vbraun merged 3 commits intosagemath:developfrom
mkoeppe:sagelib_remove_direct_installation

Conversation

@mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented May 10, 2024 •
edited
Loading

This mode, no longer the default, was using the deprecated setup.py install.

With the change in this PR, --disable-editable now implies --enable-wheels.

This simplifies sage_setup: The entire "install cleaner", a relic from pre-modern Python packaging, is removed.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@github-actions
Copy link

github-actions bot commented May 10, 2024 •
edited
Loading

Documentation preview for this PR (built with commit 016fcf2; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 11, 2024

The failure in test-new is an unrelated CI malfunction: #37985

@antonio-rojas
Copy link
Contributor

antonio-rojas commented May 13, 2024

Looks fine as far as distro packaging is concerned. Seems to fix installing a pyc file that was somehow missing before:

/usr/lib/python3.12/site-packages/sage/ext_data/nbconvert/__pycache__/postprocess.cpython-312.pyc

Can't comment on the sage-the-distro specific changes.

@kiwifb
Copy link
Member

kiwifb commented May 13, 2024

Looks fine as far as distro packaging is concerned. Seems to fix installing a pyc file that was somehow missing before:

/usr/lib/python3.12/site-packages/sage/ext_data/nbconvert/__pycache__/postprocess.cpython-312.pyc

I have had a specific fix for that in sage-on-gentoo for ages (manual generation of optimization). I'll be glad if it fixes that.

Can't comment on the sage-the-distro specific changes.

Well, I can't either. I just welcome things that narrows the differences with sage-on-distros and that ticket does drop a lot of old baggages that I am quite happy to see off (goodbye clean).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 14, 2024

Seems to fix installing a pyc file that was somehow missing before: [...] sage/ext_data/nbconvert/__pycache__/postprocess.cpython-312.pyc

Thanks for testing. Yes, this is a weird one; parts of the sagelib build system consider this Python file a data file of sorts.
Glad that this changes makes it a bit easier for you.

@jhpalmieri
Copy link
Member

jhpalmieri commented May 14, 2024

Is SAGE_WHEELS documented anywhere?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 14, 2024

The variable itself is internal, but configure --enable-wheels and the commands make wheels and make pypi-wheels are documented in
src/doc/en/developer/packaging_sage_library.rst

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 17, 2024

Thanks!

vbraun pushed a commit to vbraun/sage that referenced this pull request May 18, 2024
446a4fc 
sagemathgh-37973: Remove legacy direct installation of sagelib (`configure --disable-editable --disable-wheels`)
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

This mode, no longer the default, was using the deprecated `setup.py
install`.

With the change in this PR, `--disable-editable` now implies `--enable-
wheels`.

This simplifies `sage_setup`: The entire "install cleaner", a relic from
pre-modern Python packaging, is removed.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37973
Reported by: Matthias Köppe
Reviewer(s):
@vbraun vbraun merged commit 7473e4f into sagemath:develop May 25, 2024
vbraun pushed a commit to vbraun/sage that referenced this pull request May 29, 2024
7576a07 
sagemathgh-37857: Replace special handling of optional extensions (bliss, coxeter3, ....)
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

We relieve the distribution **sagemath-standard** (in both versions -
`SAGE_ROOT/pkgs/sagemath-standard` and `SAGE_ROOT/src`) from the duty to
build "optional extensions" based on what Sage packages are installed.

The installation is now done uniformly using the modularized
distributions **sagemath-bliss**, **sagemath-coxeter3** etc.

We introduce the missing features `coxeter3` and `sirocco` so that the
doctester does not have to rely on the sage-the-distro installation
records any more.

The wheels of the distributions now build correctly even when not going
through building an sdist first, which previously was required to apply
MANIFEST-based filtering. This is achieved using the new
`sage_setup.command.build_py`.

User-visible change:
- To install these options, use `./configure --enable-sagemath_bliss`
before building, or use `./sage -i sagemath_bliss` or `make
sagemath_bliss`.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->

- Depends on sagemath#37737 (merged here)
- Depends on sagemath#37973 (merged here)
    
URL: sagemath#37857
Reported by: Matthias Köppe
Reviewer(s): François Bissey
vbraun pushed a commit to vbraun/sage that referenced this pull request May 31, 2024
2d5477e 
sagemathgh-37857: Replace special handling of optional extensions (bliss, coxeter3, ....)
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

We relieve the distribution **sagemath-standard** (in both versions -
`SAGE_ROOT/pkgs/sagemath-standard` and `SAGE_ROOT/src`) from the duty to
build "optional extensions" based on what Sage packages are installed.

The installation is now done uniformly using the modularized
distributions **sagemath-bliss**, **sagemath-coxeter3** etc.

We introduce the missing features `coxeter3` and `sirocco` so that the
doctester does not have to rely on the sage-the-distro installation
records any more.

The wheels of the distributions now build correctly even when not going
through building an sdist first, which previously was required to apply
MANIFEST-based filtering. This is achieved using the new
`sage_setup.command.build_py`.

User-visible change:
- To install these options, use `./configure --enable-sagemath_bliss`
before building, or use `./sage -i sagemath_bliss` or `make
sagemath_bliss`.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->

- Depends on sagemath#37737 (merged here)
- Depends on sagemath#37973 (merged here)
    
URL: sagemath#37857
Reported by: Matthias Köppe
Reviewer(s): François Bissey
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kiwifb kiwifb Awaiting requested review from kiwifb

@antonio-rojas antonio-rojas Awaiting requested review from antonio-rojas

@kwankyu kwankyu Awaiting requested review from kwankyu

@jhpalmieri jhpalmieri Awaiting requested review from jhpalmieri

Assignees

@mkoeppe mkoeppe

Labels

c: build v: large

Projects

None yet

Milestone

sage-10.4

Development

Successfully merging this pull request may close these issues.

5 participants

@mkoeppe @antonio-rojas @kiwifb @jhpalmieri @vbraun