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

spkg-configure.m4 for singular #29024

Closed
mkoeppe opened this issue Jan 16, 2020 · 196 comments
Closed

spkg-configure.m4 for singular #29024

mkoeppe opened this issue Jan 16, 2020 · 196 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 16, 2020

  1. For singular, the location of the dynamic library needs to be communicated to the sage runtime.

    We add a configuration variable SINGULAR_SO to sage_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:

  1. src/bin/sage-env unconditionally sets environment variable SINGULARPATH="$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 Obtain singular.hlp location via libsingular_resources #32254, SINGULARPATH is only used by qepcad -- this last use should be removed in QEPCAD: improve installation locations #31275.)

  2. src/bin/sage-env unconditionally sets environment variable SINGULAR_EXECUTABLE="$SAGE_LOCAL/bin/Singular".

    It should be investigated whether this is actually needed in any supported configuration. If not, remove.

    (Removed in Drop SINGULAR_EXECUTABLE from src/bin/sage-env #32302.)

  3. src/sage/interfaces/singular.py tries to use SINGULARPATH 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)

Upstream: Reported upstream. No feedback yet.

CC: @dimpase @kiwifb @slel @isuruf @orlitzky @tobiasdiez @antonio-rojas @dkwo

Component: build: configure

Keywords: sd111

Author: Michael Orlitzky, Samuel Lelièvre

Branch: f01ff1f

Reviewer: Dima Pasechnik, Matthias Koeppe

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

@mkoeppe mkoeppe added this to the sage-9.1 milestone Jan 16, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 16, 2020

comment:1

Which versions of singular are good?

@mwageringel
Copy link

comment:2

Replying to @mkoeppe:

Which versions of singular are good?

Sage does not currently work correctly with any version of Singular, see #25993.

@dimpase
Copy link
Member

dimpase commented Jan 16, 2020

comment:3

well, Debian does have a Singular version which is compatible with their Sage package (v8.6 or so, I guess).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 16, 2020

comment:4

Thanks for the info. I'll check back when #25993 is done.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 Jan 16, 2020
@thierry-FreeBSD
Copy link

comment:5

This ticket should be listed in #27330 (ATM singular is listed in "No Tickets yet").

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 16, 2020

comment:6

Replying to @thierry-FreeBSD:

This ticket should be listed in #27330 (ATM singular is listed in "No Tickets yet").

I've added it

@thierry-FreeBSD
Copy link

Attachment: spkg-configure.m4.gz

spkg-configure.m4 to be put under /build/pkgs/singular/

@thierry-FreeBSD
Copy link

comment:8

[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 to spkg-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 and src/bin/sage-env must be modified so that SINGULARPATH is set to singlar's share dir (and not SAGE_SHARE / DESTDIR), and SINGULAR_EXECUTABLE is set to the real Singular and not under the DESTDIR directory.

Now it is OK for me (on FreeBSD).

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Aug 13, 2020
@dimpase
Copy link
Member

dimpase commented Sep 11, 2020

comment:12

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 11, 2020

comment:13

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.

@dimpase
Copy link
Member

dimpase commented Sep 11, 2020

comment:14

Replying to @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 to spkg-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 and src/bin/sage-env must be modified so that SINGULARPATH is set to singlar's share dir (and not SAGE_SHARE / DESTDIR), and SINGULAR_EXECUTABLE is set to the real Singular and not under the DESTDIR 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.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 11, 2020

comment:15

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``.

@dimpase
Copy link
Member

dimpase commented Sep 11, 2020

comment:16

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

@dimpase
Copy link
Member

dimpase commented Sep 11, 2020

Attachment: homebrew-singular-experiment.patch.gz

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 12, 2020

comment:17

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)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 12, 2020

comment:19

I have opened #30561 for this

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title spkg-configure.m4 for singular Make location of the Singular shared library configurable through sage-config Sep 16, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 24, 2021

comment:151

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?

@orlitzky
Copy link
Contributor

comment:152

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 of singular 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.

@vbraun
Copy link
Member

vbraun commented Oct 24, 2021

Changed branch from u/mjo/ticket/29024 to f01ff1f

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 24, 2021

Changed commit from f01ff1f to none

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 24, 2021

comment:154

Replying to @orlitzky:

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 of singular 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.

@dimpase
Copy link
Member

dimpase commented Oct 24, 2021

comment:155

using Homebrew with non-standard prefix is not something the upstream is keen on supporting.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 24, 2021

comment:156

This is true, but for us it is a valuable mechanism to test homebrew configurations.

@dimpase
Copy link
Member

dimpase commented Oct 26, 2021

comment:157

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)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 26, 2021

comment:158

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.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 26, 2021

comment:159

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

@dimpase
Copy link
Member

dimpase commented Oct 26, 2021

comment:160

well, yes, what I don't understand is the purpose of having a nonstandard Homebrew location (as well as what this location actually is).

@orlitzky
Copy link
Contributor

comment:161

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?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 26, 2021

comment:162

Yes, this is a solution that I would support.

@dimpase
Copy link
Member

dimpase commented Oct 28, 2021

comment:163

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.

@orlitzky
Copy link
Contributor

comment:164

Replying to @dimpase:

While it's not hard to test for libsingular-Singular.so in place of libSingular.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.

@dimpase
Copy link
Member

dimpase commented Oct 28, 2021

comment:165

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.

@orlitzky
Copy link
Contributor

comment:166

Replying to @dimpase:

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.

Looks like a pre-emptive change:

https://sources.debian.org/src/singular/1:4.1.1-p2+ds-4/debian/patches/debianization-rename-libraries.patch/

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 29, 2021

comment:167

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.

@jhpalmieri
Copy link
Member

comment:168

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?

@dimpase
Copy link
Member

dimpase commented Nov 16, 2021

comment:169

could this be due to the path to libsingular.dylib not being added to LIBRARY_PATH in .homebrew_env_config?

@orlitzky
Copy link
Contributor

comment:170

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.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 16, 2021

comment:171

Replying to @dimpase:

could this be due to the path to libsingular.dylib not being added to LIBRARY_PATH in .homebrew_env_config?

.homebrew-build-env is for build time, not for run time

@orlitzky
Copy link
Contributor

comment:172

Will fix it in #32880.

@jhpalmieri
Copy link
Member

comment:173

Replying to @orlitzky:

Will fix it in #32880.

Great, thank you.

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

10 participants