ci: fix CodeQL build failures by using correct Python version - #724
Conversation
Summary of ChangesHello @tianjianjiang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves recurring CodeQL build failures that were caused by a Python version mismatch in the CI environment. The core change involves modifying the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
There was a problem hiding this comment.
Code Review
This pull request addresses a CodeQL build failure by making the Python interpreter used in the Source/Data/Makefile configurable. The change introduces a PYTHON variable, defaulting to python3, and replaces all hardcoded python3 calls with $(PYTHON). This is a clean and effective solution that correctly uses Makefile conventions to allow overriding the Python version from the environment, which will resolve the CI issue described. The changes are well-implemented and I have no further suggestions.
There was a problem hiding this comment.
Pull Request Overview
This PR makes the Python interpreter configurable in the build process by introducing a PYTHON Makefile variable and ensuring it's propagated through the GitHub Actions CI pipeline.
- Adds a
PYTHONvariable to the Makefile with a default value ofpython3, allowing it to be overridden - Updates all Python invocations in the Makefile to use
$(PYTHON)instead of hardcodedpython3 - Configures the CodeQL workflow to set up Python 3.12 explicitly and pass the Python path to xcodebuild
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Source/Data/Makefile | Introduces configurable PYTHON variable and replaces all hardcoded python3 commands with $(PYTHON) references |
| .github/workflows/codeql.yml | Adds Python 3.12 setup step and passes Python path as environment variable to xcodebuild |
The CodeQL workflow was failing for Swift and C++ analyses because: 1. The workflow sets up Python 3.12 via actions/setup-python 2. But xcodebuild doesn't inherit this Python in its environment 3. The Makefile uses hardcoded python3 which resolves to Xcode's bundled Python 3.9 4. This causes build failures when the Makefile is executed during the Data target build phase Changes: - Add Python 3.12 setup step to CodeQL workflow - Pass PYTHON environment variable to xcodebuild command - Update Makefile to use $(PYTHON) variable instead of hardcoded python3 - Allows environment to override Python executable while maintaining backward compatibility This ensures CodeQL builds use the correct Python version and fixes the cascading Swift/C++ build failures. Fixes: https://github.com/openvanilla/McBopomofo/actions/runs/18998321918 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
67131da to
c007d2b
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review Summary
This PR makes clean, focused changes to fix the CodeQL Python environment issue. The approach is solid:
Strengths:
- ✅ Consistently replaces hardcoded
python3calls with the$(PYTHON)variable throughout the Makefile - ✅ Uses
PYTHON ?= python3to allow environment override while providing a sensible default - ✅ Fixes a Makefile indentation issue (line 28) that could have caused problems
- ✅ The workflow change correctly passes the Python path to xcodebuild
Overall: The changes are well-executed and follow best practices. The PR successfully addresses the issue while improving the build system's flexibility. Nice work! 🎉
| if: matrix.language == 'swift' || matrix.language == 'cpp' | ||
| run: | | ||
| xcodebuild -scheme McBopomofoInstaller -configuration Debug build ARCHS=arm64 ONLY_ACTIVE_ARCH=YES | ||
| PYTHON=$(which python3) xcodebuild -scheme McBopomofoInstaller -configuration Debug build ARCHS=arm64 ONLY_ACTIVE_ARCH=YES |
There was a problem hiding this comment.
Good fix! Setting the PYTHON environment variable ensures xcodebuild uses the correct Python interpreter. This addresses the CodeQL Python environment issue.
One suggestion: Consider documenting why this is needed in a comment, as it may not be immediately obvious to future maintainers that the build scripts rely on this variable.
| @@ -1,16 +1,18 @@ | |||
| PYTHON ?= python3 | |||
There was a problem hiding this comment.
Excellent addition! Using PYTHON ?= python3 with the conditional assignment operator allows the Python executable to be overridden via environment variables or command-line arguments, while defaulting to python3. This follows Makefile best practices and makes the build system more flexible.
|
|
||
| associated-phrases-v2.txt: data.txt curation/builders/phrase_deriver.py associated-punctuation.txt | ||
| python3 -m curation.builders.phrase_deriver $< $@ associated-punctuation.txt | ||
| $(PYTHON) -m curation.builders.phrase_deriver $< $@ associated-punctuation.txt |
There was a problem hiding this comment.
Nice catch on the indentation! This line was previously indented with spaces instead of a tab, which is incorrect for Makefiles. The change to use $(PYTHON) is good, and fixing the indentation ensures the recipe will execute properly.
User description
Summary
Fixes CodeQL Swift and C++ analysis failures caused by Python version mismatch.
Problem
The CodeQL workflow was failing with these errors:
ExternalBuildToolExecution Data(Python-related)SwiftEmitModule normal arm64 Emitting module for SQLite(cascading failure)Root cause:
actions/setup-pythonxcodebuilddoesn't inherit this Python in its environmentpython3which resolves to Xcode's bundled Python 3.9Solution
This PR makes three key changes:
PYTHONenvironment variable to xcodebuild:PYTHON=$(which python3) xcodebuild ...$(PYTHON)variable instead of hardcodedpython3This ensures:
python3if not set)Testing
🤖 Generated with Claude Code
PR Type
Enhancement, Bug fix
Description
Ensure CodeQL uses Python 3.12
Pass PYTHON env to xcodebuild
Makefile uses configurable PYTHON variable
Stabilize Swift/C++ CodeQL builds
Diagram Walkthrough
File Walkthrough
codeql.yml
CodeQL workflow uses Python 3.12 and passes env.github/workflows/codeql.yml
Makefile
Makefile parameterizes Python executableSource/Data/Makefile