Skip to content

Commit da18f60

Browse files
frenzymadnessclaude
andcommitted
Fix epoch handling in version comparisons
- If epoch is not specified in new version, it's inherited from current packages. - Epoch is used only for some of the provides. Fixes: #5 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent c1738a8 commit da18f60

File tree

5 files changed

+180
-1
lines changed

5 files changed

+180
-1
lines changed

fedora_revdep_check.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,16 @@ def simulate_version_change(self, srpm_name: str, new_version: str) -> Dict:
228228
'binary_packages': []
229229
}
230230

231+
# If new_version doesn't include an epoch, inherit it from current packages
232+
if ':' not in new_version:
233+
# Get epoch from first package (all packages from same SRPM should have same epoch)
234+
current_epoch = binary_packages[0].get_epoch()
235+
if current_epoch and current_epoch != '0':
236+
new_version = f"{current_epoch}:{new_version}"
237+
if self.verbose:
238+
print(f"No epoch specified in new version, using epoch {current_epoch} from current package")
239+
print(f"Testing with version: {new_version}\n")
240+
231241
if self.verbose:
232242
print(f"Found {len(binary_packages)} binary package(s) from {srpm_name}:")
233243
for pkg in binary_packages:
@@ -406,7 +416,19 @@ def _check_requirement_conflict(
406416
# Check if new version satisfies all constraints
407417
if constraints:
408418
for constraint in constraints:
409-
if not self._version_satisfies(new_version, constraint['op'], constraint['version']):
419+
# Determine which new version to use based on whether the provide uses epochs
420+
# Check if any current provide has an epoch in its version
421+
provide_uses_epoch = False
422+
for pkg, prov_str, prov_version in prov_info_list:
423+
current_ver = prov_version if prov_version else pkg.get_version()
424+
if ':' in current_ver:
425+
provide_uses_epoch = True
426+
break
427+
428+
# Use appropriate version: with or without epoch depending on provide format
429+
version_to_check = new_version if provide_uses_epoch else new_version.split(':', 1)[-1]
430+
431+
if not self._version_satisfies(version_to_check, constraint['op'], constraint['version']):
410432
# New version fails - now check if current version also fails
411433
# to determine if this is a new problem or already broken
412434
current_version_also_fails = False

tests/conftest.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
create_multi_binary_scenario,
3434
create_same_srpm_dependency_scenario,
3535
create_already_broken_scenario,
36+
create_epoch_package_scenario,
37+
create_epoch_with_dist_provides_scenario,
3638
)
3739
from fedora_revdep_check import FedoraRevDepChecker # noqa: E402
3840

@@ -120,6 +122,18 @@ def already_broken_base():
120122
return create_already_broken_scenario()
121123

122124

