Fix build for Python 3.12

[OCopping]

As of Python 3.12, distutils has been fully deprecated and removed from stlib. This PR aims to replace all imports of distutils with either a direct replacement in setuptools, or to use the exact copy of the function/object in the copy of distutils inside of setuptools. Fixes #24

[OCopping]

@mdavidsaver How do you feel about these changes?

[mdavidsaver]

@mdavidsaver How do you feel about these changes?

I would like for setuptools-dso to continue to support as wide a range of python versions as is practical. I do not think I could accept a change to only support the latest cpython series.

[OCopping]

@mdavidsaver How do you feel about these changes?

I would like for setuptools-dso to continue to support as wide a range of python versions as is practical. I do not think I could accept a change to only support the latest cpython series.

I did have another look at it. I reworked the import try/excepts that were there before and they seem to work (I have locally imported this into a local project and it appears to build and run with 3.12).

[OCopping]

@mdavidsaver I have tried to reproduced this Test 3.12/windows-latest build failure on a local Diamond Light Source server running Server 2022 Standard, however the build passes (using C++ Compiler 19.39.33523). More specifically I made a virtual enironment with Python 3.12 and ran

python -m setuptools.probe

And got the output:

<frozen runpy>:128: RuntimeWarning: 'setuptools_dso.probe' found in sys.modules after import of package 'setuptools_dso', but prior to execution of 'setuptools_dso.probe'; this may result in unpredictable behaviour
Patch _fix_compile_args() to avoid modification to compiler.include_dirs
_patch_fix_compile_args include_dirs=None
Microsoft (R) C/C++ Optimizing Compiler Version 19.39.33523 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

eval_macros_in.c
_patch_fix_compile_args include_dirs=None
try_compile.c
ToolchainInfo(address_width=64, compiler='msvc', compiler_type='msvc', compiler_version=(19, 39, 33523), endian='little', gnuish=False, target_arch='amd64', target_os='windows')

Do you have ideas why it could be faiiling on the Actions Runner?

EDIT: It seemed like this line was failing silently for some reason, as the next line it tried to open the "eval_macros_out.c" file it generates, but the build error states that the file doesn't exist:

https://github.com/OCopping/setuptools_dso/blob/bc5bd6604aac29b079e97fda7f21b051f2b7fc52/src/setuptools_dso/probe.py#L291

[mdavidsaver]

Do you have ideas why it could be faiiling on the Actions Runner?

Can you compare setuptools versions?

My first suspicion would be that my hacky logic to patch in preprocess() did not anticipate some change in setuptools.

https://github.com/mdavidsaver/setuptools_dso/blob/5274ba4bcdcb98a73a1a3f3f2c58f6344eecd0f7/src/setuptools_dso/compiler.py#L77-L86

[OCopping]

Can you compare setuptools versions?

So it seems like the Actions build is using setuptools 69.5.1 https://github.com/mdavidsaver/setuptools_dso/actions/runs/8739877637/job/23990331048?pr=31#step:6:72

This is the same as the version on my local Windows Server 2022 Standard environment

$ pip list
Package        Version
-------------- -------
pip            24.0
setuptools     69.5.1
setuptools_dso 2.10

What is even more weird is that from investigsting further, the windows-latest image the Actions runner is using is also Windows Server 2022... (From https://github.com/actions/runner-images):

Windows Server 2022 | windows-latest or windows-2022

[OCopping]

@mdavidsaver could it be worth re-running the two failing builds to see if they have the same outcome?

[OCopping]

I made a rookie mistake, I was working on the wrong branch on the local Windows server. I switched to this branch and now the build fails, so I am looking into it further.

[OCopping]

@mdavidsaver you were right, the issue is associated with patching preprocess. More specifically, it's that the if statement on line 93 doesn't get triggered. https://github.com/mdavidsaver/setuptools_dso/blob/caed4ec8ca5ef75bc576eff347f53a546dd06999/src/setuptools_dso/compiler.py#L93-L95

I can confirm that the preprocess function in MSVCCompiler is the same as in CCompiler (i.e. MSVCCompiler.preprocess == CCompiler.preprocess returns True), but when comparing to compiler.__class__.preprocess both return False. compiler.__class__ is <class 'distutils._msvccompiler.MSVCCompiler'> The output of the checks are as follows:

        a = compiler.__class__.preprocess
        b = CCompiler.preprocess
        print(a)
        print(b)
        print(a==b)

Output:

<function CCompiler.preprocess at 0x000001A0F7C4FD80>
<function CCompiler.preprocess at 0x000001A0F7CE9800>
False

The only thing I can think of is that the check sees they are at different memory addresses and determines they must be different. Unless it's down to how the distutils compilers are imported inside of setuptools in new_compiler: https://github.com/pypa/setuptools/blob/1ed759173983656734c3606e9c97a348895e5e0c/setuptools/_distutils/ccompiler.py#L1153-L1158

The only success I have had is to compare their qualified names, e.g.

if compiler.__class__.preprocess.__qualname__ == CCompiler.preprocess.__qualname__:

which does return True. If preprocess is patched with the partial function the code works flawlessly.

EDIT: Using __code__ also works, but still seems very hacky

if compiler.__class__.preprocess.__code__ == CCompiler.preprocess.__code__:

[mdavidsaver]

if compiler.class.preprocess is CCompiler.preprocess:

hmmm. I wonder what monkey patching is going on?

Presumably isinstance(compiler, CCompiler) will be False for you, while I assumed it would always be True.

The only thing I can think of is that the check sees they are at different memory addresses and determines they must be different.

inspect.getsourcefile(cls) may shed some light on what is happening. eg. inspect.getsourcefile(CCompiler) or inspect.getsourcefile(compiler.__class__). Or maybe inspect.getsourcefile(cc.preprocess) will show if the method is already being monkey patched.

[OCopping]

Presumably isinstance(compiler, CCompiler) will be False for you, while I assumed it would always be True.

Yeah this is the case, not sure why though as I thought it would always be True too...

inspect.getsourcefile(cls) may shed some light on what is happening. eg. inspect.getsourcefile(CCompiler) or inspect.getsourcefile(compiler.__class__). Or maybe inspect.getsourcefile(cc.preprocess) will show if the method is already being monkey patched.

So

        print(inspect.getsourcefile(CCompiler))
        print(inspect.getsourcefile(compiler.__class__))
        print(inspect.getsourcefile(compiler.preprocess))

returns

c:\Users\ollie\Documents\setuptools_dso\venv\Lib\site-packages\setuptools\_distutils\ccompiler.py
c:\Users\ollie\Documents\setuptools_dso\venv\Lib\site-packages\setuptools\_distutils\_msvccompiler.py
c:\Users\ollie\Documents\setuptools_dso\venv\Lib\site-packages\setuptools\_distutils\ccompiler.py

which shows it is using the default impl...

[AlexanderWells-diamond]

I've done some digging into this problem, and it all comes down to the fact that setuptools in 3.12 actually still publishes distutils - i.e. it is still valid to do from distutils.ccompiler import ...

The issue is that when we get a CCompiler from the _new_compiler function, it has a different namespace than the one we get by doing from setuptools._distutils.ccompiler import CCompiler due to manual calls to the import dunder method:

print(inspect.getmro(CCompiler))
print(inspect.getmro(compiler.__class__))

Gives:

So none of the inheritance/instance checks will ever work, as functionally these are two separate classes with different method chains.

The fix appears to be simple; as distutils is still published by setuptools nothing seems to stop us doing:

from distutils.ccompiler import (new_compiler as _new_compiler, gen_preprocess_options, CCompiler)

Unconditionally, outside of the current fix's try-except parsing. This seems to have some support from the setuptools maintainers, and so doesn't seem like the worst fix to do...

[OCopping]

've done some digging into this problem, and it all comes down to the fact that setuptools in 3.12 actually still publishes distutils - i.e. it is still valid to do from distutils.ccompiler import ...

It is worth noting this is only the case when SETUPTOOLS_USE_DISTUTILS=local, but this option will be removed in the future.

[OCopping]

Seems like the macos-latest builds are failing as the newest version (macOS 14.4.1) doesn't support Python<3.8 https://github.com/actions/setup-python/issues/856

[OCopping]

@mdavidsaver I ran the actions workflow on my forked repo and all the tests/builds passed. What do you think of the changes?

[OCopping]

@mdavidsaver have you had a chance to look at these changes since your last comment? I know of some people who are interested in Python 3.12 support. :slightly_smiling_face:

[mdavidsaver]

I'm sorry I have been short on time recently. Before merging I would like to get test results for building epicscorelibs, pvxslibs, and p4p with say py3.11 and py3.12 with this change. Mostly to see that nothing regresses with py3.11. I intend to get to this myself, although if one of you has time sooner that would be great.

[OCopping]

I would like to get test results for building epicscorelibs, pvxslibs, and p4p with say py3.11 and py3.12 with this change.

@mdavidsaver I decided to build these packages with Python 3.8 and 3.12 to see that they worked with this branch of setuptools_dso, and found that epicscorelibs and pvxslibs built fine both times. However, when it came to building p4p with the local copies of these two packages, the make nose tests failed with Python < 3.10. More specifically, this fails:

ERROR: testStoreBad (p4p.test.test_nt.TestEnum)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/scratch/controls/dev/p4p/python3.9/linux-x86_64/p4p/test/utils.py", line 107, in wrapper
    return meth(*args, **kws)
  File "/scratch/controls/dev/p4p/python3.9/linux-x86_64/p4p/test/test_nt.py", line 294, in testStoreBad
    self.assertWarns(UserWarning, fn)  # warns of empty choices
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.9/lib/python3.9/unittest/case.py", line 767, in assertWarns
    return context.handle('assertWarns', args, kwargs)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.9/lib/python3.9/unittest/case.py", line 200, in handle
    with self:
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.9/lib/python3.9/unittest/case.py", line 255, in __enter__
    for v in sys.modules.values():
RuntimeError: dictionary changed size during iteration

I found an issue on cython which, apart from this being a race condition, claimed (at least for Python 3.7) there was a memory allocation issue with accessing the sys.modules dict directly and not through a copy. (See https://github.com/python/cpython/issues/84507) Have you ever seen this before when testing against Python 3.7/8/9? As I said previously, building and running these tests in Python 3.10 and 3.12 does not have this issue.

[mdavidsaver]

Have you ever seen this before when testing against Python 3.7/8/9?

No I don't think so. Not with any of the Debian builds I have used. I had GHA re-run with master and don't see any test failures.

Which specific cpython and nose versions are involved?

(In case this is a .0 situation like https://github.com/mdavidsaver/p4p/issues/98#issuecomment-1421015618)

File "/scratch/controls/dev/p4p/python3.9/linux-x86_64/p4p/test/test_nt.py", line 294, in testStoreBad self.assertWarns(UserWarning, fn) # warns of empty choices

I don't understand how sys.modules could be changing here. Which I guess leaves GC?

It might be worth wrapping this line with gc.disable() and gc.enable().

Or to replace this line with the following to, maybe, see how sys.modules is changing.

            _before = list(sys.modules)
            try:
                self.assertWarns(UserWarning, fn)  # warns of empty choices
            except:
                self.assertListEqual(_before, list(sys.modules))
                raise

If neither turns up anything interesting, then I can only suggest testing with a different environment. (eg. docker/podman w/ docker.io/library/python:...)

[OCopping]

Which specific cpython and nose versions are involved?

Cython = 3.0.10 nose2 = 0.15.1

---

            _before = list(sys.modules)
            try:
                self.assertWarns(UserWarning, fn)  # warns of empty choices
            except:
                self.assertListEqual(_before, list(sys.modules))
                raise

Using this seems to suggest during the execution the dataclasses module is imported?

Traceback (most recent call last):
  File "/scratch/controls/dev/p4p/python3.9/linux-x86_64/p4p/test/utils.py", line 107, in wrapper
    return meth(*args, **kws)
  File "/scratch/controls/dev/p4p/python3.9/linux-x86_64/p4p/test/test_nt.py", line 298, in testStoreBad
    self.assertListEqual(_before, list(sys.modules))
AssertionError: Lists differ: ['sys[10778 chars]'p4p.test.asynciotest', 'p4p.test.test_asyncio'] != ['sys[10778 chars]'p4p.test.asynciotest', 'p4p.test.test_asyncio', 'dataclasses']

Second list contains 1 additional elements.
First extra element 543:
'dataclasses'

[OCopping]

After following the Python Build commands listed in https://github.com/mdavidsaver/p4p/blob/70b030de2f30cf690bb317429f59bd092e65420e/.github/workflows/build.yml Now the make nose tests for p4p all pass... I wonder why building with the local wheel makes a difference? :thinking:

[coretl]

@mdavidsaver can we merge this and make an alpha release please? Does it make sense to add me, @OCopping and @AlexanderWells-diamond to all the relevant modules so we can make alpha releases of the whole stack now?