PR#3: Resolve RHBZ#1923075; greatly simplify spec file with a number of improvements - rpms/pgcli

		`@@ -0,0 +1,68 @@`
		`+ diff -Naur pgcli-3.1.0-original/tests/metadata.py pgcli-3.1.0/tests/metadata.py`
		`+ --- pgcli-3.1.0-original/tests/metadata.py 2020-08-14 18:08:15.000000000 -0400`
		`+ +++ pgcli-3.1.0/tests/metadata.py 2021-03-29 12:48:22.105962073 -0400`
		`+ @@ -3,7 +3,7 @@`
		`+ from pgcli.packages.parseutils.meta import FunctionMetadata, ForeignKey`
		`+ from prompt_toolkit.completion import Completion`
		`+ from prompt_toolkit.document import Document`
		`+ -from mock import Mock`
		`+ +from unittest.mock import Mock`
		`+ import pytest`
		`+`
		`+ parametrize = pytest.mark.parametrize`
		`+ diff -Naur pgcli-3.1.0-original/tests/test_completion_refresher.py pgcli-3.1.0/tests/test_completion_refresher.py`
		`+ --- pgcli-3.1.0-original/tests/test_completion_refresher.py 2020-08-14 18:08:15.000000000 -0400`
		`+ +++ pgcli-3.1.0/tests/test_completion_refresher.py 2021-03-29 12:48:25.523982262 -0400`
		`+ @@ -1,6 +1,6 @@`
		`+ import time`
		`+ import pytest`
		`+ -from mock import Mock, patch`
		`+ +from unittest.mock import Mock, patch`
		`+`
		`+`
		`+ @pytest.fixture`
		`+ diff -Naur pgcli-3.1.0-original/tests/test_main.py pgcli-3.1.0/tests/test_main.py`
		`+ --- pgcli-3.1.0-original/tests/test_main.py 2020-08-14 18:08:15.000000000 -0400`
		`+ +++ pgcli-3.1.0/tests/test_main.py 2021-03-29 12:48:01.263838960 -0400`
		`+ @@ -1,6 +1,6 @@`
		`+ import os`
		`+ import platform`
		`+ -import mock`
		`+ +from unittest import mock`
		`+`
		`+ import pytest`
		`+`
		`+ diff -Naur pgcli-3.1.0-original/tests/test_naive_completion.py pgcli-3.1.0/tests/test_naive_completion.py`
		`+ --- pgcli-3.1.0-original/tests/test_naive_completion.py 2020-08-14 18:08:15.000000000 -0400`
		`+ +++ pgcli-3.1.0/tests/test_naive_completion.py 2021-03-29 12:48:28.710001082 -0400`
		`+ @@ -13,7 +13,7 @@`
		`+`
		`+ @pytest.fixture`
		`+ def complete_event():`
		`+ - from mock import Mock`
		`+ + from unittest.mock import Mock`
		`+`
		`+ return Mock()`
		`+`
		`+ diff -Naur pgcli-3.1.0-original/tests/test_pgexecute.py pgcli-3.1.0/tests/test_pgexecute.py`
		`+ --- pgcli-3.1.0-original/tests/test_pgexecute.py 2020-08-14 18:08:15.000000000 -0400`
		`+ +++ pgcli-3.1.0/tests/test_pgexecute.py 2021-03-29 12:47:49.041766766 -0400`
		`+ @@ -2,7 +2,7 @@`
		`+`
		`+ import psycopg2`
		`+ import pytest`
		`+ -from mock import patch, MagicMock`
		`+ +from unittest.mock import patch, MagicMock`
		`+ from pgspecial.main import PGSpecial, NO_QUERY`
		`+ from utils import run, dbtest, requires_json, requires_jsonb`
		`+`
		`+ diff -Naur pgcli-3.1.0-original/tests/test_rowlimit.py pgcli-3.1.0/tests/test_rowlimit.py`
		`+ --- pgcli-3.1.0-original/tests/test_rowlimit.py 2020-08-14 18:08:15.000000000 -0400`
		`+ +++ pgcli-3.1.0/tests/test_rowlimit.py 2021-03-29 12:48:06.210868182 -0400`
		`+ @@ -1,5 +1,5 @@`
		`+ import pytest`
		`+ -from mock import Mock`
		`+ +from unittest.mock import Mock`
		`+`
		`+ from pgcli.main import PGCli`
		`+`

pgcli.spec

file modified

