-
Notifications
You must be signed in to change notification settings - Fork 330
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
python-setup: Fix venv creation in Ubuntu 22.04 #1257
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From how I read the given context (thanks!) this looks fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable. I'd like to start running these tests on Ubuntu 22.04, so we can verify this is fixed and to catch it if it breaks again in the future. Could you add ubuntu-22.04
to the matrix in python-deps.yml
(and any other relevant tests)?
Done 👍 (but only for the python-setup tests) |
I don't think Python 2 is preinstalled in the |
we have a bit of an internal discussion on how to handle this. Let me get back to this PR once we have figured out what to do. |
this should fail the Ubuntu 22.04 test, but just want to be able to point to a log file to show that the logic works before disabling the test. If only I had designed the test system to be more flexible, maybe we could have kept the test alive under a form of expected failure mode 🤔 |
Another way to test this might be adding a step that calls |
Example of failure here: https://github.com/github/codeql-action/actions/runs/3097944471/jobs/5015231587#step:4:140 (revealed that I had forgotten a few spaces) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just a copy editing suggestion
Co-authored-by: Henry Mercer <henrymercer@github.com>
Taking this out of draft since that reruns the PR checks. |
Removes the hotfix from #1257
Fixes #1249
As described here, when using Ubuntu 22.04 with new enough versions of
setuptools
(60.0.0+), the virtual environment created withvirtualenv
will put binaries in<venv-path>/local/bin
instead of<venv-path>/bin
.Next release after
20.16.5
ofvirtualenv
will include a fix for this (PR).Merge / deployment checklist