Skip to content

feat: added python/pip version check to make install #165

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.

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
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mike9ant
Copy link
Contributor

Make install was failing on bad python version or if missing python with pip. Added checks for that.

@mike9ant mike9ant requested a review from a team as a code owner March 21, 2025 01:39
@checkmarx-do
Copy link

Logo
Checkmarx One – Scan Summary & Details7b22e45b-afed-4688-8c0e-73f2aba852f8

Great job, no security vulnerabilities found in this Pull Request

Comment on lines +46 to +51
@echo "Checking python version, venv and pip availability..."
@python3 -c 'import sys; exit(0 if sys.version_info >= (3,10) else 1)' \
|| (echo "You need Python 3.10 or higher. Please install it, e.g.: \n sudo apt install python3.10 python3.10-venv python3.10-dev" && exit 1)
@(python3 -m venv --help >/dev/null 2>&1 && python3 -m pip --version >/dev/null 2>&1) \
|| (echo "Python venv and/or pip are not available. Please install them, e.g.: \n sudo apt install python3.10-venv python3.10-dev python3-pip" && exit 1)

Copy link
Collaborator

Choose a reason for hiding this comment

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

this won't work on macOS

Copy link
Collaborator

Choose a reason for hiding this comment

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

in general this is still a suboptimal solution, because we need specific things installed on the host, and we can't really control them. the way forward is to use something like uv to manage all the deps we need internally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

by saying it won't work because the commands a bad, i.e. you have to use 'brew' instead? But the makefile will actually run, correct? if so maybe we check in and update later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or you saying version check may fail incorrectly, just causing install to fail where it wouldn't have?

Copy link
Collaborator

Choose a reason for hiding this comment

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

this line:

	@python3 -c 'import sys; exit(0 if sys.version_info >= (3,10) else 1)' \

is problematic, for example, on my computer:

❯ python3 --version
Python 3.9.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants