PR#5: work around possible setuptools-scm issue with importlib-metadata - rpms/python-keyring

rpms / python-keyring

#5 work around possible setuptools-scm issue with importlib-metadata

Merged 4 years ago by ctubbsii. Opened 4 years ago by decathorpe.

Unknown source master into master

drop (BuildRequires) on python3-importlib-metadata for fedora 32+

Fabio Valentini • 4 years ago

12cd719

python-keyring.spec

file modified

+8 -1

		`@@ -8,7 +8,7 @@`

		`Name: python-keyring`
		`Version: 19.3.0`
		`- Release: 1%{?dist}`
		`+ Release: 2%{?dist}`
		`Summary: Store and access your passwords safely`
		`License: MIT and Python`
		`URL: https://github.com/jaraco/keyring`
		`@@ -73,14 +73,18 @@`
		`BuildRequires: python%{python3_pkgversion}-setuptools`
		`BuildRequires: python%{python3_pkgversion}-setuptools_scm`
		`%if 0%{?with_tests}`
		`+ %if 0%{?fedora} < 32`
		`BuildRequires: python%{python3_pkgversion}-importlib-metadata`
		`+ %endif`
		`BuildRequires: python%{python3_pkgversion}-pytest`
		`BuildRequires: python%{python3_pkgversion}-pytest-cov`
		`BuildRequires: python%{python3_pkgversion}-pytest-flakes`
		`%endif`

		`Requires: python%{python3_pkgversion}-SecretStorage`
		`+ %if 0%{?fedora} < 32`
		`Requires: python%{python3_pkgversion}-importlib-metadata`
		`+ %endif`

		`%{?python_provide:%python_provide python%{python3_pkgversion}-keyring}`

		`@@ -146,6 +150,9 @@`
		`%endif`

		`%changelog`
		`+ * Wed Dec 04 2019 Fabio Valentini <decathorpe@gmail.com> - 19.3.0-2`
		`+ - Drop manual (Build)Requires on python3-importlib-metadata in rawhide/f32.`
		`+`
		`* Tue Dec 03 2019 Christopher Tubbs <ctubbsii@fedoraproject.org> - 19.3.0-1`
		`- new version 19.3.0 (rhbz#1778416)`

decathorpe commented 4 years ago

Despite the restriction of "python < 3.8", the dependency is included with python 3.8.

The workaround is to patch out this dependency entirely on fedora 32.

Safe to merge back to fedora 31 (since the patch isn't applied there), if it's necessary for pushing some update or something, but not necessary.

ctubbsii commented 4 years ago

@decathorpe Thanks for the fix. FWIW, you can put the %if %{?fedora} > 31 conditional around the Patch0 line instead, so that you don't need to change the %autosetup line at all. I prefer to use %autosetup over %setup and %patch, because it keeps the spec file cleaner.

ignatenkobrain commented 4 years ago

Did you report this as a bug to python dependency generator?

It is supposed to work as the syntax is correct.

decathorpe commented 4 years ago

@decathorpe Thanks for the fix. FWIW, you can put the %if %{?fedora} > 31 conditional around the Patch0 line instead, so that you don't need to change the %autosetup line at all. I prefer to use %autosetup over %setup and %patch, because it keeps the spec file cleaner.

Well, if you wrap the Patch0 line as well, srpm files you build on <f32 will not build on f32 because they are missing the patch file.

decathorpe commented 4 years ago

Did you report this as a bug to python dependency generator?
It is supposed to work as the syntax is correct.

Will do. Though I'm not sure which component has the bug here

ignatenkobrain commented 4 years ago

https://github.com/rpm-software-management/rpm

decathorpe commented 4 years ago

@ignatenkobrain I think you already reported this same issue a year ago:

https://github.com/rpm-software-management/rpm/issues/382

rebased onto 12cd719

4 years ago

decathorpe commented 4 years ago

@ignatenkobrain Sorry, I'm a bit stupid. The dependency isn't inserted by the dep generator. It's manually specified in the .spec despite the dep generator being on by default since ... fedora 30?

PR rewritten to just drop the manual dependency on fedora 32+ ...

The .spec file could use some serious cleanups though

ctubbsii commented 4 years ago

Well, if you wrap the Patch0 line as well, srpm files you build on <f32 will not build on f32 because they are missing the patch file.

I'm okay with that. As long as the spec file in the f31 branch builds an srpm appropriate for f31, I don't care that the srpm built for f32 is targeted for f32.

The .spec file could use some serious cleanups though

Certainly. I'd like to remove a lot of the python2 and related conditionals.

ctubbsii commented 4 years ago

I have a question regarding the latest update to this PR. Is the required functionality now built-in in newer Python versions? Why don't we need the dependency in rawhide?

decathorpe commented 4 years ago

Exactly. importlib.metadata has been included in the standard library with python 3.8.

You also shouldn't need to specify all dependencies manually. The Requires are generated automatically for you. But that's probably better addressed as part of a bigger cleanup.

ctubbsii commented 4 years ago

You also shouldn't need to specify all dependencies manually. The Requires are generated automatically for you.

Good to know. I know xmvn did similar things with Java/Maven projects, but I picked this python project up from another dev and kept things mostly the way they had done it.