Opened 22 months ago

Last modified 3 months ago

#31348 new enhancement

build/pkgs/{gmp,mpfr}/spkg-configure.m4: Check pkg-config first

Reported by: Matthias Köppe Owned by:
Priority: critical Milestone: sage-9.8
Component: build: configure Keywords: mpfr
Cc: Dima Pasechnik, John Palmieri, Zachary L Scherr, Michael Orlitzky Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: u/gh-zlscherr/add_gmp_mpfr_cython (Commits, GitHub, GitLab) Commit: 07e00b608d79e90c8854c2b4c157a1d6456e9985
Dependencies: #31344, #32549 Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

Currently gmp.pc, which may be provided at least on some platforms, is not used at all -- neither by configure nor by sage.env.cython_aliases, sage.misc.cython.

For example, on homebrew, using it will provide the specific library search directory -L/usr/local/Cellar/gmp/6.2.1/lib instead of having to rely on /usr/local/ (which is disabled when compiler/linker are invoked with -isysroot)

Currently, gmp is detected using AC_CHECK_HEADER, and the result is communicated to sage-build-env-config as variables SAGE_GMP_PREFIX and SAGE_GMP_INCLUDE; but it is not available in the runtime environment (sage_conf.py, sage-env-config).

The failures reported in

can be fixed by simply changing sage.misc.cython to pick up gmp.pc. But to avoid unpleasant surprises, we need to make sure that it is used consistently when present.

