Add macOS .dmg installer - #1250
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds macOS support for ArduPilot Methodic Configurator by implementing a .dmg installer build pipeline, complementing the existing Windows installer infrastructure. The changes enable dual-platform automated builds through GitHub Actions.
Changes:
- Added macOS-specific build dependencies and PyInstaller spec file for creating .app bundles
- Extended CI workflow to build both Windows .exe and macOS .dmg installers in parallel
- Updated gitignore to exclude macOS build artifacts
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pyproject.toml | Adds mac_dist optional dependency group with pip, pyinstaller, and packaging for macOS builds |
| macos/ardupilot_methodic_configurator.spec | New PyInstaller spec file with macOS-specific configuration including path resolution, version reading, and app bundle creation with icon and info.plist |
| .gitignore | Adds patterns to ignore macOS build artifacts (.icns, .iconset, Output/, dmg-staging/) |
| .github/workflows/build_windows_macos.yml | Extends workflow with build_macos job including icon generation from PNG, PyInstaller build, DMG creation with Applications symlink, and dual-platform artifact handling in release job |
Comments suppressed due to low confidence (4)
.github/workflows/build_windows_macos.yml:176
- The Windows build job uses 'uv' for dependency management (lines 39-47 in the Windows section), while the macOS build uses standard pip (lines 173-176). This inconsistency may lead to different dependency resolution behavior between platforms.
According to the coding guidelines (CodingGuidelineID: 1000000), "We use uv for dependency management, not pip directly." The macOS build should use uv for consistency with the project standards and the Windows build.
.github/workflows/build_windows_macos.yml:232
- The DMG creation step uses HFS+ filesystem (line 230), which is deprecated in favor of APFS on modern macOS systems. While HFS+ DMGs are still compatible, consider using APFS for better compatibility with macOS 10.13+ systems.
Additionally, the format UDZO creates a compressed read-only image. Ensure this is the intended format - UDBZ (bzip2 compression) might provide better compression for some use cases, though UDZO (zlib) is typically faster and more compatible.
.github/workflows/build_windows_macos.yml:283
- The PowerShell rename script filters files by extension using
-in '.exe', '.dmg'(line 283). However, the array literal syntax should use@('.exe', '.dmg')for better PowerShell compatibility, though the current syntax works in PowerShell Core 6+.
Consider using the more explicit array syntax for broader compatibility:
Where-Object { $_.Extension -in @('.exe', '.dmg') }.github/workflows/build_windows_macos.yml:207
- The icon preparation step uses
set -euo pipefailwhich will cause the workflow to fail if any icon generation command fails. However, the spec file handles missing icons gracefully (lines 96-97 in spec file set icon_path to None if not found).
Consider adding error handling or a fallback in the icon preparation step to ensure the build can continue even if icon generation fails. Alternatively, verify that the source PNG file has sufficient resolution (at least 1024x1024) to avoid sips errors when generating the largest icon size.
| debug=False, | ||
| strip=False, | ||
| upx=False, | ||
| console=False, |
There was a problem hiding this comment.
The macOS spec file sets console=False (line 82) while the Windows spec sets console=True (line 32 in windows spec). This means the macOS app will run without a console window (typical for macOS .app bundles), while Windows shows a console.
This is likely intentional for platform conventions, but should be verified that the application properly handles logging and error output on macOS when no console is available. The application should either log to files or use system logging APIs on macOS.
| def _find_project_root(base_dir: str) -> str: | ||
| candidates = [base_dir, os.path.abspath(os.path.join(base_dir, ".."))] | ||
| for candidate in candidates: | ||
| package_init = os.path.join(candidate, "ardupilot_methodic_configurator", "__init__.py") | ||
| if os.path.exists(package_init): | ||
| return candidate | ||
| return base_dir | ||
|
|
||
|
|
||
| PROJECT_ROOT = _find_project_root(spec_dir) | ||
| PACKAGE_ROOT = os.path.join(PROJECT_ROOT, "ardupilot_methodic_configurator") | ||
|
|
||
|
|
||
| def _read_version() -> str: | ||
| version_file = os.path.join(PACKAGE_ROOT, "__init__.py") | ||
| with open(version_file, encoding="utf-8") as fh: | ||
| for line in fh: | ||
| if line.startswith("__version__"): | ||
| return line.split("=", 1)[1].strip().strip("\"'") | ||
| return "0.0.0" | ||
|
|
||
|
|
||
| version = _read_version() |
There was a problem hiding this comment.
The macOS spec file includes custom path resolution logic and version reading that is not tested. According to the project's testing philosophy, these functions should have test coverage to ensure they work correctly across different execution contexts (running from project root vs. macos directory).
Consider adding tests to verify:
- The _find_project_root function correctly locates the project root from different starting directories
- The _read_version function correctly parses version from init.py
- The paths resolve correctly when the spec is run from different directories
This is especially important since the Windows spec doesn't have this complexity and they should behave consistently.
| if os.path.exists(git_hash_path): | ||
| datas.append((git_hash_path, "ardupilot_methodic_configurator")) | ||
|
|
||
| datas += collect_data_files("ardupilot_methodic_configurator") |
There was a problem hiding this comment.
The macOS spec file uses collect_data_files which is not imported, but the Windows spec doesn't use it at all. The import on line 8 includes collect_data_files, but looking at the Windows spec, it doesn't use collect_data_files.
However, line 51 uses collect_data_files("ardupilot_methodic_configurator") to collect package data files. This is important for including templates, translations, and other data files that the application needs. Without this, the macOS build may be missing critical data files.
The Windows spec should likely also use collect_data_files for consistency, or there may be a reason it's handled differently. This inconsistency should be verified to ensure both builds include all necessary data files.
| return candidate | ||
| return base_dir | ||
|
|
||
|
|
There was a problem hiding this comment.
The macOS spec file runs from the project root (line 64 uses PROJECT_ROOT, PACKAGE_ROOT paths), while the Windows workflow copies the spec file into the ardupilot_methodic_configurator directory before running pyinstaller (lines 77-80 in the Windows section). However, the macOS workflow runs pyinstaller from the project root with the spec file path "macos/ardupilot_methodic_configurator.spec" (line 211).
This difference in execution context is correctly handled by the macOS spec's path resolution logic (lines 19-29), but should be documented in a comment to explain why the two platforms use different approaches.
| # NOTE: | |
| # The macOS GitHub Actions workflow runs pyinstaller from the project root | |
| # using the spec file path "macos/ardupilot_methodic_configurator.spec". | |
| # In contrast, the Windows workflow copies its spec file into the | |
| # "ardupilot_methodic_configurator" package directory and then runs | |
| # pyinstaller from there. The logic below (PROJECT_ROOT/PACKAGE_ROOT | |
| # derived via _find_project_root) is intentionally designed to make the | |
| # macOS build robust to being executed from the project root, so we do | |
| # not need to copy the spec file on macOS. |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
|
@aero-oli can you tests that the .dmg file works correctly? It is available in the development release: |
Add macOS support and update CI workflows for dual-platform builds