Skip to content

Conversation

@cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Jul 14, 2025

User description

DO NOT MERGE - This PR depends on #16045 and #16046 landing first.

🔗 Related Issues

#15801

💥 What does this PR do?

Work is being done towards supporting ARM binaries for Selenium Manager (see: #15801, #16045, #16046). Once we begin publishing ARM64 binaries for Windows and Linux, the bindings will need to be updated to select the correct binary at runtime based on architecture.

This PR contains the updates to Python packaging and the Python bindings code to support this change.

💡 Additional Considerations

This PR assumes we are building/publishing new binaries named selenium-manager-arm64 (Linux) and selenium-manager-arm64.exe (Windows). It will need to be adapted to reflect whatever names are actually used when the work is done.

🔄 Types of changes

  • Build/Packaging
  • New feature

PR Type

Enhancement


Description

  • Add ARM64 binary support for Selenium Manager

  • Update binary selection logic for architecture detection

  • Modify build configuration for ARM64 binaries

  • Update packaging to include ARM64 executables


Changes diagram

flowchart LR
  A["Platform Detection"] --> B["Architecture Check"]
  B --> C["Binary Selection"]
  C --> D["ARM64 Binary"]
  C --> E["x86_64 Binary"]
  F["Build System"] --> G["Package ARM64 Binaries"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
selenium_manager.py
ARM64 binary selection logic implementation                           

py/selenium/webdriver/common/selenium_manager.py

  • Enhanced binary selection logic to support ARM64 architecture
  • Added platform-specific ARM64 binary paths for Windows and Linux
  • Updated architecture detection to use platform.machine().lower()
  • Extended allowed binary mappings for ARM64 support
  • +9/-3     
    Configuration changes
    BUILD.bazel
    Build configuration for ARM64 binaries                                     

    py/BUILD.bazel

  • Added copy rules for ARM64 Selenium Manager binaries
  • Created manager-linux-arm64 and manager-windows-arm64 targets
  • Updated build configuration to include ARM64 executables
  • +12/-0   
    pyproject.toml
    Package data configuration for ARM64                                         

    py/pyproject.toml

  • Added ARM64 binary files to package data
  • Included selenium-manager-arm64 and selenium-manager-arm64.exe
  • Updated packaging configuration for ARM64 support
  • +2/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.

  • PR Type

    Enhancement


    Description

    • Add ARM64 binary selection support for Selenium Manager

    • Refactor binary selection logic to detect architecture dynamically

    • Support ARM64 binaries on Windows and Linux platforms

    • Update build configuration and packaging for ARM64 executables


    Diagram Walkthrough

    flowchart LR
      A["Platform Detection<br/>sys.platform"] --> B["Architecture Detection<br/>platform.machine()"]
      B --> C["Binary Name Selection"]
      C --> D["x86_64: selenium-manager"]
      C --> E["ARM64: selenium-manager-arm64"]
      D --> F["Location Selection"]
      E --> F
      F --> G["windows/linux/macos"]
      G --> H["Final Binary Path"]
    
    Loading

    File Walkthrough

    Relevant files
    Enhancement
    selenium_manager.py
    Implement dynamic ARM64 binary selection logic                     

    py/selenium/webdriver/common/selenium_manager.py

    • Refactored binary selection logic to support ARM64 architecture
      detection
    • Replaced static platform/architecture mapping with dynamic detection
      using platform.machine()
    • Added support for aarch64 and arm64 architecture identifiers
    • Updated documentation to list all supported binary paths for each
      platform and architecture
    +27/-16 
    Tests
    selenium_manager_tests.py
    Add ARM64 architecture tests and update assertions             

    py/test/selenium/webdriver/common/selenium_manager_tests.py

    • Added new test test_uses_windows_arm() to verify ARM64 binary
      selection on Windows
    • Updated test_uses_linux_arm64() to expect successful ARM64 binary
      resolution instead of exception
    • Added platform.machine() mocking to test_uses_windows() for
      consistency
    • Updated error message assertion in test_errors_if_invalid_os() to
      match new error format
    +15/-4   
    Configuration changes
    BUILD.bazel
    Add Bazel build rules for ARM64 binaries                                 

    py/BUILD.bazel

    • Added manager-linux-arm64 copy rule to include ARM64 Linux binary in
      build
    • Added manager-windows-arm64 copy rule to include ARM64 Windows binary
      in build
    • Both rules reference corresponding targets from //common/manager build
      target
    +12/-0   
    pyproject.toml
    Include ARM64 binaries in Python package data                       

    py/pyproject.toml

    • Added selenium-manager-arm64 to package data for Linux ARM64 binary
    • Added selenium-manager-arm64.exe to package data for Windows ARM64
      binary
    • Ensures ARM64 executables are included in wheel distribution
    +2/-0     

    @cgoldberg cgoldberg marked this pull request as draft July 14, 2025 13:25
    @selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations labels Jul 14, 2025
    @qodo-code-review
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Architecture Mapping

    The architecture detection logic maps platform.machine() output to binary names, but ARM64 architectures can have different identifiers (aarch64, arm64, etc.). The current implementation only checks for 'arm64' which may not cover all ARM64 variants returned by platform.machine().

    arch = "any" if sys.platform == "darwin" else platform.machine().lower()
    if sys.platform in ["freebsd", "openbsd"]:
        logger.warning("Selenium Manager binary may not be compatible with %s; verify settings", sys.platform)
    
    location = allowed.get((sys.platform, arch))
    Missing Fallback

    The code removes the fallback mechanism for Windows platforms that previously used 'any' architecture. If ARM64 detection fails or returns an unexpected value, there's no fallback to the standard x86_64 binary, which could cause runtime failures.

        ("win32", "amd64"): "windows/selenium-manager.exe",
        ("win32", "arm64"): "windows/selenium-manager-arm64.exe",
        ("cygwin", "amd64"): "windows/selenium-manager.exe",
        ("cygwin", "arm64"): "windows/selenium-manager-arm64.exe",
        ("linux", "x86_64"): "linux/selenium-manager",
        ("linux", "arm64"): "linux/selenium-manager-arm64",
        ("freebsd", "x86_64"): "linux/selenium-manager",
        ("freebsd", "arm64"): "linux/selenium-manager-arm64",
        ("openbsd", "x86_64"): "linux/selenium-manager",
        ("openbsd", "arm64"): "linux/selenium-manager-arm64",
    }
    
    arch = "any" if sys.platform == "darwin" else platform.machine().lower()
    if sys.platform in ["freebsd", "openbsd"]:
        logger.warning("Selenium Manager binary may not be compatible with %s; verify settings", sys.platform)
    
    location = allowed.get((sys.platform, arch))

    @qodo-code-review
    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Normalize architecture detection values

    The architecture detection may fail on some systems where platform.machine()
    returns unexpected values. Add architecture normalization to handle common ARM64
    variants like 'aarch64' and 'arm64'.

    py/selenium/webdriver/common/selenium_manager.py [93]

     arch = "any" if sys.platform == "darwin" else platform.machine().lower()
    +# Normalize ARM64 architecture names
    +if arch in ("aarch64", "arm64"):
    +    arch = "arm64"
    +elif arch in ("x86_64", "amd64"):
    +    arch = "amd64" if sys.platform.startswith("win") else "x86_64"
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies that platform.machine() can return different values for the same architecture (e.g., aarch64 vs arm64), and the proposed normalization makes the implementation more robust.

    Medium
    • More

    @nvborisenko
    Copy link
    Member

    Something has been changed? Should I be aware?..

    @nvborisenko
    Copy link
    Member

    image

    @cgoldberg
    Copy link
    Member Author

    Something has been changed? Should I be aware?..

    No, I have a script that syncs my fork's branches with the main repo.. it triggers CI every time I update. I'll see if I can fix it so I don't do so many unnecessary merges in a branch with no changes.

    @titusfortner
    Copy link
    Member

    Is creating a huge hard coded list the best way to do this? (curious how painful the other bindings will be)

    @cgoldberg
    Copy link
    Member Author

    Is creating a huge hard coded list the best way to do this? (curious how painful the other bindings will be)

    I just refactored the way it chooses the binary name and location so it's more flexible and we don't have to hard code a big list.

    But it still has to go through some kind of nasty conditional logic to figure out the binary name:

    • selenium-manager
    • selenium-manager.exe
    • selenium-manager-arm64
    • selenium-manager-arm64.exe

    and the directory it's in:

    • linux/
    • macos/
    • windows/

    @cgoldberg cgoldberg changed the title [py] WIP - Support ARM binary selection for Selenium Manager [py] Support ARM binary selection for Selenium Manager Dec 30, 2025
    @cgoldberg cgoldberg marked this pull request as ready for review December 30, 2025 14:38
    @cgoldberg cgoldberg marked this pull request as draft December 30, 2025 14:38
    @qodo-code-review
    Copy link
    Contributor

    PR Compliance Guide 🔍

    Below is a summary of compliance checks for this PR:

    Security Compliance
    🟢
    No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
    Ticket Compliance
    🎫 No ticket provided
    • Create ticket/issue
    Codebase Duplication Compliance
    Codebase context is not defined

    Follow the guide to enable codebase context checks.

    Custom Compliance
    🟢
    Generic: Comprehensive Audit Trails

    Objective: To create a detailed and reliable record of critical system actions for security analysis
    and compliance.

    Status: Passed

    Learn more about managing compliance generic rules or creating your own custom rules

    Generic: Meaningful Naming and Self-Documenting Code

    Objective: Ensure all identifiers clearly express their purpose and intent, making code
    self-documenting

    Status: Passed

    Learn more about managing compliance generic rules or creating your own custom rules

    Generic: Secure Error Handling

    Objective: To prevent the leakage of sensitive system information through error messages while
    providing sufficient detail for internal debugging.

    Status: Passed

    Learn more about managing compliance generic rules or creating your own custom rules

    Generic: Secure Logging Practices

    Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
    information like PII, PHI, or cardholder data.

    Status: Passed

    Learn more about managing compliance generic rules or creating your own custom rules

    🔴
    Generic: Robust Error Handling and Edge Case Management

    Objective: Ensure comprehensive error handling that provides meaningful context and graceful
    degradation

    Status:
    Unknown arch fallback: For non-ARM architectures on Linux/BSD, the code silently falls back to the x86_64 binary
    path instead of explicitly rejecting/handling unknown architectures, which can lead to
    runtime failures on unsupported CPUs.

    Referred Code
    is_windows = any(("win" in sys.platform, "cygwin" in sys.platform))
    
    # choose the binary name for the architecture and platform/os
    bin_name = "selenium-manager"
    if platform.machine() in ("aarch64", "arm64"):
        bin_name = f"{bin_name}-arm64"
    if is_windows:
        bin_name = f"{bin_name}.exe"
    
    # choose the directory of the binary for the platform/os
    if is_windows:
        location = f"windows/{bin_name}"
    elif sys.platform == "darwin":
        location = f"macos/{bin_name}"
    elif sys.platform == "linux":
        location = f"linux/{bin_name}"
    elif "bsd" in sys.platform:
        location = f"linux/{bin_name}"
        logger.warning(f"Selenium Manager binary may not be compatible with {sys.platform}; verify settings")
    else:
        raise WebDriverException(f"Unsupported platform: {sys.platform}")

    Learn more about managing compliance generic rules or creating your own custom rules

    Generic: Security-First Input Validation and Data Handling

    Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
    vulnerabilities

    Status:
    Missing arch validation: The architecture value from platform.machine() is not validated/normalized beyond two ARM
    identifiers, allowing unexpected values to select an incorrect binary rather than failing
    closed.

    Referred Code
    bin_name = "selenium-manager"
    if platform.machine() in ("aarch64", "arm64"):
        bin_name = f"{bin_name}-arm64"
    if is_windows:
        bin_name = f"{bin_name}.exe"
    
    # choose the directory of the binary for the platform/os
    if is_windows:
        location = f"windows/{bin_name}"
    elif sys.platform == "darwin":
        location = f"macos/{bin_name}"
    elif sys.platform == "linux":
        location = f"linux/{bin_name}"
    elif "bsd" in sys.platform:
        location = f"linux/{bin_name}"
        logger.warning(f"Selenium Manager binary may not be compatible with {sys.platform}; verify settings")
    else:
        raise WebDriverException(f"Unsupported platform: {sys.platform}")

    Learn more about managing compliance generic rules or creating your own custom rules

    Compliance status legend 🟢 - Fully Compliant
    🟡 - Partial Compliant
    🔴 - Not Compliant
    ⚪ - Requires Further Human Verification
    🏷️ - Compliance label

    @qodo-code-review
    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    High-level
    Incomplete macOS ARM support breaks functionality

    The logic for selecting an ARM64 binary on macOS is flawed because no such
    binary is packaged, which will break functionality for Apple Silicon users. The
    code should be updated to either package a native macOS ARM binary or fall back
    to the x86_64 version.

    Examples:

    py/selenium/webdriver/common/selenium_manager.py [100-114]
                if platform.machine() in ("aarch64", "arm64"):
                    bin_name = f"{bin_name}-arm64"
                if is_windows:
                    bin_name = f"{bin_name}.exe"
    
                # choose the directory of the binary for the platform/os
                if is_windows:
                    location = f"windows/{bin_name}"
                elif sys.platform == "darwin":
                    location = f"macos/{bin_name}"
    
     ... (clipped 5 lines)
    py/BUILD.bazel [103-113]
    copy_file(
        name = "manager-linux-arm64",
        src = "//common/manager:selenium-manager-linux-arm64",
        out = "selenium/webdriver/common/linux/selenium-manager-arm64",
    )
    
    copy_file(
        name = "manager-macos",
        src = "//common/manager:selenium-manager-macos",
        out = "selenium/webdriver/common/macos/selenium-manager",
    
     ... (clipped 1 lines)

    Solution Walkthrough:

    Before:

    # py/selenium/webdriver/common/selenium_manager.py
    
    def _get_binary():
        # ...
        bin_name = "selenium-manager"
        if platform.machine() in ("aarch64", "arm64"):
            bin_name = f"{bin_name}-arm64"
        
        # ...
        if sys.platform == "darwin":
            # This will result in "macos/selenium-manager-arm64" on Apple Silicon,
            # which is not packaged.
            location = f"macos/{bin_name}"
        # ...
        path = Path(__file__).parent.joinpath(location)
        if not path.is_file():
            raise WebDriverException(...)

    After:

    # py/selenium/webdriver/common/selenium_manager.py
    
    def _get_binary():
        # ...
        bin_name = "selenium-manager"
        # Exclude macOS from the ARM binary check until it's packaged.
        if platform.machine() in ("aarch64", "arm64") and sys.platform != "darwin":
            bin_name = f"{bin_name}-arm64"
        
        # ...
        if sys.platform == "darwin":
            # This will correctly fall back to "macos/selenium-manager" on Apple Silicon.
            location = f"macos/{bin_name}"
        # ...
        path = Path(__file__).parent.joinpath(location)
        if not path.is_file():
            raise WebDriverException(...)
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly identifies a critical regression where the new logic breaks support for macOS ARM users by searching for a non-existent ARM binary instead of using the existing x86_64 version.

    High
    Possible issue
    Use case-insensitive architecture matching

    Convert the architecture string from platform.machine() to lowercase before
    comparison to ensure case-insensitive matching.

    py/selenium/webdriver/common/selenium_manager.py [100-101]

    -if platform.machine() in ("aarch64", "arm64"):
    +if platform.machine().lower() in ("aarch64", "arm64"):
         bin_name = f"{bin_name}-arm64"
    • Apply / Chat
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion correctly identifies a case-sensitivity bug in architecture detection that would cause failures on Windows ARM systems, and the proposed fix using .lower() robustly resolves it.

    Medium
    Restrict BSD fallback to x86_64

    Restrict the BSD fallback to use the Linux binary only for the x86_64
    architecture to avoid potential incompatibility on ARM-based BSD systems.

    py/selenium/webdriver/common/selenium_manager.py [112-116]

    -elif "bsd" in sys.platform:
    +elif "bsd" in sys.platform and platform.machine() == "x86_64":
         location = f"linux/{bin_name}"
         logger.warning(f"Selenium Manager binary may not be compatible with {sys.platform}; verify settings")
     else:
    -    raise WebDriverException(f"Unsupported platform: {sys.platform}")
    +    raise WebDriverException(f"Unsupported platform: {sys.platform}/{platform.machine()}")
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly points out that reusing the Linux ARM binary on BSD ARM systems is an unverified assumption and wisely suggests reverting to the previous, safer behavior of only supporting x86_64 for BSD.

    Medium
    Correct Windows platform detection

    Use sys.platform.startswith() instead of in for a more precise and robust
    detection of Windows platforms.

    py/selenium/webdriver/common/selenium_manager.py [96]

    -is_windows = any(("win" in sys.platform, "cygwin" in sys.platform))
    +is_windows = sys.platform.startswith("win") or sys.platform.startswith("cygwin")
    • Apply / Chat
    Suggestion importance[1-10]: 3

    __

    Why: The suggestion improves the robustness of the platform detection logic by using startswith instead of in, preventing potential false positives on platforms with 'win' in their names, like 'darwin'.

    Low
    Learned
    best practice
    Validate and normalize env path

    Strip and validate SE_MANAGER_PATH (e.g., reject blank strings and
    normalize/expand the path) before constructing a Path, to avoid surprising
    failures from whitespace or shell-expanded paths.

    py/selenium/webdriver/common/selenium_manager.py [87-92]

     if (env_path := os.getenv("SE_MANAGER_PATH")) is not None:
    +    env_path = env_path.strip()
    +    if not env_path:
    +        raise WebDriverException("SE_MANAGER_PATH is set but empty")
         logger.debug(f"Selenium Manager set by env SE_MANAGER_PATH to: {env_path}")
    -    path_candidate = Path(env_path)
    +    path_candidate = Path(env_path).expanduser()
         if not path_candidate.is_file():
             raise WebDriverException(f"SE_MANAGER_PATH does not point to a file: {env_path}")
         path = path_candidate

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 6

    __

    Why:
    Relevant best practice - Add explicit validation and trimming/guards at integration boundaries (environment variables) before using them as paths.

    Low
    • More

    @cgoldberg cgoldberg self-assigned this Dec 31, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    B-build Includes scripting, bazel and CI integrations C-py Python Bindings Review effort 2/5 Review effort 3/5

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    4 participants