Respect PURE_PYTHON environment variable set to 0

[perrinjerome]

Setting to 0 should be equivalent to not setting the variable.

Refs zopefoundation/meta#283

[perrinjerome]

oh tests are failing, I set to draft

[d-maurer]

The checker module contains a docstring specifying that the value of the PURE_PYTHON envvar is not relevant. It should get changed to be in line with this modification.

[perrinjerome]

The checker module contains a docstring specifying that the value of the PURE_PYTHON envvar is not relevant. It should get changed to be in line with this modification.

Thank you, I have changed this.

oh tests are failing, I set to draft

actually it was OK, I opened the same pull request for another package, but tests here seems to be OK

[perrinjerome]

actually it was OK, I opened the same pull request for another package, but tests here seems to be OK

... sorry . No, the tests also fail here. It seems there is a circular dependency between zopefoundation/zope.proxy#71 and this one, or something I don't understand yet.

[d-maurer]

Jérome Perrin wrote at 2024-9-30 00:04 -0700:

... sorry . No, the tests also fail here. It seems there is a circular dependency between zopefoundation/zope.proxy#71 and this one, or something I don't understand yet.

I think the test is wrong: it import `_Proxy` (which is the C implementation) but apparently, it is not available. Instead, the test should import "Proxy", which is the C implementation if available otherwise the Python implementation.

[d-maurer]

Instead, the test should import "Proxy", which is the C implementation if available otherwise the Python implementation.

If we do this, we may want to add an additional test: if not PURE_PYTHON, then we should test that the C implementation is available. Currently, zope.security automatically switches to the Python implementation when the C implementation is unavailable. The test should ensure that this is only the case when we expect it to be so.

[perrinjerome]

I understood the problem with the test, it's because the changes from this pull request depend on zopefoundation/zope.proxy#71 - there's a circular dependency between these two pull requests. The solution seems to be to merge both and release both packages.

Also, the test importing from zope.security.proxy import _Proxy seems correct, this is the test for the C version so it uses the full name as the target class, not the alias, to be sure to test the C class. The equivalent python test also uses the full name of the equivalent python class (from zope.security.proxy import ProxyPy) and the alias is not used here. This seems correct after all.

---

The long story about the analysis of the test errors ImportError: cannot import name '_Proxy' from 'zope.security.proxy' (/io/.tox/py310/lib/python3.10/site-packages/zope/security/proxy.py). This error happens here when running with PURE_PYTHON=0. In that test run, zope.security understands PURE_PYTHON=0 as "use C", but zope.proxy does not understands it and consider as "do not use C", because it is the current version from pypi without the changes from zopefoundation/zope.proxy#71

The test code is here:

zope.security/src/zope/security/tests/test_proxy.py

Lines 1284 to 1291 in e93ff5f

	@unittest.skipIf(PURE_PYTHON,
	"Needs C extension")
	class ProxyCTests(AbstractProxyTestBase,
	unittest.TestCase):
	def _getTargetClass(self): # pragma: no cover
	from zope.security.proxy import _Proxy
	return _Proxy

This test is not skipped because PURE_PYTHON is False.

from zope.security.proxy import _Proxy is re-exported here:

zope.security/src/zope/security/proxy.py

Lines 352 to 357 in 66d0c3d

	_c_available = not PURE_PYTHON
	if _c_available: # pragma: no cover
	try:
	from zope.security._proxy import _Proxy
	except (ImportError, AttributeError):
	_c_available = False

The module import happens in C here:

zope.security/src/zope/security/_proxy.c

Lines 860 to 861 in 66d0c3d

	if (Proxy_Import() < 0)
	return MOD_ERROR_VAL;

zope.security/include/zope.proxy/zope/proxy/proxy.h

Lines 27 to 43 in 869deee

	static int
	Proxy_Import(void)
	{
	if (_proxy_api == NULL) {
	PyObject *m = PyImport_ImportModule("zope.proxy");
	if (m != NULL) {
	PyObject *tmp = PyObject_GetAttrString(m, "_CAPI");
	if (tmp != NULL) {
	if (PyCapsule_CheckExact(tmp))
	_proxy_api = (ProxyInterface *)
	PyCapsule_GetPointer(tmp, NULL);
	Py_DECREF(tmp);
	}
	}
	}
	return (_proxy_api == NULL) ? -1 : 0;
	}

In plain python, this would be from zope.proxy import _CAPI.

And in zope.proxy, this is still checking if 'PURE_PYTHON' not in os.environ: here

https://github.com/zopefoundation/zope.proxy/blob/cbb23c424826d89538b90bb17dc35d48bacc356a/src/zope/proxy/__init__.py#L483

and once zope.proxy is updated to use if not int(os.environ.get('PURE_PYTHON', '0')): the test will pass again.

[perrinjerome]

In short, what we can do is merge #115 and zopefoundation/zope.proxy#71 although test fail for both pull requests, once boths are merged and released, the test should pass again.

[perrinjerome]

I pushed a tox.ini change to use zope.proxy from zopefoundation/zope.proxy#71 , this change is not planned to be merged, it's just to be sure that tests will be OK with this change.

[perrinjerome]

I cannot merge this one either because of the test status, but I believe it is in a state where it can be merged.
/cc @dataflake

[d-maurer]

Jérome Perrin wrote at 2024-10-2 05:44 -0700:

@perrinjerome commented on this pull request. > @@ -24,10 +24,10 @@ (:func:`getCheckerForInstancesOf`, :func:`selectChecker`). This module is accelerated with a C implementation on CPython by -default. If the environment variable ``PURE_PYTHON`` is set (to any -value) before this module is imported, the C extensions will be -bypassed and the reference Python implementations will be used. This -can be helpful for debugging and tracing. +default. If the environment variable ``PURE_PYTHON`` is set to ``1`` I don't like to leave it like that, but I could not find better words

...environment variable ``PURE_PYTHON`` (converted to ``int``) has a nonzero value ...

[perrinjerome]

...environment variable PURE_PYTHON (converted to int) has a nonzero value ..

Thank you, I did not notice your message before this was merged. I made a PR with your suggestion #116