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

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

Merged
merged 1 commit into from Mar 26, 2021

Conversation

hroncok
Copy link
Contributor

@hroncok hroncok commented Mar 18, 2021

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

  • 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
Copy link
Contributor

Thank you.

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

@MartinBasti
Copy link
Contributor

also please add signoff to the commit msg

@hroncok
Copy link
Contributor Author

hroncok commented Mar 18, 2021

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
Copy link
Contributor Author

hroncok commented Mar 18, 2021

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 hroncok changed the title Don't require importlib_metadata on recent Pythons Don't require importlib_metadata on runtime, workaround it in tests.sh instead Mar 18, 2021
@hroncok
Copy link
Contributor Author

hroncok commented Mar 18, 2021

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.

@ben-alkov ben-alkov changed the title Don't require importlib_metadata on runtime, workaround it in tests.sh instead Drop importlib_metadata from requirements.txt as it causes dependency issues; install via test.sh Mar 18, 2021
as it causes dependency issues.
Install via test.sh instead.

Also:

- setuptools_scm >= 6 no longer supports python 2

Signed-off-by: Miro Hrončok <miro@hroncok.cz>
Signed-off-by: Ben Alkov <ben.alkov@redhat.com>
@hroncok
Copy link
Contributor Author

hroncok commented Mar 18, 2021

Thanks @ben-alkov for squashing.

@MartinBasti MartinBasti merged commit dcf2b6b into containerbuildsystem:master Mar 26, 2021
@MartinBasti
Copy link
Contributor

No release notes, minor change affecting only tests

@hroncok hroncok deleted the patch-1 branch March 26, 2021 09:33
@hroncok
Copy link
Contributor Author

hroncok commented Mar 26, 2021

Not entirely true. This removes a runtime dependency.

@MartinBasti
Copy link
Contributor

So this ins not true?

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

@hroncok
Copy link
Contributor Author

hroncok commented Mar 26, 2021

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

@MartinBasti
Copy link
Contributor

Do you agree with this?

Release notes:

runtime dependency importlib_metadata removed from install requirements

@hroncok
Copy link
Contributor Author

hroncok commented Mar 26, 2021

I agree. I'd even say:

Unused runtime dependency importlib_metadata removed from install requirements

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants