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: |
Description (last modified by )
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
- ticket:31335#comment:47
- https://groups.google.com/g/sage-release/c/6WjKQt_e_B8/m/H2I4HZ-YCwAJ (https://groups.google.com/g/sage-release/c/6WjKQt_e_B8/m/Uut_ZRaZCwAJ)
- https://groups.google.com/g/sage-release/c/0q3kywk7RP8/m/uspq0xeuAgAJ
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
- Description modified (diff)
comment:2 Changed 16 months ago by
- Description modified (diff)
comment:3 Changed 16 months ago by
- 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: ↓ 5 Changed 16 months ago by
comment:5 in reply to: ↑ 4 Changed 16 months ago by
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
I've opened #31365 for NTL.
comment:7 Changed 14 months ago by
- Branch set to u/gh-zlscherr/add_gmp_mpfr_cython
comment:8 Changed 14 months ago by
- Commit set to 07e00b608d79e90c8854c2b4c157a1d6456e9985
Branch pushed to git repo; I updated commit sha1. New commits:
07e00b6 | Modify stdlibs/incs in misc/cython.py
|
comment:9 Changed 14 months ago by
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
- Description modified (diff)
comment:11 Changed 14 months ago by
- Description modified (diff)
comment:12 Changed 13 months ago by
- Description modified (diff)
comment:13 Changed 13 months ago by
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
Yes, this is part of what this ticket intends to fix.
comment:15 Changed 13 months ago by
Great, thanks for the clarification!
comment:16 Changed 13 months ago by
- 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
- Milestone changed from sage-9.4 to sage-9.5
comment:18 Changed 8 months ago by
- Description modified (diff)
comment:19 Changed 8 months ago by
some distros, like Fedora, have an unpleasant habit of not installing .pc files, while these are available.
comment:20 Changed 8 months ago by
Yes, the ticket is "Check pkg-config first", not "Check pkg-config only".
comment:21 follow-up: ↓ 23 Changed 8 months ago by
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: ↓ 26 Changed 8 months ago by
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
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
Besides, we're getting rid of mpir spkg all together.
comment:25 Changed 8 months ago by
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
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
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: ↓ 29 Changed 8 months ago by
is this reported to Homebrew as a bug?
comment:29 in reply to: ↑ 28 ; follow-up: ↓ 31 Changed 8 months ago by
comment:30 Changed 8 months ago by
- Dependencies changed from #31344 to #31344, #32549
comment:31 in reply to: ↑ 29 ; follow-up: ↓ 34 Changed 8 months ago by
comment:32 follow-up: ↓ 33 Changed 8 months ago by
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
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: ↓ 38 Changed 5 months ago by
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
- 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
comment:36 Changed 4 months ago by
- Milestone changed from sage-9.5 to sage-9.6
comment:37 Changed 4 months ago by
- Description modified (diff)
comment:38 in reply to: ↑ 34 Changed 3 months ago by
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
- Milestone changed from sage-9.6 to sage-9.7
comment:40 Changed 7 days ago by
- 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
so this needs changes to gmp's and mpfr's spkg-configure.m4
? Anything else?
comment:42 Changed 3 days ago by
sage.env.cython_aliases
also needs to be extended
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.