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

PipPackageSystem: If EXTERNALLY-MANAGED, issue venv instructions; use Feature(spkg='pkg:pypi/DISTRO-NAME') #37250

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

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Feb 6, 2024

As discussed in #37231 (comment), when Sage is installed in a distro that protects the integrity of system Python using the PEP-0668 EXTERNALLY-MANAGED mechanism, to extend their system installation of Sage, users cannot use pip install (because of permissions and the EXTERNALLY-MANAGED mechanism) nor pip install --user (because the user scheme is disabled).

Here we detect this situation and provide instructions that follow the best practices:

sage: from sage.features import PythonModule
sage: PythonModule('admcycles', spkg='admcycles').require()
FeatureNotPresentError: admcycles is not available.
Failed to import `admcycles`: No module named 'admcycles'

No equivalent system packages for void are known to Sage.

To install admcycles using the pip package manager:
Note that this Sage is installed in an externally managed Python environment.
It is recommended to first create a virtual environment:
  !sage -python -m venv --system-site-packages ~/.sage/venv
Then quit the current Sage:
  exit
Next, in the shell, activate the new virtual environment:
 $ . ~/.sage/venv/bin/activate
 $ pip install admcycles
Finally, start Sage from the new virtual environment:
 $ sage
To exit the virtual environment after use:
 $ deactivate
sage:                                                                                                                                                                                                                             

After #37500, sage --package commands accept PURLs (strings of the form pkg:pypi/DISTRO-NAME as a nickname for the SPKG that have DISTRO-NAME in their install-requires.txt or requirements.txt); see https://peps.python.org/pep-0725/#concrete-package-specification-through-purl
We change Feature definitions to use these PURLs instead of the SPKG names.

These changes allow FeatureNotPresentError to issue the pip installation advice even in downstream deployments that do not have SAGE_ROOT/build/pkgs etc.

📝 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 Feb 6, 2024
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 6, 2024

