Fixes #112 install command doesn't use platform in nt_user scheme #113
Conversation
|
Still needs a specific test, and there are issues with the existing test that need fixing, so please do not merge until I've done those |
|
I think this is ready to commit. Bit weird that distutils.sysconfig and sysconfig disagree[d?] on the include directory, but it seems to be handled okay. |
distutils/tests/test_install.py
Outdated
| 'xx', | ||
| ) | ||
| else: | ||
| expect_headers = os.path.join(sysconfig.get_python_inc(0, ''), 'xx') |
There was a problem hiding this comment.
I'm struggling to understand the intention behind divergent tests here. It's annoying that the tests need to have such intimate knowledge of the implementation details in order to set expectations. Is there a better way that derives this expectation from the API?
Is it possible the expected value could be derived from some value in sysconfig (not distutils.sysconfig)? I'm trying to move more of the logic away from distutils.sysconfig as that module is deprecated, so this line moves in the wrong direction.
There was a problem hiding this comment.
It's because Windows doesn't include the runtime version in its headers subdirectory ({prefix}\Include\xx), while POSIX does ({prefix}\include\python3.10\xx).
But POSIX also seems to include the ABI flags in the name? Sometimes? I'm sure the logic is "fine", but I couldn't figure it out. Pretty sure it's also changed upstream with rebasing onto sysconfig, though not in any way that impacts default builds after 3.7.
If you weren't running 3.7 tests on this repo, I wouldn't even have noticed the change and would've stuck with my simpler test logic :)
Fixes #112