-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add support for building for Windows ARM64 devices #3430
base: dev
Are you sure you want to change the base?
Changes from 1 commit
afaa6f3
bae81cd
85c8853
d76e076
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -393,7 +393,10 @@ def RunCMake(context, force, extraArgs = None): | |
|
||
# Note - don't want to add -A (architecture flag) if generator is, ie, Ninja | ||
if IsVisualStudio2019OrGreater() and "Visual Studio" in generator: | ||
generator = generator + " -A x64" | ||
if "ARM" in os.environ.get('PROCESSOR_IDENTIFIER'): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be good to follow the convention in apple_utils.py and extract this into GetHostArch https://github.com/PixarAnimationStudios/OpenUSD/blob/release/build_scripts/apple_utils.py#L66 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the suggestion here to write a whole new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I'd advocate for another file necessarily (though it is the route I would take myself) But just extracting the heuristics for detecting arm into a function somewhere in this file itself should be fine? I just imagine that in the future, we'll need to modify the build parameters for certain dependencies etc based on the current arch, and so it'd be nice to have it as a function from the get go. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have now moved it out to a seperate function - it should now reliably detect ARM or x64 Critique very much welcome - I've never been much of a python dev |
||
generator = generator + " -A arm64" | ||
anthony-linaro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
else: | ||
generator = generator + " -A x64" | ||
|
||
toolset = context.cmakeToolset | ||
if toolset is not None: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the
platform
module be used here?https://docs.python.org/3/library/platform.html#module-platform
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(repost from correct account)
It could be, but problems arise when running an emulated x64 version of python (which is easy to do by mistake, as that's what the website gives you by default).
This way checks the name of the processor directly, which is not affected by emulation.