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

Fix base getattro #11

Merged
merged 7 commits into from
Feb 2, 2017
Merged

Fix base getattro #11

merged 7 commits into from
Feb 2, 2017

Conversation

stephan-hof
Copy link
Member

Explanation why this pull request is needed can be found here: #7 (comment)
For further discussion lets use this pull request.

I split the change to several commits, so its more obvious what belongs to the python3 port or to our extension.

I also wrote a unittest which worked with the C-Extension of ExtenionClass 4.1.2
This test fails now with pypy. So it looks like the python only version suffers the same problem.

@stephan-hof
Copy link
Member Author

I added a commit which fixes the python version too.
The python version of Base_getattro simulates object.__getattribute, which makes it possible to inject our conditional call to of_get on an attribute coming from the instance dict.

@mauritsvanrees
Copy link
Member

I can confirm that this fixes failures in the Zope2 tests when using an ExtensionClass (and AccessControl) checkout.

@hannosch
Copy link
Contributor

hannosch commented Feb 2, 2017

I've looked at this and it looks good to me. The customized _PyObject_GenericGetAttrWithDict hasn't changed up until Python 3.6, so we should be good there. I've opened #13 for Python 3.7 compatibility in the future.

@hannosch hannosch merged commit fcb0dfb into master Feb 2, 2017
@stephan-hof stephan-hof deleted the fix-base-getattro branch February 3, 2017 08:09
@seanupton
Copy link

FYI related to this change: in upgrading an existing Plone add-on to work with Plone 5.1b4 (which incorporates ExtensionClass 4.3.0), I had to change wrapping level used for ComputedAttribute (via a decorator on a class method) from level=1 (in ExtentionClass <= 4.2.0) to level=0 (in >= 4.2.1).

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.

4 participants