Report already broken packages as a separate category

frenzymadness · frenzymadness · commit f5e9a50fe6a5 · 2026-02-10T12:44:29.000+01:00
Fixes: #2
diff --git a/README.md b/README.md
@@ -39,23 +39,33 @@ fedora-revdep-check python-requests 2.32.0 --repo fedora-40 --repo fedora-40-sou
 
 ## Output
 
-When conflicts are found:
+When conflicts are found, they are categorized into new problems and already-broken packages:
 
 ```
 These packages would FTBFS:
   jupyter-server: python3dist(jupyterlab) < 4.7
 
 These packages would FTI:
   python3-jupyter-client-8.0.0-1.fc44: python3dist(jupyterlab) >= 4.0, < 4.7
+
+These packages already FTBFS (not a new problem):
+  some-package: python3dist(jupyterlab) < 3.0
+
+These packages already FTI (not a new problem):
+  python3-old-package-1.0.0-1.fc44: python3dist(jupyterlab) < 3.0
 ```
 
 - **FTBFS**: Fail To Build From Source (source packages that won't build)
 - **FTI**: Fail To Install (binary packages that won't install)
 
+The tool distinguishes between:
+- **New problems**: Packages that currently work but would break with the update
+- **Already broken**: Packages that already fail with the current version in repos (not caused by the update)
+
 ## Exit Codes
 
-- `0` - No conflicts detected
-- `1` - Conflicts found or error occurred
+- `0` - No new conflicts detected (already-broken packages don't cause non-zero exit)
+- `1` - New conflicts found or error occurred
 - `130` - Interrupted by user
 
 ## How It Works
diff --git a/fedora_revdep_check.py b/fedora_revdep_check.py
@@ -1,3 +1,4 @@
+#!/usr/bin/env python3
 """
 Fedora Reverse Dependency Checker
 
@@ -406,14 +407,27 @@ def _check_requirement_conflict(
         if constraints:
             for constraint in constraints:
                 if not self._version_satisfies(new_version, constraint['op'], constraint['version']):
+                    # New version fails - now check if current version also fails
+                    # to determine if this is a new problem or already broken
+                    current_version_also_fails = False
+
+                    # Get current version from prov_info_list
+                    for pkg, prov_str, prov_version in prov_info_list:
+                        # Use the package version if provide doesn't have its own version
+                        current_ver = prov_version if prov_version else pkg.get_version()
+                        if not self._version_satisfies(current_ver, constraint['op'], constraint['version']):
+                            current_version_also_fails = True
+                            break
+
                     return {
                         'rdep_package': f"{rdep_pkg.get_name()}-{rdep_pkg.get_version()}-{rdep_pkg.get_release()}",
                         'rdep_source': rdep_pkg.get_source_name(),
                         'rdep_arch': rdep_pkg.get_arch(),
                         'requirement': req_str,
                         'provide_name': prov_name,
                         'new_version': new_version,
-                        'failed_constraint': f"{prov_name} {constraint['op']} {constraint['version']}"
+                        'failed_constraint': f"{prov_name} {constraint['op']} {constraint['version']}",
+                        'already_broken': current_version_also_fails
                     }
 
         return None
@@ -479,30 +493,58 @@ def print_results(self, results: Dict):
             if self.verbose:
                 print(f"No conflicts detected for {results['srpm_name']} {results['new_version']}")
         else:
-            # Separate conflicts by type
-            ftbfs_conflicts = []
-            fti_conflicts = []
+            # Separate conflicts by type and whether they're new or already broken
+            ftbfs_new = []
+            ftbfs_already_broken = []
+            fti_new = []
+            fti_already_broken = []
 
             for conflict in conflicts:
                 is_source = conflict['rdep_arch'] == 'src'
+                is_already_broken = conflict.get('already_broken', False)
+
                 if is_source:
-                    ftbfs_conflicts.append(conflict)
+                    if is_already_broken:
+                        ftbfs_already_broken.append(conflict)
+                    else:
+                        ftbfs_new.append(conflict)
                 else:
-                    fti_conflicts.append(conflict)
+                    if is_already_broken:
+                        fti_already_broken.append(conflict)
+                    else:
+                        fti_new.append(conflict)
 
-            # Print FTBFS section
-            if ftbfs_conflicts:
+            # Print new FTBFS conflicts
+            if ftbfs_new:
                 print("These packages would FTBFS:")
-                for conflict in ftbfs_conflicts:
+                for conflict in ftbfs_new:
                     package_name = conflict['rdep_source']
                     print(f"  {package_name}: {conflict['failed_constraint']}")
 
-            # Print FTI section
-            if fti_conflicts:
-                if ftbfs_conflicts:
+            # Print new FTI conflicts
+            if fti_new:
+                if ftbfs_new:
                     print()  # Empty line between sections
                 print("These packages would FTI:")
-                for conflict in fti_conflicts:
+                for conflict in fti_new:
+                    package_name = conflict['rdep_package']
+                    print(f"  {package_name}: {conflict['failed_constraint']}")
+
+            # Print already broken FTBFS packages
+            if ftbfs_already_broken:
+                if ftbfs_new or fti_new:
+                    print()  # Empty line between sections
+                print("These packages already FTBFS (not a new problem):")
+                for conflict in ftbfs_already_broken:
+                    package_name = conflict['rdep_source']
+                    print(f"  {package_name}: {conflict['failed_constraint']}")
+
+            # Print already broken FTI packages
+            if fti_already_broken:
+                if ftbfs_new or fti_new or ftbfs_already_broken:
+                    print()  # Empty line between sections
+                print("These packages already FTI (not a new problem):")
+                for conflict in fti_already_broken:
                     package_name = conflict['rdep_package']
                     print(f"  {package_name}: {conflict['failed_constraint']}")
 
@@ -536,9 +578,15 @@ def main():
         results = checker.simulate_version_change(args.srpm_name, args.new_version)
         checker.print_results(results)
 
-        # Exit with error code if conflicts found
+        # Exit with error code if NEW conflicts found (not already-broken packages)
         if results.get('conflicts'):
-            sys.exit(1)
+            # Check if there are any new conflicts (not already broken)
+            has_new_conflicts = any(
+                not conflict.get('already_broken', False)
+                for conflict in results['conflicts']
+            )
+            if has_new_conflicts:
+                sys.exit(1)
 
     except KeyboardInterrupt:
         print("\n\nInterrupted by user.")
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -32,6 +32,7 @@
     create_bundled_provides_scenario,
     create_multi_binary_scenario,
     create_same_srpm_dependency_scenario,
+    create_already_broken_scenario,
 )
 from fedora_revdep_check import FedoraRevDepChecker  # noqa: E402
 
@@ -113,6 +114,12 @@ def same_srpm_dep_base():
     return create_same_srpm_dependency_scenario()
 
 
+@pytest.fixture
+def already_broken_base():
+    """Provide a mock DNF base with already-broken and new conflict scenarios."""
+    return create_already_broken_scenario()
+
+
 @pytest.fixture
 def checker_instance(mock_dnf_base):
     """
diff --git a/tests/e2e/test_full_workflow.py b/tests/e2e/test_full_workflow.py
@@ -140,3 +140,99 @@ def mock_init(self, verbose=False, base=None, repos=None):
 
         # Outputs should be identical (deterministic)
         assert output1.getvalue() == output2.getvalue()
+
+    def test_main_exit_code_zero_for_already_broken_only(self, monkeypatch, mock_dnf_base, capsys):
+        """Test that main() exits with 0 when only already-broken packages are found."""
+        monkeypatch.setattr('sys.argv', ['fedora-revdep-check', 'pytest', '8.0.0'])
+
+        original_init = FedoraRevDepChecker.__init__
+
+        def mock_init(self, verbose=False, base=None, repos=None):
+            original_init(self, verbose=verbose, base=mock_dnf_base if base is None else base, repos=repos)
+
+        # Mock simulate_version_change to return only already-broken conflicts
+        original_simulate = FedoraRevDepChecker.simulate_version_change
+
+        def mock_simulate(self, srpm_name, new_version):
+            return {
+                'srpm_name': srpm_name,
+                'new_version': new_version,
+                'binary_packages': ['python3-pytest-7.0.0-1.fc40'],
+                'conflicts': [
+                    {
+                        'rdep_package': 'oldpkg-1.0.0-1.fc40',
+                        'rdep_source': 'oldpkg',
+                        'rdep_arch': 'src',
+                        'requirement': 'python3dist(pytest) < 5.0',
+                        'provide_name': 'python3dist(pytest)',
+                        'new_version': new_version,
+                        'failed_constraint': 'python3dist(pytest) < 5.0',
+                        'already_broken': True
+                    }
+                ]
+            }
+
+        monkeypatch.setattr(FedoraRevDepChecker, '__init__', mock_init)
+        monkeypatch.setattr(FedoraRevDepChecker, 'simulate_version_change', mock_simulate)
+
+        # main() should exit with 0 (no NEW conflicts)
+        main()
+
+        # Check output contains already-broken message
+        captured = capsys.readouterr()
+        assert 'already FTBFS (not a new problem)' in captured.out
+
+    def test_main_exit_code_one_for_mixed_conflicts(self, monkeypatch, mock_dnf_base, capsys):
+        """Test that main() exits with 1 when there are new conflicts mixed with already-broken."""
+        monkeypatch.setattr('sys.argv', ['fedora-revdep-check', 'pytest', '8.0.0'])
+
+        original_init = FedoraRevDepChecker.__init__
+
+        def mock_init(self, verbose=False, base=None, repos=None):
+            original_init(self, verbose=verbose, base=mock_dnf_base if base is None else base, repos=repos)
+
+        # Mock simulate_version_change to return mixed conflicts
+        def mock_simulate(self, srpm_name, new_version):
+            return {
+                'srpm_name': srpm_name,
+                'new_version': new_version,
+                'binary_packages': ['python3-pytest-7.0.0-1.fc40'],
+                'conflicts': [
+                    # New conflict
+                    {
+                        'rdep_package': 'newpkg-1.0.0-1.fc40',
+                        'rdep_source': 'newpkg',
+                        'rdep_arch': 'src',
+                        'requirement': 'python3dist(pytest) < 8.0',
+                        'provide_name': 'python3dist(pytest)',
+                        'new_version': new_version,
+                        'failed_constraint': 'python3dist(pytest) < 8.0',
+                        'already_broken': False
+                    },
+                    # Already broken
+                    {
+                        'rdep_package': 'oldpkg-1.0.0-1.fc40',
+                        'rdep_source': 'oldpkg',
+                        'rdep_arch': 'src',
+                        'requirement': 'python3dist(pytest) < 5.0',
+                        'provide_name': 'python3dist(pytest)',
+                        'new_version': new_version,
+                        'failed_constraint': 'python3dist(pytest) < 5.0',
+                        'already_broken': True
+                    }
+                ]
+            }
+
+        monkeypatch.setattr(FedoraRevDepChecker, '__init__', mock_init)
+        monkeypatch.setattr(FedoraRevDepChecker, 'simulate_version_change', mock_simulate)
+
+        # main() should exit with 1 (NEW conflicts found)
+        with pytest.raises(SystemExit) as exc_info:
+            main()
+
+        assert exc_info.value.code == 1
+
+        # Check output contains both sections
+        captured = capsys.readouterr()
+        assert 'These packages would FTBFS:' in captured.out
+        assert 'already FTBFS (not a new problem)' in captured.out
diff --git a/tests/fixtures/mock_packages.py b/tests/fixtures/mock_packages.py
@@ -541,3 +541,75 @@ def create_same_srpm_dependency_scenario():
     ]
 
     return MockBase(packages=packages)
+
+
+def create_already_broken_scenario():
+    """
+    Create a scenario with both new conflicts and already-broken packages.
+
+    Scenario:
+    - library 4.0.0 currently installed
+    - old-package requires library < 3.0 (already broken with current 4.0.0)
+    - new-package requires library < 5.0 (will break when upgrading to 5.0.0)
+    """
+    packages = [
+        # Library package
+        MockPackage(
+            name='library',
+            version='4.0.0',
+            release='1.fc40',
+            arch='noarch',
+            source_name='library',
+            provides=[
+                'library',
+                'library = 4.0.0-1.fc40',
+                'python3dist(library) = 4.0.0',
+            ]
+        ),
+        # Old package (already broken)
+        MockPackage(
+            name='python3-old-package',
+            version='1.0.0',
+            release='1.fc40',
+            arch='noarch',
+            source_name='old-package',
+            requires=[
+                'python3dist(library) < 3.0',  # Already fails with 4.0.0
+            ]
+        ),
+        # Old package SRPM (for FTBFS testing)
+        MockPackage(
+            name='old-package',
+            version='1.0.0',
+            release='1.fc40',
+            arch='src',
+            source_name='old-package',
+            requires=[
+                'python3dist(library) < 3.0',  # Already fails with 4.0.0
+            ]
+        ),
+        # New package (will break with 5.0.0)
+        MockPackage(
+            name='python3-new-package',
+            version='1.0.0',
+            release='1.fc40',
+            arch='noarch',
+            source_name='new-package',
+            requires=[
+                'python3dist(library) < 5.0',  # Will fail with 5.0.0
+            ]
+        ),
+        # New package SRPM (for FTBFS testing)
+        MockPackage(
+            name='new-package',
+            version='1.0.0',
+            release='1.fc40',
+            arch='src',
+            source_name='new-package',
+            requires=[
+                'python3dist(library) < 5.0',  # Will fail with 5.0.0
+            ]
+        ),
+    ]
+
+    return MockBase(packages=packages)
diff --git a/tests/integration/test_simulation.py b/tests/integration/test_simulation.py
@@ -153,3 +153,37 @@ def test_simulate_version_change_same_srpm_dependency(self, same_srpm_dep_base):
         # python3-external-tool requires micropipenv < 1.12, so it should still be satisfied
         # (1.11.0 < 1.12 is True)
         assert len(results['conflicts']) == 0
+
+    def test_simulate_version_change_already_broken_package(self, already_broken_base):
+        """Test that already-broken packages are properly marked."""
+        checker = FedoraRevDepChecker(verbose=False, base=already_broken_base)
+
+        # Upgrading library from 4.0.0 to 5.0.0
+        # old-package requires library < 3.0, so it's already broken with 4.0.0
+        # new-package requires library < 5.0, so it will break with 5.0.0
+        results = checker.simulate_version_change('library', '5.0.0')
+
+        assert 'error' not in results
+        # Should have 4 conflicts: 2 FTBFS (src) + 2 FTI (noarch) for each package
+        assert len(results['conflicts']) == 4
+
+        # Separate conflicts into already broken and new
+        old_pkg_conflicts = []
+        new_pkg_conflicts = []
+        for conflict in results['conflicts']:
+            if 'old-package' in conflict['rdep_source']:
+                old_pkg_conflicts.append(conflict)
+            elif 'new-package' in conflict['rdep_source']:
+                new_pkg_conflicts.append(conflict)
+
+        # old-package should have 2 conflicts (FTBFS and FTI), both already broken
+        assert len(old_pkg_conflicts) == 2
+        for conflict in old_pkg_conflicts:
+            assert conflict['already_broken'] is True
+            assert 'python3dist(library) < 3.0' in conflict['failed_constraint']
+
+        # new-package should have 2 conflicts (FTBFS and FTI), both new problems
+        assert len(new_pkg_conflicts) == 2
+        for conflict in new_pkg_conflicts:
+            assert conflict['already_broken'] is False
+            assert 'python3dist(library) < 5.0' in conflict['failed_constraint']
diff --git a/tests/unit/test_output.py b/tests/unit/test_output.py
