Skip to content

Windows Arm64 support for emsdk.py only #1186

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.

Sign up for GitHub

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

Merged
sbc100 merged 1 commit into from
Feb 15, 2023
Merged

Windows Arm64 support for emsdk.py only #1186

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 37 additions & 6 deletions emsdk.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,6 @@ def exit_with_error(msg):
ARCH = 'x86'
elif machine.startswith('aarch64') or machine.lower().startswith('arm64'):
ARCH = 'aarch64'
if WINDOWS:
errlog('No support for Windows on Arm, fallback to x64')
ARCH = 'x86_64'

@sbc100 sbc100 Feb 15, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On macOS it was useful for a while to run the x86 prebuilt binaries on arm.. does that approach also work on windows (i.e. is there binary emulation?) . If so, would this change cause a regression since today you can just install the x86 binaries and have a working installation?

@Blackhex Blackhex Feb 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, as far as there are no publicly available Arm64 binaries we should install the x64 ones running in emulation. But having this code here prevents form possiblility to build the Arm64 binaries from sources using emsdk.py. For that reason, I've move this check into the install_tool() method. The code is not nice but this should be only a temporary solution.

@sbc100 sbc100 Feb 15, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One option is that we drop the hacks and folks can use EMSDK_ARCH=x86_64 emsdk install latest if they want to force the install the x86 binaries? Or do you think we should continue to make it automatic?

@Blackhex Blackhex Feb 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the hack is better because it keeps the current behaviour. I can rember when I started with emsdk that it took me a while to figure out that the EMSDK_ARCH even exists. On the other, hand note that there is this "strange" behavior: If you have Arm64 Python installed on your system and you do emsdk.bat install latest and emsdk.bat activate latest, the x64 version of emsdk Python is installed and activated and since then, emsdk.bat install xxx installs or compiles x64 versions anyway (without the error message). You either need to force Arm64 with EMSDK_ARCH=aarch64, or use system Python with python emsdk.py install xxx. If your system Python is x64, you need to force EMSDK_ARCH=aarch64 in all cases.

@p-b-o p-b-o Feb 16, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the python based system detection is a bit boring, as the architecture of your python dictates which platform you have. As you mentioned, if you have an x64 python (emulated on arm64), it will say you're on an x64 cpu.

The other way to detect this under Windows is to use PROCESSOR_ARCHITECTURE env var. Same joke, from cmd.exe, it says you're on an arm64 system, and from powershell, that you're on an x64 system (because default powershell install is x64).

For this kind of case, IMHO, having an explicit option (available through help) would be the best solution. Or at least, to mention EMSDK_ARCH in help and official documentation.

elif machine.startswith('arm'):
ARCH = 'arm'
else:
Expand Down Expand Up @@ -256,7 +253,11 @@ def vswhere(version):
if not program_files:
program_files = os.environ['ProgramFiles']
vswhere_path = os.path.join(program_files, 'Microsoft Visual Studio', 'Installer', 'vswhere.exe')
output = json.loads(subprocess.check_output([vswhere_path, '-latest', '-version', '[%s.0,%s.0)' % (version, version + 1), '-requires', 'Microsoft.VisualStudio.Component.VC.Tools.x86.x64', '-property', 'installationPath', '-format', 'json']))
# Source: https://learn.microsoft.com/en-us/visualstudio/install/workload-component-id-vs-build-tools?view=vs-2022
tools_arch = 'ARM64' if ARCH == 'aarch64' else 'x86.x64'

@sbc100 sbc100 Feb 15, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is supposed to be uppercase why isn't the else case "X86.X64"?

@Blackhex Blackhex Feb 15, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was referring to Arm64 variant only. x86.x64 should be lower case. See https://learn.microsoft.com/en-us/visualstudio/install/workload-component-id-vs-build-tools?view=vs-2022 for the reference.

# The "-products *" allows detection of Build Tools, the "-prerelease" allows detection of Preview version
# of Visual Studio and Build Tools.
output = json.loads(subprocess.check_output([vswhere_path, '-latest', '-products', '*', '-prerelease', '-version', '[%s.0,%s.0)' % (version, version + 1), '-requires', 'Microsoft.VisualStudio.Component.VC.Tools.' + tools_arch, '-property', 'installationPath', '-format', 'json']))

@Blackhex Blackhex Feb 14, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -products * allows detection of Build Tools, the -prerelease allows detection of Preview version of Visual Studio and Build Tools.

@sbc100 sbc100 Feb 15, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps put this in a comment on the line above?

return str(output[0]['installationPath'])
except Exception:
return ''
Expand Down Expand Up @@ -1014,15 +1015,41 @@ def xcode_sdk_version():
return subprocess.checkplatform.mac_ver()[0].split('.')


def cmake_target_platform(tool):
# Source: https://cmake.org/cmake/help/latest/generator/Visual%20Studio%2017%202022.html#platform-selection
if hasattr(tool, 'arch'):
if tool.arch == 'aarch64':
return 'ARM64'
elif tool.arch == 'x86_64':
return 'x64'
elif tool.arch == 'x86':
return 'Win32'
if ARCH == 'aarch64':
return 'ARM64'
else:
return 'x64' if tool.bitness == 64 else 'Win32'


def cmake_host_platform():
Comment thread
sbc100 marked this conversation as resolved.
# Source: https://cmake.org/cmake/help/latest/generator/Visual%20Studio%2017%202022.html#toolset-selection
arch_to_cmake_host_platform = {
'aarch64': 'ARM64',
'arm': 'ARM',
'x86_64': 'x64',
'x86': 'x86'
}
return arch_to_cmake_host_platform[ARCH]


def get_generator_and_config_args(tool):
args = []
cmake_generator = CMAKE_GENERATOR
if 'Visual Studio 16' in CMAKE_GENERATOR or 'Visual Studio 17' in CMAKE_GENERATOR: # VS2019 or VS2022
# With Visual Studio 16 2019, CMake changed the way they specify target arch.
# Instead of appending it into the CMake generator line, it is specified
# with a -A arch parameter.
args += ['-A', 'x64' if tool.bitness == 64 else 'x86']
args += ['-Thost=x64']
args += ['-A', cmake_target_platform(tool)]
args += ['-Thost=' + cmake_host_platform()]
elif 'Visual Studio' in CMAKE_GENERATOR and tool.bitness == 64:
cmake_generator += ' Win64'
args += ['-Thost=x64']
Expand Down Expand Up @@ -1831,6 +1858,10 @@ def install_tool(self):
elif hasattr(self, 'git_branch'):
success = git_clone_checkout_and_pull(url, self.installation_path(), self.git_branch)
elif url.endswith(ARCHIVE_SUFFIXES):
global ARCH
if WINDOWS and ARCH == 'aarch64':
errlog('No support for Windows on Arm, fallback to x64')
ARCH = 'x86_64'
success = download_and_unzip(url, self.installation_path(), filename_prefix=getattr(self, 'zipfile_prefix', ''))
else:
assert False, 'unhandled url type: ' + url
Expand Down