Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MPIR's configure fails with GCC 5.x #18247

Closed
nexttime mannequin opened this issue Apr 18, 2015 · 21 comments
Closed

MPIR's configure fails with GCC 5.x #18247

nexttime mannequin opened this issue Apr 18, 2015 · 21 comments

Comments

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 18, 2015

...
checking whether gcc-5.0 is gcc... yes
checking compiler gcc-5.0 -m64 ... no, long long reliability test 1
configure: error: could not find a working compiler, see config.log for details
Error configuring MPIR (with CFLAGS unset).

This test already failed with Clang (cf. #13948); now that GCC (5.x) defaults to STDC inlining (as opposed to GNUC inlining) semantics, we have to adapt the test (which is supposed to detect an unrelated bug in older GCC versions) again.

Upstream: Completely fixed; Fix reported upstream

CC: @wbhart @kiwifb

Component: packages: standard

Keywords: GNUC STDC inline

Author: Leif Leonhardy

Branch/Commit: 764ad4e

Reviewer: Jeroen Demeyer

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

@nexttime nexttime mannequin added this to the sage-6.7 milestone Apr 18, 2015
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 18, 2015

comment:1

The following patch to configure fixes the issue:

--- src/configure.orig	2014-09-14 21:59:13.000000000 +0200
+++ src/configure	2015-04-18 21:21:22.170454397 +0200
@@ -5663,6 +5663,9 @@
 
 #if defined(__GNUC__) && !defined(__clang__)
 typedef unsigned long long t1;typedef t1*t2;
+#if defined(__GNUC_STDC_INLINE__)  /* e.g. GCC 5.x default */
+extern
+#endif
 __inline__ t1 e(t2 rp,t2 up,int n,t1 v0)
 {t1 c,x,r;int i;if(v0){c=1;for(i=1;i<n;i++){x=up[i];r=x+1;rp[i]=r;}}return c;}
 f(){static const struct{t1 n;t1 src[9];t1 want[9];}d[]={{1,{0},{1}},};t1 got[9];int i;
@@ -6977,6 +6980,9 @@
 
 #if defined(__GNUC__) && !defined(__clang__)
 typedef unsigned long long t1;typedef t1*t2;
+#if defined(__GNUC_STDC_INLINE__)  /* e.g. GCC 5.x default */
+extern
+#endif
 __inline__ t1 e(t2 rp,t2 up,int n,t1 v0)
 {t1 c,x,r;int i;if(v0){c=1;for(i=1;i<n;i++){x=up[i];r=x+1;rp[i]=r;}}return c;}
 f(){static const struct{t1 n;t1 src[9];t1 want[9];}d[]={{1,{0},{1}},};t1 got[9];int i;

(Haven't messed with configure.ac, its source, yet.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 19, 2015

Changed upstream from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 19, 2015

comment:3

P.S.: __GNUC_STDC_INLINE__ is implied by -std=gnu11 (and -std=gnu99), which is GCC 5.x's default, while all older versions defaulted to -std=gnu89 (implying __GNUC_GNU_INLINE__).

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 26, 2015

Attachment: MPIR-2.7.0_fix_configure_with_GCC_5.x.upstream.patch.gz

Patch submitted upstream. (Only fixes acinclude.m4, not configure.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 26, 2015

comment:4

Just noticed there are bugs w.r.t. $SAGE_LOCAL/share/mp_config in both MPIR's spkg-install and $SAGE_ROOT/build/install, but that's for another ticket.

(E.g. if I install MPIR with sage -i ... before I've ever run make, MPIR is built without GMP compatibility, due to a syntax error caused by improper quoting. Also, MPIR should always get configured with --enable-gmpcompat if we're bootstrapping GCC. I'd also add an explicit message on whether GMP compatibility is enabled or not. Not sure whether we should also handle the whole stuff after a successful installation if GMP compatibility was enabled in MPIR's spkg-install, and also GMP's spkg-install, i.e., when the default MP library changed by manual installation of the packages.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 26, 2015

Changed upstream from Reported upstream. No feedback yet. to Completely fixed; Fix reported upstream

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 26, 2015

Branch: u/leif/MPIR_configure_GCC_5.x

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 26, 2015

Commit: 764ad4e

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 26, 2015

Author: Leif Leonhardy

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 26, 2015

New commits:

764ad4eAdd patch fixing MPIR (2.7.0.alpha12) 'configure' failure with GCC 5.x (#18247)

@nexttime nexttime mannequin added the s: needs review label Apr 26, 2015
@nexttime nexttime mannequin changed the title MPIR's configure fails with GCC 5 MPIR's configure fails with GCC 5.x Apr 27, 2015
@jdemeyer
Copy link

comment:8

Why not unconditionally use static __inline__? I think that should work on all compilers.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 28, 2015

comment:9

Replying to @jdemeyer:

Why not unconditionally use static __inline__? I think that should work on all compilers.

Thought of that, too (already when we were fixing the test for clang), but this way it's more explicit, and more importantly doesn't change the test at all for older compilers.

After all, upstream will decide...

@nexttime nexttime mannequin added s: needs review and removed s: needs info labels Apr 28, 2015
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 28, 2015

comment:10

P.S.: I didn't "bump" the patch level because MPIR 2.7.0-alpha12 didn't have one, although it was already patched before (meanwhile, in Sage 6.7.beta3, even twice)... ;-)

@jdemeyer
Copy link

comment:12

On the other hand, with static __inline__ I will be more at ease regarding other (non-GCC) compilers. Here, you are fixing the test just for GCC but what about other compilers with potentially the same problem?

@jdemeyer
Copy link

comment:13

Replying to @nexttime:

P.S.: I didn't "bump" the patch level because MPIR 2.7.0-alpha12 didn't have one, although it was already patched before (meanwhile, in Sage 6.7.beta3, even twice)... ;-)

Since this is purely about a package not compiling, there is not even a need to bump the patchlevel:

  • if a user already managed to compile mpir, there is no need for this fix.
  • if a user did not manage to compile mpir, the version number doesn't matter anyway.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 29, 2015

comment:14

Replying to @jdemeyer:

On the other hand, with static __inline__ I will be more at ease regarding other (non-GCC) compilers. Here, you are fixing the test just for GCC but what about other compilers with potentially the same problem?

Bike-shedding?

This test is solely for GCC. Other compilers may pretend they're GCC, but then they should also define the respective macros. (We explicitly exclude clang, we could in principle exclude ICC as well, but that's something upstream should eventually decide, and not at all an issue for Sage. I'm not aware of any other relevant compiler identifying as GCC.)

Feel free to submit some other patch upstream.

@nexttime nexttime mannequin added s: needs review and removed s: needs info labels Apr 29, 2015
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 29, 2015

comment:15

P.S.: Using static __inline__ would actually modify the test, so it wouldn't be guaranteed that it catches the situation it was written for (namely a bug in older GCC versions).

@jdemeyer
Copy link

comment:16

Replying to @nexttime:

This test is solely for GCC.

I missed this part. I still don't like your patch (in fact, I don't even like the test), but I can accept it.

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 29, 2015

comment:17

Replying to @jdemeyer:

I still don't like your patch (in fact, I don't even like the test), but I can accept it.

I don't like the test either; I'd simply require some minimal version of GCC.

But as I said, it's IMHO up to upstream to clean up / decide on the tests...

but I can accept it.

Thanks. We can discuss again when finally a new upstream release gets out... ;-)

I just don't want to have a Sage 6.7 that cannot even bootstrap Sage's GCC 4.9.2 with GCC 5.x for no real reason; distros with GCC 5.1 will presumably pop up soon.

@vbraun
Copy link
Member

vbraun commented May 1, 2015

Changed branch from u/leif/MPIR_configure_GCC_5.x to 764ad4e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants