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

build/bin/sage-spkg: Add support for installing script packages #36747

Merged

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Nov 22, 2023

Split out and updated from #29386.

The section of build/make/Makefile.in that deals with script packages can be consolidated with the section on normal packages later, in a follow-up PR after #36740, #36738.

We switch the spkg-install scripts of the sage* script packages to the templated versions (spkg-install.in), which is a simplification. spkg-check remains as is because we still invoke it directly (changing this will need #36738).

Resolves #29386.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • 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 accordingly.

⌛ Dependencies

@mkoeppe mkoeppe self-assigned this Nov 22, 2023
@mkoeppe mkoeppe force-pushed the t/29386/install_script_packages_via_sage_spkg branch from ca92398 to 6be9ba0 Compare November 22, 2023 04:27
@mkoeppe mkoeppe changed the title build/bin/sage-spkg: Add support for installing script packages, pip packages build/bin/sage-spkg: Add support for installing script packages Nov 22, 2023
@mkoeppe mkoeppe marked this pull request as draft November 22, 2023 06:23
@mkoeppe mkoeppe force-pushed the t/29386/install_script_packages_via_sage_spkg branch from d15561b to e35a72e Compare November 22, 2023 22:03
@mkoeppe mkoeppe force-pushed the t/29386/install_script_packages_via_sage_spkg branch from e35a72e to c008841 Compare November 23, 2023 00:16
@mkoeppe mkoeppe marked this pull request as ready for review November 23, 2023 00:19
@orlitzky
Copy link
Contributor

I get two errors under "normal" circumstances that don't hurt anything:

$ sage-spkg sage_conf
...
Package 'sage_conf' is currently not installed
No legacy uninstaller found for 'sage_conf'; nothing to do
Installing sage_conf-10.2.rc4
Looking in links: /home/mjo/src/sage.git/local/var/lib/sage/wheels
Processing /home/mjo/src/sage.git/pkgs/sage-conf
  Running command pip subprocess to install build dependencies
  Looking in links: /home/mjo/src/sage.git/local/var/lib/sage/wheels
  WARNING: Location '/home/mjo/src/sage.git/local/var/lib/sage/wheels' is ignored: it is either a non-existing path or lacks a specific scheme.
  ERROR: Could not find a version that satisfies the requirement setuptools (from versions: none)
  ERROR: No matching distribution found for setuptools
  error: subprocess-exited-with-error
  
  × pip subprocess to install build dependencies did not run successfully.
  │ exit code: 1
  ╰─> See above for output.
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  full command: /home/mjo/src/sage.git/local/var/lib/sage/venv-python3.11/bin/python3 /usr/lib/python3.11/site-packages/pip/__pip-runner__.py install --ignore-installed --no-user --prefix /tmp/pip-build-env-eto9101s/overlay --no-warn-script-location --no-binary :none: --only-binary :none: --no-index --find-links /home/mjo/src/sage.git/local/var/lib/sage/wheels -- setuptools wheel
  cwd: [inherit]
  Installing build dependencies ... error
error: subprocess-exited-with-error

× pip subprocess to install build dependencies did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.
Warning: building with "python3 -m pip wheel --wheel-dir=dist --verbose --no-deps --no-index --isolated --ignore-requires-python --find-links=/home/mjo/src/sage.git/local/var/lib/sage/wheels" failed.
Retrying with "python3 -m pip wheel --wheel-dir=dist --verbose --no-deps --no-index --isolated --ignore-requires-python --no-build-isolation --no-binary :all:".

The retry works, but the error is in bright red and always happens AFAICT so it would be better if, well, it didn't. It may have something to do with --enable-system-site-packages since I'm using almost everything (and in particular setuptools and wheel) from the system.

The other error can probably be fixed with an -f test?

$ sage-spkg-uninstall sage_conf
Uninstalling existing 'sage_conf'
Running pip-uninstall script for 'sage_conf'
ERROR: Could not open requirements file: [Errno 2] No such file or directory: '/home/mjo/src/sage.git/local/var/lib/sage/scripts/sage_conf/spkg-requirements.txt'
Warning: pip exited with status 0
Removing stamp file '/home/mjo/src/sage.git/local/var/lib/sage/installed/sage_conf-10.2.rc4'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 25, 2023

It may have something to do with --enable-system-site-packages since I'm using almost everything (and in particular setuptools and wheel) from the system.

The first try uses build isolation, so it needs the wheel file of setuptools.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 25, 2023

... so perhaps when using --enable-system-site-packages, you would want to disable build isolation?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 25, 2023

The other error can probably be fixed with an -f test?

$ sage-spkg-uninstall sage_conf
Uninstalling existing 'sage_conf'
Running pip-uninstall script for 'sage_conf'
ERROR: Could not open requirements file: [Errno 2] No such file or directory: '/home/mjo/src/sage.git/local/var/lib/sage/scripts/sage_conf/spkg-requirements.txt'
Warning: pip exited with status 0
Removing stamp file '/home/mjo/src/sage.git/local/var/lib/sage/installed/sage_conf-10.2.rc4'

How do you provoke this error? I don't see it

@orlitzky
Copy link
Contributor

How do you provoke this error? I don't see it

$ ./bootstrap
$ ./configure <my long list of configure flags>
$ make sage_conf  # install python deps to prevent pip fails
$ sage -sh
$ sage-spkg sage_conf  # reinstall
$ sage-spkg-uninstall sage_conf

The reinstall may not be necessary, but those were the steps I used.

@orlitzky
Copy link
Contributor

... so perhaps when using --enable-system-site-packages, you would want to disable build isolation?

I think so. Is there an easy way to do this across the board? If not, it's not that big of a deal. The make that most people use outputs the error in black and white and is a lot less scary.

@orlitzky
Copy link
Contributor

My ./configure flags can be found here:

https://gitweb.michael.orlitzky.com/?p=bash.d.git;a=blob;f=sage.sh;hb=HEAD

I've obviously got --enable-system-site-packages, but maybe --disable-editable is relevant too, who knows.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 26, 2023

Oh, that's #36737. I'll merge it here

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 26, 2023

... so perhaps when using --enable-system-site-packages, you would want to disable build isolation?

I think so. Is there an easy way to do this across the board?

It all goes through build/bin/sage-dist-helpers (sdh_pip_install, sdh_pip_editable_install); for the first one, just set build_isolation_option to a different default for this case. (Separate PR)

@orlitzky
Copy link
Contributor

Yep, that fixed it.

sage-logger -p '$$(SAGE_ROOT)/build/pkgs/$(1)/spkg-install && if [ $$$$SAGE_CHECK != no -a -x $$(SAGE_ROOT)/build/pkgs/$(1)/spkg-check ]; then $$(SAGE_ROOT)/build/pkgs/$(1)/spkg-check; fi' '$$(SAGE_LOGS)/$(1)-$(2).log' && \
rm -f "$$($(4))/$(SPKG_INST_RELDIR)/$(1)"-* && \
touch "$$($(4))/$(SPKG_INST_RELDIR)/$(1)-$(2)"; \
sage-logger -p 'SAGE_CHECK=$$(SAGE_CHECK_$(1)) PATH=$$(SAGE_SRC)/bin:$$($(4))/bin:$$$$PATH $$(SAGE_SPKG) $$(SAGE_SPKG_OPTIONS) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$$$$PATH 💪

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 26, 2023

Thank you!

@vbraun
Copy link
Member

vbraun commented Dec 10, 2023

merge conflict

…spkg

SageMath version 10.3.beta1, Release Date: 2023-12-10
@mkoeppe mkoeppe force-pushed the t/29386/install_script_packages_via_sage_spkg branch from 4cde017 to 32d95d2 Compare December 11, 2023 04:34
Copy link

Documentation preview for this PR (built with commit 32d95d2; changes) is ready! 🎉

@vbraun vbraun merged commit 6cadd84 into sagemath:develop Dec 14, 2023
18 of 25 checks passed
@mkoeppe mkoeppe added this to the sage-10.3 milestone Dec 14, 2023
@mkoeppe mkoeppe deleted the t/29386/install_script_packages_via_sage_spkg branch December 14, 2023 03:32
vbraun pushed a commit to vbraun/sage that referenced this pull request Mar 30, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
`make SPKG-check` runs the testsuite of `SPKG` via sage-spkg. This
builds on
- sagemath#36738
- sagemath#36747

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [ ] The title is concise, informative, and self-explanatory.
- [ ] 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 accordingly.

### ⌛ Dependencies

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

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#37022
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install script and pip packages via sage-spkg
3 participants