-
Notifications
You must be signed in to change notification settings - Fork 379
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
[fix] Add explicit wheel dependency for virtualenv #3363
base: master
Are you sure you want to change the base?
Conversation
I had a problem on Ubuntu 20.04 (inside WSL2) with making a new virtualenv (make venv). bdist_wheel was needed but not found when building codechecker-api and its -shared package's wheels. |
I could have injected the dependency into the requirements.txt files and/or could have specified an exact version number for wheel, but my reasoning behind not doing so is this: |
On a real Ubuntu 18.04 I was able to do
|
Python package wheel provides the bdist_wheel command required for building wheels for codechecker-api and codechecker-api-shared packages. This modification ensures the wheel dependency for each virtualenv.
4ae8434
to
af81a03
Compare
I hit errors with bdist_wheel not being found and this patch resolved them for me. |
Right now, |
@@ -98,13 +98,15 @@ venv: | |||
# Create a virtual environment which can be used to run the build package. | |||
python3 -m venv venv --prompt="CodeChecker venv" && \ | |||
$(ACTIVATE_RUNTIME_VENV) && \ | |||
pip3 install wheel && \ |
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.
Why is this better than having the package specified in the requirements.txt
?
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.
I believe, @gamesh411 had this answer for this question from earlier:
The wheel package is only used for building the other two packages, and thus will not contribute to the runtime code directly, so there are no API contracts to uphold/break. So at build time, an up-to-date version of the package seems like a good idea to me.
However, I would also agree to put dependencies in the requirements.txt
of the venv_dev
target. This target is dedicated for dependencies during development and not during usage.
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.
@bruntib @gamesh411 By "other two packages" we mean the API, right? If yes, then I do not see why we need these in the "main" CodeChecker package or venv, as these installs could be scoped into the API Makefile: https://github.com/Ericsson/codechecker/blob/v6.23.1/web/api/Makefile#L60
Python package wheel provides the bdist_wheel command required for
building wheels for codechecker-api and codechecker-api-shared
packages. This modification ensures the wheel dependency for each
virtualenv.