Fix Linux build after python-appimage release disappeared #722
Conversation
|
Warning Rate limit exceeded@jonathanperret has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 2 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request introduces modifications to several GitHub Actions workflows. Key changes include renaming the output variable for Python version handling, updating Python versions across multiple jobs to more specific minor versions, and restructuring workflow steps for clarity. The updates ensure that the workflows correctly reflect the new naming conventions and utilize precise Python versions, especially addressing issues related to the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
.github/workflows/python_tests.yml (1)
13-13: Add a comment explaining the version discrepancy.To prevent future confusion about the different Python versions used across platforms (3.11.9 for Windows/macOS vs 3.11.10 for Linux), consider adding a comment.
+ # Using Python 3.11.10 specifically for Linux builds due to python-appimage availability python-version: [3.11.10].github/actions/init-environment/action.yml (1)
Line range hint
18-33: Consider adding AppImage availability check.To prevent future build failures, it would be helpful to verify the AppImage existence before proceeding with the build.
Add a new step after Python setup:
- name: Verify AppImage availability shell: bash run: | APPIMAGE_URL="https://github.com/niess/python-appimage/releases/download/python${{steps.py.outputs.python-version}}/${{steps.vars.outputs.python-appimage}}" if ! curl --output /dev/null --silent --head --fail "$APPIMAGE_URL"; then echo "::error::AppImage not found at $APPIMAGE_URL" exit 1 fi.github/workflows/build-multi-os.yml (1)
27-27: LGTM! Consider documenting version split rationale.The Python version split (3.11.9 for Windows/macOS, 3.11.10 for Linux) is a valid workaround for the python-appimage availability issue. However, this divergence might need attention in future updates.
Consider adding a comment in the workflow file explaining why different Python versions are used across platforms to help future maintainers understand this decision.
Also applies to: 176-176, 304-304, 427-427
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- .github/actions/init-environment/action.yml (2 hunks)
- .github/workflows/build-multi-os.yml (6 hunks)
- .github/workflows/python_tests.yml (1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/build-multi-os.yml
460-460: shellcheck reported issue in this script: SC2086:info:1:19: Double quote to prevent globbing and word splitting
(shellcheck)
460-460: shellcheck reported issue in this script: SC2086:info:2:56: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (6)
.github/workflows/python_tests.yml (1)
13-13: LGTM! Verify test consistency across platforms.The update to Python 3.11.10 aligns with the Linux build requirements. However, since Windows and macOS builds use 3.11.9, we should ensure test results remain consistent across platforms.
Let's verify the Python package dependencies:
✅ Verification successful
Version difference is intentional and safe
The Python version difference between platforms (3.11.10 for Linux, 3.11.9 for Windows/macOS) is intentionally coordinated across the build workflows. The requirements files show no Python version-specific dependencies or constraints that would be affected by this minor version difference.
Key findings:
- Linux build specifically needs 3.11.10 for AppImage compatibility
- All dependencies use compatible version ranges
- No Python version constraints found in requirements files
- Core dependencies (PySide6, PyInstaller, etc.) are pinned to specific versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any version-specific dependencies that might be affected # by the Python version difference between platforms. # Test: Look for Python version constraints in requirements rg -A 5 "python_version|Python>=|Python==|Python>|Python<" requirements.txt # Test: Look for setup.py version constraints rg -A 5 "python_version|Python>=|Python==|Python>|Python<" setup.pyLength of output: 196
Script:
#!/bin/bash # Let's try different approaches to find Python dependency files and version constraints # Find all potential Python dependency files fd -e txt -e toml -e cfg -e ini -e yaml -e yml . | grep -iE 'require|depend|python|setup|pip' # Look for Python version constraints in any file rg -g '!.git' -l "python_version|Python>=|Python==|Python>|Python<" # Check if pyproject.toml exists and examine its contents if [ -f "pyproject.toml" ]; then echo "=== pyproject.toml contents ===" cat pyproject.toml fi # Look for any Python version references in workflow files rg "python-version" .github/workflows/Length of output: 849
Script:
#!/bin/bash # Let's examine the contents of requirements files to check for version constraints echo "=== requirements.txt ===" cat requirements.txt echo -e "\n=== requirements.build.txt ===" cat requirements.build.txt echo -e "\n=== windows_build_requirements.txt ===" cat windows-build/windows_build_requirements.txt # Also check the workflow files for any Python-dependent steps echo -e "\n=== build-multi-os.yml ===" cat .github/workflows/build-multi-os.ymlLength of output: 24645
.github/actions/init-environment/action.yml (3)
18-23: LGTM! Good improvements to the Python setup.The changes include:
- Using the latest version (v5) of setup-python action
- Enabling pip caching for better performance
- Proper step ordering to ensure outputs are available
32-32: LGTM! Clean version string manipulation.The sed command correctly extracts the minor version (e.g., "3.11" from "3.11.10").
11-12: Verify the impact of renaming the output variable.The output variable has been renamed from
pythontopython-minor, which better reflects its content. However, this is a breaking change that could affect other workflows.Let's check for any references to the old output name:
✅ Verification successful
Renaming output from
pythontopython-minoris safeBased on the evidence from the git history and workflow usage:
- The action was newly introduced in commit 205d651 and only modified once in e8f64f1
- The only usage of this output is in the Linux build workflow where it's used as
steps.vars.outputs.python-minorto download the Python AppImage- The rename from
pythontopython-minorwas part of the same commit that modified the AppImage download logic🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for references to the old output name in workflow files # Test: Look for references to python output. Expect: No references to the old name. rg -g '*.yml' -g '*.yaml' "steps\.vars\.outputs\.python[^-]"Length of output: 62
Script:
#!/bin/bash # Let's try a broader search pattern to catch any references to the output # This will help us find both old and new usage patterns rg -g '*.yml' -g '*.yaml' "outputs\.python" -A 2 -B 2Length of output: 55
Script:
#!/bin/bash # Let's also check for any direct references to the action itself # to understand its usage context rg -g '*.yml' -g '*.yaml' "uses.*init-environment" -A 5 -B 2Length of output: 62
Script:
#!/bin/bash # Let's examine the complete init-environment action file # to understand the full context of changes cat .github/actions/init-environment/action.ymlLength of output: 1741
Script:
#!/bin/bash # Let's search for any workflow files that use this action # and check their actual usage of outputs fd -e yml -e yaml . .github/workflows/ -x cat {} \; | grep -A 10 -B 2 "init-environment"Length of output: 2215
Script:
#!/bin/bash # Let's also check git history to see if this is a new action # or if there are previous versions we should compare git log --patch -- .github/actions/init-environment/action.ymlLength of output: 5443
.github/workflows/build-multi-os.yml (2)
443-443: LGTM! Improved URL construction for python-appimage.The dynamic URL construction using
python-minorversion makes the workflow more maintainable and resilient to version changes.
457-457: LGTM! Consistent version variable usage.The PATH configuration correctly uses the same
python-minorversion variable, maintaining consistency with the AppImage changes.Also applies to: 462-462
jonathanperret
force-pushed
the
fix-appimage-build
branch
from
e8f64f1 to
26b3151
Compare
This should give us more control over which exact Python version we're using for the build (i.e. 3.11.9 vs. 3.11.10). For the Linux build, this also lets us select a version which has a `python-appimage` build available.
jonathanperret
force-pushed
the
fix-appimage-build
branch
from
26b3151 to
491d9a5
Compare
|
|
Thanks for taking care of this...supporting multiple OS is hard :( |
Problem
Fixes #721 — at least until
python-appimagebreaks us again.Proposed solution
This PR slightly changes the logic used to download the
python-appimagebuild: instead of a hardcoded.9added to the specified Python version (3.11), the build now uses a full Python version (e.g.3.11.10) and tries to download a matchingpython-appimagebuild.Note that the mismatch between the Python versions used for Windows and macOS (
3.11.9) on one side, and Linux (3.11.10) on the other, is caused by the fact that:python-appimagebuild is now only available for Python3.11.10;actions/setup-pythoncannot install Python3.11.10on Windows or macOS (apparently because Python 3.11 is EOL, there are no longer official binaries distributions for the security updates).Note that because we previously were not specifying the full Python version, these specific versions were already being used by the build.
I'd like to investigate the https://github.com/niess/python-appimage release process a bit more to understand why they remove previously published assets, but for the immediate term this fix should let us continue building AYAB releases.
Summary by CodeRabbit
New Features
Bug Fixes
Chores