+38 -53

		`@@ -2,86 +2,71 @@`

		`Name: %{pypi_name}`
		`Version: 3.1.0`
		`- Release: 1%{?dist}`
		`+ Release: 2%{?dist}`
		`Summary: CLI for Postgres Database. With auto-completion and syntax highlighting`

		`License: BSD`
		`- URL: http://pgcli.com`
		`+ URL: https://www.pgcli.com/`
		`Source0: %{pypi_source}`
		`+`
		`+ # Backport a portion of commit 762fb4b8da98fdf6792e6c5586060ed37224f894,`
		`+ # corresponding to https://github.com/dbcli/pgcli/pull/1229 “Modernize code to`
		`+ # Python 3.6+”, to remove the dependency on PyPI mock; see`
		`+ # https://fedoraproject.org/wiki/Changes/DeprecatePythonMock.`
		`+ Patch0: %{pypi_name}-3.1.0-DeprecatePythonMock.patch`
		`+`
		`BuildArch: noarch`

		`BuildRequires: python3-devel`
		`- BuildRequires: python3dist(cli-helpers) >= 1.2`
		`- BuildRequires: python3dist(click) >= 4.1`
		`- BuildRequires: python3dist(configobj) >= 5.0.6`
		`- BuildRequires: python3dist(humanize) >= 0.5.1`
		`- #BuildRequires: python3dist(keyring) >= 12.2`
		`- BuildRequires: python3dist(prompt-toolkit)`
		`- BuildRequires: python3dist(psycopg2) >= 2.8`
		`- BuildRequires: python3dist(pygments) >= 2`
		`- BuildRequires: python3dist(setproctitle) >= 1.1.9`
		`- BuildRequires: python3dist(setuptools)`
		`- BuildRequires: python3dist(sqlparse)`
		`-`
		`- Requires: python3dist(cli-helpers) >= 1.2`
		`- Requires: python3dist(click) >= 4.1`
		`- Requires: python3dist(configobj) >= 5.0.6`
		`- Requires: python3dist(humanize) >= 0.5.1`
		`- #Requires: python3dist(keyring) >= 12.2`
		`- Requires: python3dist(prompt-toolkit)`
		`- Requires: python3dist(psycopg2) >= 2.8`
		`- Requires: python3dist(pygments) >= 2`
		`- Requires: python3dist(setproctitle) >= 1.1.9`
		`- Requires: python3dist(sqlparse)`
		`- Requires: python3dist(pendulum)`
		`-`
		`- %{?python_provide:%python_provide python3-%{pypi_name}}`
		`-`
		`- #BuildRequires for tests`
		`+ BuildRequires: pyproject-rpm-macros`
		`+`
		`+ # Additional BuildRequires for tests, not in the package metadata. Versions`
		`+ # come from tox.ini in unreleased upstream sources. Note that upstream wants`
		`+ # pytest <= 3.0.7, and we will have to unpin it and hope for the best.`
		`BuildRequires: python3dist(pytest) >= 2.7.0`
		`- BuildRequires: python3dist(pgspecial) >= 1.11.8`
		`- BuildRequires: python3dist(click) >= 4.1`
		`- BuildRequires: python3dist(pygments) >= 2.0`
		`- BuildRequires: python3dist(prompt-toolkit)`
		`- BuildRequires: python3dist(psycopg2) >= 2.8`
		`- BuildRequires: python3dist(sqlparse) >= 0.3.0`
		`- BuildRequires: python3dist(configobj) >= 5.0.6`
		`- BuildRequires: python3dist(humanize) => 0.5.1`
		`- BuildRequires: python3dist(cli-helpers) >= 1.2.0`
		`-`
		`- BuildRequires: python3dist(setproctitle) >= 1.1.9`
		`- BuildRequires: python3dist(mock) >= 1.0.1`
		`BuildRequires: python3dist(behave) >= 1.2.4`
		`BuildRequires: python3dist(pexpect) >= 3.3`
		`- BuildRequires: python3dist(keyring) >= 11.0.0`
		`- BuildRequires: python3dist(pendulum)`
		`+`
		`+ %py_provides python3-%{pypi_name}`

		`%description`
		`CLI for Postgres Database. With auto-completion and syntax highlighting`

		`+ %pyproject_extras_subpkg -n python3-%{pypi_name} keyring`
		`+`
		`+ %generate_buildrequires`
		`+ %pyproject_buildrequires -x keyring`
		`+`
		`%prep`
		`- %autosetup`
		`- # Remove bundled egg-info`
		`- rm -rf %{pypi_name}.egg-info`
		`- sed -i 's#"prompt_toolkit>=.*"#"prompt_toolkit >= 2.0.6"#' setup.py`
		`+ %autosetup -p1`

		`%build`
		`- %py3_build`
		`+ %pyproject_wheel`

		`%install`
		`- %py3_install`
		`+ %pyproject_install`
		`+ %pyproject_save_files %{pypi_name}`

		`%check`
		`- PYTHONPATH=build/lib/ py.test-3`
		`+ %pytest`

		`- %files`
		`+ %files -f %{pyproject_files}`
		`%license LICENSE.txt`
		`%doc README.rst changelog.rst`
		`%{_bindir}/%{pypi_name}`
		`- %{python3_sitelib}/%{pypi_name}`
		`- %{python3_sitelib}/%{pypi_name}-%{version}-py%{python3_version}.egg-info`

		`%changelog`
		`+ * Mon Mar 29 2021 Benjamin A. Beasley <code@musicinmybrain.net> - 3.1.0-2`
		`+ - Resolve RHBZ#1923075`
		`+ - Use pyproject-rpm-macros to eliminate error-prone manual BR’s`
		`+ - Do not manually duplicate automatic Requires`
		`+ - Drop obsolete sed invocation on setup.py`
		`+ - Do not use obsolete python_provide macro; use py_provides macro instead`
		`+ - Add the Python extras metapackage for the keyring extra`
		`+ - Use the pytest macro`
		`+ - Stop removing bundled egg-info`
		`+ - Switch to HTTPS URL`
		`+`
		`* Sat Feb 20 2021 Dick Marinus <dick@mrns.nl - 3.1.0-1`
		`- Update to v3.1.0`

