containerbuildsystem / koji-containerbuild

Container build support for Koji buildsystem

GNU Lesser General Public License v2.1

29 stars 32 forks source link

Drop importlib_metadata from requirements.txt as it causes dependency issues; install via test.sh #202

Closed hroncok closed 3 years ago

hroncok commented 3 years ago

I'd update the imports in the code as well, but they are not here. I don't understand why was this dependency added in the first place, but the commit message mentions CentOS and Python 2, so maybe it fixes a transitive depndency on importlib_metadata there, yet this can be avoided on Python 3.8+

Relevant problem in Fedora: nothing provides python3.9dist(importlib-metadata) < 3 needed by koji-containerbuild-0.11.0-2.fc35.noarch

Maintainers will complete the following section

[x] Commit messages are descriptive enough
[n/a] Code coverage from testing does not decrease and new code is covered
[n/a] JSON/YAML configuration changes are updated in the relevant schema
[n/a] Pull request has a link to an osbs-docs PR for user documentation updates

MartinBasti commented 3 years ago

Thank you.

This change caused build failure on el7, see packit error. Would you give a try to remove that dependency completely?

MartinBasti commented 3 years ago

also please add signoff to the commit msg

hroncok commented 3 years ago

This change caused build failure on el7, see packit error.

error in koji-containerbuild setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers

That is a very old setuptools we have there :(

Would you give a try to remove that dependency completely?

Sure, let me remove it and see what happens.

also please add signoff to the commit msg

What am I signing?

hroncok commented 3 years ago

So, importlib_metadata is not required on runtime but only during tests. There seem to be a dependency somewhere in the test stack that does not specify dependencies properly or that clashes with some other dependency.

Let's pin importlib_metadata in the test requires instead and see if all the failed CI jobs use that one.

hroncok commented 3 years ago

OK, this works. I cannot sign the commits because I am using the github web UI. You can squash them when merging and sign them with your own signature. Consider my addition public domain.

hroncok commented 3 years ago

Thanks @ben-alkov for squashing.

MartinBasti commented 3 years ago

No release notes, minor change affecting only tests

hroncok commented 3 years ago

Not entirely true. This removes a runtime dependency.

MartinBasti commented 3 years ago

So this ins not true?

So, importlib_metadata is not required on runtime but only during tests....

hroncok commented 3 years ago

importlib_metadata is not actually required on runtime, but it was specified as such.

MartinBasti commented 3 years ago

Do you agree with this?

Release notes:

runtime dependency importlib_metadata removed from install requirements

hroncok commented 3 years ago

I agree. I'd even say:

Unused runtime dependency importlib_metadata removed from install requirements