Skip to content

fix(tools): correct python version check in op.sh #37064

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.

Sign up for GitHub

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
TheSecurityDev wants to merge 2 commits into
base: master
Choose a base branch
from
+10 −13
Open

fix(tools): correct python version check in op.sh #37064

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db2b531
fix: improve python version check in op_check_python function
TheSecurityDev Feb 2, 2026
b38d364
simplify and use INSTALLED_PYTHON_VERSION
TheSecurityDev Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions tools/op.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,24 +153,21 @@ function op_check_os() {

function op_check_python() {
echo "Checking for compatible python version..."
REQUIRED_PYTHON_VERSION=$(grep "requires-python" $OPENPILOT_ROOT/pyproject.toml)
INSTALLED_PYTHON_VERSION=$(python3 --version 2> /dev/null || true)
REQUIRED_PYTHON_VERSION=$(grep "requires-python" $OPENPILOT_ROOT/pyproject.toml | cut -d'=' -f2- | xargs)
INSTALLED_PYTHON_VERSION=$(python3 --version 2>&1 | grep -oP '\d+\.\d+(?:\.\d+)?')

if [[ -z $INSTALLED_PYTHON_VERSION ]]; then
echo -e " ↳ [${RED}✗${NC}] python3 not found on your system. You need python version satisfying $(echo $REQUIRED_PYTHON_VERSION | cut -d '=' -f2-) to continue!"
echo -e " ↳ [${RED}✗${NC}] python3 not found on your system. You need python version satisfying '$REQUIRED_PYTHON_VERSION' to continue!"
loge "ERROR_PYTHON_NOT_FOUND"
return 1
fi

if python3 -c "from packaging.specifiers import SpecifierSet; import sys; exit(0 if '$INSTALLED_PYTHON_VERSION' in SpecifierSet('$REQUIRED_PYTHON_VERSION') else 1)" 2>/dev/null; then
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The 2>/dev/null suppresses all errors from the Python command, including import errors if the packaging module is missing. This could lead to confusing behavior where the check silently fails. Consider capturing stderr and providing a helpful error message if packaging is not installed, or document the dependency requirement more explicitly in the script's error output.

Suggested change
if python3 -c "from packaging.specifiers import SpecifierSet; import sys; exit(0 if '$INSTALLED_PYTHON_VERSION' in SpecifierSet('$REQUIRED_PYTHON_VERSION') else 1)" 2>/dev/null; then
# Ensure the required 'packaging' module is available before using it for version checks.
if ! python3 -c "import packaging.specifiers" >/dev/null 2>&1; then
echo -e " ↳ [${RED}✗${NC}] The Python 'packaging' module is required but not installed in your environment."
echo " Install it with: python3 -m pip install packaging"
loge "ERROR_PYTHON_PACKAGING_MISSING"
return 1
fi
if python3 -c "from packaging.specifiers import SpecifierSet; import sys; exit(0 if '$INSTALLED_PYTHON_VERSION' in SpecifierSet('$REQUIRED_PYTHON_VERSION') else 1)"; then

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@TheSecurityDev TheSecurityDev Feb 2, 2026

Choose a reason for hiding this comment

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

We could add an extra check if they don't mind the extra code

echo -e " ↳ [${GREEN}✔${NC}] $INSTALLED_PYTHON_VERSION detected."
else
LB=$(echo $REQUIRED_PYTHON_VERSION | tr -d -c '[0-9,]' | cut -d ',' -f1)
UB=$(echo $REQUIRED_PYTHON_VERSION | tr -d -c '[0-9,]' | cut -d ',' -f2)
VERSION=$(echo $INSTALLED_PYTHON_VERSION | grep -o '[0-9]\+\.[0-9]\+' | tr -d -c '[0-9]')
if [[ $VERSION -ge LB && $VERSION -lt UB ]]; then
echo -e " ↳ [${GREEN}✔${NC}] $INSTALLED_PYTHON_VERSION detected."
else
echo -e " ↳ [${RED}✗${NC}] You need a python version satisfying $(echo $REQUIRED_PYTHON_VERSION | cut -d '=' -f2-) to continue!"
loge "ERROR_PYTHON_VERSION" "$INSTALLED_PYTHON_VERSION"
return 1
fi
echo -e " ↳ [${RED}✗${NC}] You need a python version satisfying '$REQUIRED_PYTHON_VERSION' to continue!"
loge "ERROR_PYTHON_VERSION" "$INSTALLED_PYTHON_VERSION"
return 1
fi
}

Expand Down
Loading