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

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

Merged
merged 3 commits into from
May 25, 2024

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented May 10, 2024

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

Copy link

github-actions bot commented May 10, 2024

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

@mkoeppe mkoeppe force-pushed the sagelib_remove_direct_installation branch from 322c6d5 to c761dbb Compare May 11, 2024 03:50
@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 11, 2024

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

@mkoeppe mkoeppe force-pushed the sagelib_remove_direct_installation branch from 5108c80 to 016fcf2 Compare May 13, 2024 17:54
@antonio-rojas
Copy link
Contributor

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

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!

@mkoeppe mkoeppe added this to the sage-10.4 milestone May 18, 2024
vbraun pushed a commit to vbraun/sage that referenced this pull request May 18, 2024
…igure --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
15 of 19 checks passed
@mkoeppe mkoeppe deleted the sagelib_remove_direct_installation branch May 25, 2024 18:55
vbraun pushed a commit to vbraun/sage that referenced this pull request May 29, 2024
…iss, 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
…iss, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants