Skip to content

[build] fix native arm64 builds #4168

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bjornalm
Copy link
Collaborator

What changes were proposed in this pull request?

Overview

This Pull Request introduces a series of targeted modifications across .gitignore, the root Makefile, Makefile.sdk, and desktop/core/generate_requirements.py. These changes are primarily driven by the need to enable a stable and reliable build process for Hue on ARM64 (Apple Silicon / M1/M2) Macs, while ensuring full backward compatibility and improved robustness for all other supported architectures (e.g., x86_64 Linux/macOS).

Problem Statement

Building Hue on ARM64 Macs exposed several critical issues:

Incorrect Python Environment Resolution: The make build system often failed to correctly identify and utilize the native ARM64 Python installed via pyenv. Instead, it would sometimes default to system-installed x86_64 Python frameworks or struggle with pyenv's shim paths, leading to:

Incorrect Python header paths (PYTHON_INCLUDE_DIR).
Errors like FileNotFoundError: .../bin/pip because pip was being sought in the wrong, often non-existent, location.
Malformed pip install Commands: Various Makefile targets (in both the root Makefile and Makefile.sdk) were attempting to execute pip install commands using a syntax that, in certain contexts (especially with the misidentified Python environments), resulted in the Python interpreter trying to "open file 'install'", leading to can't open file 'install': No such file or directory errors.

Incorrect Requirements File Selection: The desktop/core/generate_requirements.py script, responsible for dynamically selecting architecture-specific Python requirements, did not correctly map platform.machine()'s 'arm64' output (from M1/M2 Macs) to its expected 'aarch64' key, causing it to fall back to x86_64-specific dependency lists.

Module Not Found at Runtime (ModuleNotFoundError: No module named 'apps'): After the build, when attempting to run Hue, the Django application couldn't find its core modules because the project's root directory wasn't consistently added to Python's search path (PYTHONPATH).

Changes Introduced

This PR addresses the above problems with the following modifications:

Makefile

  1. pyenv PATH Fix: We adjusted the PATH within make to always prioritize pyenv's Python. This ensures the correct ARM-native interpreter is consistently used for all build steps.
  2. Robust pip install: We changed pip commands to use $(ENV_PYTHON) -m pip with aggressive reinstall flags. This fixed unreliable package installations and dependency caching across architectures.
  3. Explicit PYTHONPATH for collectstatic: We added a PYTHONPATH export specifically for collectstatic. This ensures Django can always find all its application modules during static file collection.
  4. Aggressive Virtual Environment Cleanup: We enhanced the clean target to aggressively remove the entire build/venvs directory. This guarantees a completely fresh virtual environment on every rebuild.

Makefile.sdk
The Makefile.sdk was updated to enhance dependency installation reliability and cleanup. Changes included using $(ENV_PYTHON) -m pip with aggressive reinstall flags (--no-cache-dir --upgrade --force-reinstall) for all package installations, and adding a cleanup step to remove generated version-specific requirements directories like desktop/core/3.8/ during make clean.

generate_requirements.py
The generate_requirements.py script was significantly updated to properly support ARM64 (M1/M2 Mac) builds. This involved accurately mapping the arm64 architecture for correct requirements file generation and precisely specifying polars-lts-cpu for ARM64 builds, while ensuring polars[calamine] remains for x86_64 builds.

How was this patch tested?

By executing build according new ARM-native Python 3.8 instructions in "Hue Setup for M1 Macs" docs

Please review Hue Contributing Guide before opening a pull request.

@bjornalm bjornalm force-pushed the fix-ARM64-native-build branch from 40a3191 to 499f26e Compare June 17, 2025 12:47
Copy link

github-actions bot commented Jun 17, 2025

⚠️ No test files modified. Please ensure that changes are properly tested. ⚠️

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Enables reliable ARM64 (Apple Silicon) builds by refining Makefile scripts and requirements generation.

  • Prioritizes pyenv shims and standardizes pip invocations (python -m pip) across Makefiles.
  • Improves virtual environment cleanup and ensures Django’s collectstatic sees all modules.
  • Updates generate_requirements.py to correctly handle ARM64 architecture and load the right Polars package.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
desktop/core/generate_requirements.py Map ‘arm64’ to ‘aarch64’ and relocate Polars pins into version-specific lists.
Makefile.sdk Switch to $(ENV_PYTHON) -m pip with flags and clean up versioned req dirs.
Makefile Prepend pyenv shims to PATH, switch pip installs, set PYTHONPATH for static, and clean venvs.

Comment on lines +208 to +209
if self.arch == 'arm64':
self.arch = 'aarch64'
Copy link
Preview

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

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

The code maps 'arm64' to 'aarch64' but platform_map only has an 'arm64' key. After setting self.arch = 'aarch64', lookups will fail. Either map 'aarch64' in platform_map or avoid changing the key.

Suggested change
if self.arch == 'arm64':
self.arch = 'aarch64'
# No need to map 'arm64' to 'aarch64' as both are handled in arch_requirements_map

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It will not work without this fix.

Makefile.sdk Outdated
@@ -174,6 +174,7 @@ clean:: ext-clean
@rm -Rf $(BUILD_DIR) dist
@find . -name \*.egg-info -prune -exec rm -Rf {} \;
@find . -name \*.py[co] -exec rm -f {} \;
@rm -rf $(APP_ROOT)/$(PYTHON_VER)
Copy link
Preview

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

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

Removing $(APP_ROOT)/$(PYTHON_VER) may not target the generated desktop/core/$(PYTHON_VER) directory. Consider using the correct path where versioned requirements are written.

Suggested change
@rm -rf $(APP_ROOT)/$(PYTHON_VER)
@rm -rf $(APP_ROOT)/desktop/core/$(PYTHON_VER)

Copilot uses AI. Check for mistakes.

Makefile Outdated
@@ -163,9 +169,9 @@ $(BLD_DIR_ENV)/bin/python:
@$(ENV_PYTHON) -m pip install virtualenv==$(VIRTUAL_ENV_VERSION) virtualenv-make-relocatable==$(VIRTUAL_ENV_RELOCATABLE_VERSION)
@$(eval RELOCATABLE := $(shell which virtualenv-make-relocatable))
@echo "REQUIREMENT_FILE is $(REQUIREMENT_FILE)"
@$(ENV_PIP) install -r $(REQUIREMENT_FILE)
@$(ENV_PYTHON) -m pip install -r $(REQUIREMENT_FILE)
Copy link
Preview

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

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

[nitpick] For consistency with Makefile.sdk, consider adding flags --no-cache-dir --upgrade --force-reinstall to pip installs here to ensure reproducible builds across architectures.

Suggested change
@$(ENV_PYTHON) -m pip install -r $(REQUIREMENT_FILE)
@$(ENV_PYTHON) -m pip install --no-cache-dir --upgrade --force-reinstall -r $(REQUIREMENT_FILE)

Copilot uses AI. Check for mistakes.

@bjornalm bjornalm requested a review from athithyaaselvam June 17, 2025 12:48
Copy link

UI Coverage Report

Lines Statements Branches Functions
Coverage: 32%
39.15% (30527/77959) 31.01% (14247/45936) 23.89% (2130/8915)

Copy link

github-actions bot commented Jun 17, 2025

Python Code Coverage

Python Coverage Report •
FileStmtsMissCoverMissing
desktop/core
   generate_requirements.py39879%211, 229–235
TOTAL541852707850% 

Pytest Report

Tests Skipped Failures Errors Time
1186 106 💤 0 ❌ 0 🔥 6m 5s ⏱️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant