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

sage.libs.singular: Do not fail if the Singular binary is not in PATH #33440

Closed
mkoeppe opened this issue Mar 1, 2022 · 31 comments
Closed

sage.libs.singular: Do not fail if the Singular binary is not in PATH #33440

mkoeppe opened this issue Mar 1, 2022 · 31 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Mar 1, 2022

This code from #29024

    os.environ["SINGULAR_BIN_DIR"] = dirname(which("Singular"))

assumes that Singular is in PATH

This causes errors when users attempt to use sagelib outside of the environment set up by sage-env.

https://groups.google.com/g/sage-support/c/XBxpl-JjaNU/m/FtRvcoW0AgAJ

Depends on #31292
Depends on #31296

CC: @orlitzky @dimpase @koffie

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 4cd3130

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/33440

@mkoeppe mkoeppe added this to the sage-9.6 milestone Mar 1, 2022
@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 1, 2022

comment:1

Fix should be via sage.features / #31296

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 1, 2022

Dependencies: #31292, #31296

@orlitzky
Copy link
Contributor

orlitzky commented Mar 1, 2022

comment:3

We have this problem in a lot of packages and we usually usesage_confto store one of two values: the system location, or the sage location -- depending on what's detected at ./configure time. The spkg-configure.m4 for singular already detects SINGULAR_BIN; we probably just need to record its dirname() if it's not in the system's PATH.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 1, 2022

comment:4

I don't know if we need it as a configuration variable because the configure script just searches for it in PATH.

@orlitzky
Copy link
Contributor

orlitzky commented Mar 1, 2022

comment:5

Replying to @mkoeppe:

I don't know if we need it as a configuration variable because the configure script just searches for it in PATH.

The SAGE_LOCAL location will be prepended to PATH during ./configure though, so that's the time to write it down. Then later the location is known regardless of what PATH contains.

@orlitzky
Copy link
Contributor

orlitzky commented Mar 1, 2022

comment:6

Replying to @orlitzky:

The SAGE_LOCAL location will be prepended to PATH during ./configure though, so that's the time to write it down. Then later the location is known regardless of what PATH contains.

Sorry, that's half nonsense. If we don't find Singular in the PATH, we know that it's in the SAGE_LOCAL location, and we put that value into sage_conf so that later we don't need to rely on PATH.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 1, 2022

@orlitzky
Copy link
Contributor

orlitzky commented Mar 1, 2022

Commit: f44f426

@orlitzky
Copy link
Contributor

orlitzky commented Mar 1, 2022

comment:8

I know you know this already, but I think adding geographically varied runtime checks for things that are known at build time is the wrong solution.


New commits:

6a63fa0src/sage/features/singular.py: New
f44f426src/sage/libs/singular/singular.pyx: Use sage.features.singular, do not fail on FeatureNotPresentError

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 1, 2022

Changed commit from f44f426 to 13649b8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 1, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

32ce831sage.features.Executable.absolute_path: New
88860d3sage.features.Executable.absolute_path: If SAGE_LOCAL != SAGE_VENV, search first in SAGE_VENV/bin, SAGE_LOCAL/bin, then PATH
fb00b68Merge #31296
13649b8src/sage/interfaces/singular.py: Use sage.features.singular.Singular().absolute_path()

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 1, 2022

comment:10

No objection if you want to provide SINGULAR_BIN via sage_conf

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 2, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8801359src/sage/features/__init__.py: Fix import
403bcc1sage.features.Executable: Fix up
16bc8ddsrc/sage/features/singular.py: New
97c4eb4src/sage/libs/singular/singular.pyx: Use sage.features.singular, do not fail on FeatureNotPresentError
09f710fsrc/sage/interfaces/singular.py: Use sage.features.singular.Singular().absolute_path()
9ed65d8src/sage/features/singular.py: Fix doctest output

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 2, 2022

Changed commit from 13649b8 to 9ed65d8

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 2, 2022

comment:12

Replying to @mkoeppe:

No objection if you want to provide SINGULAR_BIN via sage_conf

This variable would be used in src/sage/features/singular.py

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 2, 2022

Author: Matthias Koeppe, ...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 2, 2022

Changed commit from 9ed65d8 to 727ec53

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 2, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

8bb7e3asrc/sage/features/__init__.py: Relax doctest
9fad95eMerge #31292
727ec53Merge #31296

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 2, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

001f92aconfigure, pkgs/sage-conf, sage.env, sage.features.singular: Handle SINGULAR_BIN
a2a061csrc/sage/features/singular.py: Fix up doctest output

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 2, 2022

Changed commit from 727ec53 to a2a061c

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 2, 2022

Changed author from Matthias Koeppe, ... to Matthias Koeppe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 3, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

a93e9afsrc/sage/features/__init__.py: Simplify Executable.absolute_path
5b25c1dsage.features: Refactor StaticFile, Executable through a new base class FileFeature
6c35717sage.features.FileFeature: Replace method absolute_path by absolute_filename, with deprecation
9266709Merge #31292
ffead1dMerge #31296
4cd3130sage.interfaces.singular, sage.libs.singular: Use Singular().absolute_filename()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 3, 2022

Changed commit from a2a061c to 4cd3130

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 12, 2022

comment:18

Let's get this in please

@dimpase
Copy link
Member

dimpase commented Mar 12, 2022

comment:19

How does one test this? I tried the way given in the link, but it ends up as follows:

ima@hilbert /mnt/opt/Sage $ ./sage-dev/sage -python3 -m venv --system-site-packages venv
dima@hilbert /mnt/opt/Sage $ source venv/bin/activate
(venv) dima@hilbert /mnt/opt/Sage $ python3
Python 3.9.10 (main, Mar  4 2022, 14:46:20) 
[GCC 11.2.1 20211127] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from sage.all import *
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'sage'
>>> 

Am I doing something wrong?

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 12, 2022

comment:20

The method of setting up the venv that was tried in that post only works when Sage has built its own Python. One cannot make a venv from a venv.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 12, 2022

comment:21

You can test it by just using command lines like the ones I showed in #31296

@dimpase
Copy link
Member

dimpase commented Mar 12, 2022

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Mar 12, 2022

comment:22

OK, this works.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 12, 2022

comment:23

Thank you!

@vbraun
Copy link
Member

vbraun commented Mar 21, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants