PR#56: Don't include all requirements with True-evaluating markers in extras subpackages - rpms/python-rpm-generators

rpms / python-rpm-generators

#56 Don't include all requirements with True-evaluating markers in extras subpackages

Merged 2 years ago by churchyard. Opened 2 years ago by churchyard.

rpms/ churchyard/python-rpm-generators bz2090186 into rawhide

Don't include all requirements with True-evaluating markers in extras subpackages

Miro Hrončok • 2 years ago

0bd051d

python-rpm-generators.spec

file modified

+5 -1

		`@@ -1,7 +1,7 @@`
		`Name: python-rpm-generators`
		`Summary: Dependency generators for Python RPMs`
		`Version: 12`
		`- Release: 14%{?dist}`
		`+ Release: 15%{?dist}`

		`# Originally all those files were part of RPM, so license is kept here`
		`License: GPLv2+`
		`@@ -47,6 +47,10 @@`
		`%{_rpmconfigdir}/pythonbundles.py`

		`%changelog`
		`+ * Fri May 27 2022 Miro Hrončok <mhroncok@redhat.com> - 12-15`
		`+ - Don't include all requirements with True-evaluating markers in extras subpackages`
		`+ - Fixes: rhbz#2090186`
		`+`
		`* Thu Feb 10 2022 Sandro Mani <manisandro@gmail.com> - 12-14`
		`- Add namespace option to pythodistdeps.py to allow mingw-python generatros`

pythondistdeps.py

file modified

+8 -1

		`@@ -112,10 +112,17 @@`

		`def requirements_for_extra(self, extra):`
		`extra_deps = []`
		`+ # we are only interested in dependencies with extra == 'our_extra' marker`
		`for req in self.requirements:`
		`+ # no marker at all, nothing to evaluate`
		`if not req.marker:`
		`continue`
		`- if req.marker.evaluate(get_marker_env(self, extra)):`
		`+ # does the marker include extra == 'our_extra'?`
		`+ # we can only evaluate the marker as a whole,`
		`+ # so we evaluate it twice (using 2 different marker_envs)`
		`+ # and see if it only evaluates to True with our extra`
		`+ if (req.marker.evaluate(get_marker_env(self, extra)) and`
		`+ not req.marker.evaluate(get_marker_env(self, None))):`
		`extra_deps.append(req)`
		`return extra_deps`

tests/data/scripts_pythondistdeps/test-data.yaml

file modified

		`@@ -1353,4 +1353,13 @@`
		`python3.9dist(zope-component[security])`
		`python3.9dist(zope-component[zcml])`
		`python3.9dist(zope-testing)`
		`+ --requires --normalized-names-format pep503 --require-extras-subpackages --package-name python3-build+virtualenv:`
		`+ --provides --majorver-provides --normalized-names-format pep503 --package-name python3-build+virtualenv:`
		`+ usr/lib/python3.10/site-packages/build-0.8.0.dist-info:`
		`+ provides: \|-`
		`+ python3.10dist(build[virtualenv]) = 0.8`
		`+ python3dist(build[virtualenv]) = 0.8`
		`+ requires: \|-`
		`+ python(abi) = 3.10`
		`+ python3.10dist(virtualenv) >= 20.0.35`

tests/data/scripts_pythondistdeps/test-requires.yaml

file modified

		`@@ -100,3 +100,6 @@`
		`'2.1.0': ['3.9']`
		`wheel:`
		`'2.1.0': ['3.9']`
		`+ build:`
		`+ wheel:`
		`+ '0.8.0': ['3.10']`

churchyard commented 2 years ago

The idea is that the extra subpackage only has requirements specific to that extra.
The logic however only excluded requirements without markers,
but requirements with a marker that was correct leaked to all extras subpackages.

E.g. with the following requirements:

Requires-Dist: base-dependency
Requires-Dist: base-dependency-with-matching-marker ; python_version < "3.15"
Requires-Dist: base-dependency-with-unmatching-marker ; python_version < "3.8"
Provides-Extra: an-extra
Requires-Dist: extra-only-dependency-with-matching-marker ; extra == 'an-extra' and python_version < "3.15"
Requires-Dist: extra-only-dependency-with-unmatching-marker ; extra == 'an-extra' and python_version < "3.8"

On Python 3.10, the base package generated the following requirements:

python3.10dist(base-dependency)
python3.10dist(base-dependency-with-matching-marker)

And for the [an-extra] extra:

python3.10dist(base-dependency-with-matching-marker)  <--- REDUNDANT, WRONG
python3.10dist(extra-only-dependency-with-matching-marker)

Now we no longer just check if the marker evaluates to True,
but we also check that the same marker evaluates to False when the extra is not given.

A real package with this thing issue build[virtualenv] 0.8.0, which we use for tests. The package has:

Requires-Dist: tomli (>=1.0.0) ; python_version < "3.11"

And on Python 3.10, it generated the following dependency for python3-build+virtualenv-0.8.0-2.fc37.noarch.rpm:

python3.10dist(tomli) >= 1

Now it no longer does. This is asserted in tests.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2090186

Upstream PR: https://github.com/rpm-software-management/python-rpm-packaging/pull/16

lbalhar commented 2 years ago

I like it. I wanted to test it with the package which uncovered the problem but you've already added that as a test here so this is already verified.

rebased onto 0bd051d

2 years ago

churchyard commented 2 years ago

Amended the commit message to fix a typo. Waiting for the CI.

zuul commented 2 years ago

Build succeeded.

check-for-arches : SUCCESS in 45s
rpm-scratch-build : SUCCESS in 6m 35s
rpm-scratch-build-s390x : SKIPPED (non-voting)
rpm-scratch-build-ppc64le : SKIPPED (non-voting)
rpm-scratch-build-i686 : SKIPPED (non-voting)
rpm-scratch-build-aarch64 : SKIPPED (non-voting)
rpm-linter : SUCCESS in 2m 26s (non-voting)
rpm-rpminspect : SUCCESS in 2m 55s (non-voting)
check-for-tests : SUCCESS in 24s
check-for-sti-tests : SUCCESS in 22s
check-for-fmf-tests : SUCCESS in 23s
rpm-install-test : SUCCESS in 1m 54s
rpm-test : SUCCESS in 8m 21s
rpm-tmt-test : SKIPPED
rpm-sti-test : SUCCESS in 12m 07s

Pull-Request has been merged by churchyard

2 years ago

Metadata

Assignee

None

Tags

No Tags

Changes Summary 4

+5 -1

file changed

python-rpm-generators.spec

tests/data/scripts_pythondistdeps/test-data.yaml

+3 -0

file changed

tests/data/scripts_pythondistdeps/test-requires.yaml

rpms / python-rpm-generators

Source Code

#56 Don't include all requirements with True-evaluating markers in extras subpackages Merged 2 years ago by churchyard. Opened 2 years ago by churchyard. rpms/ churchyard/python-rpm-generators bz2090186 into rawhide

Metadata

Changes Summary 4

#56 Don't include all requirements with True-evaluating markers in extras subpackages

Merged 2 years ago by churchyard. Opened 2 years ago by churchyard.

rpms/ churchyard/python-rpm-generators bz2090186 into rawhide