#31552 closed defect (fixed)

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

Reported by: Matthias Köppe Owned by:
Priority: blocker Milestone: sage-9.3
Component: packages: standard Keywords:
Cc: Dima Pasechnik, Samuel Lelièvre, John Palmieri, Zachary L Scherr, François Bissey, Vincent Delecroix, Antonio Rojas Merged in:
Authors: Matthias Koeppe Reviewers: François Bissey
Report Upstream: Fixed upstream, but not in a stable release. Work issues:
Branch: 8b5b273 (Commits, GitHub, GitLab) Commit: 8b5b2739d107271f8477cee8cd941bec8b255827
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

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: https://github.com/Singular/Singular/pull/1066

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

Attachments (1)

singular-4.2.0p1+2021-03-24+sage-2.tar.gz (13.2 MB) - added by Matthias Köppe 18 months ago.

Change History (24)

comment:1 Changed 19 months ago by Matthias Köppe

Actually, driver error, I ran into #29620 again

comment:2 Changed 19 months ago by Matthias Köppe

Priority: criticalmajor

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

Version 0, edited 19 months ago by Matthias Köppe (next)

comment:3 Changed 19 months ago by Matthias Köppe

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.

comment:4 Changed 19 months ago by Matthias Köppe

Description: modified (diff)
Report Upstream: N/AReported upstream. No feedback yet.

comment:5 Changed 19 months ago by Matthias Köppe

Priority: majorcritical

Changed 18 months ago by Matthias Köppe

comment:6 Changed 18 months ago by Matthias Köppe

Branch: u/mkoeppe/_usr_local_leaks_into_singular_build

comment:7 Changed 18 months ago by Matthias Köppe

Authors: Matthias Koeppe
Cc: François Bissey Vincent Delecroix Antonio Rojas added
Commit: 8b5b2739d107271f8477cee8cd941bec8b255827
Status: newneeds_review
Summary: /usr/local leaks into Singular buildUpdate singular to stop /usr/local from leaking into Singular build

Picking up a newer upstream spielwiese HEAD (which contains a few new commits that look like bugfixes) plus two new PRs: https://github.com/Singular/Singular/pull/1066 and https://github.com/Singular/Singular/pull/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
Last edited 18 months ago by Matthias Köppe (previous) (diff)

comment:8 Changed 18 months ago by Matthias Köppe

Upstream has merged the two PRs.

Needs review

comment:9 Changed 18 months ago by Samuel Lelièvre

Report Upstream: Reported upstream. No feedback yet.Fixed upstream, but not in a stable release.

How to review this?

comment:10 Changed 18 months ago by Matthias Köppe

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

comment:11 in reply to:  10 Changed 18 months ago by François Bissey

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 Changed 18 months ago by Matthias Köppe

Reviewers: https://github.com/mkoeppe/sage/runs/2197735803

comment:13 Changed 18 months ago by John Palmieri

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 Changed 18 months ago by Matthias Köppe

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 in reply to:  14 Changed 18 months ago by François Bissey

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 Changed 18 months ago by Matthias Köppe

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

comment:17 Changed 18 months ago by François Bissey

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 in reply to:  17 Changed 18 months ago by Matthias Köppe

Replying to fbissey:

Is there anything holding that ticket?

Not as far as I can see

comment:19 Changed 18 months ago by François Bissey

Reviewers: https://github.com/mkoeppe/sage/runs/2197735803François Bissey
Status: needs_reviewpositive_review

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

comment:20 Changed 18 months ago by Matthias Köppe

Thanks!

comment:21 Changed 18 months ago by Matthias Köppe

Next ticket in the same direction: #31562

comment:22 Changed 18 months ago by Matthias Köppe

Priority: criticalblocker

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

comment:23 Changed 18 months ago by Volker Braun

Branch: u/mkoeppe/_usr_local_leaks_into_singular_build8b5b2739d107271f8477cee8cd941bec8b255827
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.