125+
@pytest.fixture
126+
def epoch_package_base():
127+
"""Provide a mock DNF base with packages that have epochs."""
128+
return create_epoch_package_scenario()
129+
130+
131+
@pytest.fixture
132+
def epoch_with_dist_provides_base():
133+
"""Provide a mock DNF base with packages that have both RPM and dist provides."""
134+
return create_epoch_with_dist_provides_scenario()
135+
136+
123137
@pytest.fixture
124138
def checker_instance(mock_dnf_base):
125139
"""

tests/fixtures/mock_packages.py

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,3 +613,97 @@ def create_already_broken_scenario():
613613
]
614614

615615
return MockBase(packages=packages)
616+
617+
618+
def create_epoch_package_scenario():
619+
"""
620+
Create a scenario with packages that have epochs.
621+
622+
Scenario:
623+
- sphinx 1:8.0.0 currently installed (epoch 1)
624+
- reverse-dep requires sphinx >= 1:8.0.0
625+
- Upgrading to 9.0.0 without specifying epoch should inherit epoch 1
626+
- So 1:9.0.0 should satisfy the requirement (not 0:9.0.0 which would fail)
627+
"""
628+
packages = [
629+
# Sphinx package with epoch 1
630+
MockPackage(
631+
name='python3-sphinx',
632+
version='8.0.0',
633+
release='1.fc40',
634+
arch='noarch',
635+
source_name='sphinx',
636+
epoch='1',
637+
provides=[
638+
'python3-sphinx',
639+
'python3-sphinx = 1:8.0.0-1.fc40',
640+
'python3dist(sphinx) = 8.0.0',
641+
]
642+
),
643+
# Reverse dependency requiring >= 1:8.0.0
644+
MockPackage(
645+
name='python3-docs',
646+
version='1.0.0',
647+
release='1.fc40',
648+
arch='noarch',
649+
source_name='python-docs',
650+
requires=[
651+
'python3-sphinx >= 1:8.0.0',
652+
]
653+
),
654+
]
655+
656+
return MockBase(packages=packages)
657+
658+
659+
def create_epoch_with_dist_provides_scenario():
660+
"""
661+
Create a scenario with packages that have both RPM and dist provides.
662+
663+
Scenario:
664+
- sphinx 1:8.0.0 currently installed (epoch 1)
665+
- Provides both python3-sphinx (with epoch) and python3dist(sphinx) (without epoch)
666+
- reverse-dep-rpm requires python3-sphinx >= 1:8.0.0 (RPM provide with epoch)
667+
- reverse-dep-dist requires python3dist(sphinx) < 10~~ (dist provide without epoch)
668+
- Upgrading to 9.1.0 should satisfy both (use epoch for RPM, not for dist)
669+
"""
670+
packages = [
671+
# Sphinx package with epoch 1
672+
MockPackage(
673+
name='python3-sphinx',
674+
version='8.0.0',
675+
release='1.fc40',
676+
arch='noarch',
677+
source_name='sphinx',
678+
epoch='1',
679+
provides=[
680+
'python3-sphinx',
681+
'python3-sphinx = 1:8.0.0-1.fc40',
682+
'python3dist(sphinx) = 8.0.0',
683+
]
684+
),
685+
# Reverse dependency requiring RPM package with epoch
686+
MockPackage(
687+
name='python3-docs',
688+
version='1.0.0',
689+
release='1.fc40',
690+
arch='noarch',
691+
source_name='python-docs',
692+
requires=[
693+
'python3-sphinx >= 1:8.0.0',
694+
]
695+
),
696+
# Reverse dependency requiring dist provide without epoch
697+
MockPackage(
698+
name='python3-myst-parser',
699+
version='5.0.0',
700+
release='1.fc40',
701+
arch='noarch',
702+
source_name='python-myst-parser',
703+
requires=[
704+
'python3dist(sphinx) < 10~~',
705+
]
706+
),
707+
]
708+
709+
return MockBase(packages=packages)

tests/integration/test_simulation.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,3 +187,40 @@ def test_simulate_version_change_already_broken_package(self, already_broken_bas
187187
for conflict in new_pkg_conflicts:
188188
assert conflict['already_broken'] is False
189189
assert 'python3dist(library) < 5.0' in conflict['failed_constraint']
190+
191+
def test_simulate_version_change_inherits_epoch(self, epoch_package_base):
192+
"""Test that new version without epoch inherits epoch from current package."""
193+
checker = FedoraRevDepChecker(verbose=False, base=epoch_package_base)
194+
195+
# Current package is 1:8.0.0, upgrading to 9.0.0 (without epoch)
196+
# should be interpreted as 1:9.0.0, not 0:9.0.0
197+
# reverse-dep requires >= 1:8.0.0, so 1:9.0.0 should satisfy it
198+
results = checker.simulate_version_change('sphinx', '9.0.0')
199+
200+
assert 'error' not in results
201+
# The new_version should be transformed to include epoch
202+
assert results['new_version'] == '1:9.0.0'
203+
204+
# Should have no conflicts (1:9.0.0 >= 1:8.0.0)
205+
assert len(results['conflicts']) == 0
206+
207+
def test_simulate_version_change_epoch_only_for_rpm_provides(self, epoch_with_dist_provides_base):
208+
"""Test that epoch is only used for RPM provides, not for dist provides."""
209+
checker = FedoraRevDepChecker(verbose=False, base=epoch_with_dist_provides_base)
210+
211+
# Current package is 1:8.0.0 with both RPM and dist provides
212+
# Upgrading to 9.1.0 should:
213+
# - Use 1:9.1.0 for RPM package provides (with epoch)
214+
# - Use 9.1.0 for dist provides (without epoch)
215+
# reverse-dep-rpm requires python3-sphinx >= 1:8.0.0 (should be satisfied)
216+
# reverse-dep-dist requires python3dist(sphinx) < 10~~ (should be satisfied)
217+
results = checker.simulate_version_change('sphinx', '9.1.0')
218+
219+
assert 'error' not in results
220+
assert results['new_version'] == '1:9.1.0'
221+
222+
# Should have no conflicts
223+
# Both requirements should be satisfied:
224+
# - 1:9.1.0 >= 1:8.0.0 (True)
225+
# - 9.1.0 < 10~~ (True)
226+
assert len(results['conflicts']) == 0

tests/unit/test_version_operations.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,18 @@ def test_version_satisfies_invalid_operator(self, checker):
137137
assert result is False
138138

139139

140+
@pytest.mark.parametrize("version,op,required,expected", [
141+
# Test that new version without epoch is correctly compared
142+
# to requirements with epoch (after epoch is inherited)
143+
("1:9.1.0", ">=", "1:8.2.0", True), # Should satisfy after inheriting epoch 1
144+
("1:9.1.0", ">=", "8.2.0", True), # Epoch 1 > epoch 0
145+
])
146+
def test_version_satisfies_inherited_epoch(self, checker, version, op, required, expected):
147+
"""Test version comparison when epoch is inherited from current package."""
148+
result = checker._version_satisfies(version, op, required)
149+
assert result == expected, f"{version} {op} {required} should be {expected}"
150+
151+
140152
@pytest.mark.parametrize("version,op,required,expected", [
141153
# Double tilde (very pre-release)
142154
("4.7.0~~alpha", "<", "4.7.0~beta", True),

0 commit comments

Comments
 (0)