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

Most extensions don't need to be listed in module_list #7987

Closed
robertwb opened this issue Jan 19, 2010 · 13 comments
Closed

Most extensions don't need to be listed in module_list #7987

robertwb opened this issue Jan 19, 2010 · 13 comments

Comments

@robertwb
Copy link
Contributor

Unless special libraries or C flags are needed, we can auto-generate almost this whole list, which simplifies the making of new .pyx files in the standard library.

I am sure this needs rebasing WRT any new modules that have been added.

See also #15410.

CC: @williamstein @mwhansen @jasongrout @nexttime

Component: build

Reviewer: Jeroen Demeyer

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

@robertwb
Copy link
Contributor Author

comment:1

Eventually, the library, include, and language information should be able to be pulled out of the files themselves by Cython...

@robertwb
Copy link
Contributor Author

Attachment: 7987-module_list-cleanup.patch.gz

@robertwb
Copy link
Contributor Author

comment:3

Eventually, this should be part of Cython. Also, clang, clib, etc. should be allowed in .pxd files and be transitive (for example, everything using Pynac or NTL would automatically be C++ and get the right library).

@robertwb

This comment has been minimized.

@robertwb
Copy link
Contributor Author

comment:5

I will probably implement something like this in Cython directly, though of course heavily inspired by what we want for Sage.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 3, 2010

comment:7

Not surprising (see ticket description), the patches need to be rebased.

The merged Cygwin patch involved many changes to module_list.py, too.

Btw, IMHO libcsage and libstdcxx should not be "linked" unconditionally (especially regardless of the module's language) to each and every module.
(I recently started sorting out which modules really directly use libcsage, and did add "stdcxx" to libraries only if language=="c++". Currently suspended work in progress...)

@nexttime nexttime mannequin added s: needs work and removed s: needs review labels Jun 3, 2010
@robertwb
Copy link
Contributor Author

robertwb commented Jun 3, 2010

comment:8

Replying to @nexttime:

Btw, IMHO libcsage and libstdcxx should not be "linked" unconditionally (especially regardless of the module's language) to each and every module.
(I recently started sorting out which modules really directly use libcsage, and did add "stdcxx" to libraries only if language=="c++". Currently suspended work in progress...)

For sure, but I figured it'd be better to refractor and clean things up in separate steps (in case one or the other has unintended consequences).

For the record, I plan to add this functionality to Cython soon (including transitivity of library dependance), so that may make this patch invalid. Sorting what modules actually need what will be very useful though.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 3, 2010

comment:9

Replying to @robertwb:

Replying to @nexttime:

Btw, IMHO libcsage and libstdcxx should not be "linked" unconditionally (especially regardless of the module's language) to each and every module.
(I recently started sorting out which modules really directly use libcsage, and did add "stdcxx" to libraries only if language=="c++". Currently suspended work in progress...)

For sure, but I figured it'd be better to refractor and clean things up in separate steps (in case one or the other has unintended consequences).

Yes. The unconditional inclusion is anyhow performed in setup.py.

For the record, I plan to add this functionality to Cython soon (including transitivity of library dependance), so that may make this patch invalid. Sorting what modules actually need what will be very useful though.

I just wanted to decrease the number of tickets needing review. ;-)

P.S.: s/stdcxx/stdc++/

@nexttime nexttime mannequin added s: needs info and removed s: needs work labels Mar 15, 2013
@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@jdemeyer

This comment has been minimized.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 13, 2015

Replying to @jdemeyer:

See also #15140.

Sure?

@nexttime nexttime mannequin modified the milestones: sage-6.4, sage-6.6 Apr 13, 2015
@nexttime nexttime mannequin added t: enhancement and removed t: bug labels Apr 13, 2015
@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link

comment:18

This is way too outdated to apply, it also incorrectly adds the libraries to .pyx files instead of .pxd files (see #18450) and several parts of this have already been done.

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

3 participants