Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds macOS support to the dlib Python wheels repository, which previously only supported Linux. The changes introduce a new automated installation script for macOS, update documentation to include macOS instructions, and add a GitHub Actions workflow to build wheels for both Intel and Apple Silicon Macs.
Changes:
- Added macOS-specific wheel installer script with architecture detection and automated download
- Updated documentation (README.md and docs/usage.md) to include macOS installation instructions, supported platforms, and troubleshooting
- Added GitHub Actions workflow for building and releasing macOS wheels (Python 3.10-3.13, both x86_64 and arm64)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| scripts/install_wheel_macos.sh | New installer script for macOS that detects architecture (Intel/Apple Silicon) and Python version, then downloads and installs the appropriate wheel |
| docs/usage.md | Updated installation guide with macOS sections, platform-specific instructions, and expanded troubleshooting for cross-platform support |
| README.md | Updated quick start guide and system requirements to include macOS platforms alongside Linux |
| .github/workflows/build-wheels-macos.yml | New CI/CD workflow to build, repair (bundle dependencies), and release dlib wheels for macOS platforms |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| elif command -v python &>/dev/null; then | ||
| python -c "import sys; print(f'{sys.version_info.major}.{sys.version_info.minor}')" | ||
| else | ||
| error "python 3 not found"; exit 1 |
There was a problem hiding this comment.
The error message "python 3 not found" is inconsistent with how Python is referenced elsewhere in the script (using "python" with lowercase). The message should read "Python 3 not found" for consistency and proper capitalization of the product name.
| error "python 3 not found"; exit 1 | |
| error "Python 3 not found"; exit 1 |
| if curl -s --head "$url" | head -n1 | grep -q "200 OK"; then | ||
| info "downloading wheel from releases" | ||
| curl -L -o "$wheel" "$url" | ||
| else | ||
| url="$REPO_URL/raw/main/$wheel" | ||
| if curl -s --head "$url" | head -n1 | grep -q "200 OK"; then |
There was a problem hiding this comment.
The URL check using curl -s --head and parsing with grep -q "200 OK" may be unreliable. GitHub redirects (302) are common for raw content, and HTTP/2 responses don't always include "OK" in the status line. A more reliable approach would be to check the exit code of curl or use curl --fail --head.
| if curl -s --head "$url" | head -n1 | grep -q "200 OK"; then | |
| info "downloading wheel from releases" | |
| curl -L -o "$wheel" "$url" | |
| else | |
| url="$REPO_URL/raw/main/$wheel" | |
| if curl -s --head "$url" | head -n1 | grep -q "200 OK"; then | |
| if curl --silent --head --fail --location "$url" >/dev/null 2>&1; then | |
| info "downloading wheel from releases" | |
| curl -L -o "$wheel" "$url" | |
| else | |
| url="$REPO_URL/raw/main/$wheel" | |
| if curl --silent --head --fail --location "$url" >/dev/null 2>&1; then |
| if curl -s --head "$url" | head -n1 | grep -q "200 OK"; then | ||
| info "downloading wheel from releases" | ||
| curl -L -o "$wheel" "$url" | ||
| else | ||
| url="$REPO_URL/raw/main/$wheel" | ||
| if curl -s --head "$url" | head -n1 | grep -q "200 OK"; then |
There was a problem hiding this comment.
The same URL check reliability issue exists here. The script may incorrectly determine that a wheel is not found when it actually exists, or attempt to download from an invalid URL.
| if curl -s --head "$url" | head -n1 | grep -q "200 OK"; then | |
| info "downloading wheel from releases" | |
| curl -L -o "$wheel" "$url" | |
| else | |
| url="$REPO_URL/raw/main/$wheel" | |
| if curl -s --head "$url" | head -n1 | grep -q "200 OK"; then | |
| if curl -fsS --head "$url" >/dev/null; then | |
| info "downloading wheel from releases" | |
| curl -L -o "$wheel" "$url" | |
| else | |
| url="$REPO_URL/raw/main/$wheel" | |
| if curl -fsS --head "$url" >/dev/null; then |
| curl -L -o "$wheel" "$url" | ||
| else | ||
| error "wheel not found for python $pyver on macOS $arch" | ||
| info "you can build from source with: pip install dlib" |
There was a problem hiding this comment.
The error message suggests building from source with "pip install dlib", but this may fail on macOS if the necessary build dependencies aren't installed. The message should clarify that build dependencies (cmake, boost) must be installed first, or reference the homebrew installation instructions.
| info "you can build from source with: pip install dlib" | |
| info "no compatible prebuilt wheel is available." | |
| info "to build dlib from source on macOS, you must first install build dependencies (cmake, boost, and a C++ compiler)." | |
| info "for example, with Homebrew: brew install cmake boost && pip install dlib" |
| automated installer (macOS): | ||
|
|
||
| ```bash | ||
| curl -sSL https://raw.githubusercontent.com/comethrusws/Dlib_linux_python_3.x/main/scripts/install_wheel_macos.sh | bash |
There was a problem hiding this comment.
The repository URL "comethrusws/Dlib_linux_python_3.x" is inconsistent with "EqualByte/Dlib_linux_python_3.x" used in the documentation. All repository references should use the same GitHub organization/user for the automated installer to work correctly.
| python --version | ||
| ``` | ||
|
|
||
| - **macOS platform error**: make sure you download the macOS wheel (macosx*\*), not linux wheel (linux*_ or manylinux\__) |
There was a problem hiding this comment.
The markdown formatting is incorrect. There are two asterisks at the end ("macosx**") which should be backticks or regular formatting. The intent appears to be "macosx_*" but the backslash escaping is incorrect for markdown.
| - **macOS platform error**: make sure you download the macOS wheel (macosx*\*), not linux wheel (linux*_ or manylinux\__) | |
| - **macOS platform error**: make sure you download the macOS wheel (`macosx_*`), not Linux wheel (`linux_*` or `manylinux_*`) |
|
|
||
| set -e | ||
|
|
||
| REPO_URL="https://github.com/comethrusws/Dlib_linux_python_3.x" |
There was a problem hiding this comment.
The REPO_URL points to "comethrusws/Dlib_linux_python_3.x" but the documentation in docs/usage.md and README.md uses "EqualByte/Dlib_linux_python_3.x". This inconsistency could cause the script to fail when trying to download wheels if these are different repositories. All references should point to the same repository.
| REPO_URL="https://github.com/comethrusws/Dlib_linux_python_3.x" | |
| REPO_URL="https://github.com/EqualByte/Dlib_linux_python_3.x" |
|
|
||
| ```bash | ||
| # automated installer (recommended) | ||
| curl -sSL https://raw.githubusercontent.com/EqualByte/Dlib_linux_python_3.x/main/scripts/install_wheel_macos.sh | bash |
There was a problem hiding this comment.
The repository URL references "EqualByte/Dlib_linux_python_3.x" here but the install script uses "comethrusws/Dlib_linux_python_3.x". These should be consistent across all files to avoid confusion and ensure the script works correctly.
| wget https://github.com/EqualByte/Dlib_linux_python_3.x/releases/download/v19.24/dlib-19.24-cp312-cp312-macosx_10_9_x86_64.whl | ||
| pip install dlib-19.24-cp312-cp312-macosx_10_9_x86_64.whl | ||
|
|
||
| # or for Apple Silicon (arm64) | ||
| wget https://github.com/EqualByte/Dlib_linux_python_3.x/releases/download/v19.24/dlib-19.24-cp312-cp312-macosx_11_0_arm64.whl |
There was a problem hiding this comment.
The repository URL is inconsistent with the one used in the install script (comethrusws) and should be standardized across all documentation.
No description provided.