sagelib setup.py: Fix dependencies on header files of packages gmp, ntl

Question

sagelib setup.py: Fix dependencies on header files of packages gmp, ntl

Closed this issue 5 years ago · 22 comments

These header file locations (for use in Cython dependencies only) are currently hardcoded as living in SAGE_INC, which is no longer true.

This is a step toward #29711, and also preparation for #29847 because it removes use of sage.env.SAGE_INC.

Depends on #29702

CC: @orlitzky @kliem @kiwifb @dimpase @jhpalmieri

Component: build

Author: Matthias Koeppe

Branch/Commit: 18aae7f

Reviewer: François Bissey, Jonathan Kliem

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

Answer 1 · 2020-06-14T05:32:05.000Z

Branch: u/mkoeppe/sagelib_setup_py__fix_dependencies_on_header_files_of_packages_gmp__ntl

Answer 2 · 2020-06-14T05:32:36.000Z

Commit: 041c9e8

Answer 3 · 2020-06-14T05:32:36.000Z

Last 10 new commits:

`5db5318`	`Trac #29345: remove "break" statements from AC_SEARCH_LIBS.`
`e810ad1`	`Trac #29345: don't use sage's config.status for the lrcalc build.`
`93c9921`	`Trac #29345: replace the function that populates the CVXOPT_* variables.`
`0e66a0a`	`Trac #29345: add Dima's SPKG patches for ksh compatibility.`
`df3f05e`	`build/make/Makefile.in [SCRIPT_PACKAGE_templ]: cd into the SPKG directory; adjust spkg-install scripts`
`5372065`	`Merge branch 't/29793/script_packages_should_cd_into_the_spkg_directory' into t/29411/make_sagelib_a_script_package`
`c166b97`	`Merge branch 't/29411/make_sagelib_a_script_package' into t/29702/move_all_code_from_src_setup_py__src_fpickle_setup_py_to_sage_setup`
`cc30471`	`build/bin/write-dockerfile.sh: Do not ADD removed file src/Makefile.in`
`8a41326`	`Merge branch 't/29411/make_sagelib_a_script_package' into t/29702/move_all_code_from_src_setup_py__src_fpickle_setup_py_to_sage_setup`
`041c9e8`	`sage_setup.command.sage_build_cython: Use SAGE_GMP_PREFIX, SAGE_NTL_PREFIX for header dependencies instead of SAGE_INC`

Answer 4 · 2020-06-14T05:32:56.000Z

comment:3

Branch is on top of #29702; there is only one new commit.

Answer 5 · 2020-06-21T01:15:40.000Z

comment:5

Needs review

Answer 6 · 2020-06-21T01:19:16.000Z

comment:6

Replying to @mkoeppe:

Needs review

This ticket is only the last commit? Or should I look at more commits?

Answer 7 · 2020-06-21T01:19:56.000Z

comment:7

That's right, only the last commit.

Answer 8 · 2020-06-21T01:24:01.000Z

Reviewer: François Bissey

Answer 9 · 2020-06-21T01:24:01.000Z

comment:8

Hum, you even said so in an earlier. Must be post lunch fatigue :)

LGTM this is much cleaner and much more appropriate.

I really have the feeling that you are breaking sage apart to rebuild it. And I am not complaining.

Answer 10 · 2020-06-21T01:25:15.000Z

comment:9

Thanks!

Answer 11 · 2020-06-24T01:27:04.000Z

comment:11

Discovered a stupid mistake in the last commit.

Answer 12 · 2020-06-24T01:29:11.000Z

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

`18aae7f`	`sage_setup.command.sage_build_cython: Fix up - add list brackets`

Answer 13 · 2020-06-24T01:29:11.000Z

Changed commit from 041c9e8 to 18aae7f

Answer 14 · 2020-06-24T01:41:28.000Z

comment:13

I hadn't spotted that! So, they need to be dictionaries?

Answer 15 · 2020-06-24T01:42:20.000Z

comment:14

the values of that dictionary must be lists.

Answer 16 · 2020-06-27T02:49:45.000Z

comment:16

Ready for another round of review

Answer 17 · 2020-06-27T03:02:11.000Z

comment:17

I feel especially stupid to have missed the syntax the first time so I think it should be fair for someone else to check.

Answer 18 · 2020-06-27T05:29:39.000Z

Changed reviewer from François Bissey to François Bissey, ...

Answer 19 · 2020-06-29T10:12:01.000Z

Changed reviewer from François Bissey, ... to François Bissey, Jonathan Kliem

Answer 20 · 2020-06-29T10:12:01.000Z

comment:19

LGTM.

Answer 21 · 2020-06-29T17:42:05.000Z

comment:20

Thank you!

Answer 22 · 2020-07-08T19:34:12.000Z

Changed branch from u/mkoeppe/sagelib_setup_py__fix_dependencies_on_header_files_of_packages_gmp__ntl to 18aae7f