music commented 2 years ago

Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1923075.

Addresses https://fedoraproject.org/wiki/Changes/DeprecatePythonMock by backporting a patch from upstream.

Adds the metapackage needed for the “keyring” extra (https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_python_extras).

Removes manual Requires (relying instead on automatic Requires generation); uses pyproject-rpm-macros to generate BuildRequires as well. This greatly simplifies the spec file and keeps the version specifications from getting out of sync with the upstream project metadata.

The result builds on F35 and F34 (older releases lack proper versions of one or more dependencies for the 3.1.0 release) and resolves the current FTBFS. It should also be much easier to maintain.

Let me know if you have questions or complaints, or need/want a co-maintainer.

churchyard commented on line 80 of pgcli.spec 2 years ago

I'd include this before %generate_buildrequires, but it is just a matter of style.

churchyard commented on line 10 of pgcli.spec 2 years ago

As an unrelated matter of general cleanup: They do have a HTTPS website: https://www.pgcli.com/

churchyard commented 2 years ago

There are no actual guidelines about this, but it is abit weird that the packages are called like this:

pgcli-3.1.0-2.fc35.noarch.rpm
python3-pgcli+keyring-3.1.0-2.fc35.noarch.rpm

Should the package with extra be called just:

pgcli+keyring-3.1.0-2.fc35.noarch.rpm

WDYT?

churchyard commented on line 86 of pgcli.spec 2 years ago

There is no reason to remove bundled egg-info.

meeuw commented 2 years ago

There is no reason to remove bundled egg-info.

That had been done IIRC because pgcli was at one time bundled with outdated egg-info

LGTM, I learned a lot of nice Python packaging tricks from this!

music commented 2 years ago

I'd include this before %generate_buildrequires, but it is just a matter of style.

I am indifferent, so I will adjust to your preference.

As an unrelated matter of general cleanup: They do have a HTTPS website: https://www.pgcli.com/

Good catch; will adjust.

There is no reason to remove bundled egg-info.

That had been done IIRC because pgcli was at one time bundled with outdated egg-info

Will drop this.

I see this in a majority of Python packages, probably because of pyp2rpm. I’ve not run into a case where it was needed yet, but I hadn’t taken time to convince myself I wasn’t missing something here.

Should the package with extra be called just:

That’s a very interesting question. My first reaction is that if you see the extra as being related to the importable Python module or package, python3-pgcli+keyring-3.1.0-2.fc35.noarch.rpm is correct; it goes well with %py_provides python3-%{pypi_name}. If you see the extra as being related to an optional feature for the CLI tool, then pgcli+keyring-3.1.0-2.fc35.noarch.rpm might be better. I think I lean toward the former but I am not firmly committed to that opinion.

A determined packager should be able to offer both by manually specifying the metapackage, but as far as I can tell this would prevent using one of the nice macro helpers (%pyproject_extras_subpkg or %python_extras_subpkg).

Either way, I think the most important thing is for BuildRequires: python3dist(pgcli[keyring]) to work as expected. I defer to the maintainer’s opinion on which way to do it.

3 new commits added

Order %pyproject_extras_subpkg before %generate_buildrequires
Switch to HTTPS URL
Stop removing bundled egg-info

2 years ago

music commented 2 years ago

P.S. Thanks to both @churchyard and @meeuw for looking over this PR. Always nice to get a few sets of eyeballs on things.

churchyard commented 2 years ago

I see this in a majority of Python packages, probably because of pyp2rpm. I’ve not run into a case where it was needed yet, but I hadn’t taken time to convince myself I wasn’t missing something here.

Rule of thumb: packages that use %pyproject_wheel/%pyproject_install should not ever need to remove the bundled egg info.