Skip to content

Improve platform architecture detection for arm on Linux and Windows #290

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

Open
juliabeliaeva wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Improve platform architecture detection for arm on Linux and Windows #290

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f653395
Improve platform architecture detection for arm on Linux and Windows
juliabeliaeva Dec 28, 2025
b5a5c1a
Include detected platform into the error message
juliabeliaeva Dec 28, 2025
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
36 changes: 21 additions & 15 deletions src/choreographer/cli/_cli_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,29 @@
supported_platform_strings = ["linux64", "win32", "win64", "mac-x64", "mac-arm64"]


def get_google_supported_platform_string() -> str | None:
arch_size_detected = "64" if sys.maxsize > 2**32 else "32"
arch_detected = "arm" if platform.processor() == "arm" else "x"
def get_google_platform_string() -> str | None:

@camdecoster camdecoster May 28, 2026

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.

Suggested change
def get_google_platform_string() -> str | None:
# Returns the detected platform string, including ones not in
# supported_platform_strings (e.g. linux-arm64, win-arm64) so callers
# can surface them in error messages.
def get_google_platform_string() -> str | None:

arch_size_detected = "64" if sys.maxsize > 2 ** 32 else "32"
is_x86 = platform.processor() not in {"arm", "aarch64"}

@camdecoster camdecoster May 28, 2026

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.

Could you please refactor your PR to use platform.machine() instead? platform.processor() on my Linux machine just returns an empty string. In that case, is_x86 would be True which would lead to the incorrect download.

arch_detected = "x" if is_x86 else "arm"

chrome_platform_detected: str | None = None
if platform.system() == "Windows":
chrome_platform_detected = "win" + arch_size_detected
elif platform.system() == "Linux":
chrome_platform_detected = "linux" + arch_size_detected
elif platform.system() == "Darwin":
chrome_platform_detected = "mac-" + arch_detected + arch_size_detected
if is_x86:
return "win" + arch_size_detected
return "win-" + arch_detected + arch_size_detected
if platform.system() == "Linux":
if is_x86:
return "linux" + arch_size_detected
return "linux-" + arch_detected + arch_size_detected
if platform.system() == "Darwin":
return "mac-" + arch_detected + arch_size_detected
return None

platform_string = ""
if chrome_platform_detected in supported_platform_strings:
platform_string = chrome_platform_detected

return platform_string
def get_google_supported_platform_string() -> str | None:
chrome_platform_detected = get_google_platform_string()
if chrome_platform_detected in supported_platform_strings:
return chrome_platform_detected
return ""

@camdecoster camdecoster May 28, 2026

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.

Let's return None instead of an empty string to match the type annotation.

Suggested change
return ""
return None



def get_chrome_download_path(
Expand Down Expand Up @@ -106,8 +112,8 @@ def get_chrome_sync( # noqa: C901, PLR0912
if isinstance(path, str):
path = Path(path)

arch = arch or get_google_supported_platform_string()
if not arch:
arch = arch or get_google_platform_string()
if arch not in supported_platform_strings:
raise RuntimeError(
"You must specify an arch, one of: "
f"{', '.join(supported_platform_strings)}. "
Expand Down