Restore RPM builds from pyproject configuration #165
Conversation
Signed-off-by: Plamen Dimitrov <[email protected]>
RPM builds with the previous configuration bring to
ValueError: invalid pyproject.toml config: `project.license`.
configuration error: `project.license` must be valid exactly by one definition (2 matches found):
- keys:
'file': {type: string}
required: ['file']
- keys:
'text': {type: string}
required: ['text']
error: subprocess-exited-with-error
× Preparing metadata (pyproject.toml) did not run successfully.
│ exit code: 1
╰─> See above for output.
Signed-off-by: Plamen Dimitrov <[email protected]>
The RPM spec file already contains a fixed or non-dynamic version so having a dynamic version for the pyproject.toml doesn't bring any benefit. Exporting env variables to force such a version is complicated when mixed with the isolated bash environments for the separate stages and sed-ing the pyproject file is invasive and might not work with all autosetup magic. Signed-off-by: Plamen Dimitrov <[email protected]>
WalkthroughThe changes update project build and metadata configuration. In Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
pevogam
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
2-2: Remove setuptools-scm dependency and configuration when using static versioning.With an explicit static version on line 9,
setuptools-scmis no longer needed. Its presence adds unnecessary build complexity and dependency overhead. The[tool.setuptools_scm]configuration (lines 44-46) is now ineffective.🔎 Proposed fix to remove setuptools-scm
[build-system] -requires = ["setuptools>=61.0", "setuptools-scm>=8"] +requires = ["setuptools>=61.0"] build-backend = "setuptools.build_meta"Also remove the unused configuration section:
-[tool.setuptools_scm] -version_scheme = "post-release" -local_scheme = "node-and-date" -And from line 36, remove it from dev dependencies:
-dev = ["aexpect[test,lint]", "setuptools-scm>=8", "build", "twine"] +dev = ["aexpect[test,lint]", "build", "twine"]Also applies to: 9-9, 44-46
🧹 Nitpick comments (1)
python-aexpect.spec (1)
39-40: Consider using %pyproject_buildrequires for automatic dependency management.While the current explicit
BuildRequireswork correctly, the%pyproject_buildrequiresmacro can automatically generate build dependencies frompyproject.toml, reducing maintenance burden.🔎 Optional refactor to use %pyproject_buildrequires
BuildArch: noarch -BuildRequires: python3-devel -BuildRequires: python3-setuptools +BuildRequires: pyproject-rpm-macros +BuildRequires: python3-devel +%generate_buildrequires +%pyproject_buildrequiresThis automatically pulls
python3-setuptoolsand other build dependencies from yourpyproject.toml.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pyproject.tomlpython-aexpect.spec
🔇 Additional comments (3)
python-aexpect.spec (2)
63-63: LGTM! Modern pyproject macros are correctly implemented.The migration from legacy
%py3_build/%py3_installto modern%pyproject_wheel/%pyproject_install/%pyproject_save_filesmacros is correct and follows current Fedora/RHEL Python packaging guidelines. This approach provides better integration with PEP 517 build systems and automatic file list generation.Also applies to: 66-67
76-76: LGTM! File listing automation improves maintainability.Using
-f %{pyproject_files}automates Python file listing, reducing manual maintenance. Explicit entries for%license,%doc, and binaries remain appropriate.pyproject.toml (1)
10-10: No changes needed. The pyproject.toml uses the SPDX identifierGPL-2.0-or-laterand the .spec file uses the Fedora/RPM identifierGPLv2+— both correctly represent the same license using their respective ecosystem standards. Using different formats across different packaging systems is standard practice and expected.Likely an incorrect or invalid review comment.
|
I will wait for comments on the approach towards fixed version and why the arguments for it could be wrong before removing the |
| description = "Python library used to control interactive programs" | ||
| readme = "README.rst" | ||
| dynamic = ["version"] | ||
| version = "1.8.0" |
There was a problem hiding this comment.
I'd prefer to keep away from hardcoded versions as much as possible. As I'm not really a spec-file person, I asked AI and it came-up with this:
diff --git a/aexpect/__init__.py b/aexpect/__init__.py
index 3e32262..6d714d1 100644
--- a/aexpect/__init__.py
+++ b/aexpect/__init__.py
@@ -14,6 +14,8 @@ Aexpect module, see help('aexpect.client') to get info about the main
entry-points.
"""
+# _version is generated by tool.setuptools_scm
+from ._version import version as __version__
from . import remote, rss_client
from .client import (
Expect,
diff --git a/pyproject.toml b/pyproject.toml
index 085928f..dfb3782 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -6,7 +6,7 @@ build-backend = "setuptools.build_meta"
name = "aexpect"
description = "Python library used to control interactive programs"
readme = "README.rst"
-version = "1.8.0"
+dynamic = ["version"]
license = {text = "GPL-2.0-or-later"}
authors = [{name = "Aexpect developers", email = "[email protected]"}]
maintainers = [{name = "Aexpect developers", email = "[email protected]"}]
@@ -44,6 +44,7 @@ include = ["aexpect*"]
[tool.setuptools_scm]
version_scheme = "post-release"
local_scheme = "node-and-date"
+write_to = "aexpect/_version.py"
[tool.black]
line-length = 79would it work for you?
There was a problem hiding this comment.
Alternatively it suggested:
%build
export SETUPTOOLS_SCM_PRETEND_VERSION=%{version}
%pyproject_wheel
There was a problem hiding this comment.
I tried identical export of a variable suggested by AI and it didn't work - in fact I stated the various problems I encountered in the commit messages.
There was a problem hiding this comment.
Hmm, not sure what the problem is, I built the previous version as well as my suggested one and they all worked like a charm on F42. Could you please share the steps that fail?
There was a problem hiding this comment.
Ok, so let me elaborate with details about the error here. The initial error I would get without any change regarding the version is
Error: The version in the Python package metadata 0.0.0 normalizes to zero.
It's likely a packaging error caused by missing version information
(e.g. when using a version control system snapshot as a source).
Try providing the version information manually when building the Python package,
for example by setting the SETUPTOOLS_SCM_PRETEND_VERSION environment variable if the package uses setuptools_scm.
If you are confident that the version of the Python package is intentionally zero,
you may %define the _python_dist_allow_version_zero macro in the spec file to disable this check.
error: Dependency tokens must begin with alpha-numeric, '_' or '/': *** PYTHON_PROVIDED_VERSION_NORMALIZES_TO_ZERO___SEE_STDERR ***
Provides: python-aexpect = 1.8.0-1.fc42 python3-aexpect = 1.8.0-1.fc42 python3.13-aexpect = 1.8.0-1.fc42
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PartialHardlinkSets) <= 4.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
Requires: /usr/bin/python3 python(abi) = 3.13
bogus date in %changelog: Tue Feb 28 2022 Lucas Meneghel Rodrigues <[email protected]> - 1.7.0-1
bogus date in %changelog: Tue Feb 28 2022 Lucas Meneghel Rodrigues <[email protected]> - 1.7.0-1
Dependency tokens must begin with alpha-numeric, '_' or '/': *** PYTHON_PROVIDED_VERSION_NORMALIZES_TO_ZERO___SEE_STDERR ***
and will remain the same error whether I implement the diff in 262b9a8#r2661195752 or the exported variable or both (have already tried the exported variable before too).
My environment is the same OS like yours, the fact it is a container should not matter:
# cat /etc/os-release
NAME="Fedora Linux"
VERSION="42 (Container Image)"
RELEASE_TYPE=stable
ID=fedora
VERSION_ID=42
VERSION_CODENAME=""
PLATFORM_ID="platform:f42"
PRETTY_NAME="Fedora Linux 42 (Container Image)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:42"
DEFAULT_HOSTNAME="fedora"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f42/system-administrators-guide/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=42
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=42
SUPPORT_END=2026-05-13
VARIANT="Container Image"
VARIANT_ID=container
I tried identical export of a variable suggested by AI and it didn't work - in fact I stated the various problems I encountered in the commit messages.
These are other things I tried from the commit message:
The RPM spec file already contains a fixed or non-dynamic version
so having a dynamic version for the pyproject.toml doesn't bring
any benefit. Exporting env variables to force such a version is
complicated when mixed with the isolated bash environments for the
separate stages and sed-ing the pyproject file is invasive and
might not work with all autosetup magic.
@ldoktor Do you think any different in rpmbuild versions might affect this? Just in case:
# rpmbuild --version
RPM version 4.20.1
|
Hello @pevogam, thanks for notice me. I also suggest keeping dynamic version. Refer your comments, I tried on my machines, and did some changes with using diff --git a/Makefile b/Makefile
index 4ca686f..35b74a2 100644
--- a/Makefile
+++ b/Makefile
@@ -70,19 +70,19 @@ build-deb-all: prepare-source
srpm: source
mkdir -p BUILD/SRPM
- mock -r $(MOCK_CONFIG) --resultdir BUILD/SRPM -D "rel_build 0" -D "commit $(COMMIT)" -D "commit_date $(COMMIT_DATE)" --buildsrpm --spec python-$(PROJECT).spec --sources SOURCES
+ mock -r $(MOCK_CONFIG) --resultdir BUILD/SRPM -D "rel_build 0" -D "commit $(COMMIT)" -D "commit_date $(COMMIT_DATE)" -D "package_version $(VERSION)" --buildsrpm --spec python-$(PROJECT).spec --sources SOURCES
rpm: srpm
mkdir -p BUILD/RPM
- mock -r $(MOCK_CONFIG) --resultdir BUILD/RPM -D "rel_build 0" -D "commit $(COMMIT)" -D "commit_date $(COMMIT_DATE)" --rebuild BUILD/SRPM/python-$(PROJECT)-$(VERSION)-*.src.rpm
+ mock -r $(MOCK_CONFIG) --resultdir BUILD/RPM -D "rel_build 0" -D "commit $(COMMIT)" -D "commit_date $(COMMIT_DATE)" -D "package_version $(VERSION)" --rebuild BUILD/SRPM/python-$(PROJECT)-$(VERSION)-*.src.rpm
srpm-release: source-release
mkdir -p BUILD/SRPM
- mock -r $(MOCK_CONFIG) --resultdir BUILD/SRPM -D "rel_build 1" --buildsrpm --spec python-$(PROJECT).spec --sources SOURCES
+ mock -r $(MOCK_CONFIG) --resultdir BUILD/SRPM -D "rel_build 1" -D "package_version $(VERSION)" --buildsrpm --spec python-$(PROJECT).spec --sources SOURCES
rpm-release: srpm-release
mkdir -p BUILD/RPM
- mock -r $(MOCK_CONFIG) --resultdir BUILD/RPM -D "rel_build 1" --rebuild BUILD/SRPM/python-$(PROJECT)-$(VERSION)-*.src.rpm
+ mock -r $(MOCK_CONFIG) --resultdir BUILD/RPM -D "rel_build 1" -D "package_version $(VERSION)" --rebuild BUILD/SRPM/python-$(PROJECT)-$(VERSION)-*.src.rpm
clean:
$(MAKE) -f $(CURDIR)/debian/rules clean || true
diff --git a/python-aexpect.spec b/python-aexpect.spec
index 0e72551..734b460 100644
--- a/python-aexpect.spec
+++ b/python-aexpect.spec
@@ -22,7 +22,7 @@
%global with_tests 0
Name: python-aexpect
-Version: 1.8.0
+Version: %{package_version}
Release: 1%{?gitrel}%{?dist}
Summary: Aexpect is a python library to control interactive applications
@@ -36,8 +36,10 @@ Source0: %{url}/archive/%{commit}/%{gittar}
%endif
BuildArch: noarch
+BuildRequires: pyproject-rpm-macros
BuildRequires: python3-devel
BuildRequires: python3-setuptools
+BuildRequires: python3-setuptools-scm
%description
Aexpect is a python library used to control interactive applications, very
@@ -54,16 +56,22 @@ sftp, telnet, among others.
%prep
%if 0%{?rel_build}
-%autosetup -n aexpect-%{version} -p 1
+%autosetup -n aexpect-%{package_version} -p 1
%else
%autosetup -n aexpect-%{commit} -p 1
%endif
+%generate_buildrequires
+export SETUPTOOLS_SCM_PRETEND_VERSION=%{package_version}
+%pyproject_buildrequires
+
%build
-%py3_build
+export SETUPTOOLS_SCM_PRETEND_VERSION=%{package_version}
+%pyproject_wheel
%install
-%py3_install
+%pyproject_install
+%pyproject_save_files -l aexpect
ln -s aexpect_helper %{buildroot}%{_bindir}/aexpect_helper-%{python3_pkgversion}
ln -s aexpect_helper %{buildroot}%{_bindir}/aexpect_helper-%{python3_version}
@@ -72,11 +80,9 @@ ln -s aexpect_helper %{buildroot}%{_bindir}/aexpect_helper-%{python3_version}
selftests/checkall
%endif
-%files -n python3-aexpect
+%files -n python3-aexpect -f %{pyproject_files}
%license LICENSE
%doc README.rst
-%{python3_sitelib}/aexpect/
-%{python3_sitelib}/aexpect-%{version}-py%{python3_version}.egg-info/
%{_bindir}/aexpect_helper*
%changelogAnd I am able to build the rpm in fedora43 container, but failed on RHEL10 machine. This may depends on pyproject-rpm-macros or python version. |
|
Hi both, I will check the state of this pull request on Friday, sorry for the delay. EDIT: Still couldn't do it, Monday evening is my next candidate and a more likely one. |
There is a typo, the package is called |
3 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.