Skip to content
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

Update python to 3.11 #473

Merged
merged 16 commits into from
Oct 16, 2023
Merged

Update python to 3.11 #473

merged 16 commits into from
Oct 16, 2023

Conversation

lekcyjna123
Copy link
Contributor

As pointed in #472 we need to update python to 3.11 to be allowed to inherit __init__ in Protocol. This pull request update docker files, docker images and documentation.

It introduced venv to jobs which uses custom dockers because in python 3.11 installing packages system wide is disabled by default and setup-python don't work with custom docker images. There are reported issues (eg. actions/setup-python#719), but they aren't solved.

I also updated pip3 install ... to python3 -m pip install ... to always use the same python version (setup-python don't configure pip so it is used system wide instead of uploaded by setup-python).

@lekcyjna123
Copy link
Contributor Author

@lekcyjna123 lekcyjna123 marked this pull request as ready for review October 14, 2023 13:53
Copy link
Member

@tilk tilk left a comment

Choose a reason for hiding this comment

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

Seems to work fine.

Copy link
Contributor

@Arusekk Arusekk left a comment

Choose a reason for hiding this comment

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

Should work fine :) especially with #384 (comment) out of the way.

- name: Install dependencies
run: |
python3 -m venv venv
Copy link
Contributor

Choose a reason for hiding this comment

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

setup-python should make python enough; also, no need for venv I think, as the python that has been set up has one by default already (or is it the --break-system-packages thing?)

Suggested change
python3 -m venv venv
python -m venv venv

or even

Suggested change
python3 -m venv venv

python3 -m pip install --upgrade pip
pip3 install -r requirements-dev.txt
python3 -m pip install -r requirements-dev.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be enough with setup-python (especially when using venv, but even without it)

Suggested change
python3 -m pip install -r requirements-dev.txt
pip install -r requirements-dev.txt


RUN apt-get update && \
DEBIAN_FRONTEND=noninteractive \
apt-get install -y --no-install-recommends \
python3.10 python3-pip git yosys \
python3.11 python3-pip git yosys lsb-release \
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed? Would love to see some explanation.

@tilk tilk merged commit bdf722e into master Oct 16, 2023
6 checks passed
@tilk tilk deleted the lekcyjna/bump-python311 branch October 16, 2023 12:41
@Arusekk Arusekk mentioned this pull request Oct 16, 2023
github-actions bot pushed a commit that referenced this pull request Oct 16, 2023
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