@tornaria @orlitzky @kiwifb
This still invokes the sage-get-system-packages script (and hence uses the build/pkgs/*/install-requires.txt, build/pkgs/*/requirements.txt files, just like it needs build/pkgs/*/distros/{gentoo,void,...}.txt to provide the system package information).
Probably you don't want to install these files (even if made available as a pip-installable distribution sage-bootstrap, #31662), so what should we do?

@orlitzky
Copy link
Contributor

orlitzky commented Feb 6, 2024

I don't really have a problem with the way it works now. Isolating a python program from the python environment gets a "well, don't do that" from me. We have --enable-system-site-packages now as a band-aid, but it should be the default. Either way, none of this pseudo-package-management stuff belongs in sagelib. It can only hinder the one approach that actually works well.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 6, 2024

Are you sure you're posting on the right PR?

@orlitzky
Copy link
Contributor

orlitzky commented Feb 6, 2024

Yeah. We're trying to figure out a goofy hack (pip instructions) to an existing goofy hack (package management in sage) that is only needed because sage-the-distribution has historically caused problems for itself.

The best long-term solution is for people to be able to pip install sagelib, or emerge sagelib, and have everything just work normally. In other words, to not use sage-the-distribution. It's cool if we can make the distro experience better in the meantime, but not at the expense of the thing that might actually work right some day (sagelib).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 7, 2024

No, you're missing the point. That users need info how to install components is independent of how Sage is installed or deployed.
My request for input is exactly how, from the viewpoint of downstream packagers, we should provide the necessary cross-referencing information -- currently only available in SAGE_ROOT/build/pkgs and therefore, at least currently, not available at the runtime of Sage in downstream packaging.

@orlitzky
Copy link
Contributor

orlitzky commented Feb 7, 2024

No, you're missing the point. That users need info how to install components is independent of how Sage is installed or deployed. My request for input is exactly how, from the viewpoint of downstream packagers, we should provide the necessary cross-referencing information -- currently only available in SAGE_ROOT/build/pkgs and therefore, at least currently, not available at the runtime of Sage in downstream packaging.

I understand. What it should say is please install <package>, and give the name of the upstream package. If sage is installed via pip, or the system package manager, or using meson install, that should suffice -- however they decide to install it. Sage-the-distro won't be able to see user packages because it poisons PYTHONUSERBASE but... well, don't do that.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 7, 2024

Thanks for the clarification, that's all certainly very consistent. And I'll guess within this framework, "the one approach that actually works well" = "compiling monolithically from source on the user's (= developer's) machine with a static configuration of everything known at compile time"?

@orlitzky
Copy link
Contributor

orlitzky commented Feb 7, 2024

Thanks for the clarification, that's all certainly very consistent. And I'll guess within this framework, "the one approach that actually works well" = "compiling monolithically from source on the user's (= developer's) machine with a static configuration of everything known at compile time"?

In this case all I meant was building and installing sagelib "normally," outside of sage-the-distribution. Though, yes, using the standard meson setup to do what it is designed to do would eventually be more reliable and efficient than sage.features. In this case, passing -Denable_admcycles=true could add the package as a dependency of sagelib and enable the integration code. Then there's no need to worry about how precisely it's supposed to appear one day at runtime. And no risk of sage silently changing if the package was installed for some other reason and is subsequently removed.

@tornaria
Copy link
Contributor

tornaria commented Feb 7, 2024

Copied from #37231 (comment):

Anyway, I discovered that sage already supports this idea. That is why PYTHONUSERBASE is set:

$ sage -pip install --break-system-packages admcycles
Defaulting to user installation because normal site-packages is not writeable
Collecting admcycles
  Using cached admcycles-1.4-py3-none-any.whl (351 kB)
Installing collected packages: admcycles
Successfully installed admcycles-1.4

$ ls ~/.sage/local/lib/python3.12/site-packages/
admcycles  admcycles-1.4.dist-info

$ sage -q
sage: from sage.features import PythonModule
sage: PythonModule("admcycles").is_present()
FeatureTestResult('admcycles', True)


$ python -m admcycles     #### CHECK THAT IT DID NOT MESS WITH MY SYSTEM PYTHON
/usr/bin/python: No module named admcycles

So this gives a possible way to move forward.

@tornaria
Copy link
Contributor

tornaria commented Feb 8, 2024

@tornaria @orlitzky @kiwifb This still invokes the sage-get-system-packages script (and hence uses the build/pkgs/*/install-requires.txt, build/pkgs/*/requirements.txt files, just like it needs build/pkgs/*/distros/{gentoo,void,...}.txt to provide the system package information). Probably you don't want to install these files (even if made available as a pip-installable distribution sage-bootstrap, #31662), so what should we do?

Maybe the feature should be PythonModule('module_name', distribution='distro-name') in case it is available from pypi? (analogous to spkg='spkg_name').

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 8, 2024

Maybe the feature should be PythonModule('module_name', distribution='distro-name') in case it is available from pypi? (analogous to spkg='spkg_name').

Yes, that would be a possible solution, thanks for the input!

If we go with this solution, I would extend sage-print-system-package-command with an option --pypi to indicate that inputs are PyPI distribution names. (Likewise sage-get-system-packages.) These scripts would then map it back to the SPKG name (via the contents of install-requires.txt or requirements.txt; note #31136) and from there to the name of the Debian, Arch, ... package, whatever is needed.

The more verbose solution PythonModule('module_name', spkg='spkg-name', distribution='distro-name') would avoid the "mapping back" step, but it would duplicate information that is already in build/pkgs.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 8, 2024

Maybe the feature should be PythonModule('module_name', distribution='distro-name') in case it is available from pypi? (analogous to spkg='spkg_name').

@tornaria I have implemented a version of this now. Using spkg='pypi:distro-name' though, which flows through the scripts more easily

@mkoeppe mkoeppe changed the title PipPackageSystem: If EXTERNALLY-MANAGED, issue venv instructions PipPackageSystem: If EXTERNALLY-MANAGED, issue venv instructions; support Feature(spkg='pypi:DISTRO-NAME') Feb 10, 2024
@mkoeppe mkoeppe requested a review from videlec February 14, 2024 08:00
@mkoeppe mkoeppe requested a review from kiwifb February 25, 2024 18:32
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 26, 2024

Instead of the ad-hoc nickname scheme pypi:DISTRO-NAME invented here, it's probably better to use PURLs, see https://peps.python.org/pep-0725/#concrete-package-specification-through-purl, pkg:pypi/DISTRO-NAME.

@mkoeppe mkoeppe changed the title PipPackageSystem: If EXTERNALLY-MANAGED, issue venv instructions; support Feature(spkg='pypi:DISTRO-NAME') PipPackageSystem: If EXTERNALLY-MANAGED, issue venv instructions; support Feature(spkg='pkg:pypi/DISTRO-NAME') Feb 26, 2024
Copy link

github-actions bot commented Feb 27, 2024

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

vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 28, 2024
…rt PURLs `pkg:pypi/DISTRO-NAME`, obtain dependencies of wheels from PyPI

    
<!-- ^ 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 make it possible to refer to Python packages via their PURL (see
[draft PEP 725](https://peps.python.org/pep-0725/#concrete-package-
specification-through-purl)) instead of their SPKG name.

For now a string of the form `pkg:pypi/DISTRO-NAME` is simply a nickname
for the (unique) SPKG that has DISTRO-NAME in their `install-
requires.txt` or `requirements.txt`. The scheme can also be omitted:
`pypi/DISTRO-NAME` also works. And we also map `pkg:generic/PACKAGE-
NAME` to `PACKAGE_NAME`.

Based on code by @culler, `sage --package create --pypi` now also fills
`dependencies` from the PyPI metadata of wheel packages. When some of
the Python dependencies obtained in this way do not have SPKGs yet, they
are also automatically created.

- Preparation for sagemath#31136.
- Split out from sagemath#37250.


### 📝 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 accordingly.

### ⌛ 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#37500
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Marc Culler, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request May 12, 2024
…rt PURLs `pkg:pypi/DISTRO-NAME`, obtain dependencies of wheels from PyPI

    
<!-- ^ 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 make it possible to refer to Python packages via their PURL (see
[draft PEP 725](https://peps.python.org/pep-0725/#concrete-package-
specification-through-purl)) instead of their SPKG name.

For now a string of the form `pkg:pypi/DISTRO-NAME` is simply a nickname
for the (unique) SPKG that has DISTRO-NAME in their
`version_requirements.txt` or `requirements.txt`. The scheme can also be
omitted: `pypi/DISTRO-NAME` also works. And we also map
`pkg:generic/PACKAGE-NAME` to `PACKAGE_NAME`.

Based on code by @culler, `sage --package create --pypi` now also fills
`dependencies` from the PyPI metadata of wheel packages. When some of
the Python dependencies obtained in this way do not have SPKGs yet, they
are also automatically created.

- Preparation for sagemath#31136.
- Split out from sagemath#37250.


### 📝 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 accordingly.

### ⌛ 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#37500
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Marc Culler, Matthias Köppe
@mkoeppe mkoeppe changed the title PipPackageSystem: If EXTERNALLY-MANAGED, issue venv instructions; support Feature(spkg='pkg:pypi/DISTRO-NAME') PipPackageSystem: If EXTERNALLY-MANAGED, issue venv instructions; use Feature(spkg='pkg:pypi/DISTRO-NAME') May 31, 2024
@antonio-rojas
Copy link
Contributor

antonio-rojas commented Sep 15, 2024

For distros that implement EXTERNALLY-MANAGED, the message provided in the EXTERNALLY-MANAGED file should be displayed, instead of some generic instructions that may or may not be suitable for the given distro. At least on Arch, the proposed message is not the recommended way to proceed.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 26, 2024

@antonio-rojas What does Arch recommend?

@antonio-rojas
Copy link
Contributor

@antonio-rojas What does Arch recommend?

To install the distro packages using the package manager

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.

4 participants