(After #32549, the relevant file is build/pkgs/gmp/spkg-configure.m4.)

The above applies to mpfr as well. See failure report in https://groups.google.com/g/sage-release/c/8ctwsUGl6LQ/m/Ob38Iyh1AAAJ

Similar situation with readline (#29000): The spkg-configure script already uses pkg-config - but forgets to tell the build system what it found.

Previous tickets: #27212

Change History (45)

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

Description: modified (diff)

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

Description: modified (diff)

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

Description: modified (diff)
Summary: build/pkgs/mpir/spkg-configure.m4: Check pkg-config firstbuild/pkgs/{mpir,mpfr}/spkg-configure.m4: Check pkg-config first

comment:4 Changed 22 months ago by Zachary L Scherr

I started to play around with this a bit, and I can use python’s pkgconfig to locate gmp and mpfr. After doing that though, I get complaints about not being able to locagte -lntl, which I don’t think I can find with pkgconfig since it doesn’t have an ntl.pc file.

This may be a bit too hacky, but would it be possible in the build process to copy the various includes and libraries to SAGE_LOCAL/{include, lib} that are picked up by configure? That seems like it would solve a whole host of problems at the expense of if the user updates one of those libraries then the new version would not be picked up.

comment:5 in reply to:  4 Changed 22 months ago by Matthias Köppe

Replying to gh-zlscherr:

I started to play around with this a bit, and I can use python’s pkgconfig to locate gmp and mpfr. After doing that though, I get complaints about not being able to locagte -lntl, which I don’t think I can find with pkgconfig since it doesn’t have an ntl.pc file.

For NTL, the configure script already determines SAGE_NTL_PREFIX. This variable just needs to be used more.

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

I've opened #31365 for NTL.

comment:7 Changed 20 months ago by Zachary L Scherr

Branch: u/gh-zlscherr/add_gmp_mpfr_cython

comment:8 Changed 20 months ago by git

Commit: 07e00b608d79e90c8854c2b4c157a1d6456e9985

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

07e00b6Modify stdlibs/incs in misc/cython.py

comment:9 Changed 20 months ago by Zachary L Scherr

I pushed very simple code for trying to deal with this. It's probably better to use regular expressions, but I just wanted a proof of concept. This allows make ptest to work with homebrew without having to source .homebrew-build-env.

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

Description: modified (diff)

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

Description: modified (diff)

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

Description: modified (diff)

comment:13 Changed 20 months ago by David Roe

I've started getting failures that are fixed by sourcing .homebrew-build-env even when I'm just changing Cython files and recompiling (not changing any spkgs). Is this expected?

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

Yes, this is part of what this ticket intends to fix.

comment:15 Changed 20 months ago by David Roe

Great, thanks for the clarification!

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

Milestone: sage-9.3sage-9.4

Moving to 9.4, as 9.3 has been released.

comment:17 Changed 16 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

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

Description: modified (diff)

comment:19 Changed 15 months ago by Dima Pasechnik

some distros, like Fedora, have an unpleasant habit of not installing .pc files, while these are available.

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

Yes, the ticket is "Check pkg-config first", not "Check pkg-config only".

comment:21 Changed 15 months ago by Michael Orlitzky

IMO homebrew (and anyone else installing libgmp to a nonstandard directory) should be pestered to install it to the system's default location. I'm pretty sure that the gmp and mpir packages then conflict, but you don't need to install them both at the same time. In fact, mpir can probably be deprecated.

Using pkg-config just changes this potential problem (libraries in wrong locations) to another one (pc files in wrong locations) and adds an extra layer of indirection in the process. We may have to go ahead and fix it anyway, but we shouldn't lose track of the simple and easy way that this was all meant to work.

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

The problem on homebrew is really a very specific one, which I have explained in the ticket description: The GMP headers *are* installed in what you would call the "standard location" (/usr/local/), but when compiler/linker are invoked with -isysroot, then /usr/local is not a standard location. Yes, we fix it in the *build* environment using .homebrew-build-env but that does not affect the compiler use in the *runtime* environment.

comment:23 in reply to:  21 Changed 15 months ago by Dima Pasechnik

Replying to mjo:

IMO homebrew (and anyone else installing libgmp to a nonstandard directory) should be pestered to install it to the system's default location. I'm pretty sure that the gmp and mpir packages then conflict, but you don't need to install them both at the same time. In fact, mpir can probably be deprecated.

No, brew does not install mpir in gmp-compatible mode, as far as I can tell from reading https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/mpir.rb

  • so there is no interference with gmp.

comment:24 Changed 15 months ago by Dima Pasechnik

Besides, we're getting rid of mpir spkg all together.

comment:25 Changed 15 months ago by Matthias Köppe

This ticket has really nothing to do with MPIR. It's just (pre #32549) that the spkg-configure duties for gmp are done by MPIR's spkg-configure.

comment:26 in reply to:  22 Changed 15 months ago by Michael Orlitzky

Replying to mkoeppe:

The problem on homebrew is really a very specific one, which I have explained in the ticket description: The GMP headers *are* installed in what you would call the "standard location" (/usr/local/), but when compiler/linker are invoked with -isysroot, then /usr/local is not a standard location. Yes, we fix it in the *build* environment using .homebrew-build-env but that does not affect the compiler use in the *runtime* environment.

Oh, the description makes it sounds like libgmp being in /usr/local/Cellar/gmp/6.2.1/lib is the problem. Is this another case of trying to use cython (which in turn uses the C toolchain) without sourcing .homebrew-build-env to make the C toolchain work?

comment:27 Changed 15 months ago by Matthias Köppe

It's really just the broken design of homebrew's python3. They use -isysroot in the build configuration of python, which goes into the compiler/flags of any extensions built via distutils/setuptools. Then they add back a few homebrew directories via distutils.cfg. It's hopelessly broken because these directories take priority over other user/package-provided include and linker directories. At build time, we work around it by disabling use of the system python's vendored distutils and use our own. At runtime, we would be best off by using paths explicitly that can be discovered via pkg-config -- because in the Sage library we already do that for all other packages.

comment:28 Changed 15 months ago by Dima Pasechnik

is this reported to Homebrew as a bug?

comment:29 in reply to:  28 ; Changed 15 months ago by Matthias Köppe

Replying to dimpase:

is this reported to Homebrew as a bug?

I didn't report it when I came across it in #31335. I figured a bug report titled "distutils.cfg considered harmful" would drain a lot of energy...

comment:30 Changed 15 months ago by Matthias Köppe

Dependencies: #31344#31344, #32549

comment:31 in reply to:  29 ; Changed 15 months ago by Dima Pasechnik

Replying to mkoeppe:

Replying to dimpase:

is this reported to Homebrew as a bug?

I didn't report it when I came across it in #31335. I figured a bug report titled "distutils.cfg considered harmful" would drain a lot of energy...

You don't need to fight a battle, merely report the problem, please.

comment:32 Changed 14 months ago by Matthias Köppe

They're pretty quick with closing as wontfix if one doesn't also provide a patch

comment:33 in reply to:  32 Changed 14 months ago by Dima Pasechnik

Replying to mkoeppe:

They're pretty quick with closing as wontfix if one doesn't also provide a patch

Sprecht du kein Ruby, oder? :-)

comment:34 in reply to:  31 ; Changed 12 months ago by Matthias Köppe

Replying to dimpase:

Replying to mkoeppe:

Replying to dimpase:

is this reported to Homebrew as a bug?

I didn't report it when I came across it in #31335. I figured a bug report titled "distutils.cfg considered harmful" would drain a lot of energy...

You don't need to fight a battle, merely report the problem, please.

I sent a first PR to reduce homebrew's distutils.cfg - https://github.com/Homebrew/homebrew-core/pull/91043

comment:35 Changed 11 months ago by Samuel Lelièvre

Description: modified (diff)
Keywords: mpfr added
Summary: build/pkgs/{mpir,mpfr}/spkg-configure.m4: Check pkg-config firstbuild/pkgs/mpfr/spkg-configure.m4: Check pkg-config first

Removed mpir from ticket summary after its removal in #32549, #32727.

comment:36 Changed 11 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:37 Changed 11 months ago by Matthias Köppe

Description: modified (diff)

comment:38 in reply to:  34 Changed 10 months ago by Matthias Köppe

Replying to mkoeppe:

I sent a first PR to reduce homebrew's distutils.cfg - https://github.com/Homebrew/homebrew-core/pull/91043

They have merged a broader change now, eliminating all of distutils.cfg (https://github.com/Homebrew/homebrew-core/pull/91043#issuecomment-1032079055), at least for their Python 3.10 package. That's a good improvement, but it changes nothing for the present ticket.

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

Milestone: sage-9.6sage-9.7

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

Summary: build/pkgs/mpfr/spkg-configure.m4: Check pkg-config firstbuild/pkgs/{gmp,mpfr}/spkg-configure.m4: Check pkg-config first

comment:41 Changed 7 months ago by Dima Pasechnik

so this needs changes to gmp's and mpfr's spkg-configure.m4 ? Anything else?

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

sage.env.cython_aliases also needs to be extended

comment:43 Changed 6 months ago by Dima Pasechnik

should we remove all the crud needed for gmp and mpfr without .pc files available?

I suppose almost every platform we support nowadays will have them installed.

comment:44 in reply to:  43 Changed 6 months ago by Matthias Köppe

Replying to dimpase:

I suppose almost every platform we support nowadays will have them installed.

That would be worth checking.

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

Milestone: sage-9.7sage-9.8
Note: See TracTickets for help on using tickets.