Skip to content

Require Python >= 3.6.1 for PySlice_AdjustIndices - #25

Closed
jschueller wants to merge 1 commit into
conda-forge:masterfrom
jschueller:patch-1
Closed

jschueller wants to merge 1 commit into
conda-forge:masterfrom
jschueller:patch-1

Conversation

@jschueller

@jschueller jschueller commented Apr 25, 2017 •

Copy link
Copy Markdown
Contributor

with python 3.6.0 from miniconda-latest: PyQt5/QtCore.so: undefined symbol: PySlice_AdjustIndices, this is a symbol that was added in python 3.6.1, which is used for build

with python 3.6.0: PyQt5/QtCore.so: undefined symbol: PySlice_AdjustIndices
@conda-forge-linter

Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@astaric

astaric commented Apr 25, 2017

Copy link
Copy Markdown
Contributor

+1 for this, I have encountered this error as well

@astaric

astaric commented Apr 25, 2017

Copy link
Copy Markdown
Contributor

Related python ticket: https://bugs.python.org/issue29943

After reading the comments there, I am not sure whether 3.6.2 is going to be compatible with 3.6.0 or 3.6.1. Should python be pinned to the exact version (=3.6.1)?

@jakirkham

Copy link
Copy Markdown
Member

It looks like the author of that patch is reverting it. Maybe excluding 3.6.1 is the right solution.

@jschueller

jschueller commented Apr 25, 2017 •

Copy link
Copy Markdown
Contributor Author

@jakirkham as build dependency ? how ?

@jakirkham

Copy link
Copy Markdown
Member

Adding python !=3.6.1. Both build and run, right? Otherwise PySlice_GetIndicesEx will be missing.

@jakirkham

Copy link
Copy Markdown
Member

Does this sound right @njsmith? Also here is another case to add to your issue if needed.

@jschueller

Copy link
Copy Markdown
Contributor Author

what happens if we install py361 then pyqt with your fix ? I'd only exclude 361 from build.

@jakirkham

Copy link
Copy Markdown
Member

...PySlice_GetIndicesEx was converted into a macro that calls this new function [PySlice_AdjustIndices]. The patch was backported to both the 3.5 and 3.6 branches, was released in 3.6.1, and is currently slated to be released as part of 3.5.4.

Quoted from the OP in the bug report with clarification as to the new function.

@jakirkham

Copy link
Copy Markdown
Member

what happens if we install py361 then pyqt with your fix ? I'd only exclude 361 from build.

Then it will be missing the PySlice_GetIndicesEx symbol and fail to load. Also patches to Python 2.7 and 3.5 have already been merged it appears. Not seeing a patch for Python 3.6 ATM.

@jschueller

Copy link
Copy Markdown
Contributor Author

PySlice_GetIndicesEx already exists, its PySlice_AdjustIndices that was added

@jakirkham

Copy link
Copy Markdown
Member

No, PySlice_GetIndicesEx use to be a function. Now it is a macro. Please see commit ( python/cpython@b2a5be0 ) for details. This broke CPython's API as this function was suppose to be part of the CPython API including in Python 3.6. The addition of PySlice_AdjustIndices is also bad, but the reason it shows up at all is the PySlice_GetIndicesEx macro calls it.

@njsmith

njsmith commented Apr 25, 2017

Copy link
Copy Markdown

After reading the comments there, I am not sure whether 3.6.2 is going to be compatible with 3.6.0 or 3.6.1. Should python be pinned to the exact version (=3.6.1)?

The change in 3.6.1 only broke compatibility in one direction. Anything built against 3.6.0 to will definitely run on 3.6.1 and 3.6.2; the problem is that things built against 3.6.1 can't necessarily run on 3.6.0. I expect that things built against 3.6.1 will also run on 3.6.2 (ie even if they revert the patch they'll be careful to do it in a way that avoids adding new breakage). I don't know what will happen if you build on 3.6.2.

So yeah, the most sensible options I think are to either make sure to build against 3.6.0 (possibly as a general policy across conda-forge...), or else add a run requirement for >=3.6.1.

@jschueller

jschueller commented Apr 26, 2017 •

Copy link
Copy Markdown
Contributor Author

I chose the second option: add a run requirement for >=3.6.1, that's what I already did for openturns.

@jschueller

Copy link
Copy Markdown
Contributor Author

ping, this is good to go only a connection failure in 1/6 appveyor job

@mingwandroid

Copy link
Copy Markdown
Contributor

The third option is to patch Python.h until Python 3.8 so that extensions remain compatible across all of Python 3.7.x. This is what we did on defaults.

@mingwandroid

Copy link
Copy Markdown
Contributor

The third option: conda-forge/python-feedstock#145

@jschueller

Copy link
Copy Markdown
Contributor Author

@mingwandroid is that the upstream patch ?

@mingwandroid

Copy link
Copy Markdown
Contributor

We don't know what upstream will do yet but we did ask on the issue whether this approach was reasonable.

It disables the fix that the breakage was meant to address until Python 3.7 (when the patch would be removed).

Read the issue for details.

@jschueller jschueller closed this May 20, 2017
@jschueller
jschueller deleted the patch-1 branch May 20, 2017 12:51
@lanzagar lanzagar mentioned this pull request Jul 6, 2018
1 of 3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants