#29024 closed enhancement (fixed)
spkg-configure.m4 for singular
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.5 |
Component: | build: configure | Keywords: | sd111 |
Cc: | dimpase, fbissey, slelievre, isuruf, mjo, gh-tobiasdiez, arojas, gh-dkwo | Merged in: | |
Authors: | Michael Orlitzky, Samuel Lelièvre | Reviewers: | Dima Pasechnik, Matthias Koeppe |
Report Upstream: | Reported upstream. No feedback yet. | Work issues: | |
Branch: | f01ff1f (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
- For singular, the location of the dynamic library needs to be communicated to the sage runtime.
We add a configuration variable
SINGULAR_SO
tosage_conf.py.in
.
The filename can be found from
src/Singular/libSingular.la
(currently not installed):# The name that we can dlopen(3). dlname='libSingular-4.1.1.dylib' ... # Directory that this library needs to be installed in: libdir='/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/lib'
This will:
- reduce the need for downstream patching (see for example https://salsa.debian.org/science-team/sagemath/-/blob/master/debian/patches/d0-singular.patch),
- provide preparation for adding an
spkg-configure.m4
forsingular
.
src/bin/sage-env
unconditionally sets environment variableSINGULARPATH="$SAGE_LOCAL/share/singular"
If
SINGULARPATH
is already set by the user, perhaps the directory in $SAGE_LOCAL should be prepended instead of overwriting it.
(After #32254,
SINGULARPATH
is only used byqepcad
-- this last use should be removed in #31275.)
src/bin/sage-env
unconditionally sets environment variableSINGULAR_EXECUTABLE="$SAGE_LOCAL/bin/Singular"
.
It should be investigated whether this is actually needed in any supported configuration. If not, remove.
(Removed in #32302.)
src/sage/interfaces/singular.py
tries to useSINGULARPATH
as if it is a directory name (it is a colon-separated list of directories):singular_docdir = SINGULARPATH + "/../info/"
This should be fixed so that it works when
SINGULARPATH
is set by a user to a colon-separated list.
(Solved in #32254 - Obtain singular.hlp location via libsingular_resources)
Attachments (2)
Change History (175)
comment:1 follow-up: ↓ 2 Changed 2 years ago by
comment:2 in reply to: ↑ 1 Changed 2 years ago by
comment:3 Changed 2 years ago by
well, Debian does have a Singular version which is compatible with their Sage package (v8.6 or so, I guess).
comment:4 Changed 2 years ago by
- Milestone changed from sage-9.1 to sage-9.2
Thanks for the info. I'll check back when #25993 is done.
comment:5 follow-up: ↓ 6 Changed 2 years ago by
This ticket should be listed in #27330 (ATM singular is listed in "No Tickets yet").
comment:6 in reply to: ↑ 5 Changed 2 years ago by
Replying to gh-thierry-FreeBSD:
This ticket should be listed in #27330 (ATM singular is listed in "No Tickets yet").
I've added it
comment:7 Changed 2 years ago by
- Component changed from packages: standard to build: configure
comment:8 follow-up: ↓ 14 Changed 2 years ago by
[Copying elements from #29024]
- my 1st problem was a build failure with Singular taken from an external package: to solve it, Singular must be built
--with-ntl
: it would be fine to add this check tospkg-configure.m4
(but I do not know how!)
- my 2nd error was when building dochtml:
NameError: Singular library 'freegb.lib' not found
. To solve it,src/sage/env.py
andsrc/bin/sage-env
must be modified so thatSINGULARPATH
is set to singlar's share dir (and notSAGE_SHARE
/DESTDIR
), andSINGULAR_EXECUTABLE
is set to the real Singular and not under theDESTDIR
directory.
Now it is OK for me (on FreeBSD).
comment:9 Changed 2 years ago by
- Cc mjo added
comment:10 Changed 23 months ago by
- Description modified (diff)
comment:11 Changed 21 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:12 Changed 21 months ago by
the error message
raise RuntimeError( "libSingular not found--a working Singular install in $SAGE_LOCAL " "is required for Sage to work")
should be modified, too.
Perhaps just remove in $SAGE_LOCAL
comment:13 Changed 21 months ago by
Moving this ticket forward is great, but note that it is unrelated to the build problem on homebrew reported on sage-devel... and it is also unlikely that the homebrew version of singular actually works for us (homebrew does not like to patch).
As I explained in https://groups.google.com/d/msg/sage-devel/KXK_zxzfhIQ/RO3FryYWAwAJ, on the reporter's system the computed order of -I
options is wrong -- something that I was not able reproduce on my system.
comment:14 in reply to: ↑ 8 Changed 21 months ago by
Replying to gh-thierry-FreeBSD:
[Copying elements from #29024]
- my 1st problem was a build failure with Singular taken from an external package: to solve it, Singular must be built
--with-ntl
: it would be fine to add this check tospkg-configure.m4
(but I do not know how!)
- my 2nd error was when building dochtml:
NameError: Singular library 'freegb.lib' not found
. To solve it,src/sage/env.py
andsrc/bin/sage-env
must be modified so thatSINGULARPATH
is set to singlar's share dir (and notSAGE_SHARE
/DESTDIR
), andSINGULAR_EXECUTABLE
is set to the real Singular and not under theDESTDIR
directory.Now it is OK for me (on FreeBSD).
The fix in SageMath needs some programming - specifically, the code in singular.pyx calls dlopen on SINGULAR_SO, which is computed by _get_shared_lib_filename dynamically in env.py, and assuming that it is in SAGE_LOCAL. So this probably can instead by handled using cython_aliases from the same module (which does compute all the needed info for Singular, it seems). Or by fixing _get_shared_lib_filename so that it also searches elsewhere.
comment:15 Changed 21 months ago by
Is it known whether this comment added in 2009 to singular.pyx is still valid:
This initializes the SINGULAR library. This is a hack to some extent. SINGULAR has a concept of compiled extension modules similar to Sage. For this, the compiled modules need to see the symbols from the main program. However, SINGULAR is a shared library in this context these symbols are not known globally. The work around so far is to load the library again and to specify ``RTLD_GLOBAL``.
comment:16 follow-up: ↓ 17 Changed 21 months ago by
Someone working on innards of Singular might know. (I'd be surprised if it was changed).
Singular is not alone - this hack is also applied to libgap.
I'm inclined to fix _get_shared_lib_filename
so that it can accept other libdirs, not only
SAGE_LOCAL/lib
Changed 21 months ago by
comment:17 in reply to: ↑ 16 Changed 21 months ago by
Replying to dimpase:
I'm inclined to fix
_get_shared_lib_filename
so that it can accept other libdirs, not only SAGE_LOCAL/lib
Yes, this function certainly would need changing if we continue to use it.
For example, sysconfig.get_config_var('LIBDIR')
gives a directory of the system python, not of the current venv.
But I think better solution would be to wrap loading of modules that are linked to singular by sys.setdlopenflags.
This is what pytorch does when it has to use RTLD_GLOBAL (see https://github.com/pytorch/pytorch/blob/master/torch/__init__.py#L132)
comment:18 Changed 21 months ago by
comment:19 Changed 21 months ago by
I have opened #30561 for this
comment:20 Changed 20 months ago by
- Description modified (diff)
- Milestone changed from sage-9.3 to sage-9.2
- Summary changed from spkg-configure.m4 for singular to Make location of the Singular shared library configurable through sage-config
comment:21 Changed 20 months ago by
- Description modified (diff)
comment:22 Changed 20 months ago by
- Description modified (diff)
comment:23 Changed 19 months ago by
- Cc gh-tobiasdiez added
- Description modified (diff)
comment:24 Changed 19 months ago by
- Description modified (diff)
comment:25 Changed 19 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:26 Changed 18 months ago by
- Keywords sd111 added
Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111
comment:27 Changed 17 months ago by
- Cc arojas added
comment:28 Changed 14 months ago by
- Milestone changed from sage-9.3 to sage-9.4
Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.
comment:29 Changed 12 months ago by
- Dependencies set to #31642
With #31642 taking us to a released version of Singular, it is time to work on an spkg-configure.m4
for using system singular. Arch already has the latest version (https://repology.org/project/singular/versions)
comment:30 Changed 11 months ago by
- Summary changed from Make location of the Singular shared library configurable through sage-config to spkg-configure.m4 for singular
comment:31 Changed 10 months ago by
The SINGULARPATH
is being used to find the singular's docdir so that sage can manually parse its GNU info file and retrieve the docstring for Singular functions.
Currently the help file is mis-named (https://github.com/Singular/Singular/issues/1107), but even so, there's a much more reliable way to get this information without parsing the file ourselves (and therefore, without SINGULARPATH
):
$ info "(singular.hlp)align" | tail -n+2 5.1.1 align ----------- `*Syntax:*' `align (' vector_expression, int_expression `)' `align (' module_expression, int_expression `)' `*Type:*' type of the first argument `*Purpose:*' maps module generators `gen(i)' to `gen(i+s)' for all i. `*Example:*' ring r=0,(x,y,z),(c,dp); align([1,2,3],3); ==> [0,0,0,1,2,3] align([0,0,1,2,3],-1); ==> [0,1,2,3] align(freemodule(2),1); ==> _[1]=[0,1] ==> _[2]=[0,0,1]
comment:32 follow-up: ↓ 33 Changed 10 months ago by
- Branch set to u/mjo/ticket/29024
- Commit set to a8610506141953cd344b699ca01e329c4a67fe2b
This branch is only half-baked, but it already eliminates problems 2, 3, and 4 from the description at the cost of requiring GNU Info to be installed.
Can the first issue be avoided as well? Why are we using dlopen
to load SINGULAR_SO
instead of just linking to it?
comment:33 in reply to: ↑ 32 Changed 10 months ago by
comment:34 Changed 10 months ago by
- Branch u/mjo/ticket/29024 deleted
- Commit a8610506141953cd344b699ca01e329c4a67fe2b deleted
comment:35 Changed 10 months ago by
- Dependencies changed from #31642 to #32302
- Description modified (diff)
comment:36 Changed 10 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:37 Changed 9 months ago by
- Dependencies changed from #32302 to #32302, #31275
comment:38 Changed 9 months ago by
- Dependencies changed from #32302, #31275 to #32302, #31275, #32243
comment:39 Changed 9 months ago by
- Branch set to u/mjo/ticket/29024
- Commit set to a3b5983adf9e89e75ce3295f0ace0307b5cdb2d2
- Status changed from new to needs_review
I've left the dlopen()
alone because I don't feel like trying to eliminate it. This branch uses autoconf to find out the extension of shared libraries on the system (SHLIBEXT
), and then tries to dlopen("libSingular.$SHLIBEXT")
. If it works, we use the system copy, and use the same dlopen()
strategy in singular.pyx
. Otherwise we fall back to the SPKG and pass the full SAGE_LOCAL
path to dlopen()
. Works well enough on Gentoo.
New commits:
5fa1125 | Trac #32302: drop SINGULAR_EXECUTABLE from src/bin/sage-env.
|
5395116 | Trac #32243: look for cddlib-0.94m headers in the right place.
|
ea42a9b | Trac #32243: patch gfan to look for cddlib headers in new location.
|
c0340b3 | Trac #29024: put the shared library extension in sage_conf.
|
3919061 | Trac #29024: new spkg-configure.m4 for singular.
|
fc4dc8c | Trac #29024: don't set SINGULARPATH in sage-env.
|
ae26f76 | Trac #29024: use SINGULAR_LIB_BASE and SHLIBEXT to replace SINGULAR_SO.
|
a3b5983 | Trac #29024: add Gentoo package information.
|
comment:40 follow-up: ↓ 44 Changed 9 months ago by
by the way, why there is no hint for pkgconfig
in build/pkgs/pkgconfig/distros/gentoo.txt
(and for many other obvious distros)?
I only notices as Gentoo's emerge
was telling be about the switch to dev-util/pkgconf
.
comment:41 follow-up: ↓ 53 Changed 9 months ago by
OK, what's this:
[dochtml] make doc-inventory--Bug [dochtml] cd /mnt/opt/Sage/sage-dev && ./sage --docbuild --no-pdf-links Bug inventory [dochtml] Bug >>Could not get expanded executable from "libSingular.so"<< at feResource.cc:434 [dochtml] 'Bug' is not a recognized document. Type 'sage --docbuild -D' for a list [dochtml] of documents, or 'sage --docbuild --help' for more help. [dochtml] make[5]: *** [Makefile:20: doc-inventory--Bug] Error 1
also,
$ ./sage ┌────────────────────────────────────────────────────────────────────┐ │ SageMath version 9.4.rc2, Release Date: 2021-08-12 │ │ Using Python 3.8.9. Type "help()" for help. │ └────────────────────────────────────────────────────────────────────┘ ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Warning: this is a prerelease version, and it may be unstable. ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ Bug >>Could not get expanded executable from "libSingular.so"<< at feResource.cc:434 sage: 2+2 4 sage:
comment:42 follow-up: ↓ 43 Changed 9 months ago by
Can this be coming from Gentoo's Singular install? Weird.
comment:43 in reply to: ↑ 42 Changed 9 months ago by
Replying to dimpase:
Can this be coming from Gentoo's Singular install? Weird.
Oh, right, I saw that too. Singular is trying to guess the path of its own executable and is failing. It doesn't hurt anything (that I noticed...), and is an upstream bug like the message says. I'll dig deeper as this gets closer to being merged and figure out what to report upstream.
comment:44 in reply to: ↑ 40 ; follow-up: ↓ 47 Changed 9 months ago by
Replying to dimpase:
by the way, why there is no hint for
pkgconfig
inbuild/pkgs/pkgconfig/distros/gentoo.txt
(and for many other obvious distros)?I only notices as Gentoo's
emerge
was telling be about the switch todev-util/pkgconf
.
The Sage "pkgconfig" package is a python module and not the tool itself. There's nothing we can do with it yet, but eventually it's dev-python/pkgconfig.
comment:45 Changed 9 months ago by
You can make the BUG warning go away with,
$ SINGULAR_BIN_DIR=$(dirname $(type -P Singular)) sage
but hopefully that is not needed long-term.
comment:46 Changed 9 months ago by
Upstream bug: https://github.com/Singular/Singular/issues/1113
comment:47 in reply to: ↑ 44 ; follow-up: ↓ 49 Changed 9 months ago by
Replying to mjo:
The Sage "pkgconfig" package is a python module and not the tool itself. There's nothing we can do with it yet, but eventually it's dev-python/pkgconfig.
build/pkgs/pkgconf/distros
also does not have an entry for gentoo (and neither for arch)
comment:48 Changed 9 months ago by
build/pkgs/singular/distros
looks like it needs more entries for some distributions
comment:49 in reply to: ↑ 47 Changed 9 months ago by
Replying to mkoeppe:
Replying to mjo:
The Sage "pkgconfig" package is a python module and not the tool itself. There's nothing we can do with it yet, but eventually it's dev-python/pkgconfig.
build/pkgs/pkgconf/distros
also does not have an entry for gentoo (and neither for arch)
You can't bootstrap without pkgconf and you would be hard-pressed to find a Gentoo machine without it (being source-based), but it certainly can't hurt. The message that Dima saw says that we are getting rid of the old implementation, so now there's only dev-util/pkgconf
to put in gentoo.txt.
comment:50 Changed 9 months ago by
- Branch changed from u/mjo/ticket/29024 to u/slelievre/29024
- Commit changed from a3b5983adf9e89e75ce3295f0ace0307b5cdb2d2 to a5cfcec18a1eb65068151189b22dd0ba461ca2cf
Here are Singular distro files for Arch Linux, Cygwin and Fedora.
Revert to your branch if those are not helping.
New commits:
a5cfcec | 29024: Add Singular distro info
|
comment:51 Changed 9 months ago by
-# As of 2021-03-20, build of homebrew's singular from source fails +# [2021-03-20] building from source with Homebrew's singular fails
this rewording changes the meaning. What failed at this time was really the from-source build of the homebrew package (via brew install -s
, or implied when using brew
with a non-standard prefix)
comment:52 Changed 9 months ago by
- Commit changed from a5cfcec18a1eb65068151189b22dd0ba461ca2cf to fe67a2174fb5076228c13a8b6b5b94a697002348
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
fe67a21 | 29024: Add Singular distro info
|
comment:53 in reply to: ↑ 41 Changed 9 months ago by
Replying to dimpase:
OK, what's this:
[dochtml] make doc-inventory--Bug
This is likely caused by unexpected output (on stdout!) from the command ./sage --docbuild --all-documents reference
comment:54 Changed 9 months ago by
- Status changed from needs_review to needs_work
comment:55 follow-up: ↓ 56 Changed 8 months ago by
We probably also need to do something with SINGULARPATH
, a variable hold, if needed, paths to Singular's .lib files, something that may be installed by Sage's optional packages - it is done e.g. by p_group_cohomology
. So to make the latter work with system-wide Singular one needs to export SINGULARPATH=$SAGE_LOCAL/share/singular/LIB
.
comment:56 in reply to: ↑ 55 Changed 8 months ago by
Replying to dimpase:
We probably also need to do something with
SINGULARPATH
, a variable hold, if needed, paths to Singular's .lib files, something that may be installed by Sage's optional packages - it is done e.g. byp_group_cohomology
. So to make the latter work with system-wide Singular one needs to exportSINGULARPATH=$SAGE_LOCAL/share/singular/LIB
.
What we really need is a system for optional packages to set their own environment variables, so we don't have to set every possible environment variable (for an unknowable number of optional packages) in sage-env. Of course Gentoo and everyone else already figured this out 20 years ago. We can have sage-env source any files in local/etc/env.d
, and then tell optional packages to install what they need (e.g. a script that exports SINGULARPATH
) there.
I guess I will waste my time reimplementing that so that some day I can stop wasting my time rebuilding the same copy of singular that I already have installed.
comment:57 Changed 8 months ago by
Best to plan to move away from any such dependence on setting environment variables.
Instead, it should be set at runtime when initializing the singular interface / libsingular.
comment:58 Changed 8 months ago by
comment:59 follow-up: ↓ 60 Changed 8 months ago by
In fact, do we actually install any custom Singular packages in SAGE_LOCAL/share/singular/LIB
? I don't think we do.
src/sage/rings/function_field/function_field.py
uses the correct approach -- it loads its custom Singular library using a fully qualified path to the library, which is just in the installed package data of the Sage library.
If the p_group_cohomology
Sage code relies on SINGULARPATH
, it should be changed so it uses the same approach.
comment:60 in reply to: ↑ 59 ; follow-ups: ↓ 61 ↓ 62 Changed 8 months ago by
Replying to mkoeppe:
In fact, do we actually install any custom Singular packages in
SAGE_LOCAL/share/singular/LIB
? I don't think we do.
Not sure what "we" is here, but
p_group_cohomology
does install a custom Singular lib, called dickson.lib
,
and it ends up in $SAGE_LOCAL/share/singular/LIB
, as spkg-install
has
sdh_install singular_helper/dickson.lib "$SAGE_SHARE/singular/LIB/"
- which is fine if Singular is built by Sage, but not with the external Singular.
src/sage/rings/function_field/function_field.py
uses the correct approach -- it loads its custom Singular library using a fully qualified path to the library, which is just in the installed package data of the Sage library.If the
p_group_cohomology
Sage code relies onSINGULARPATH
, it should be changed so it uses the same approach.
It doesn't rely on SINGULARPATH
is seems to me. SINGULARPATH
can be used by Singular,
and system-wide Singular doesn't know about $SAGE_LOCAL/share/singular/LIB
, obviously, unless something is done.
Should p_group_cohomology
be using $SAGE_SHARE/singular/LIB/
to install dickson.lib
, or some
other location?
Regarding src/sage/ext_data/singular/function_field/core.lib
, one sees
from sage.env import SAGE_EXTCODE lib(SAGE_EXTCODE + '/singular/function_field/core.lib')
but
$ find . -name core.lib ./pkgs/sagemath-standard/build/lib.linux-x86_64-3.8/sage/ext_data/singular/function_field/core.lib ./src/sage/ext_data/singular/function_field/core.lib ./local/lib/python3.8/site-packages/sage/ext_data/singular/function_field/core.lib
so it also gets installed into site-packages. Is it a bug?
comment:61 in reply to: ↑ 60 Changed 8 months ago by
Replying to dimpase:
Regarding
src/sage/ext_data/singular/function_field/core.lib
, one seesfrom sage.env import SAGE_EXTCODE lib(SAGE_EXTCODE + '/singular/function_field/core.lib')but
$ find . -name core.lib ./pkgs/sagemath-standard/build/lib.linux-x86_64-3.8/sage/ext_data/singular/function_field/core.lib ./src/sage/ext_data/singular/function_field/core.lib ./local/lib/python3.8/site-packages/sage/ext_data/singular/function_field/core.libso it also gets installed into site-packages.
core.lib
is package_data
of the distribution package sagemath-standard
.
As such it gets installed alongside the Python files, in site-packages
.
This works as designed.
comment:62 in reply to: ↑ 60 ; follow-up: ↓ 65 Changed 8 months ago by
Replying to dimpase:
Replying to mkoeppe:
In fact, do we actually install any custom Singular packages in
SAGE_LOCAL/share/singular/LIB
? I don't think we do.Not sure what "we" is here
... any package in the Sage distribution ...
Should
p_group_cohomology
be using$SAGE_SHARE/singular/LIB/
to installdickson.lib
, or some other location?
As I said, this should be changed to follow the same model as src/sage/ext_data/singular/function_field/core.lib
comment:63 follow-up: ↓ 64 Changed 8 months ago by
Is there any code that actually loads dickson.lib
?
comment:64 in reply to: ↑ 63 Changed 8 months ago by
Replying to mkoeppe:
Is there any code that actually loads
dickson.lib
?
sage -t src/sage/tests/modular_group_cohomology.py
will trigger loading it, if p_group_cohomology
is installed.
comment:65 in reply to: ↑ 62 Changed 8 months ago by
- Report Upstream changed from N/A to Reported upstream. No feedback yet.
Replying to mkoeppe:
Replying to dimpase:
Replying to mkoeppe:
In fact, do we actually install any custom Singular packages in
SAGE_LOCAL/share/singular/LIB
? I don't think we do.Not sure what "we" is here
... any package in the Sage distribution ...
Should
p_group_cohomology
be using$SAGE_SHARE/singular/LIB/
to installdickson.lib
, or some other location?As I said, this should be changed to follow the same model as
src/sage/ext_data/singular/function_field/core.lib
I've opened https://github.com/sagemath/p_group_cohomology/issues/5 for this
comment:66 Changed 8 months ago by
As p_group_cohomology
is broken since Sage 9.4 anyway (https://github.com/sagemath/p_group_cohomology/issues/4), I think we can proceed here on the ticket without worrying about SINGULARPATH
.
comment:67 follow-up: ↓ 70 Changed 8 months ago by
So are we waiting for the upstream fix here, or proceed with making sure that
export SINGULAR_BIN_DIR=$(dirname $(type -P Singular))
is added at the correct place?
comment:68 Changed 8 months ago by
I think we'll have to add it. Upstream started to work on the issue but we're nowhere near having a complete fix in a released version of Singular. The BUG warning is harmless I think; but if we don't hide it, we're going to have to deal with a million bug reports about it.
I'll add the workaround and create a ticket to remove it whenever the fix is in an upstream release.
comment:69 Changed 8 months ago by
Actually, I just looked at the upstream issue and see that I missed a notification about a new commit. Let me go test it. We may still want to add the workaround for a few months until a new release is made and distros catch up.
comment:70 in reply to: ↑ 67 ; follow-up: ↓ 71 Changed 8 months ago by
Replying to dimpase:
So are we waiting for the upstream fix here, or proceed with making sure that
export SINGULAR_BIN_DIR=$(dirname $(type -P Singular))is added at the correct place?
Can we put this environment setting in src/sage/libs/singular/singular.pyx
(init_libsingular
)?
comment:71 in reply to: ↑ 70 ; follow-ups: ↓ 72 ↓ 73 Changed 8 months ago by
Replying to mkoeppe:
Replying to dimpase:
So are we waiting for the upstream fix here, or proceed with making sure that
export SINGULAR_BIN_DIR=$(dirname $(type -P Singular))is added at the correct place?
Can we put this environment setting in
src/sage/libs/singular/singular.pyx
(init_libsingular
)?
Both type -P
and command -v
are shell built-ins on my system, making them a bit hard to use from python code. Does putting it in sage-env cause a problem I've overlooked, or are you just thinking it would be more appropriate in init_singular()
? I don't disagree -- but considering that this is a temporary workaround, it might just be easier to add the one line in sage-env instead of the python code to launch a full-blown shell and parse its output.
comment:72 in reply to: ↑ 71 Changed 8 months ago by
Replying to mjo:
Does putting it in sage-env cause a problem I've overlooked
Yes, we are working on making sage pip-installable and importable into normal Python environments (import sage.all
), and for this depending on sage-env
needs to be removed
comment:73 in reply to: ↑ 71 Changed 8 months ago by
Replying to mjo:
temporary workaround
Well, versions of singular that need this workaround may be in relevant distributions for years...
comment:74 follow-up: ↓ 75 Changed 8 months ago by
And I think this just https://docs.python.org/3/library/shutil.html#shutil.which
comment:75 in reply to: ↑ 74 Changed 8 months ago by
- Branch changed from u/slelievre/29024 to u/mjo/ticket/29024
- Commit changed from fe67a2174fb5076228c13a8b6b5b94a697002348 to 3cebd4f26aba00ed35aa6956e2a69e323353a4a3
Replying to mkoeppe:
And I think this just https://docs.python.org/3/library/shutil.html#shutil.which
Yep, here's an untested branch. It's going to take me all night to rebuild sage after the rebase but I'll make sure it works in the morning. And report back to upstream... that last commit looks strange to me.
New commits:
28837b9 | Trac #29024: put the shared library extension in sage_conf.
|
c383777 | Trac #29024: new spkg-configure.m4 for singular.
|
8e80875 | Trac #29024: don't set SINGULARPATH in sage-env.
|
9b8244f | Trac #29024: use SINGULAR_LIB_BASE and SHLIBEXT to replace SINGULAR_SO.
|
f6e4f89 | Trac #29024: add Gentoo package information.
|
1055247 | 29024: Add Singular distro info
|
3cebd4f | Trac #29024: work around Singular issue 1113 in init_libsingular().
|
comment:76 Changed 8 months ago by
It works.
comment:77 Changed 8 months ago by
Ready for review?
comment:78 Changed 8 months ago by
- Status changed from needs_work to needs_review
comment:79 Changed 8 months ago by
This will need a GH Actions run
comment:80 Changed 8 months ago by
comment:81 Changed 8 months ago by
- Reviewers set to Dima Pasechnik, https://github.com/sagemath/sagetrac-mirror/actions/runs/1240853423
comment:82 follow-up: ↓ 85 Changed 8 months ago by
Doesn't accept singular on cygwin-standard
(https://github.com/mkoeppe/sage/runs/3625055086)
----------------------------------------------------------------------------- Checking whether SageMath should install SPKG singular... checking whether any of gmp mpir ntl flint readline mpfr cddlib is installed as or will be installed as SPKG... no checking for Singular... /usr/bin/Singular checking for SINGULAR... no checking if we can dlopen(libSingular.dll)... configure: no suitable system package found for SPKG singular
comment:83 Changed 8 months ago by
Works OK on archlinux-latest-standard
(https://github.com/mkoeppe/sage/runs/3625059112)
---------------------------------------------------------------------------- Checking whether SageMath should install SPKG singular... checking whether any of gmp mpir ntl flint readline mpfr cddlib is installed as or will be installed as SPKG... no checking for Singular... /usr/sbin/Singular checking for SINGULAR... yes checking if we can dlopen(libSingular.so)... configure: will use system package and not install SPKG singular
(but looks like an AC_MSG_RESULT
is missing)
comment:84 Changed 8 months ago by
Also OK (as expected) on gentoo-standard
No luck on debian
, fedora
, and homebrew
because of the cddlib
dependency. It works correctly but does not accept system singular.
comment:85 in reply to: ↑ 82 Changed 8 months ago by
Replying to mkoeppe:
Doesn't accept singular on
cygwin-standard
(https://github.com/mkoeppe/sage/runs/3625055086)
I think that's expected, since it has an older version of Singular than the pkg-config test looks for.
comment:86 Changed 8 months ago by
- Commit changed from 3cebd4f26aba00ed35aa6956e2a69e323353a4a3 to d8118acd08ade1751ad1ecd8394415d32a643c2d
Branch pushed to git repo; I updated commit sha1. New commits:
d8118ac | Trac #29024: add missing AC_MSG_RESULTs in singular/spkg-configure.m4.
|
comment:87 Changed 8 months ago by
- Reviewers changed from Dima Pasechnik, https://github.com/sagemath/sagetrac-mirror/actions/runs/1240853423 to Dima Pasechnik, Matthias Koeppe
- Status changed from needs_review to positive_review
comment:88 Changed 8 months ago by
- Status changed from positive_review to needs_work
Actually this needs more work on Cygwin: https://github.com/mkoeppe/sage/runs/3638292833?check_suite_focus=true
./sage -t -p --all --logfile=logs/ptest.log no stored timings available Running doctests with ID 2021-09-18-08-09-52-cb78f982. Git branch: HEAD Using --optional=build,cygwin,dochtml,pip,sage,sage_spkg Doctesting entire Sage library. Sorting sources by runtime so that slower doctests are run first.... Doctesting 4363 files using 3 threads. Traceback (most recent call last): File "/cygdrive/d/a/sage/sage/src/bin/sage-runtests", line 144, in <module> err = DC.run() File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/doctest/control.py", line 1207, in run self.run_doctests() File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/doctest/control.py", line 904, in run_doctests self.dispatcher = DocTestDispatcher(self) File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/doctest/forker.py", line 1624, in __init__ init_sage(controller) File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/doctest/forker.py", line 192, in init_sage controller.load_environment() File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/doctest/control.py", line 525, in load_environment return import_module(self.options.environment) File "/usr/lib/python3.8/importlib/__init__.py", line 127, in import_module return _bootstrap._gcd_import(name[level:], package, level) File "<frozen importlib._bootstrap>", line 1014, in _gcd_import File "<frozen importlib._bootstrap>", line 991, in _find_and_load File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 671, in _load_unlocked File "<frozen importlib._bootstrap_external>", line 848, in exec_module File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/repl/ipython_kernel/all_jupyter.py", line 5, in <module> from sage.all_cmdline import * File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/all_cmdline.py", line 19, in <module> from sage.all import * File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/all.py", line 141, in <module> from sage.rings.all import * File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/rings/all.py", line 83, in <module> from .polynomial.all import * File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/rings/polynomial/all.py", line 42, in <module> from sage.rings.polynomial.laurent_polynomial_ring import LaurentPolynomialRing File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/rings/polynomial/laurent_polynomial_ring.py", line 50, in <module> from sage.rings.polynomial.laurent_polynomial import LaurentPolynomial_mpair, LaurentPolynomial_univariate File "sage/rings/polynomial/laurent_polynomial.pyx", line 1, in init sage.rings.polynomial.laurent_polynomial (build/cythonized/sage/rings/polynomial/laurent_polynomial.c:40817) File "sage/matrix/matrix0.pyx", line 28, in init sage.matrix.matrix0 (build/cythonized/sage/matrix/matrix0.c:44570) File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/modules/free_module.py", line 167, in <module> import sage.matrix.matrix_space File "/opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/python3.8/site-packages/sage/matrix/matrix_space.py", line 55, in <module> from . import matrix_mpolynomial_dense File "sage/matrix/matrix_mpolynomial_dense.pyx", line 1, in init sage.matrix.matrix_mpolynomial_dense (build/cythonized/sage/matrix/matrix_mpolynomial_dense.cpp:8666) File "sage/rings/polynomial/multi_polynomial_libsingular.pyx", line 1, in init sage.rings.polynomial.multi_polynomial_libsingular (build/cythonized/sage/rings/polynomial/multi_polynomial_libsingular.cpp:49840) File "sage/libs/singular/singular.pyx", line 808, in init sage.libs.singular.singular (build/cythonized/sage/libs/singular/singular.cpp:11287) File "sage/libs/singular/singular.pyx", line 785, in sage.libs.singular.singular.init_libsingular (build/cythonized/sage/libs/singular/singular.cpp:8697) ImportError: cannot load Singular library from /opt/sage-032178416da49f83f86c3b8ce2797134fbb97bd8/lib/libSingular.dll (b'No such file or directory') make: *** [Makefile:233: ptest-nodoc] Error 1
Note that on cygwin, dlls are installed in bin/
, not in lib/
comment:89 Changed 8 months ago by
- Commit changed from d8118acd08ade1751ad1ecd8394415d32a643c2d to da802ef6c996c37611ef0be910b714e41acc68b7
comment:90 Changed 8 months ago by
- Status changed from needs_work to needs_review
I think that will fix it. I thought about eliminating SINGULAR_LIB_BASE
entirely, but I can't say for sure that we won't encounter a platform where dlopen()
does not search the library path, so I've left it in and invented a way to reference the libdir.
The hacks in sage_conf are starting to pile up, so I've opened #32540 with a proposal to refactor things.
comment:91 Changed 8 months ago by
Have you tested this on Cygwin?
comment:92 follow-up: ↓ 98 Changed 8 months ago by
Asking because the situation with DLLs on Cygwin is not really related to anything "LIBDIR". Only the naked DLLs end up in bin/
on the platform, but lib/
is still used for .a
and .dll.a
files
comment:93 follow-up: ↓ 99 Changed 8 months ago by
Homebrew's Singular needs some fixes; 1st of all,
-
.homebrew-build-env
a b export PKG_CONFIG_PATH 23 23 LIBRARY_PATH="$HOMEBREW/lib$LIBRARY_PATH" 24 24 [ -z "$CPATH" ] || CPATH=":${CPATH}" 25 25 CPATH="$HOMEBREW/include$CPATH" 26 for l in readline bzip2 ntl ; do26 for l in readline bzip2 ntl singular; do 27 27 if [ -d "$HOMEBREW/opt/$l/lib" ]; then 28 28 LIBRARY_PATH="$HOMEBREW/opt/$l/lib:$LIBRARY_PATH" 29 29 fi
Then, it appears that dlopen()
call in singular.pyx
needs a full path to .dylib on macOS.
(it might be a Cython bug, or the plarform limitation, I don't know).
That is, if I make sure that
SINGULAR_LIB_BASE = "/usr/local/opt/singular/lib/libSingular"
in sage_conf.py
then everything works. I am not sure how to achieve the latter automatically.
One way might be to export a special env. var. in .homebrew-build-env
and use it to set up the proper
SINGULAR_LIB_BASE
in spkg-configure.m4
- but there could be a better way.
comment:94 Changed 8 months ago by
Once the latter is fixed, we should apply
-
build/pkgs/singular/distros/homebrew.txt
a b 1 # [2021-03-20] Homebrew's Singular can be built from source 2 # (via `brew install -s`, or implied when using `brew` with 3 # a non-standard prefix), but that currently fails. 4 5 #singular 1 singular
comment:95 follow-up: ↓ 96 Changed 8 months ago by
- Status changed from needs_review to needs_work
This breaks distro installs without sage_conf
. Please keep the old _get_shared_lib_path
code path as a fallback in case sage_conf
gives an ImportError
.
comment:96 in reply to: ↑ 95 ; follow-up: ↓ 97 Changed 8 months ago by
Replying to arojas:
This breaks distro installs without
sage_conf
. Please keep the old_get_shared_lib_path
code path as a fallback in casesage_conf
gives anImportError
.
Do distros use env. vars from env.py
? (That's where the change was, the env. var. there is not computed any more.
comment:97 in reply to: ↑ 96 Changed 8 months ago by
Replying to dimpase:
Replying to arojas:
This breaks distro installs without
sage_conf
. Please keep the old_get_shared_lib_path
code path as a fallback in casesage_conf
gives anImportError
.Do distros use env. vars from
env.py
? (That's where the change was, the env. var. there is not computed any more.
Yes, env.py
is part of sagelib. Actually, a simple fallback to SINGULAR_LIB_PATH="libSingular.so"
would be fine for me (if any distro modifies the default library name, I guess it's their job to patch sage accordingly or provide a custom sage_conf)
comment:98 in reply to: ↑ 92 ; follow-up: ↓ 100 Changed 8 months ago by
Replying to mkoeppe:
Asking because the situation with DLLs on Cygwin is not really related to anything "LIBDIR". Only the naked DLLs end up in
bin/
on the platform, butlib/
is still used for.a
and.dll.a
files
No, I haven't tested yet. I'll have to run it on GH.
Even if $libdir isn't being used... at some point, the build system is deciding to put those dlls into a $directory, and $directory=bin on cygwin while $directory=lib everywhere else, no? I don't suppose you know where that decision is made?
comment:99 in reply to: ↑ 93 ; follow-up: ↓ 101 Changed 8 months ago by
Replying to dimpase:
Then, it appears that
dlopen()
call insingular.pyx
needs a full path to .dylib on macOS. (it might be a Cython bug, or the plarform limitation, I don't know).
This was a conscious choice, as da802ef now makes explicit. POSIX doesn't say what will happen if you use a filename without a path in dlopen()
, so the SPKG is still used as a fallback on systems where it doesn't work.
There are a couple options to improve this, of varying difficulty:
- We could try to get the libdir from Singular itself. The
feResource.cc
file provides a bunch of other configuration information, but not the bare libdir at the moment. We could provide a patch upstream that lets us obtain its libdir. (However, the cygwin discussion is suggesting that libdir may not always be the right place?) - We can eliminate the
dlopen()
, if possible. We already link directly to libSingular and that's easy to do portably with-lSingular
. I'm not entirely sure whydlopen()
is needed, but theinit_libsingular()
documentation says""" This initializes the SINGULAR library. This is a hack to some extent. SINGULAR has a concept of compiled extension modules similar to Sage. For this, the compiled modules need to see the symbols from the main program. However, SINGULAR is a shared library in this context these symbols are not known globally. The work around so far is to load the library again and to specify ``RTLD_GLOBAL``. """
- We can invent some hacks that guess the right location on most of the platforms we support.
comment:100 in reply to: ↑ 98 ; follow-up: ↓ 102 Changed 8 months ago by
Replying to mjo:
Even if $libdir isn't being used... at some point, the build system is deciding to put those dlls into a $directory, and $directory=bin on cygwin while $directory=lib everywhere else, no? I don't suppose you know where that decision is made?
I think it has to do with cygwin fork() design, Windows address space, rebasing... DLL needs to get a fixed memory address allocation, for fork() to work), and so bin/
is what's beging scanned by the corresponding Cygwin utility rebaseall
.
Not running away screaming yet?
comment:101 in reply to: ↑ 99 ; follow-up: ↓ 103 Changed 8 months ago by
Replying to mjo:
Replying to dimpase:
Then, it appears that
dlopen()
call insingular.pyx
needs a full path to .dylib on macOS. (it might be a Cython bug, or the plarform limitation, I don't know).This was a conscious choice, as da802ef now makes explicit. POSIX doesn't say what will happen if you use a filename without a path in
dlopen()
, so the SPKG is still used as a fallback on systems where it doesn't work.
This does not explain why no-full-path dlopen()
succeeds in the test
configure:40299: checking if we can dlopen(libSingular.dylib) configure:40330: gcc -o conftest -g -O2 -Ddarwin -I/usr/local/opt/gmp/include -I/usr/local/Cellar/ecl/21.2.1/include conftest.c -lmpfi -lmpc -lnauty -lglpk -lpari -lncurses -lcurl -lcddgmp -lbz2 -larb -lflint -lmpfr -lgmp -lm -lntl -lppl -lgmpxx -lgmp -ldl >&5 configure:40330: $? = 0 configure:40330: ./conftest configure:40330: $? = 0 configure:40333: result: yes
but fails in singular.pyx
.
comment:102 in reply to: ↑ 100 Changed 8 months ago by
Replying to dimpase:
Replying to mjo:
Even if $libdir isn't being used... at some point, the build system is deciding to put those dlls into a $directory, and $directory=bin on cygwin while $directory=lib everywhere else, no? I don't suppose you know where that decision is made?
I think it has to do with cygwin fork() design, Windows address space, rebasing... DLL needs to get a fixed memory address allocation, for fork() to work), and so
bin/
is what's beging scanned by the corresponding Cygwin utilityrebaseall
.Not running away screaming yet?
Is that an option? Can I wrap the whole thing in if [ $(whoami) = "mjo" ]
?
The rebaseall thing makes some amount of sense to me (I used cygwin back when Windows XP was around), but I'm still not sure exactly when the dlls either get installed directly to bin, or moved from lib to bin.
comment:103 in reply to: ↑ 101 ; follow-up: ↓ 104 Changed 8 months ago by
Replying to dimpase:
This does not explain why no-full-path
dlopen()
succeeds in the test... but fails insingular.pyx
.
Oh, no. That's interesting. What error message does it give you?
comment:104 in reply to: ↑ 103 Changed 8 months ago by
Replying to mjo:
Replying to dimpase:
This does not explain why no-full-path
dlopen()
succeeds in the test... but fails insingular.pyx
.Oh, no. That's interesting. What error message does it give you?
... --> 808 init_libsingular() ... 782 handle = dlopen(lib, RTLD_GLOBAL|RTLD_LAZY) 783 if not handle: 784 err = dlerror() --> 785 raise ImportError(f"cannot load Singular library from {SINGULAR_LIB_PATH} ({err})") global ImportError = undefined 786 787 # load SINGULAR 788 siInit(lib) 789 ... ImportError: cannot load Singular library from libSingular.dylib (b'dlopen(libSingular.dylib, 9): image not found')
I tried setting LD_LIBRARY_PATH or DYLD_FALLBACK_LIBRARY_PATH (cf. Apple's manpage on dlopen()), to no effect.
PS. related reading https://developer.apple.com/forums/thread/13161 sounds as if Apple broke the latter stuff in the name of "security"...
comment:105 Changed 8 months ago by
The cygwin magic happens in libtool.m4
:
cygwin* | mingw* | pw32* | cegcc*) ... # DLL is installed to $(libdir)/../bin by postinstall_cmds postinstall_cmds='base_file=`basename \$file`~ dlpath=`$SHELL 2>&1 -c '\''. $dir/'\''\$base_file'\''i; echo \$dlname'\''`~ dldir=$destdir/`dirname \$dlpath`~ test -d \$dldir || mkdir -p \$dldir~ $install_prog $dir/$dlname \$dldir/$dlname~ chmod a+x \$dldir/$dlname~ if test -n '\''$stripme'\'' && test -n '\''$striplib'\''; then eval '\''$striplib \$dldir/$dlname'\'' || exit \$?; fi'
I guess I'll just undo the last few commits and special-case cygwin*
.
comment:106 Changed 8 months ago by
As far as I know, on Cygwin dlopen() looks up things in $PATH
rather than $LD_...
, so this appears to be good as it is, no?
Although Conda and macOS still need an action.
comment:107 Changed 8 months ago by
- Commit changed from da802ef6c996c37611ef0be910b714e41acc68b7 to 8cd60dde05a5421500ddcfdce3f48d1a3efe8a14
comment:108 follow-up: ↓ 109 Changed 8 months ago by
On cygwin the SPKG will always be used because its Singular is too old. But even that was failing, because the absolute path involving "lib" was wrong. It should be fixed now.
Remaining questions:
- Do I need to use "cygSingular" as the library name on Cygwin?
- WTF is happening on macOS?
comment:109 in reply to: ↑ 108 Changed 8 months ago by
Replying to mjo:
On cygwin the SPKG will always be used because its Singular is too old.
I've asked for an update: https://cygwin.com/pipermail/cygwin/2021-September/249449.html
But even that was failing, because the absolute path involving "lib" was wrong. It should be fixed now.
Remaining questions:
- Do I need to use "cygSingular" as the library name on Cygwin?
Yes, I think so.
- WTF is happening on macOS?
I suppose we need to supply the full path to DLL (.e.g. .dylib), via an env. var in .homebrew-build-env
and forget about it.
(Not sure about Conda).
One discrepancy I noticed is that that singular.pyx is compiled as C++, while the test in spkg-configure is done with plain C; I tried with C++, saw no difference.
comment:110 Changed 8 months ago by
- Branch changed from u/mjo/ticket/29024 to u/dimpase/ticket/29024
- Commit changed from 8cd60dde05a5421500ddcfdce3f48d1a3efe8a14 to f72df456ba4c29b5b5353948af02d3ed9a8a66d3
comment:111 Changed 8 months ago by
This is not a good change.
--- a/.homebrew-build-env +++ b/.homebrew-build-env @@ -23,7 +23,7 @@ export PKG_CONFIG_PATH LIBRARY_PATH="$HOMEBREW/lib$LIBRARY_PATH" [ -z "$CPATH" ] || CPATH=":${CPATH}" CPATH="$HOMEBREW/include$CPATH" -for l in readline bzip2 ntl; do +for l in readline bzip2 ntl singular; do if [ -d "$HOMEBREW/opt/$l/lib" ]; then LIBRARY_PATH="$HOMEBREW/opt/$l/lib:$LIBRARY_PATH" fi
Adding to LIBRARY_PATH is only needed for "keg-only" packages.
singular
is a regular package, linked into /usr/local
.
@@ -45,3 +45,5 @@ for l in gettext; do fi done export ACLOCAL_PATH +SINGULAR_HOMEBREW_LIBDIR="$HOMEBREW/opt/singular/lib/" +export SINGULAR_HOMEBREW_LIBDIR
Pretty sure you can find the information you need during configure using pkg-config
.
comment:112 Changed 8 months ago by
Indeed, we can instead call PKG_CHECK_VAR([SINGULAR_LIBDIR], [Singular], [libdir])
to get SINGULAR_LIBDIR
.
I suppose, to be on the safe side we should prepend it to SINGULAR_LIB_BASE
unconditionally, not only on macOS.
comment:113 Changed 8 months ago by
- Commit changed from f72df456ba4c29b5b5353948af02d3ed9a8a66d3 to f11791b35962276ae007802d5397572a58a0e3bd
Branch pushed to git repo; I updated commit sha1. New commits:
f11791b | pkg-config to get the libdir, properly nest tests
|
comment:114 follow-up: ↓ 116 Changed 8 months ago by
I was working on this at the same time =(
I've simplified the variables a bit, so the "base" and extension are no longer separate. I also added the special-case for cygwin into the system-singular code branch.
comment:115 Changed 8 months ago by
- Commit changed from f11791b35962276ae007802d5397572a58a0e3bd to e6c41f7d7d13fca5dd6f2802c75e8c829b040e92
Branch pushed to git repo; I updated commit sha1. New commits:
e6c41f7 | Fedora needs devel pkg
|
comment:116 in reply to: ↑ 114 Changed 8 months ago by
Replying to mjo:
I was working on this at the same time =(
I've simplified the variables a bit, so the "base" and extension are no longer separate. I also added the special-case for cygwin into the system-singular code branch.
I'll try to merge your improvements
comment:117 Changed 8 months ago by
- Cc gh-dkwo added
comment:118 Changed 8 months ago by
- Dependencies changed from #32302, #31275, #32243 to #32302, #31275, #32243, #32549
there is also #32549 removing mpir
spkg, so mpir
should disappear in spkg-config.m4
comment:119 Changed 8 months ago by
- Dependencies changed from #32302, #31275, #32243, #32549 to #32549
comment:120 Changed 8 months ago by
- Commit changed from e6c41f7d7d13fca5dd6f2802c75e8c829b040e92 to 3404652e8897d662a83953c32d9b1ef8266ef443
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
d5b8ac8 | Trac #29024: add Gentoo package information.
|
9589a1d | 29024: Add Singular distro info
|
c76a975 | Trac #29024: work around Singular issue 1113 in init_libsingular().
|
3730d72 | Trac #29024: add missing AC_MSG_RESULTs in singular/spkg-configure.m4.
|
30e7fd6 | Trac #29024: clarify a comment in singular's spkg-configure.
|
f64594c | Trac #29024: fix libSingular path on Cygwin.
|
73d301d | macOS dlopen() prefers full path sometimes
|
604b0c4 | pkg-config to get the libdir, properly nest tests
|
ca4aa03 | Fedora needs devel pkg
|
3404652 | dependance on gmp/mpir already covered by mpfr
|
comment:121 Changed 8 months ago by
- Dependencies #32549 deleted
comment:122 Changed 7 months ago by
- Commit changed from 3404652e8897d662a83953c32d9b1ef8266ef443 to d8db64bda6e1939e1af277b2925dddd0f8167d30
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
a61b3a5 | Trac #29024: add Gentoo package information.
|
fa65fa5 | 29024: Add Singular distro info
|
0846173 | Trac #29024: work around Singular issue 1113 in init_libsingular().
|
dd69ac1 | Trac #29024: add missing AC_MSG_RESULTs in singular/spkg-configure.m4.
|
9d6080f | Trac #29024: clarify a comment in singular's spkg-configure.
|
e0c0e05 | Trac #29024: fix libSingular path on Cygwin.
|
93b2f47 | macOS dlopen() prefers full path sometimes
|
dca5125 | pkg-config to get the libdir, properly nest tests
|
f85d09c | Fedora needs devel pkg
|
d8db64b | dependance on gmp/mpir already covered by mpfr
|
comment:123 Changed 7 months ago by
so what's exactly is different in functionality in the current ticket branch, u/dimpase/ticket/29024
vs
u/mjo/ticket/29024
?
comment:124 Changed 7 months ago by
Now that we're just asking pkg-config for the correct location, the two variables SINGULAR_LIB_BASE
and SINGULAR_LIB_FILE
are redundant. (The "base" variable would be left empty in the past to let dlopen()
search for the library.) I think the main difference in my branch is that I've refactored things to use only a single variable, LIBSINGULAR_PATH
. The rest should be cosmetic.
comment:125 follow-up: ↓ 126 Changed 7 months ago by
you do non-nested calls where they can and should be nested. slows down ./configure
for no reason.
comment:126 in reply to: ↑ 125 ; follow-up: ↓ 127 Changed 7 months ago by
Replying to dimpase:
you do non-nested calls where they can and should be nested. slows down
./configure
for no reason.
I find the logic much easier to understand when the "else" branch of a test is not separated from the test by an entire page. Ideally I would be able to put something like return;
after each sage_spkg_install_singular=yes
, but I don't know of a reliable way to accomplish that.
Running "make" takes many hours, so an extra second or two on systems without Singular didn't seem like a huge problem. And those of us who work on the build system and run ./configure
repeatedly should be clever enough to install it. But I could still nest them if that's a deal-breaker for you.
comment:127 in reply to: ↑ 126 Changed 7 months ago by
Replying to mjo:
Replying to dimpase:
you do non-nested calls where they can and should be nested. slows down
./configure
for no reason.I find the logic much easier to understand when the "else" branch of a test is not separated from the test by an entire page.
Do some coding in Lisp, then, for the training purposes ;-)
Ideally I would be able to put something like
return;
after eachsage_spkg_install_singular=yes
, but I don't know of a reliable way to accomplish that.Running "make" takes many hours, so an extra second or two on systems without Singular didn't seem like a huge problem. And those of us who work on the build system and run
./configure
repeatedly should be clever enough to install it. But I could still nest them if that's a deal-breaker for you.
Also, nesting makes sure that the output in case of errors is meaningful - it just errors on the 1st failing place, and that's it.
comment:128 Changed 7 months ago by
I'm willing to review a branch with nesting - or, you can review my branch...
comment:129 Changed 7 months ago by
- Branch changed from u/dimpase/ticket/29024 to u/mjo/ticket/29024
- Commit changed from d8db64bda6e1939e1af277b2925dddd0f8167d30 to eb3b4e559096605857584054ecdfe3556e6444ed
- Status changed from needs_work to needs_review
Here it is fully nested. I cherry-picked your fix for fedora.txt
, but I left "gmp" in the DEPCHECK because it's easier to just list them all than it is to try to keep track of which DEPCHECKs need to be modified when e.g. mpfr loses a dependency.
Last 10 new commits:
dfb5860 | Trac #29024: add Gentoo package information.
|
175e6f6 | 29024: Add Singular distro info
|
dc1dcc3 | Trac #29024: work around Singular issue 1113 in init_libsingular().
|
ef79af8 | Trac #29024: add missing AC_MSG_RESULTs in singular/spkg-configure.m4.
|
2f32b69 | Trac #29024: clarify a comment in singular's spkg-configure.
|
8ec888a | Trac #29024: fix libSingular path on Cygwin.
|
62065de | Trac #29024: always obtain the absolute path to libSingular.
|
2aa55e7 | Trac #29024: fully nest tests in Singular's spkg-configure.m4.
|
eecd43c | Fedora needs devel pkg
|
eb3b4e5 | Trac #29024: drop "mpir" from singular's DEPCHECK.
|
comment:130 Changed 7 months ago by
build/pkgs/singular/distros/homebrew.txt
should be like in my branch:
-
build/pkgs/singular/distros/homebrew.txt
a b 1 # [2021-03-20] Homebrew's Singular can be built from source 2 # (via `brew install -s`, or implied when using `brew` with 3 # a non-standard prefix), but that currently fails. 4 5 #singular 1 singular
comment:131 follow-up: ↓ 135 Changed 7 months ago by
- Commit changed from eb3b4e559096605857584054ecdfe3556e6444ed to a3c53051a5961226700bdb8a40f831092f8dffcd
Branch pushed to git repo; I updated commit sha1. New commits:
a3c5305 | Trac #29024: re-enable the system singular hint on homebrew.
|
comment:133 follow-up: ↓ 134 Changed 7 months ago by
- Status changed from positive_review to needs_work
comment:95 hasn't been addressed. This breaks sage_conf
-less installs.
comment:134 in reply to: ↑ 133 ; follow-up: ↓ 138 Changed 7 months ago by
- Status changed from needs_work to positive_review
Replying to arojas:
comment:95 hasn't been addressed. This breaks
sage_conf
-less installs.
Please open a followup ticket. It's not clear to me how or what exactly needs to be be fixed.
comment:135 in reply to: ↑ 131 ; follow-up: ↓ 149 Changed 7 months ago by
- Status changed from positive_review to needs_work
comment:136 Changed 7 months ago by
needs work ? On what exactly?
comment:137 Changed 7 months ago by
I suggest we run tests on the followup ticket for sage_conf
-less installs.
The most one might need to adjust here is the list of distros on which this works.
comment:138 in reply to: ↑ 134 ; follow-up: ↓ 140 Changed 7 months ago by
Replying to dimpase:
It's not clear to me how or what exactly needs to be be fixed.
This unconditionally imports sage_conf
so it breaks on systems without it. As I said in comment:97 adding a simple fallback should fix it, here is a patch that works for me
-
src/sage/libs/singular/singular.pyx
diff --git a/src/sage/libs/singular/singular.pyx b/src/sage/libs/singular/singular.pyx index ea6d0ab01d..8dfa490de5 100644
a b cdef init_libsingular(): 768 768 769 769 cdef void *handle = NULL 770 770 771 from sage_conf import LIBSINGULAR_PATH 771 try: 772 from sage_conf import LIBSINGULAR_PATH 773 except ImportError: 774 # No sage_conf, assume everything is properly set up by the distro 775 LIBSINGULAR_PATH="libSingular.so" 772 776 lib = str_to_bytes(LIBSINGULAR_PATH, FS_ENCODING, "surrogateescape") 773 777 774 778 # This is a workaround for https://github.com/Singular/Singular/issues/1113
comment:139 Changed 7 months ago by
- Status changed from needs_work to needs_review
comment:140 in reply to: ↑ 138 Changed 7 months ago by
- Status changed from needs_review to needs_work
Replying to arojas:
Replying to dimpase:
It's not clear to me how or what exactly needs to be be fixed.
This unconditionally imports
sage_conf
so it breaks on systems without it. As I said in comment:97 adding a simple fallback should fix it, here is a patch that works for me
src/sage/libs/singular/singular.pyx
diff --git a/src/sage/libs/singular/singular.pyx b/src/sage/libs/singular/singular.pyx index ea6d0ab01d..8dfa490de5 100644
a b cdef init_libsingular(): 768 768 769 769 cdef void *handle = NULL 770 770 771 from sage_conf import LIBSINGULAR_PATH 771 try: 772 from sage_conf import LIBSINGULAR_PATH 773 except ImportError: 774 # No sage_conf, assume everything is properly set up by the distro 775 LIBSINGULAR_PATH="libSingular.so" 772 776 lib = str_to_bytes(LIBSINGULAR_PATH, FS_ENCODING, "surrogateescape") 773 777 774 778 # This is a workaround for https://github.com/Singular/Singular/issues/1113
Actually this should be rewritten so that it goes through a variable declared in sage.env
instead of using sage_conf
directly.
sage.env
already takes care of conditional import from sage_conf
.
comment:141 Changed 7 months ago by
- Status changed from needs_work to needs_review
Please either propose a concrete patch, or move this over to a followup tiket.
comment:142 Changed 7 months ago by
I think what he means is that one should keep using sage.env
instead of sage_conf
here
https://github.com/sagemath/sagetrac-mirror/compare/develop...u/mjo/ticket/29024#diff-25a6d752c56f179f8798c63d0938849853c5c6d61b21a171aae63bee0f6eb6cfL771-R773
I'm not sure about the conventions but for cblas pkgconfig is used in sage.env
and not in in the spk_configure script, see https://github.com/sagemath/sage/blob/056b8d4e7bcdc936af39a33d3885925f1c850f80/src/sage/env.py#L401-L407. I guess the latter is more flexible as it would also work outside of sage's build environment, i.e. without LIBSINGULAR_PATH set.
comment:143 Changed 7 months ago by
This sounds more and more like "move this over to a followup ticket" --- for pkg-config and its generated files are a special can of worms to open.
comment:144 follow-up: ↓ 150 Changed 7 months ago by
- Commit changed from a3c53051a5961226700bdb8a40f831092f8dffcd to f01ff1f0f076fe550ffb6a6a657c5fda9dc0a023
Branch pushed to git repo; I updated commit sha1. New commits:
f01ff1f | Trac #29024: obtain LIBSINGULAR_PATH from sage.env instead of sage_conf.
|
comment:145 follow-up: ↓ 146 Changed 7 months ago by
arojas, does this work?
comment:146 in reply to: ↑ 145 Changed 7 months ago by
comment:147 follow-up: ↓ 151 Changed 7 months ago by
By the way, while trying to combine this and u/mkoeppe/spkg_configure_m4_for_polymake
on Arch I see some messages about conflicting BLAS for Polymake (BLAS) and Singular (OpenBLAS). Is this normal?
comment:148 Changed 7 months ago by
- Status changed from needs_review to positive_review
OK, good, thanks.
comment:149 in reply to: ↑ 135 ; follow-up: ↓ 152 Changed 7 months ago by
Replying to mkoeppe:
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
a3c5305 Trac #29024: re-enable the system singular hint on homebrew.
This needs testing on GH Actions.
In the cited commit, you removed the comment that explained why the singular
package was not included on homebrew. Unless homebrew packaging of singular
has changed in the meantime, this will break all of our automated tests on homebrew.
This is what would have needed testing.
comment:150 in reply to: ↑ 144 Changed 7 months ago by
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
f01ff1f Trac #29024: obtain LIBSINGULAR_PATH from sage.env instead of sage_conf.
Thanks for rewriting it "so that it goes through a variable declared in sage.env
instead of using sage_conf
directly", as suggested in comment:140.
This is exactly what I meant.
comment:151 in reply to: ↑ 147 Changed 7 months ago by
Replying to dimpase:
By the way, while trying to combine this and
u/mkoeppe/spkg_configure_m4_for_polymake
on Arch I see some messages about conflicting BLAS for Polymake (BLAS) and Singular (OpenBLAS). Is this normal?
What messages are these, and when are they issued?
comment:152 in reply to: ↑ 149 ; follow-up: ↓ 154 Changed 7 months ago by
Replying to mkoeppe:
In the cited commit, you removed the comment that explained why the
singular
package was not included on homebrew. Unless homebrew packaging ofsingular
has changed in the meantime, this will break all of our automated tests on homebrew.This is what would have needed testing.
The impetus for the last round of major changes was comment:93 which found that homebrew worked with some modification -- particularly using the full path to libSingular.
comment:153 Changed 7 months ago by
- Branch changed from u/mjo/ticket/29024 to f01ff1f0f076fe550ffb6a6a657c5fda9dc0a023
- Resolution set to fixed
- Status changed from positive_review to closed
comment:154 in reply to: ↑ 152 ; follow-up: ↓ 161 Changed 7 months ago by
- Commit f01ff1f0f076fe550ffb6a6a657c5fda9dc0a023 deleted
Replying to mjo:
Replying to mkoeppe:
In the cited commit, you removed the comment that explained why the
singular
package was not included on homebrew. Unless homebrew packaging ofsingular
has changed in the meantime, this will break all of our automated tests on homebrew.This is what would have needed testing.
The impetus for the last round of major changes was comment:93 which found that homebrew worked with some modification -- particularly using the full path to libSingular.
I don't have a comment on using the homebrew singular package; I haven't tested this ticket.
The issue lies in *installing* the singular package on homebrew when a non-standard prefix (= not /usr/local) is in use -- which is what our testing workflows on GH Actions do.
comment:155 Changed 7 months ago by
using Homebrew with non-standard prefix is not something the upstream is keen on supporting.
comment:156 Changed 7 months ago by
This is true, but for us it is a valuable mechanism to test homebrew configurations.
comment:157 follow-up: ↓ 159 Changed 7 months ago by
well, one ends up running into homebrew bugs for no good reason this way.
(I must say I looked at tox.ini and stuff in .github, and failed to see how this is done)
comment:158 Changed 7 months ago by
Let me explain the purpose in case it is really unclear: By using a local installation of homebrew, a developer can actually do testing against different homebrew package configurations without compromising the normal environment on one's machine that one uses for ... you know ... other work.
comment:159 in reply to: ↑ 157 Changed 7 months ago by
Replying to dimpase:
(I must say I looked at tox.ini and stuff in .github, and failed to see how this is done)
There are 2 lines around line 512 in tox.ini, which just follow the instructions at https://docs.brew.sh/Installation
comment:160 Changed 7 months ago by
well, yes, what I don't understand is the purpose of having a nonstandard Homebrew location (as well as what this location actually is).
comment:161 in reply to: ↑ 154 Changed 7 months ago by
Replying to mkoeppe:
I don't have a comment on using the homebrew singular package; I haven't tested this ticket.
The issue lies in *installing* the singular package on homebrew when a non-standard prefix (= not /usr/local) is in use -- which is what our testing workflows on GH Actions do.
The homebrew docs pretty clearly say "Pick another prefix at your peril!"
If singular works well in a supported homebrew configuration, it's mean to hide the system package suggestion from everyone for the sake of an unsupported, non-user-facing test scenario.
Can't we add a sed "/singular/d"
somewhere in that GH action, closer to where the problem originates, to fix it?
comment:162 Changed 7 months ago by
Yes, this is a solution that I would support.
comment:163 follow-up: ↓ 164 Changed 7 months ago by
On Debian 11 (a.k.a. bullseye, stable) Singular's libs have different names, and are failing to be picked up: they all are installed in /usr/lib/x86_64-linux-gnu/
and we have the following: I add Sage's names next to their
Debian Sage libsingular-factory.so libfactory.so libsingular-gfan.so ??? libsingular-omalloc.so libomalloc.so libsingular-polys.so libpolys.so libsingular-resources.so libsingular_resources.so libsingular-Singular.so libSingular.so
While it's not hard to test for libsingular-Singular.so
in place of libSingular.so
, it's some more work to use it.
Library names may be obtained from $ pkg-config --libs-only-l singular
, which on Debian produces
-lsingular-Singular -ldl -lsingular-polys -lntl -lgmp -ldl -lsingular-factory -lntl -lgmp -lsingular-omalloc -lsingular-resources
I suppose this needs a followup ticket.
comment:164 in reply to: ↑ 163 Changed 7 months ago by
Replying to dimpase:
While it's not hard to test for
libsingular-Singular.so
in place oflibSingular.so
, it's some more work to use it.
File a bug? Sometimes, people are going to want to use non-debian packages on debian. The name of the library is libSingular, and that's what everyone will (and should) try to use. It's as fundamental to the API as anything can be.
There's probably a name conflict with some other library that is better avoided some other way, e.g. by having the two packages with conflicting files block each other. Or at the very least, by renaming only the problematic library. I doubt some other package is installing libSingular.so
.
comment:165 follow-up: ↓ 166 Changed 7 months ago by
other packages may be installing libfactory.so
- I guess they wanted to prefix all the libs produced by Singular with singular-
.
bug report or not, knowing Debian pace it ought to be fixed regardless.
comment:166 in reply to: ↑ 165 Changed 7 months ago by
Replying to dimpase:
other packages may be installing
libfactory.so
- I guess they wanted to prefix all the libs produced by Singular withsingular-
.bug report or not, knowing Debian pace it ought to be fixed regardless.
Looks like a pre-emptive change:
comment:167 Changed 7 months ago by
I have opened #32789 for the follow up. I'd suggest to give up on trying to educate Debian on doing things in the same that Gentoo does. This is not a viable approach for our project.
comment:168 follow-up: ↓ 170 Changed 6 months ago by
On an OS X machine I used homebrew
to install cddlib
(ignoring the comment in build/pkgs/cddlib/distros/homebrew.txt
) and singular
, and now Sage does not build them anymore. I get a related doctest failure, though:
File "src/sage/env.py", line 280, in sage.env._get_shared_lib_path Failed example: fnmatch(str(lib_filename), pattern) Expected: True Got: False ********************************************************************** 1 item had failures: 1 of 8 in sage.env._get_shared_lib_path [47 tests, 1 failure, 2.57 s]
This is because lib_filename
is defined by lib_filename = _get_shared_lib_path("Singular", "singular-Singular")
, and on my machine it ends up being None
. On other platforms that use an external Singular installation, how does this doctest pass?
comment:169 follow-up: ↓ 171 Changed 6 months ago by
could this be due to the path to libsingular.dylib
not being added to LIBRARY_PATH
in .homebrew_env_config
?
comment:170 in reply to: ↑ 168 Changed 6 months ago by
Replying to jhpalmieri:
On other platforms that use an external Singular installation, how does this doctest pass?
Accidentally, like anything that uses _get_shared_lib_path
. It's not a good function. This doctest should be replaced with one that looks for libgap, since we no longer care if it can find libSingular (the full path to libSingular is contained in LIBSINGULAR_PATH
). Since GAP -- at least for the moment -- is guaranteed to come from our SPKG, an analogous test has some chance of passing.
comment:171 in reply to: ↑ 169 Changed 6 months ago by
Replying to dimpase:
could this be due to the path to
libsingular.dylib
not being added toLIBRARY_PATH
in.homebrew_env_config
?
.homebrew-build-env
is for build time, not for run time
comment:172 follow-up: ↓ 173 Changed 6 months ago by
Will fix it in #32880.
Which versions of singular are good?