classification
Title: "xxlimited" extension declared incorrectly in setup.py
Type: Stage: resolved
Components: Build Versions: Python 3.3, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, loewis, ned.deily, python-dev, r.david.murray
Priority: low Keywords: patch

Created on 2013-07-21 08:02 by ned.deily, last changed 2013-08-02 07:05 by ned.deily. This issue is now closed.

Files
File name Uploaded Description Edit
issue_XXXXX_setup_xxlimited.patch ned.deily, 2013-07-21 08:04 review
Messages (5)
msg193428 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2013-07-21 08:02
In the top-level setup.py, the code to define the Extension instance for the "xxlimited" extension is currently incorrectly located near the end of detect_tkinter() rather than in its parent detect_modules(). This has the effect of skipping the build of "xxlimited" on OS X since detect_tkinter() shortcuts to detect_tkinter_darwin() there.  The attached patch corrects that.

However, it does raise the question of why "xxlimited" is built at all for normal installable builds, since "xxlimited" is a dummy template module.  The definition for the somewhat similar "xx" is commented out by default.  Perhaps "xxlimited" should be as well.  Or perhaps both should only be built for --with-pydebug configs.  Opinions?
msg193450 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-07-21 14:15
My expectation would be that we want the tests to be runnable (and be run, not skipped) with Python installed, and thus we build the test module.  (Whether or not our tests actually *can* be run with Python installed is an open question, though.)  And skipping some tests when installed is probably OK, too, so if I'm right about the rationale we could still change our minds.
msg194051 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2013-08-01 12:05
I would be fine with it being build only in a debug build. The rationale for the module is that it should test whether the header files actually compile under the limited API (or whether e.g. some functions are exposed that rely on unexposed structures). So the main test is really whether it compiles.
msg194157 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-08-02 06:54
New changeset e5607874e8ff by Ned Deily in branch '3.3':
Issue #18517: Move definition of "xxlimited" extension to detect_modules().
http://hg.python.org/cpython/rev/e5607874e8ff

New changeset 1d832bc857e2 by Ned Deily in branch 'default':
Issue #18517: merge from 3.3
http://hg.python.org/cpython/rev/1d832bc857e2
msg194159 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2013-08-02 07:05
On further reflection, only building on a debug build partially defeats part of the reason for building xxlimited; the current setup.py skips building in the debug case.  Perhaps another approach would be to comment out the build in setup.py and instead add a new test case that builds xxlimited, similar to how test_distutils builds xxmodule. In any case, I've applied the original patch for 3.3.3 and 3.4.0 and closing this issue.  Feel free to reopen if anyone wants to pursue this further.
History
Date User Action Args
2013-08-02 07:05:03ned.deilysetstatus: open -> closed
versions: + Python 3.3
messages: + msg194159

resolution: fixed
stage: patch review -> resolved
2013-08-02 06:54:03python-devsetnosy: + python-dev
messages: + msg194157
2013-08-01 17:41:18Arfreversetnosy: + Arfrever
2013-08-01 12:05:27loewissetmessages: + msg194051
2013-07-21 14:15:21r.david.murraysetnosy: + r.david.murray
messages: + msg193450
2013-07-21 08:04:11ned.deilysetfiles: + issue_XXXXX_setup_xxlimited.patch
keywords: + patch
2013-07-21 08:02:40ned.deilycreate