sagemath/sage

Update singular to stop /usr/local from leaking into Singular build

mkoeppe opened this issue · 29 comments

On macOS, even when using gcc -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk (which disables use of /usr/local), Singular's configure script sneaks in /usr/local/include and /usr/local/lib.

It was created by singular configure 4.2.0p1+2021-03-13+sage, which was
generated by GNU Autoconf 2.69.  Invocation command line was

  $ ./configure --prefix=/var/tmp/sage-9.3.rc0-cpython-38-darwin/local --libdir=/var/tmp/sage-9.3.rc0-cpython-38-darwin/local/lib --disable-maintainer-mode
 --disable-dependency-tracking --exec-prefix=/var/tmp/sage-9.3.rc0-cpython-38-darwin/local --bindir=/var/tmp/sage-9.3.rc0-cpython-38-darwin/local/bin --wit
h-ntl=/var/tmp/sage-9.3.rc0-cpython-38-darwin/local --with-flint=/var/tmp/sage-9.3.rc0-cpython-38-darwin/local --enable-gfanlib --enable-Singular --enable-
factory --disable-doc --disable-polymake --without-python --without-pythonmodule --disable-python --disable-python_module --disable-python-module --disable
-static

PATH: /private/var/tmp/sage-9.3.rc0-cpython-38-darwin/build/bin
PATH: /private/var/tmp/sage-9.3.rc0-cpython-38-darwin/src/bin
PATH: /var/tmp/sage-9.3.rc0-cpython-38-darwin/local/bin
PATH: /private/var/tmp/sage-9.3.rc0-cpython-38-darwin/build/bin
PATH: /private/var/tmp/sage-9.3.rc0-cpython-38-darwin/src/bin
PATH: /var/tmp/sage-9.3.rc0-cpython-38-darwin/local/bin
PATH: /private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-build-env-dcsf70hx/overlay/bin
PATH: /private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-build-env-dcsf70hx/normal/bin
PATH: /Users/mkoeppe/s/sage/sage-rebasing/worktree-dist/src/pkgs/sage_conf-relocatable/.tox/python-macos-10.15-python3.8/bin
PATH: /usr/local/opt/xz/bin
PATH: /usr/local/opt/gpatch/bin
PATH: /usr/bin
PATH: /bin
PATH: /usr/sbin
PATH: /sbin

configure:20714: checking gmp.h usability
configure:20714: gcc -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -c -O2 -g  -pipe -fno-common -g0 -O3 -Wno-unused-function -Wno-trigraphs -Wno-unused-parameter -Wunknown-pragmas -Wno-unused-variable -fomit-frame-pointer -fwrapv -fvisibility=default -finline-functions -fno-exceptions -fno-threadsafe-statics -funroll-loops -fno-delete-null-pointer-checks -Qunused-arguments -pthread -I/usr/local/include  conftest.c >&5
configure:20714: $? = 0
configure:20714: result: yes

If NTL from homebrew is installed, this can lead to an NTL threading mismatch:

    [singular-4.2.0p1+2021-03-13+sage]   ld: illegal thread local variable reference to regular symbol __ZN3NTL13ErrorCallbackE for architecture x86_64
    [singular-4.2.0p1+2021-03-13+sage]   clang: error: linker command failed with exit code 1 (use -v to see invocation)

PR: Singular/Singular#1066

See also: #31348 build/pkgs/{mpir,mpfr}/spkg-configure.m4: Check pkg-config first

Upstream: Fixed upstream, but not in a stable release.

CC: @dimpase @slel @jhpalmieri @zlscherr @kiwifb @videlec @antonio-rojas

Component: packages: standard

Author: Matthias Koeppe

Branch/Commit: 8b5b273

Reviewer: François Bissey

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

comment:1

Actually, driver error, I ran into #29620 again

comment:2

But the problem is real - Singular will not use gmp that can be found by compiler and linker if it finds /usr/local/include/gmp.h
This ought to be fixed upstream.

comment:3

It also ignores an explicitly passed gmp directory (using --with-gmp=...) when there is an installation in /usr/local because the search loop is missing a break. https://github.com/Singular/Singular/blob/spielwiese/m4/gfanlib-check.m4#L40

This can only be fixed by making a PR + new singular tarball.

Upstream: Reported upstream. No feedback yet.

Description changed:

--- 
+++ 
@@ -39,7 +39,8 @@
 ```
 
 
-We fix this by passing gmp configuration to the Singular configure script.
+PR: https://github.com/Singular/Singular/pull/1066
+
 
 See also: #31348 `build/pkgs/{mpir,mpfr}/spkg-configure.m4`: Check pkg-config first
 

Author: Matthias Koeppe

Commit: 8b5b273

comment:7

Picking up a newer upstream spielwiese HEAD (which contains a few new commits that look like bugfixes) plus two new PRs: Singular/Singular#1066 and Singular/Singular#1067; the new tarball is made from tag https://github.com/mkoeppe/Singular/tree/singular-4.2.0p1+2021-03-24%2Bsage-2

Recipe for making the tarball: From within a Sage shell, I used ./configure --with-ntl=/usr/local --with-gmp=/usr/local --disable-pyobject-module --enable-doc-build --with-libparse && make && make dist


New commits:

8b5b273build/pkgs/singular/checksums.ini: Use 4.2.0p1+2021-03-24+sage-2
comment:8

Upstream has merged the two PRs.

Needs review

slel commented
comment:9

How to review this?

slel commented

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

comment:10

This is a tiny increment over the update done in #25993.
If it builds, it ships, I'd think

comment:11

Replying to @mkoeppe:

This is a tiny increment over the update done in #25993.
If it builds, it ships, I'd think

I'd agree with that. I have been thinking of giving the detection of other libraries the treatment I gave to flint and I decided to give up because I don't need that kind of problem right now. I like the simplicity of your minimal fix.

comment:12

Tests have been running at https://github.com/mkoeppe/sage/runs/2197735803

comment:13

Rather than using our own tarball, it's more common to use patches (and then delete the patches when they're merged in a stable upstream release). Why not do that here?

comment:14

https://doc.sagemath.org/html/en/developer/packaging.html#when-to-patch-when-to-repackage-when-to-autoconfiscate explains why patching is not always possible.
(In short: Changes to the autotooling are done, and thus the build system is regenerated)

comment:15

Replying to @mkoeppe:

https://doc.sagemath.org/html/en/developer/packaging.html#when-to-patch-when-to-repackage-when-to-autoconfiscate explains why patching is not always possible.
(In short: Changes to the autotooling are done, and thus the build system is regenerated)

The fact that autotools files need to be regenerated is one of the strongest point in favor of cmake compared to autotools for downstream re-packagers in my opinion.

comment:16

Beyond the scope of this ticket to switch upstream to another build system.

comment:17

That was just a general comment. Is there anything holding that ticket? It mostly looks good to me, some of your testruns appear to have failed, one at least for lack of space. Anything more serious?

comment:18

Replying to @kiwifb:

Is there anything holding that ticket?

Not as far as I can see

Changed reviewer from https://github.com/mkoeppe/sage/runs/2197735803 to François Bissey

comment:19

I am putting my name on the reviewer field then. Ship it.

comment:20

Thanks!

comment:21

Next ticket in the same direction: #31562

comment:22

Setting priority to blocker to bring this ticket to the attention of the release bot.