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

[fix] Add explicit wheel dependency for virtualenv #3363

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gamesh411
Copy link
Collaborator

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.

@gamesh411
Copy link
Collaborator Author

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.

@gamesh411
Copy link
Collaborator Author

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:
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.

@whisperity
Copy link
Contributor

On a real Ubuntu 18.04 I was able to do pip install codechecker-api codechecker-api-shared, however, pip install codechecker failed:

Building wheels for collected packages: codechecker
  Running setup.py bdist_wheel for codechecker ... error
  Complete output from command /home/w/TEMPORARY_CRAP_ENV/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-build-x9i4yi32/codechecker/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" bdist_wheel -d /tmp/tmpg1_qjztfpip-wheel- --python-tag cp36:
  /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
    warnings.warn(msg)
  usage: -c [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
     or: -c --help [cmd1 cmd2 ...]
     or: -c --help-commands
     or: -c cmd --help

  error: invalid command 'bdist_wheel'

  ----------------------------------------
  Failed building wheel for codechecker
  Running setup.py clean for codechecker
Failed to build codechecker

@whisperity whisperity added dependencies 📦 Pull requests that update a dependency file dev env ⛑️ Development environment labels Jul 5, 2021
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.
@tomrittervg
Copy link

I hit errors with bdist_wheel not being found and this patch resolved them for me.

@whisperity
Copy link
Contributor

Right now, pip install codechecker seems to work on all major systems. @gamesh411 Can you check in let's say a Docker env whether most of the LTS Linuxes (Ubuntu 20, 22, and maybe latest Debian, and Alpine Linux as well?) and their default Python versions can do the pip install. If so, this change is actually not needed? If not, then we should move forward with reviewing this patch.

@@ -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 && \
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 better than having the package specified in the requirements.txt?

Copy link
Contributor

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.

Copy link
Contributor

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

@whisperity whisperity added the python Pull requests that update Python code (used by DependaBot) label Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies 📦 Pull requests that update a dependency file dev env ⛑️ Development environment python Pull requests that update Python code (used by DependaBot)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants