[refactor] Use minimal versions instead of pinned for setuptools
It is not recommended by the Python Packaging User Guide to pin dependencies. This is a naive approach to set the project's pinned dependencies as the package's minimal dependencies.
Fixes #4500
Hi, Thanks for the change. However, I wouldn't agree with changing dependency versions compared to what we describe in requirements.txt files. There should be a single source of dependencies and versions. Also, I would say that at only minor version changes should be allowed in an automatic manner.
Yes, as stated, this was a very naive attempt, but I still think the referred issue is a valid one. Any alternative suggestions?
Actually, requiring specific versions has historical reasons. In the beginning of the project's lifecycle it caused a lot of headache to detect whether an issue is due to a dependency version mismatch, or CodeChecker itself. So, we decided to use specific version numbers so it is common for everyone. I wouldn't be against trying to be more flexible in this question, but I would suggest the following things:
- Dependency version numbers should be maintained in
requirements.txtfiles only. I wouldn't think that using CodeChecker in different contexts should come with different version numbers (i.e. having CodeChecker via "pip install" vs. "make package"). - The minimum version numbers should be provided in this format:
lxml~=5.3. This is the same as:lxml>=5.3,<6. Major version change may result backward incompatible change and it wouldn't be nice if such a change would break the CI. https://packaging.python.org/en/latest/specifications/version-specifiers/#compatible-release
I wouldn't be against trying to be more flexible in this question
Great!
- Dependency version numbers should be maintained in
requirements.txtfiles only. I wouldn't think that using CodeChecker in different contexts should come with different version numbers (i.e. having CodeChecker via "pip install" vs. "make package").
It is common however to use tooling (pip-tools for example) to "fix" testing with a known and reproducible version, but not imposing it onto the end-user.
- The minimum version numbers should be provided in this format:
lxml~=5.3. This is the same as:lxml>=5.3,<6. Major version change may result backward incompatible change and it wouldn't be nice if such a change would break the CI. packaging.python.org/en/latest/specifications/version-specifiers#compatible-release
I've the updated the PR, is this how you prefer the changes?
There are many other files, too. Otherwise the change looks fine. Thank you!
./tools/tu_collector/requirements_py/dev/requirements.txt ./tools/bazel/requirements_py/dev/requirements.txt ./tools/report-converter/requirements_py/dev/requirements.txt ./web/requirements_py/db_psycopg2/requirements.txt ./web/requirements_py/osx/requirements.txt ./web/requirements_py/auth/requirements.txt ./web/requirements_py/dev/requirements.txt ./web/requirements_py/db_pg8000/requirements.txt ./web/requirements.txt ./requirements_py/docs/requirements.txt ./analyzer/tools/build-logger/requirements_py/dev/requirements.txt ./analyzer/tools/merge_clang_extdef_mappings/requirements_py/dev/requirements.txt ./analyzer/tools/statistics_collector/requirements_py/dev/requirements.txt ./analyzer/requirements_py/osx/requirements.txt ./analyzer/requirements_py/dev/requirements.txt ./analyzer/requirements.txt ./codechecker_common/requirements_py/dev/requirements.txt
There are many other files, too. Otherwise the change looks fine. Thank you!
Sure, didn't know it was good if I touched all those files, instead of the ones used for pypi packaging. Updated
Now it occurred that an automatic upgrade of a dependency (mypy in this case) caused a failing CI. I'm starting to be skeptic with the usefulness of the recommendation of auto version upgrade. It feels like it will cause more headache in the future. Shouldn't we rather stay with the current way of using specific 3pp versions and upgrade them right before each release? We could add this action to a check-list before the releases.
Normally you pin ci versions, but not the package versions.
Ps. Its very very useful in others ci environment which may pull in stuff like sphinx to build docs and other tools.
If codechecker pins its packages, these will conflict with other projects pins. Which is why it should not be pinned in the package description.
Also. I you dont use Poetry/uv lock files or pip compile to ensure a stable CI environment. You are still susceptible to sub package dependency upgrades.
Normally you pin ci versions, but not the package versions.
Exactly, moreover you pin using hashes to avoid tampered versions.
Now it occurred that an automatic upgrade of a dependency (mypy in this case) caused a failing CI. I'm starting to be skeptic with the usefulness of the recommendation of auto version upgrade. It feels like it will cause more headache in the future. Shouldn't we rather stay with the current way of using specific 3pp versions and upgrade them right before each release? We could add this action to a check-list before the releases.
@bruntib what's the plan then? Is there a clear distinction between package dependencies vs testing dependencies?
One approach I can recommend is using tooling to pin the dependencies for you. You provide a requirements.in file that has the minimal and/or excluded versions, and run
$ pip-compile --generate-hashes --output-file=requirements.txt requirements.in
The requirements.txt file is the source of truth for CI, and requirements.in for package depedencies.
Next to that, add a dependabot configuration that checks for updates, which can be tested in CI before pushing to the main branch.
@bruntib any more feedback on this?
I often need to uninstall CodeChecker because it forces versions of dependencies for an entire environment, which isn't ideal. Projects should pin in CI and during testing, so users can always fallback to those if issues arise.
@pdgendt, Sorry for disappearing this long. Alright, let's try using the ~= in requirements.txt files, like in the current version of this PR. Could you, please, resolve the conflicts, and then we could try this approach. Please, also make sure that the required tests are passing. Thank you!
@pdgendt seems some checks are failing?