Skip to content

Commit 94d8290

Browse files
committed
reverse-dependences: Fix bug in find-package-dependents script
find-package-dependents was passing the user's max_results limit to the generate_direct_dependents function. This would be fine, except we sometimes need more than max_results generated if some of the results are filtered by the user's filter command. The upshot is the script would sometimes traverse the dependents tree deep instead of wide, leading to performance issues. This commit drops the max_result limit on generate_direct_dependents, since it's wrong and not needed anyway (because max_results is already handled elsewhere). This commit also adds a couple of test cases to make sure the problem doesn't creep in again.
1 parent 1427692 commit 94d8290

File tree

2 files changed

+99
-1
lines changed

2 files changed

+99
-1
lines changed

scripts/find-package-dependents.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -997,7 +997,7 @@ def build_dependents_graph(
997997
any_filtered_dependents = False
998998
for dependent in generate_direct_dependents(
999999
package, repository_paths, metrics, dependency_cache, verbose,
1000-
cache_only=result_limit_hit, max_results=max_results
1000+
cache_only=result_limit_hit
10011001
):
10021002
if show_source_packages:
10031003
dependent = query_source_package(

scripts/tests/test-find-package-dependents.py

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1876,6 +1876,104 @@ def mock_dependents(package_name, *args, **kwargs):
18761876
self.assertIn("mutter", result["libwayland-server"]["dependents"])
18771877
self.assertNotIn("libwayland-server", result["mutter"]["dependents"])
18781878

1879+
@patch.object(find_package_dependents, 'run_command')
1880+
def test_breadth_first_with_max_results_and_filtering(self, mock_run_command):
1881+
"""Test that breadth-first traversal works correctly with max_results even when some dependents are filtered out.
1882+
1883+
This test verifies the fix for the bug where max_results was incorrectly applied to generate_direct_dependents,
1884+
causing performance issues due to deep traversal when filters would reject some results.
1885+
"""
1886+
def mock_dnf_calls(command_args):
1887+
full_command = ' '.join(command_args)
1888+
if "libwayland-server" in full_command:
1889+
return {"return_code": 0, "output": "mutter\nweston\nsway\nkwin"}
1890+
else:
1891+
return {"return_code": 0, "output": ""}
1892+
1893+
mock_run_command.side_effect = mock_dnf_calls
1894+
1895+
def mock_filter_command(package_name, filter_cmd, *args, **kwargs):
1896+
return package_name in ["mutter", "kwin"]
1897+
1898+
with patch.object(find_package_dependents, 'run_filter_command', side_effect=mock_filter_command):
1899+
result = find_package_dependents.build_dependents_graph(
1900+
root_package="libwayland-server",
1901+
repository_paths=self.repositories,
1902+
show_source_packages=False,
1903+
source_cache=self.source_cache,
1904+
metrics=self.metrics,
1905+
filter_cache=self.filter_cache,
1906+
dependency_cache=self.dependency_cache,
1907+
max_results=1, # Very small limit to test breadth-first behavior
1908+
keep_cycles=False,
1909+
verbose=False,
1910+
filter_command="test filter", # Enable filtering
1911+
allow_missing=False
1912+
)
1913+
1914+
self.assertIn("libwayland-server", result)
1915+
root_dependents = result["libwayland-server"]["dependents"]
1916+
1917+
self.assertGreaterEqual(len(root_dependents), 1)
1918+
1919+
valid_dependents = [dep for dep in root_dependents if dep in ["mutter", "kwin"]]
1920+
self.assertGreaterEqual(len(valid_dependents), 1)
1921+
1922+
self.assertNotIn("sway", root_dependents)
1923+
self.assertNotIn("weston", root_dependents)
1924+
1925+
self.assertTrue(result["libwayland-server"]["partial"])
1926+
1927+
1928+
@patch.object(find_package_dependents, 'run_command')
1929+
def test_breadth_first_multi_level_traversal(self, mock_run_command):
1930+
"""Test breadth-first traversal works correctly across multiple dependency levels with filtering.
1931+
"""
1932+
def mock_dnf_calls(command_args):
1933+
full_command = ' '.join(command_args)
1934+
if "glibc" in full_command:
1935+
return {"return_code": 0, "output": "systemd\nbash\npodman\nbuildah\nkernel"}
1936+
elif "podman" in full_command:
1937+
return {"return_code": 0, "output": "cockpit-podman\ntoolbox"}
1938+
elif "buildah" in full_command:
1939+
return {"return_code": 0, "output": "container-tools\nskopeo"}
1940+
else:
1941+
return {"return_code": 0, "output": ""}
1942+
1943+
mock_run_command.side_effect = mock_dnf_calls
1944+
1945+
def mock_filter_command(package_name, filter_cmd, *args, **kwargs):
1946+
return package_name in ["podman", "buildah", "cockpit-podman", "toolbox", "container-tools", "skopeo"]
1947+
1948+
with patch.object(find_package_dependents, 'run_filter_command', side_effect=mock_filter_command):
1949+
result = find_package_dependents.build_dependents_graph(
1950+
root_package="glibc",
1951+
repository_paths=self.repositories,
1952+
show_source_packages=False,
1953+
source_cache=self.source_cache,
1954+
metrics=self.metrics,
1955+
filter_cache=self.filter_cache,
1956+
dependency_cache=self.dependency_cache,
1957+
max_results=4,
1958+
keep_cycles=False,
1959+
verbose=False,
1960+
filter_command="test filter",
1961+
allow_missing=False
1962+
)
1963+
1964+
self.assertIn("glibc", result)
1965+
self.assertIn("podman", result)
1966+
self.assertIn("buildah", result)
1967+
1968+
podman_children = result["podman"]["dependents"]
1969+
self.assertIn("cockpit-podman", podman_children)
1970+
self.assertIn("toolbox", podman_children)
1971+
1972+
root_dependents = result["glibc"]["dependents"]
1973+
self.assertNotIn("systemd", root_dependents)
1974+
self.assertNotIn("bash", root_dependents)
1975+
self.assertNotIn("kernel", root_dependents)
1976+
18791977

18801978
def print_header():
18811979
print("─" * 70)

0 commit comments

Comments
 (0)