Opened 5 years ago

Closed 5 years ago

#22554 closed enhancement (fixed)

Write custom create_extension() for Cython

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-7.6
Component: build Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Dima Pasechnik
Report Upstream: Fixed upstream, but not in a stable release. Work issues:
Branch: 0c0c603 (Commits, GitHub, GitLab) Commit: 0c0c6034c220f2658cf8b6b1512e11720d956c1e
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

Implement a custom create_extension() function for Cython. This new function does some things which were done in setup.py in different places, but now have one dedicated place. It should also fix the problem where -std=c99 is used for a C++ extension, which is an error for Clang. Also duplicate flags (like -msse -mmse) are removed.

This requires an upstream patch https://github.com/cython/cython/pull/466

Change History (18)

comment:1 Changed 5 years ago by jdemeyer

  • Summary changed from Use create_extension() from Cython to Write custom create_extension() for Cython

comment:2 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/use_create_extension___from_cython

comment:3 Changed 5 years ago by git

  • Commit set to 4446cb42735d2b754a50b2fcb31d00044c91e458

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4446cb4Write custom create_extension() for Cython

comment:4 Changed 5 years ago by git

  • Commit changed from 4446cb42735d2b754a50b2fcb31d00044c91e458 to 47452a34f83b7752f4449f662533590e2f4b8fc7

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

47452a3Write custom create_extension() for Cython

comment:5 Changed 5 years ago by git

  • Commit changed from 47452a34f83b7752f4449f662533590e2f4b8fc7 to f59e4e14775c324f5f93f64635df38871fd3aac3

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f59e4e1Write custom create_extension() for Cython

comment:6 Changed 5 years ago by git

  • Commit changed from f59e4e14775c324f5f93f64635df38871fd3aac3 to 0c0c6034c220f2658cf8b6b1512e11720d956c1e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

0c0c603Write custom create_extension() for Cython

comment:7 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:9 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:10 Changed 5 years ago by jdemeyer

  • Status changed from new to needs_review

comment:11 follow-up: Changed 5 years ago by mkoeppe

I can't really comment on this patch, but I've successfully built sage with it on my Mac with homebrew (which provides gfortran, so the Sage gcc is not installed), using Apple's "gcc" (clang)

comment:12 in reply to: ↑ 11 Changed 5 years ago by jdemeyer

Replying to mkoeppe:

I can't really comment on this patch, but I've successfully built sage with it on my Mac with homebrew (which provides gfortran, so the Sage gcc is not installed), using Apple's "gcc" (clang)

Just to be clear: I assume you mean with SAGE_INSTALL_GCC=no, otherwise Sage will build and use GCC anyway.

comment:13 Changed 5 years ago by mkoeppe

Turns out I used a different "feature" of the build system. I had configured using ./configure CC=gcc-6 CXX=g++-6 (they are from homebrew) and then configure was happy; but then the build just used /usr/bin/gcc anyway. (This bug is #22646.)

comment:14 Changed 5 years ago by jdemeyer

  • Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, but not in a stable release.

comment:15 follow-up: Changed 5 years ago by dimpase

What does "This requires an upstream patch" mean? That it should be applied to the upstream tarball?

(in fact one does not seem to need it to be applied to Sage, so this probably needs to be clarified in the ticket description)

Last edited 5 years ago by dimpase (previous) (diff)

comment:16 in reply to: ↑ 15 Changed 5 years ago by jdemeyer

Replying to dimpase:

What does "This requires an upstream patch" mean?

That the patch from the linked github pull request should be applied to the Cython sources. In this case, the patch is added as build/pkgs/cython/patches/create_extension.patch and the Cython version number is bumped to force rebuilding Cython with that patch.

comment:17 Changed 5 years ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

comment:18 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/use_create_extension___from_cython to 0c0c6034c220f2658cf8b6b1512e11720d956c1e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.