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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

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

wants to merge 3 commits into from

Conversation

mkoeppe
Copy link
Member

@mkoeppe mkoeppe commented May 10, 2024 •
edited

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

@mkoeppe mkoeppe self-assigned this May 10, 2024
@mkoeppe mkoeppe mentioned this pull request May 10, 2024
Closed
@mkoeppe mkoeppe mentioned this pull request May 10, 2024
Open
5 tasks
@github-actions GitHub Actions
Copy link

github-actions bot commented May 10, 2024 •
edited

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
Member 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
Member 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
Member 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
Member 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
sagemathgh-37973: Remove legacy direct installation of sagelib (`conf…
446a4fc 
…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 pushed a commit to vbraun/sage that referenced this pull request May 24, 2024
sagemathgh-37973: Remove legacy direct installation of sagelib (`conf…
7473e4f 
…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):
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 s: positive review v: large
Projects
None yet
Milestone
sage-10.4
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mkoeppe @antonio-rojas @kiwifb @jhpalmieri @vbraun