codechecker icon indicating copy to clipboard operation
codechecker copied to clipboard

[refactor] Use minimal versions instead of pinned for setuptools

Open pdgendt opened this issue 9 months ago • 12 comments

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

pdgendt avatar Mar 21 '25 09:03 pdgendt

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?

pdgendt avatar Mar 21 '25 11:03 pdgendt

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.txt files 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

bruntib avatar Mar 21 '25 14:03 bruntib

I wouldn't be against trying to be more flexible in this question

Great!

  • Dependency version numbers should be maintained in requirements.txt files 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.

I've the updated the PR, is this how you prefer the changes?

pdgendt avatar Mar 21 '25 14:03 pdgendt

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

bruntib avatar Mar 21 '25 15:03 bruntib

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

pdgendt avatar Mar 21 '25 15:03 pdgendt

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 avatar Mar 24 '25 16:03 bruntib

Normally you pin ci versions, but not the package versions.

elupus avatar Mar 24 '25 16:03 elupus

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.

elupus avatar Mar 24 '25 16:03 elupus

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.

elupus avatar Mar 24 '25 16:03 elupus

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.

pdgendt avatar Mar 25 '25 08:03 pdgendt

@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 avatar May 02 '25 15:05 pdgendt

@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!

bruntib avatar Jun 10 '25 11:06 bruntib

@pdgendt seems some checks are failing?

elupus avatar Jul 01 '25 07:07 elupus