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

fix ecm on macOS Xcode 12 #30600

Closed
dimpase opened this issue Sep 18, 2020 · 22 comments
Closed

fix ecm on macOS Xcode 12 #30600

dimpase opened this issue Sep 18, 2020 · 22 comments

Comments

@dimpase
Copy link
Member

dimpase commented Sep 18, 2020

as discussed on #30494
and fixed upstream on
https://gforge.inria.fr/tracker/index.php?func=detail&aid=21856&group_id=135&atid=623

Upstream: Fixed upstream, but not in a stable release.

CC: @mkoeppe @jhpalmieri @zimmermann6

Component: packages: standard

Author: Dima Pasechnik

Branch: 1991357

Reviewer: John Palmieri

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

@dimpase dimpase added this to the sage-9.2 milestone Sep 18, 2020
@dimpase
Copy link
Member Author

dimpase commented Sep 18, 2020

Branch: u/dimpase/packages/ecm/xcode12patch

@dimpase
Copy link
Member Author

dimpase commented Sep 18, 2020

Commit: 9a2408c

@dimpase
Copy link
Member Author

dimpase commented Sep 18, 2020

New commits:

9a2408cupstream patch to fix ecm configure on XCode 12 clang

@jhpalmieri
Copy link
Member

comment:2

I think that most of the changes to ./configure are unnecessary, and it would be cleaner to use just

diff -u a/configure b/configure
--- a/configure	2016-10-11 02:27:12.000000000 -0700
+++ b/configure	2020-09-18 09:03:06.000000000 -0700
@@ -13265,8 +13253,10 @@
   cat >conftes1.c <<EOF
 #ifdef __cplusplus
 extern "C" { void underscore_test(); }
+#else
+extern void underscore_test();
 #endif
-main () { underscore_test(); }
+int main () { underscore_test(); return 1;}
 EOF
 for tmp_underscore in "" "_"; do
   cat >conftes2.s <<EOF

(I assume that you got this by running autoreconf after making the changes to acinclude.m4. Maybe upstream's configure was already out of sync, which is where the other changes came from?)

@dimpase
Copy link
Member Author

dimpase commented Sep 18, 2020

comment:4

sure, it's just the result of an autoreconf run. I don't think we should be bothered by this, it's clearly a temporary patch, to go away with a new ecm release.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@vbraun
Copy link
Member

vbraun commented Sep 19, 2020

comment:6

Build depends on autotools:

Building ecm-7.0.4.p2
CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash /var/lib/buildbot/slave/sage_git/build/local/var/tmp/sage/build/ecm-7.0.4.p2/src/missing aclocal-1.16 -I m4
/var/lib/buildbot/slave/sage_git/build/local/var/tmp/sage/build/ecm-7.0.4.p2/src/missing: line 81: aclocal-1.16: command not found
WARNING: 'aclocal-1.16' is missing on your system.
         You should only need it if you modified 'acinclude.m4' or
         'configure.ac' or m4 files included by 'configure.ac'.
         The 'aclocal' program is part of the GNU Automake package:
         <http://www.gnu.org/software/automake>
         It also requires GNU Autoconf, GNU m4 and Perl in order to run:
         <http://www.gnu.org/software/autoconf>
         <http://www.gnu.org/software/m4/>
         <http://www.perl.org/>
Makefile:808: recipe for target 'aclocal.m4' failed
make[5]: *** [aclocal.m4] Error 127
make[5]: Failed to remake makefile 'Makefile'.
make[5]: Target 'all' not remade because of errors.

@zimmermann6
Copy link

comment:7

Build depends on autotools: [...]

[speaking as upstream developer] I believe this has to be fixed on the Sage side, since the vanilla 7.0.4 tarball should not depend on autotools.

@jhpalmieri
Copy link
Member

comment:8

I think we can just patch configure, not acinclude.m4, and that shouldn't trigger running aclocal. I see this problem intermittently (running make ecm-clean; make), but if I remove the part of the patch for acinclude.m4, I don't see it any more.

@jhpalmieri
Copy link
Member

@jhpalmieri
Copy link
Member

Changed commit from 9a2408c to 1991357

@jhpalmieri
Copy link
Member

New commits:

1991357trac 30600: don't patch acinclude.m4, to avoid running aclocal

@dimpase
Copy link
Member Author

dimpase commented Sep 19, 2020

comment:11

ok, good catch!

@zimmermann6
Copy link

comment:12

it would be cleaner to run autoreconf after having patched acinclude.m4, and use the updated configure file (which is auto-generated)

@jhpalmieri
Copy link
Member

comment:13

Replying to @zimmermann6:

it would be cleaner to run autoreconf after having patched acinclude.m4, and use the updated configure file (which is auto-generated)

I think that's how Dima produced the patch in the first place. I just removed the part of the patch affecting acinclude.m4.

@dimpase
Copy link
Member Author

dimpase commented Sep 19, 2020

comment:14

Replying to @zimmermann6:

it would be cleaner to run autoreconf after having patched acinclude.m4, and use the updated configure file (which is auto-generated)

this what I did - but I only took updates of acinclude.m4 and ./configure, not the complete stuff. My fault.

@vbraun
Copy link
Member

vbraun commented Sep 21, 2020

Changed branch from u/jhpalmieri/packages/ecm/xcode12patch to 1991357

@mkoeppe
Copy link
Member

mkoeppe commented Feb 1, 2021

Changed commit from 1991357 to none

@mkoeppe
Copy link
Member

mkoeppe commented Feb 1, 2021

comment:16

Paul, is there any chance that a new release will be made soon with these updates?

@zimmermann6
Copy link

comment:17

I've put a candidate tarball at https://members.loria.fr/PZimmermann/ecm-7.0.5-dev.tar.gz, please can you confirm it is ok? If so, I will make a new release.

@zimmermann6
Copy link

comment:18

Replying to @zimmermann6:

I've put a candidate tarball at https://members.loria.fr/PZimmermann/ecm-7.0.5-dev.tar.gz, please can you confirm it is ok? If so, I will make a new release.

still waiting for an answer...

@mkoeppe
Copy link
Member

mkoeppe commented Apr 25, 2022

comment:19

Sorry, let's take this to #31325

This was referenced Jan 1, 2023
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

5 participants