setuptools.msvc: set host_dir correctly on ARM64 hosts#4786
setuptools.msvc: set host_dir correctly on ARM64 hosts#4786
Conversation
5ef90ee to
cf436b4
Compare
jaraco
left a comment
There was a problem hiding this comment.
Since this PR isn't linked to any issue, can you update the PR description to elaborate on the issue? In particular, answer questions like: Why does it matter? How did you encounter it? Under what conditions is it relevant (e.g. does it affect all users of Setuptools on ARM, or some subset)?
I see lots of other places in this module where the assumption is that there is x86 and x64. I'm not sure it makes sense to add ARM support here, but not elsewhere. What's the value in fixing just this issue and not other parts in the module that deal with processor architecture?
I thought the description was pretty clear about what the problem was (it really is just about the This is the only problem that I'm actually aware of with setuptools.msvc on Windows ARM64. I just did another look through the file and while yes, there are a lot of cases where it assumes there is only x86 and x64, I didn't see any cases where it is actually doing the wrong thing on ARM64. The rest of the output of EnvironmentInfo.return_env() looks fine. |
cf436b4 to
b8f9800
Compare
b8f9800 to
e1d81b8
Compare
792e753 to
d69bf04
Compare
|
Thanks @jaraco for the fixups and adding a test. I wasn't very creative in thinking about how to test this without a Windows ARM64 host. |
Summary of changes
This corrects the setting of the host directory name on ARM64 hosts so that the
EnvironmentInfo.VCToolsproperty andEnvironmentInfo.return_env()['path']includes the correct path to CL.EXE. Currently, it returns the X64 directory on ARM64 hosts. See below; noteHostX64in first listed path.Pull Request Checklist
newsfragments/.(See documentation for details)