Opened 16 months ago

Last modified 3 days ago

#31348 new enhancement

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

Reported by: mkoeppe Owned by:
Priority: critical Milestone: sage-9.7
Component: build: configure Keywords: mpfr
Cc: dimpase, jhpalmieri, gh-zlscherr, mjo 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 mkoeppe)

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 (42)

comment:1 Changed 16 months ago by mkoeppe

  • Description modified (diff)

comment:2 Changed 16 months ago by mkoeppe

  • Description modified (diff)

comment:3 Changed 16 months ago by mkoeppe

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

comment:4 follow-up: Changed 16 months ago by 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.

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 16 months ago by mkoeppe

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 16 months ago by mkoeppe

I've opened #31365 for NTL.

comment:7 Changed 14 months ago by gh-zlscherr

  • Branch set to u/gh-zlscherr/add_gmp_mpfr_cython

comment:8 Changed 14 months ago by git

  • Commit set to 07e00b608d79e90c8854c2b4c157a1d6456e9985

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

07e00b6Modify stdlibs/incs in misc/cython.py

comment:9 Changed 14 months ago by gh-zlscherr

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 14 months ago by mkoeppe

  • Description modified (diff)

comment:11 Changed 14 months ago by mkoeppe

  • Description modified (diff)

comment:12 Changed 13 months ago by mkoeppe

  • Description modified (diff)

comment:13 Changed 13 months ago by roed

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 13 months ago by mkoeppe

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

comment:15 Changed 13 months ago by roed

Great, thanks for the clarification!

comment:16 Changed 13 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Moving to 9.4, as 9.3 has been released.

comment:17 Changed 9 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:18 Changed 8 months ago by mkoeppe

  • Description modified (diff)

comment:19 Changed 8 months ago by dimpase

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

comment:20 Changed 8 months ago by mkoeppe

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

comment:21 follow-up: Changed 8 months ago by 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.

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 follow-up: Changed 8 months ago by 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.

comment:23 in reply to: ↑ 21 Changed 8 months ago by dimpase

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 8 months ago by dimpase

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

comment:25 Changed 8 months ago by mkoeppe

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 8 months ago by mjo

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 8 months ago by mkoeppe

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 follow-up: Changed 8 months ago by dimpase

is this reported to Homebrew as a bug?

comment:29 in reply to: ↑ 28 ; follow-up: Changed 8 months ago by 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...

comment:30 Changed 8 months ago by mkoeppe

  • Dependencies changed from #31344 to #31344, #32549

comment:31 in reply to: ↑ 29 ; follow-up: Changed 8 months ago by 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.

comment:32 follow-up: Changed 8 months ago by mkoeppe

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

comment:33 in reply to: ↑ 32 Changed 8 months ago by dimpase

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 ; follow-up: Changed 5 months ago by mkoeppe

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 5 months ago by slelievre

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

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

comment:36 Changed 4 months ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.6

comment:37 Changed 4 months ago by mkoeppe

  • Description modified (diff)

comment:38 in reply to: ↑ 34 Changed 3 months ago by mkoeppe

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 7 weeks ago by mkoeppe

  • Milestone changed from sage-9.6 to sage-9.7

comment:40 Changed 7 days ago by mkoeppe

  • Summary changed from build/pkgs/mpfr/spkg-configure.m4: Check pkg-config first to build/pkgs/{gmp,mpfr}/spkg-configure.m4: Check pkg-config first

comment:41 Changed 4 days ago by dimpase

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

comment:42 Changed 3 days ago by mkoeppe

sage.env.cython_aliases also needs to be extended

Note: See TracTickets for help on using tickets.